[PATCH] usb: dwc2: Add in version 4xx compatibility

[PATCH] usb: dwc2: Add in version 4xx compatibility

[Greg Malysa]

From: Nathan Barrett-Morrison <nathan.morrison@timesys.com>

This adds the Synopsys device id for version 4xx of the designware
IP block and extends the version check to include it to permit
new hardware to run. It does not add any 4xx-specific features.

Signed-off-by: Ian Roberts <ian.roberts@timesys.com>
Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>

---


---
 drivers/usb/host/dwc2.c | 3 ++-
 drivers/usb/host/dwc2.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 637eb2dd06..6fdde6a9a7 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -1180,7 +1180,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv)
 		 snpsid >> 12 & 0xf, snpsid & 0xfff);
 
 	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
-	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
+	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx &&
+	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_4xx) {
 		dev_info(dev, "SNPSID invalid (not DWC2 OTG device): %08x\n",
 			 snpsid);
 		return -ENODEV;
diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
index 6f022e33a1..f202d55eb2 100644
--- a/drivers/usb/host/dwc2.h
+++ b/drivers/usb/host/dwc2.h
@@ -739,6 +739,7 @@ struct dwc2_core_regs {
 #define DWC2_PCGCCTL_DEEP_SLEEP_OFFSET			7
 #define DWC2_SNPSID_DEVID_VER_2xx			(0x4f542 << 12)
 #define DWC2_SNPSID_DEVID_VER_3xx			(0x4f543 << 12)
+#define DWC2_SNPSID_DEVID_VER_4xx			(0x4f544 << 12)
 #define DWC2_SNPSID_DEVID_MASK				(0xfffff << 12)
 #define DWC2_SNPSID_DEVID_OFFSET			12
 
-- 
2.43.2

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Marek Vasut]

On 3/26/24 3:32 AM, Greg Malysa wrote:
> From: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> 
> This adds the Synopsys device id for version 4xx of the designware
> IP block and extends the version check to include it to permit
> new hardware to run. It does not add any 4xx-specific features.
> 
> Signed-off-by: Ian Roberts <ian.roberts@timesys.com>
> Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> 
> ---
> 
> 
> ---
>   drivers/usb/host/dwc2.c | 3 ++-
>   drivers/usb/host/dwc2.h | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index 637eb2dd06..6fdde6a9a7 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -1180,7 +1180,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv)
>   		 snpsid >> 12 & 0xf, snpsid & 0xfff);
>   
>   	if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
> -	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
> +	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx &&
> +	    (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_4xx) {
>   		dev_info(dev, "SNPSID invalid (not DWC2 OTG device): %08x\n",
>   			 snpsid);
>   		return -ENODEV;
> diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
> index 6f022e33a1..f202d55eb2 100644
> --- a/drivers/usb/host/dwc2.h
> +++ b/drivers/usb/host/dwc2.h
> @@ -739,6 +739,7 @@ struct dwc2_core_regs {
>   #define DWC2_PCGCCTL_DEEP_SLEEP_OFFSET			7
>   #define DWC2_SNPSID_DEVID_VER_2xx			(0x4f542 << 12)
>   #define DWC2_SNPSID_DEVID_VER_3xx			(0x4f543 << 12)
> +#define DWC2_SNPSID_DEVID_VER_4xx			(0x4f544 << 12)
>   #define DWC2_SNPSID_DEVID_MASK				(0xfffff << 12)
>   #define DWC2_SNPSID_DEVID_OFFSET			12

Maybe it would be better/easier/futureproof to simply check if (snpsid & 
0xffff0 == 0x4f540) ?

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Greg Malysa]

I'd be happy with that change. Does anyone have access to the
associated designware databook (I do not)? We could also check to see
if those four bits are all always allocated to the 2/3/4/x version
number. I can submit v2 with that change instead once we know.

On Tue, Mar 26, 2024 at 7:50 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/26/24 3:32 AM, Greg Malysa wrote:
> > From: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> >
> > This adds the Synopsys device id for version 4xx of the designware
> > IP block and extends the version check to include it to permit
> > new hardware to run. It does not add any 4xx-specific features.
> >
> > Signed-off-by: Ian Roberts <ian.roberts@timesys.com>
> > Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
> > Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> >
> > ---
> >
> >
> > ---
> >   drivers/usb/host/dwc2.c | 3 ++-
> >   drivers/usb/host/dwc2.h | 1 +
> >   2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> > index 637eb2dd06..6fdde6a9a7 100644
> > --- a/drivers/usb/host/dwc2.c
> > +++ b/drivers/usb/host/dwc2.c
> > @@ -1180,7 +1180,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv)
> >                snpsid >> 12 & 0xf, snpsid & 0xfff);
> >
> >       if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
> > -         (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
> > +         (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx &&
> > +         (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_4xx) {
> >               dev_info(dev, "SNPSID invalid (not DWC2 OTG device): %08x\n",
> >                        snpsid);
> >               return -ENODEV;
> > diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
> > index 6f022e33a1..f202d55eb2 100644
> > --- a/drivers/usb/host/dwc2.h
> > +++ b/drivers/usb/host/dwc2.h
> > @@ -739,6 +739,7 @@ struct dwc2_core_regs {
> >   #define DWC2_PCGCCTL_DEEP_SLEEP_OFFSET                      7
> >   #define DWC2_SNPSID_DEVID_VER_2xx                   (0x4f542 << 12)
> >   #define DWC2_SNPSID_DEVID_VER_3xx                   (0x4f543 << 12)
> > +#define DWC2_SNPSID_DEVID_VER_4xx                    (0x4f544 << 12)
> >   #define DWC2_SNPSID_DEVID_MASK                              (0xfffff << 12)
> >   #define DWC2_SNPSID_DEVID_OFFSET                    12
>
> Maybe it would be better/easier/futureproof to simply check if (snpsid &
> 0xffff0 == 0x4f540) ?



-- 
Greg Malysa
Timesys Corporation

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Mattijs Korpershoek]

Hi Greg,

Thank you for the contribution.

On mar., mars 26, 2024 at 11:36, Greg Malysa <greg.malysa@timesys.com> wrote:

Please avoid top-posting when replying, it makes following the
discussion more difficult:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions

> I'd be happy with that change. Does anyone have access to the
> associated designware databook (I do not)? We could also check to see
> if those four bits are all always allocated to the 2/3/4/x version
> number. I can submit v2 with that change instead once we know.

I don't have access to the databooks either. I usually refer to the
linux kernel code since it's a more up to date version of this driver.

Looking at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c

we can see that the following is added:
#define DWC2_CORE_REV_MASK	0x0000ffff

This makes me believe that the versioning follows a well known pattern.

>
> On Tue, Mar 26, 2024 at 7:50 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/26/24 3:32 AM, Greg Malysa wrote:
>> > From: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
>> >
>> > This adds the Synopsys device id for version 4xx of the designware
>> > IP block and extends the version check to include it to permit
>> > new hardware to run. It does not add any 4xx-specific features.
>> >
>> > Signed-off-by: Ian Roberts <ian.roberts@timesys.com>
>> > Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
>> > Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
>> >
>> > ---
>> >
>> >
>> > ---
>> >   drivers/usb/host/dwc2.c | 3 ++-
>> >   drivers/usb/host/dwc2.h | 1 +
>> >   2 files changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>> > index 637eb2dd06..6fdde6a9a7 100644
>> > --- a/drivers/usb/host/dwc2.c
>> > +++ b/drivers/usb/host/dwc2.c
>> > @@ -1180,7 +1180,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv)
>> >                snpsid >> 12 & 0xf, snpsid & 0xfff);
>> >
>> >       if ((snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_2xx &&
>> > -         (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx) {
>> > +         (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_3xx &&
>> > +         (snpsid & DWC2_SNPSID_DEVID_MASK) != DWC2_SNPSID_DEVID_VER_4xx) {

Note that this change is also part of:
https://lore.kernel.org/all/20240328131811.94559-1-seashell11234455@gmail.com/

>> >               dev_info(dev, "SNPSID invalid (not DWC2 OTG device): %08x\n",
>> >                        snpsid);
>> >               return -ENODEV;
>> > diff --git a/drivers/usb/host/dwc2.h b/drivers/usb/host/dwc2.h
>> > index 6f022e33a1..f202d55eb2 100644
>> > --- a/drivers/usb/host/dwc2.h
>> > +++ b/drivers/usb/host/dwc2.h
>> > @@ -739,6 +739,7 @@ struct dwc2_core_regs {
>> >   #define DWC2_PCGCCTL_DEEP_SLEEP_OFFSET                      7
>> >   #define DWC2_SNPSID_DEVID_VER_2xx                   (0x4f542 << 12)
>> >   #define DWC2_SNPSID_DEVID_VER_3xx                   (0x4f543 << 12)
>> > +#define DWC2_SNPSID_DEVID_VER_4xx                    (0x4f544 << 12)
>> >   #define DWC2_SNPSID_DEVID_MASK                              (0xfffff << 12)
>> >   #define DWC2_SNPSID_DEVID_OFFSET                    12
>>
>> Maybe it would be better/easier/futureproof to simply check if (snpsid &
>> 0xffff0 == 0x4f540) ?
>
>
>
> -- 
> Greg Malysa
> Timesys Corporation

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Greg Malysa]

Hi Mattijs,

> Please avoid top-posting when replying, it makes following the
> discussion more difficult:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions

Will do. Sorry about that; I'm still learning about this approach to email.

> Looking at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
>
> we can see that the following is added:
> #define DWC2_CORE_REV_MASK      0x0000ffff
>
> This makes me believe that the versioning follows a well known pattern.

I can submit a v2 next week to bring it in line with the kernel's approach.

> Note that this change is also part of:
> https://lore.kernel.org/all/20240328131811.94559-1-seashell11234455@gmail.com/

Or if you prefer I can also drop our patch and we can pursue this
linked patch with both 4xx compatibility and the 420a reset handling.

Thanks,
Greg

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Mattijs Korpershoek]

Hi Greg,

On ven., avril 19, 2024 at 15:21, Greg Malysa <greg.malysa@timesys.com> wrote:

> Hi Mattijs,
>
>> Please avoid top-posting when replying, it makes following the
>> discussion more difficult:
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
>
> Will do. Sorry about that; I'm still learning about this approach to email.

No worries. There are quite some things to learn and we probably all did
this wrong when starting. Thank you for taking the time to learn and to contribute!

>
>> Looking at
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
>>
>> we can see that the following is added:
>> #define DWC2_CORE_REV_MASK      0x0000ffff
>>
>> This makes me believe that the versioning follows a well known pattern.
>
> I can submit a v2 next week to bring it in line with the kernel's approach.

To me, it's fine as is.

>
>> Note that this change is also part of:
>> https://lore.kernel.org/all/20240328131811.94559-1-seashell11234455@gmail.com/
>
> Or if you prefer I can also drop our patch and we can pursue this
> linked patch with both 4xx compatibility and the 420a reset handling.

I think the patch you submitted is fine by itself, but I'd let Marek
decide since he is the maintainer for this part.

If you have access to the hardware that has a 4.20a dwc2 controller,
maybe you can help testing the patch above patch as well?

>
> Thanks,
> Greg

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Marek Vasut]

On 4/23/24 9:09 AM, Mattijs Korpershoek wrote:
> Hi Greg,
> 
> On ven., avril 19, 2024 at 15:21, Greg Malysa <greg.malysa@timesys.com> wrote:
> 
>> Hi Mattijs,
>>
>>> Please avoid top-posting when replying, it makes following the
>>> discussion more difficult:
>>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
>>
>> Will do. Sorry about that; I'm still learning about this approach to email.
> 
> No worries. There are quite some things to learn and we probably all did
> this wrong when starting. Thank you for taking the time to learn and to contribute!
> 
>>
>>> Looking at
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
>>>
>>> we can see that the following is added:
>>> #define DWC2_CORE_REV_MASK      0x0000ffff
>>>
>>> This makes me believe that the versioning follows a well known pattern.
>>
>> I can submit a v2 next week to bring it in line with the kernel's approach.
> 
> To me, it's fine as is.
> 
>>
>>> Note that this change is also part of:
>>> https://lore.kernel.org/all/20240328131811.94559-1-seashell11234455@gmail.com/
>>
>> Or if you prefer I can also drop our patch and we can pursue this
>> linked patch with both 4xx compatibility and the 420a reset handling.
> 
> I think the patch you submitted is fine by itself, but I'd let Marek
> decide since he is the maintainer for this part.
> 
> If you have access to the hardware that has a 4.20a dwc2 controller,
> maybe you can help testing the patch above patch as well?

+CC Liu on this thread, maybe it is best if the two of you figure out 
the best common approach that works for you both ?

Re: [PATCH] usb: dwc2: Add in version 4xx compatibility

[Greg Malysa]

> >
> > If you have access to the hardware that has a 4.20a dwc2 controller,
> > maybe you can help testing the patch above patch as well?

My hardware unfortunately only has a 4.00a controller so I cannot test
the 4.20a reset functionality. However, Kongyang Liu's patch works for
me as a replacement for our submission and functions correctly
otherwise on our hardware, so I am fine with moving forward on his
patch. If that's meaningful enough I can add a tested by tag to the
other patch from me.

>
> +CC Liu on this thread, maybe it is best if the two of you figure out
> the best common approach that works for you both ?

I am also open to collaborating on any other changes as needed.