| Message ID | 20160829110456.12206-1-sr@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 29, 2016 at 01:04:55PM +0200, Stefan Roese wrote: > This patch provides a stub function for mvebu_mbus_get_io_win_info(), > which will be used for all non-Orion (ARM32 MVEBU) platforms for > compile test coverage. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Gregory CLEMENT <gregory.clement@free-electrons.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Marcin Wojtas <mw@semihalf.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Vinod Koul <vinod.koul@intel.com> > --- > include/linux/mbus.h | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/linux/mbus.h b/include/linux/mbus.h > index d610232..a884405 100644 > --- a/include/linux/mbus.h > +++ b/include/linux/mbus.h > @@ -55,6 +55,8 @@ struct mbus_dram_target_info > #ifdef CONFIG_PLAT_ORION > extern const struct mbus_dram_target_info *mv_mbus_dram_info(void); > extern const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(void); > +int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, > + u8 *attr); > #else > static inline const struct mbus_dram_target_info *mv_mbus_dram_info(void) > { > @@ -64,14 +66,17 @@ static inline const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(vo > { > return NULL; > } > +static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, > + u8 *target, u8 *attr) > +{ > + return 0; > +} > #endif Hi Stefan If i understand it correctly, this should never be used for real, it is here only for compile testing? Maybe add a WARN_ON_ONCE(1) here? And a comment, why it should never happen. Thanks Andrew
Hi Andrew, On 29.08.2016 14:57, Andrew Lunn wrote: > On Mon, Aug 29, 2016 at 01:04:55PM +0200, Stefan Roese wrote: >> This patch provides a stub function for mvebu_mbus_get_io_win_info(), >> which will be used for all non-Orion (ARM32 MVEBU) platforms for >> compile test coverage. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com> >> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >> Cc: Marcin Wojtas <mw@semihalf.com> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Vinod Koul <vinod.koul@intel.com> >> --- >> include/linux/mbus.h | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/mbus.h b/include/linux/mbus.h >> index d610232..a884405 100644 >> --- a/include/linux/mbus.h >> +++ b/include/linux/mbus.h >> @@ -55,6 +55,8 @@ struct mbus_dram_target_info >> #ifdef CONFIG_PLAT_ORION >> extern const struct mbus_dram_target_info *mv_mbus_dram_info(void); >> extern const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(void); >> +int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, >> + u8 *attr); >> #else >> static inline const struct mbus_dram_target_info *mv_mbus_dram_info(void) >> { >> @@ -64,14 +66,17 @@ static inline const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(vo >> { >> return NULL; >> } >> +static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, >> + u8 *target, u8 *attr) >> +{ >> + return 0; >> +} >> #endif > > Hi Stefan > > If i understand it correctly, this should never be used for real, it > is here only for compile testing? Correct. > Maybe add a WARN_ON_ONCE(1) here? And a comment, why it should never > happen. Yes, I could do this. I (as others too) just followed this example here: git ID 63a9332b "ARM: Orion: Get address map from plat-orion instead of via platform_data" Should I add these WARN_ON_ONCE(1) macros to the other stub functions in this header as well? Perhaps as a new 3rd patch in this series? Thanks, Stefan
Hi Andrew, On lun., août 29 2016, Andrew Lunn <andrew@lunn.ch> wrote: > On Mon, Aug 29, 2016 at 01:04:55PM +0200, Stefan Roese wrote: >> This patch provides a stub function for mvebu_mbus_get_io_win_info(), >> which will be used for all non-Orion (ARM32 MVEBU) platforms for >> compile test coverage. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com> >> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >> Cc: Marcin Wojtas <mw@semihalf.com> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Cc: Vinod Koul <vinod.koul@intel.com> >> --- >> include/linux/mbus.h | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/mbus.h b/include/linux/mbus.h >> index d610232..a884405 100644 >> --- a/include/linux/mbus.h >> +++ b/include/linux/mbus.h >> @@ -55,6 +55,8 @@ struct mbus_dram_target_info >> #ifdef CONFIG_PLAT_ORION >> extern const struct mbus_dram_target_info *mv_mbus_dram_info(void); >> extern const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(void); >> +int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, >> + u8 *attr); >> #else >> static inline const struct mbus_dram_target_info *mv_mbus_dram_info(void) >> { >> @@ -64,14 +66,17 @@ static inline const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(vo >> { >> return NULL; >> } >> +static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, >> + u8 *target, u8 *attr) >> +{ >> + return 0; >> +} >> #endif > > Hi Stefan > > If i understand it correctly, this should never be used for real, it > is here only for compile testing? > > Maybe add a WARN_ON_ONCE(1) here? And a comment, why it should never > happen. Actually arm64 mvebu SoC such as the Armada 3700 do not have mbus bus but can still reuse the same driver than the other mvebu SoC. So in this case I would prefer that the function return an error. Thanks, Gregory > > Thanks > > Andrew
> Actually arm64 mvebu SoC such as the Armada 3700 do not have mbus bus > but can still reuse the same driver than the other mvebu SoC. So in > this case I would prefer that the function return an error. O.K. An error would be good, assuming the code actually catches it. I think the code change in patch [2/2] did not catch the error. Andrew
Hi Gregory, On 29.08.2016 16:02, Gregory CLEMENT wrote: > On lun., août 29 2016, Andrew Lunn <andrew@lunn.ch> wrote: > >> On Mon, Aug 29, 2016 at 01:04:55PM +0200, Stefan Roese wrote: >>> This patch provides a stub function for mvebu_mbus_get_io_win_info(), >>> which will be used for all non-Orion (ARM32 MVEBU) platforms for >>> compile test coverage. >>> >>> Signed-off-by: Stefan Roese <sr@denx.de> >>> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com> >>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >>> Cc: Marcin Wojtas <mw@semihalf.com> >>> Cc: Arnd Bergmann <arnd@arndb.de> >>> Cc: Vinod Koul <vinod.koul@intel.com> >>> --- >>> include/linux/mbus.h | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/linux/mbus.h b/include/linux/mbus.h >>> index d610232..a884405 100644 >>> --- a/include/linux/mbus.h >>> +++ b/include/linux/mbus.h >>> @@ -55,6 +55,8 @@ struct mbus_dram_target_info >>> #ifdef CONFIG_PLAT_ORION >>> extern const struct mbus_dram_target_info *mv_mbus_dram_info(void); >>> extern const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(void); >>> +int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, >>> + u8 *attr); >>> #else >>> static inline const struct mbus_dram_target_info *mv_mbus_dram_info(void) >>> { >>> @@ -64,14 +66,17 @@ static inline const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(vo >>> { >>> return NULL; >>> } >>> +static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, >>> + u8 *target, u8 *attr) >>> +{ >>> + return 0; >>> +} >>> #endif >> >> Hi Stefan >> >> If i understand it correctly, this should never be used for real, it >> is here only for compile testing? >> >> Maybe add a WARN_ON_ONCE(1) here? And a comment, why it should never >> happen. > > Actually arm64 mvebu SoC such as the Armada 3700 do not have mbus bus > but can still reuse the same driver than the other mvebu SoC. So in > this case I would prefer that the function return an error. I missed thinking about Armada 3700 while adding this IO (PCIe) support to the XOR DMA driver. Most likely because the Armada 3700 support was added after I started working on this IO (non SDRAM) DMA support for Armada XP. Its probably not possible to support these IO (PCIe) areas on Armada 3700 this way without any information from the MBus mapping as possible for the other ARM32 SoCs. So its perhaps best if I just disable this "feature" for Armada 3700 for now. What do you think? Thanks, Stefan
Hi Stefan, On lun., août 29 2016, Stefan Roese <sr@denx.de> wrote: > Hi Gregory, > > On 29.08.2016 16:02, Gregory CLEMENT wrote: >> On lun., août 29 2016, Andrew Lunn <andrew@lunn.ch> wrote: >> >>> On Mon, Aug 29, 2016 at 01:04:55PM +0200, Stefan Roese wrote: >>>> This patch provides a stub function for mvebu_mbus_get_io_win_info(), >>>> which will be used for all non-Orion (ARM32 MVEBU) platforms for >>>> compile test coverage. >>>> >>>> Signed-off-by: Stefan Roese <sr@denx.de> >>>> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com> >>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >>>> Cc: Marcin Wojtas <mw@semihalf.com> >>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>> Cc: Vinod Koul <vinod.koul@intel.com> >>>> --- >>>> include/linux/mbus.h | 9 +++++++-- >>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/include/linux/mbus.h b/include/linux/mbus.h >>>> index d610232..a884405 100644 >>>> --- a/include/linux/mbus.h >>>> +++ b/include/linux/mbus.h >>>> @@ -55,6 +55,8 @@ struct mbus_dram_target_info >>>> #ifdef CONFIG_PLAT_ORION >>>> extern const struct mbus_dram_target_info *mv_mbus_dram_info(void); >>>> extern const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(void); >>>> +int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, >>>> + u8 *attr); >>>> #else >>>> static inline const struct mbus_dram_target_info *mv_mbus_dram_info(void) >>>> { >>>> @@ -64,14 +66,17 @@ static inline const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(vo >>>> { >>>> return NULL; >>>> } >>>> +static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, >>>> + u8 *target, u8 *attr) >>>> +{ >>>> + return 0; >>>> +} >>>> #endif >>> >>> Hi Stefan >>> >>> If i understand it correctly, this should never be used for real, it >>> is here only for compile testing? >>> >>> Maybe add a WARN_ON_ONCE(1) here? And a comment, why it should never >>> happen. >> >> Actually arm64 mvebu SoC such as the Armada 3700 do not have mbus bus >> but can still reuse the same driver than the other mvebu SoC. So in >> this case I would prefer that the function return an error. > > I missed thinking about Armada 3700 while adding this IO (PCIe) support > to the XOR DMA driver. Most likely because the Armada 3700 support was > added after I started working on this IO (non SDRAM) DMA support for > Armada XP. > > Its probably not possible to support these IO (PCIe) areas on Armada > 3700 this way without any information from the MBus mapping as > possible for the other ARM32 SoCs. So its perhaps best if I just > disable this "feature" for Armada 3700 for now. > > What do you think? My concern to return an error with mbus was not for the XOR driver. It was the neta driver that I had in mind (which was not converted yet). Thanks, Gregory > > Thanks, > Stefan
diff --git a/include/linux/mbus.h b/include/linux/mbus.h index d610232..a884405 100644 --- a/include/linux/mbus.h +++ b/include/linux/mbus.h @@ -55,6 +55,8 @@ struct mbus_dram_target_info #ifdef CONFIG_PLAT_ORION extern const struct mbus_dram_target_info *mv_mbus_dram_info(void); extern const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(void); +int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, + u8 *attr); #else static inline const struct mbus_dram_target_info *mv_mbus_dram_info(void) { @@ -64,14 +66,17 @@ static inline const struct mbus_dram_target_info *mv_mbus_dram_info_nooverlap(vo { return NULL; } +static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, + u8 *target, u8 *attr) +{ + return 0; +} #endif int mvebu_mbus_save_cpu_target(u32 __iomem *store_addr); void mvebu_mbus_get_pcie_mem_aperture(struct resource *res); void mvebu_mbus_get_pcie_io_aperture(struct resource *res); int mvebu_mbus_get_dram_win_info(phys_addr_t phyaddr, u8 *target, u8 *attr); -int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size, u8 *target, - u8 *attr); int mvebu_mbus_add_window_remap_by_id(unsigned int target, unsigned int attribute, phys_addr_t base, size_t size,
This patch provides a stub function for mvebu_mbus_get_io_win_info(), which will be used for all non-Orion (ARM32 MVEBU) platforms for compile test coverage. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Marcin Wojtas <mw@semihalf.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Vinod Koul <vinod.koul@intel.com> --- include/linux/mbus.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)