[v4,2/2] dt-bindings: usb: snps,dwc3: Add the compatible name 'snps,dwc3-rtk-soc'

Message ID 20230502050452.27276-2-stanley_chang@realtek.com
State New
Headers
Series [v4,1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address |

Commit Message

Stanley Chang[昌育德] May 2, 2023, 5:04 a.m. UTC
  Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek dwc3
core to adjust the global register start address

The RTK DHC SoCs were designed, the global register address offset at
0x8100. The default address offset is constant at DWC3_GLOBALS_REGS_START
(0xc100). Therefore, add the compatible name of device-tree to specify
the SoC custom's global register start address.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
 v3 to v4 change:
Use the compatible name to specify the global register address offset.
If the compatible name is "snps,dwc3-rtk-soc", then the offset use 0x8100.
Otherwise, the offset is default value 0xc100.

 v2 to v3 change:
1.  Fix the dtschema validation error.

 v1 to v2 change:
1. Change the name of the property "snps,global-regs-starting-offset".
2. Adjust the format of comment.
3. Add initial value of the global_regs_starting_offset
4. Remove the log of dev_info.
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Krzysztof Kozlowski May 2, 2023, 7:40 a.m. UTC | #1
On 02/05/2023 07:04, Stanley Chang wrote:
> Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek dwc3
> core to adjust the global register start address
> 
> The RTK DHC SoCs were designed, the global register address offset at

What are: "RTK" and "DHC"? These are manufactured by Synopsys as you
suggest in the patch?


> 0x8100. The default address offset is constant at DWC3_GLOBALS_REGS_START
> (0xc100). Therefore, add the compatible name of device-tree to specify
> the SoC custom's global register start address.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>

Based on your email, rtk could mean Realtek, so the compatible is
clearly wrong.

> ---
>  v3 to v4 change:
> Use the compatible name to specify the global register address offset.
> If the compatible name is "snps,dwc3-rtk-soc", then the offset use 0x8100.
> Otherwise, the offset is default value 0xc100.
> 

Best regards,
Krzysztof
  
Stanley Chang[昌育德] May 2, 2023, 8:05 a.m. UTC | #2
Hi Krzysztof,

> On 02/05/2023 07:04, Stanley Chang wrote:
> > Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek dwc3
> > core to adjust the global register start address
> >
> > The RTK DHC SoCs were designed, the global register address offset at
> 
> What are: "RTK" and "DHC"? These are manufactured by Synopsys as you
> suggest in the patch?

RTK is Realtek.
DHC is the department name in Realtek and the abbreviation of the Digital Home Center.
The USB controller of RTK DHC SoCs used the DWC3 IP of Synopsys.

> > 0x8100. The default address offset is constant at
> > DWC3_GLOBALS_REGS_START (0xc100). Therefore, add the compatible
> name
> > of device-tree to specify the SoC custom's global register start address.
> >
> > Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> 
> Based on your email, rtk could mean Realtek, so the compatible is clearly
> wrong.

The compatible name "snps,dwc3-rtk-soc" wants to represent the dwc3 driver, which requires a different offset for Realtek SoCs

> > ---
> >  v3 to v4 change:
> > Use the compatible name to specify the global register address offset.
> > If the compatible name is "snps,dwc3-rtk-soc", then the offset use 0x8100.
> > Otherwise, the offset is default value 0xc100.
> >
> 
> Best regards,
> Krzysztof
> 
> 
> ------Please consider the environment before printing this e-mail.
  
Krzysztof Kozlowski May 2, 2023, 8:44 a.m. UTC | #3
On 02/05/2023 10:05, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
> 
>> On 02/05/2023 07:04, Stanley Chang wrote:
>>> Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek dwc3
>>> core to adjust the global register start address
>>>
>>> The RTK DHC SoCs were designed, the global register address offset at
>>
>> What are: "RTK" and "DHC"? These are manufactured by Synopsys as you
>> suggest in the patch?
> 
> RTK is Realtek.
> DHC is the department name in Realtek and the abbreviation of the Digital Home Center.
> The USB controller of RTK DHC SoCs used the DWC3 IP of Synopsys.

Then entire compatible is not correct. Vendor is Realtek not Synopsys.
DHC is not even device name. Use real device names.

> 
>>> 0x8100. The default address offset is constant at
>>> DWC3_GLOBALS_REGS_START (0xc100). Therefore, add the compatible
>> name
>>> of device-tree to specify the SoC custom's global register start address.
>>>
>>> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
>>
>> Based on your email, rtk could mean Realtek, so the compatible is clearly
>> wrong.
> 
> The compatible name "snps,dwc3-rtk-soc" wants to represent the dwc3 driver, which requires a different offset for Realtek SoCs

No. The compatible represents hardware, not driver. Use compatible
matching real hardware.

Best regards,
Krzysztof
  
Stanley Chang[昌育德] May 2, 2023, 8:56 a.m. UTC | #4
Hi Krzysztof,

> >> On 02/05/2023 07:04, Stanley Chang wrote:
> >>> Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek dwc3
> >>> core to adjust the global register start address
> >>>
> >>> The RTK DHC SoCs were designed, the global register address offset
> >>> at
> >>
> >> What are: "RTK" and "DHC"? These are manufactured by Synopsys as you
> >> suggest in the patch?
> >
> > RTK is Realtek.
> > DHC is the department name in Realtek and the abbreviation of the Digital
> Home Center.
> > The USB controller of RTK DHC SoCs used the DWC3 IP of Synopsys.
> 
> Then entire compatible is not correct. Vendor is Realtek not Synopsys.
> DHC is not even device name. Use real device names.

So, can we use the compatible name as 'realtek,dwc3' ?
For example,
@@ -2224,10 +2230,16 @@ static const struct dev_pm_ops dwc3_dev_pm_ops = {
 #ifdef CONFIG_OF
 static const struct of_device_id of_dwc3_match[] = {
        {
-               .compatible = "snps,dwc3"
+               .compatible = "snps,dwc3",
+               .data = (void *)DWC3_GLOBALS_REGS_START,
+       },
+       {
+               .compatible = "realtek,dwc3",
+               .data = (void *)DWC3_GLOBALS_REGS_START_FOR_RTK,
        },
        {
-               .compatible = "synopsys,dwc3"
+               .compatible = "synopsys,dwc3",
+               .data = (void *)DWC3_GLOBALS_REGS_START,
        },
        { },
 };

> >
> >>> 0x8100. The default address offset is constant at
> >>> DWC3_GLOBALS_REGS_START (0xc100). Therefore, add the compatible
> >> name
> >>> of device-tree to specify the SoC custom's global register start address.
> >>>
> >>> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> >>
> >> Based on your email, rtk could mean Realtek, so the compatible is
> >> clearly wrong.
> >
> > The compatible name "snps,dwc3-rtk-soc" wants to represent the dwc3
> > driver, which requires a different offset for Realtek SoCs
> 
> No. The compatible represents hardware, not driver. Use compatible matching
> real hardware.
> 
> Best regards,
> Krzysztof
> 
> 
> ------Please consider the environment before printing this e-mail.
  
Krzysztof Kozlowski May 2, 2023, 10:15 a.m. UTC | #5
On 02/05/2023 10:56, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
> 
>>>> On 02/05/2023 07:04, Stanley Chang wrote:
>>>>> Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek dwc3
>>>>> core to adjust the global register start address
>>>>>
>>>>> The RTK DHC SoCs were designed, the global register address offset
>>>>> at
>>>>
>>>> What are: "RTK" and "DHC"? These are manufactured by Synopsys as you
>>>> suggest in the patch?
>>>
>>> RTK is Realtek.
>>> DHC is the department name in Realtek and the abbreviation of the Digital
>> Home Center.
>>> The USB controller of RTK DHC SoCs used the DWC3 IP of Synopsys.
>>
>> Then entire compatible is not correct. Vendor is Realtek not Synopsys.
>> DHC is not even device name. Use real device names.
> 
> So, can we use the compatible name as 'realtek,dwc3' ?

dwc3 is not a real device name for Realtek.

Best regards,
Krzysztof
  
Stanley Chang[昌育德] May 2, 2023, 10:37 a.m. UTC | #6
Hi Krzysztof,

> >>>> On 02/05/2023 07:04, Stanley Chang wrote:
> >>>>> Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek
> >>>>> dwc3 core to adjust the global register start address
> >>>>>
> >>>>> The RTK DHC SoCs were designed, the global register address offset
> >>>>> at
> >>>>
> >>>> What are: "RTK" and "DHC"? These are manufactured by Synopsys as
> >>>> you suggest in the patch?
> >>>
> >>> RTK is Realtek.
> >>> DHC is the department name in Realtek and the abbreviation of the
> >>> Digital
> >> Home Center.
> >>> The USB controller of RTK DHC SoCs used the DWC3 IP of Synopsys.
> >>
> >> Then entire compatible is not correct. Vendor is Realtek not Synopsys.
> >> DHC is not even device name. Use real device names.
> >
> > So, can we use the compatible name as 'realtek,dwc3' ?
> 
> dwc3 is not a real device name for Realtek.

We still use dwc3 IP in Realtek's SoC. Why is the name "dwc3" inappropriate?

Should compatibility names use the SoC name?
For example, our SoC name
RTD129x, RTD139x, RTD161x, RTD161xB, etc.
Should we use these names in compatible names?
"realtek, rtd129x", "realtek, rtd139x", "realtek, rtd161x"...etc.

Thanks,
Stanley
  
Krzysztof Kozlowski May 2, 2023, 7:27 p.m. UTC | #7
On 02/05/2023 12:37, Stanley Chang[昌育德] wrote:
> Hi Krzysztof,
> 
>>>>>> On 02/05/2023 07:04, Stanley Chang wrote:
>>>>>>> Add a new compatible name 'snps,dwc3-rtk-soc' of DT for realtek
>>>>>>> dwc3 core to adjust the global register start address
>>>>>>>
>>>>>>> The RTK DHC SoCs were designed, the global register address offset
>>>>>>> at
>>>>>>
>>>>>> What are: "RTK" and "DHC"? These are manufactured by Synopsys as
>>>>>> you suggest in the patch?
>>>>>
>>>>> RTK is Realtek.
>>>>> DHC is the department name in Realtek and the abbreviation of the
>>>>> Digital
>>>> Home Center.
>>>>> The USB controller of RTK DHC SoCs used the DWC3 IP of Synopsys.
>>>>
>>>> Then entire compatible is not correct. Vendor is Realtek not Synopsys.
>>>> DHC is not even device name. Use real device names.
>>>
>>> So, can we use the compatible name as 'realtek,dwc3' ?
>>
>> dwc3 is not a real device name for Realtek.
> 
> We still use dwc3 IP in Realtek's SoC. Why is the name "dwc3" inappropriate?

dwc3 is the name of design coming from Synopsys. Your device is probably
called differently. Why it is inappropriate? Because your device is not
called DWC3, even though you use IP from Synopsys.

Although vendor,dwc3 is already used as compatible in several cases, I
don't think it is a good pattern.

> 
> Should compatibility names use the SoC name?
> For example, our SoC name
> RTD129x, RTD139x, RTD161x, RTD161xB, etc.
> Should we use these names in compatible names?
> "realtek, rtd129x", "realtek, rtd139x", "realtek, rtd161x"...etc.

Regular rules apply, because your device is not special.
https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42

Therefore either SoC-based device specific name or followed by:
1. SoC-based device specific fallback,
2. Family-device generic fallback,


Best regards,
Krzysztof
  
Stanley Chang[昌育德] May 3, 2023, 3:14 a.m. UTC | #8
Hi Krzysztof,

> Regular rules apply, because your device is not special.
> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bin
> dings/writing-bindings.rst#L42
> 
> Therefore either SoC-based device specific name or followed by:
> 1. SoC-based device specific fallback,
> 2. Family-device generic fallback,
> 

Thanks for your suggestion. I will follow these rules.
Thinh has a good suggestion for this problem.
His solution is simply. And no modify the compatible name and property of dwc3.

Thanks,
Stanley
  

Patch

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 50edc4da780e..40d9a461ed93 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -33,6 +33,7 @@  properties:
     contains:
       oneOf:
         - const: snps,dwc3
+        - const: snps,dwc3-rtk-soc
         - const: synopsys,dwc3
           deprecated: true