[PATCH] usb: musb-new: Extend Allwinner quirk to newer SoCs

[PATCH] usb: musb-new: Extend Allwinner quirk to newer SoCs

[Andre Przywara]

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
 	/* <Sigh> allwinner saves a reg, and we need to hardcode this */
 	return 0xde;
 #else
-- 
2.17.5

Re: [PATCH] usb: musb-new: Extend Allwinner quirk to newer SoCs

[Marek Vasut]

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 ?

Re: [PATCH] usb: musb-new: Extend Allwinner quirk to newer SoCs

[Andre Przywara]

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

Re: [PATCH] usb: musb-new: Extend Allwinner quirk to newer SoCs

[Marek Vasut]

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

Re: [PATCH] usb: musb-new: Extend Allwinner quirk to newer SoCs

[Jagan Teki]

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.