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

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

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