| Message ID | 20210427000323.18285-1-andre.przywara@arm.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | usb: musb-new: Extend Allwinner quirk to newer SoCs | expand |
On 4/27/21 2:03 AM, Andre Przywara wrote: > As the comment in musb_regs.h describes, Allwinner saves the > MUSB_CONFIGDATA register, which always return 0 on those SoCs. > > This is also true for the H6 and H616, so extend the quirk to those > controllers as well. > > This fixes USB peripheral mode on H6 and H616 boards. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/usb/musb-new/musb_regs.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/musb-new/musb_regs.h b/drivers/usb/musb-new/musb_regs.h > index c4d7203b851..bee1b715a95 100644 > --- a/drivers/usb/musb-new/musb_regs.h > +++ b/drivers/usb/musb-new/musb_regs.h > @@ -432,7 +432,8 @@ static inline u8 musb_read_ulpi_buscontrol(void __iomem *mbase) > static inline u8 musb_read_configdata(void __iomem *mbase) > { > #if defined CONFIG_MACH_SUN8I_A33 || defined CONFIG_MACH_SUN8I_A83T || \ > - defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I > + defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I || \ > + defined CONFIG_SUN50I_GEN_H6 Isn't there some better solution then ever-growing list of macros to check, like e.g. a single CONFIG_MACH_SUNXI ?
On Tue, 27 Apr 2021 02:37:20 +0200 Marek Vasut <marex@denx.de> wrote: Hi, > On 4/27/21 2:03 AM, Andre Przywara wrote: > > As the comment in musb_regs.h describes, Allwinner saves the > > MUSB_CONFIGDATA register, which always return 0 on those SoCs. > > > > This is also true for the H6 and H616, so extend the quirk to those > > controllers as well. > > > > This fixes USB peripheral mode on H6 and H616 boards. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > drivers/usb/musb-new/musb_regs.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/musb-new/musb_regs.h b/drivers/usb/musb-new/musb_regs.h > > index c4d7203b851..bee1b715a95 100644 > > --- a/drivers/usb/musb-new/musb_regs.h > > +++ b/drivers/usb/musb-new/musb_regs.h > > @@ -432,7 +432,8 @@ static inline u8 musb_read_ulpi_buscontrol(void __iomem *mbase) > > static inline u8 musb_read_configdata(void __iomem *mbase) > > { > > #if defined CONFIG_MACH_SUN8I_A33 || defined CONFIG_MACH_SUN8I_A83T || \ > > - defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I > > + defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I || \ > > + defined CONFIG_SUN50I_GEN_H6 > > Isn't there some better solution then ever-growing list of macros to > check, like e.g. a single CONFIG_MACH_SUNXI ? I was wondering the same, but I think this does not apply to the older SoCs (we use ARCH_SUNXI in the two functions above and below, so I guess the differentiation here is deliberate). I will test this later. So we could probably use the quirk also for the older, working(?) SoCs, but I am not sure we should do that. CONFIG_SUN50I_GEN_H6 is already a symbol covering multiple SoCs, so ideally we won't need to add many more. I can have a look if we have other checks like that in the code, then maybe define a collective symbol for newer SoCs? Cheers, Andre
On 4/27/21 10:10 AM, Andre Przywara wrote: > On Tue, 27 Apr 2021 02:37:20 +0200 > Marek Vasut <marex@denx.de> wrote: > > Hi, > >> On 4/27/21 2:03 AM, Andre Przywara wrote: >>> As the comment in musb_regs.h describes, Allwinner saves the >>> MUSB_CONFIGDATA register, which always return 0 on those SoCs. >>> >>> This is also true for the H6 and H616, so extend the quirk to those >>> controllers as well. >>> >>> This fixes USB peripheral mode on H6 and H616 boards. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> drivers/usb/musb-new/musb_regs.h | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/musb-new/musb_regs.h b/drivers/usb/musb-new/musb_regs.h >>> index c4d7203b851..bee1b715a95 100644 >>> --- a/drivers/usb/musb-new/musb_regs.h >>> +++ b/drivers/usb/musb-new/musb_regs.h >>> @@ -432,7 +432,8 @@ static inline u8 musb_read_ulpi_buscontrol(void __iomem *mbase) >>> static inline u8 musb_read_configdata(void __iomem *mbase) >>> { >>> #if defined CONFIG_MACH_SUN8I_A33 || defined CONFIG_MACH_SUN8I_A83T || \ >>> - defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I >>> + defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I || \ >>> + defined CONFIG_SUN50I_GEN_H6 >> >> Isn't there some better solution then ever-growing list of macros to >> check, like e.g. a single CONFIG_MACH_SUNXI ? > > I was wondering the same, but I think this does not apply to the older > SoCs (we use ARCH_SUNXI in the two functions above and below, so I > guess the differentiation here is deliberate). I will test this later. > > So we could probably use the quirk also for the older, working(?) SoCs, > but I am not sure we should do that. CONFIG_SUN50I_GEN_H6 is already a > symbol covering multiple SoCs, so ideally we won't need to add many > more. > I can have a look if we have other checks like that in the code, then > maybe define a collective symbol for newer SoCs? Yes please
On Tue, Apr 27, 2021 at 1:41 PM Andre Przywara <andre.przywara@arm.com> wrote: > > On Tue, 27 Apr 2021 02:37:20 +0200 > Marek Vasut <marex@denx.de> wrote: > > Hi, > > > On 4/27/21 2:03 AM, Andre Przywara wrote: > > > As the comment in musb_regs.h describes, Allwinner saves the > > > MUSB_CONFIGDATA register, which always return 0 on those SoCs. > > > > > > This is also true for the H6 and H616, so extend the quirk to those > > > controllers as well. > > > > > > This fixes USB peripheral mode on H6 and H616 boards. > > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > --- > > > drivers/usb/musb-new/musb_regs.h | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/musb-new/musb_regs.h b/drivers/usb/musb-new/musb_regs.h > > > index c4d7203b851..bee1b715a95 100644 > > > --- a/drivers/usb/musb-new/musb_regs.h > > > +++ b/drivers/usb/musb-new/musb_regs.h > > > @@ -432,7 +432,8 @@ static inline u8 musb_read_ulpi_buscontrol(void __iomem *mbase) > > > static inline u8 musb_read_configdata(void __iomem *mbase) > > > { > > > #if defined CONFIG_MACH_SUN8I_A33 || defined CONFIG_MACH_SUN8I_A83T || \ > > > - defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I > > > + defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I || \ > > > + defined CONFIG_SUN50I_GEN_H6 > > > > Isn't there some better solution then ever-growing list of macros to > > check, like e.g. a single CONFIG_MACH_SUNXI ? > > I was wondering the same, but I think this does not apply to the older > SoCs (we use ARCH_SUNXI in the two functions above and below, so I > guess the differentiation here is deliberate). I will test this later. > > So we could probably use the quirk also for the older, working(?) SoCs, > but I am not sure we should do that. CONFIG_SUN50I_GEN_H6 is already a > symbol covering multiple SoCs, so ideally we won't need to add many > more. > I can have a look if we have other checks like that in the code, then > maybe define a collective symbol for newer SoCs? The better approach is to support config_read via musb_platform_ops. If so, we can identify the offsets to endpoint registers using SoC musb reg space and return the relevant 16-bit config value. Jagan.
diff --git a/drivers/usb/musb-new/musb_regs.h b/drivers/usb/musb-new/musb_regs.h index c4d7203b851..bee1b715a95 100644 --- a/drivers/usb/musb-new/musb_regs.h +++ b/drivers/usb/musb-new/musb_regs.h @@ -432,7 +432,8 @@ static inline u8 musb_read_ulpi_buscontrol(void __iomem *mbase) static inline u8 musb_read_configdata(void __iomem *mbase) { #if defined CONFIG_MACH_SUN8I_A33 || defined CONFIG_MACH_SUN8I_A83T || \ - defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I + defined CONFIG_MACH_SUNXI_H3_H5 || defined CONFIG_MACH_SUN50I || \ + defined CONFIG_SUN50I_GEN_H6 /* <Sigh> allwinner saves a reg, and we need to hardcode this */ return 0xde; #else
As the comment in musb_regs.h describes, Allwinner saves the MUSB_CONFIGDATA register, which always return 0 on those SoCs. This is also true for the H6 and H616, so extend the quirk to those controllers as well. This fixes USB peripheral mode on H6 and H616 boards. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/usb/musb-new/musb_regs.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)