[PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[Stanley Chang]

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.
---
 drivers/usb/dwc3/core.c | 18 +++++++++++++++---
 drivers/usb/dwc3/core.h |  5 +++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0beaab932e7d..4f69b26d7dab 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/acpi.h>
 #include <linux/pinctrl/consumer.h>
@@ -1793,12 +1794,17 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc->xhci_resources[0].flags = res->flags;
 	dwc->xhci_resources[0].name = res->name;
 
+	dwc->global_regs_starting_offset = (u32)(uintptr_t)
+		    of_device_get_match_data(dev);
+	if (!dwc->global_regs_starting_offset)
+		dwc->global_regs_starting_offset = DWC3_GLOBALS_REGS_START;
+
 	/*
 	 * Request memory region but exclude xHCI regs,
 	 * since it will be requested by the xhci-plat driver.
 	 */
 	dwc_res = *res;
-	dwc_res.start += DWC3_GLOBALS_REGS_START;
+	dwc_res.start += dwc->global_regs_starting_offset;
 
 	regs = devm_ioremap_resource(dev, &dwc_res);
 	if (IS_ERR(regs))
@@ -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 = "snps,dwc3-rtk-soc",
+		.data = (void *)DWC3_GLOBALS_REGS_START_FOR_RTK,
 	},
 	{
-		.compatible = "synopsys,dwc3"
+		.compatible = "synopsys,dwc3",
+		.data = (void *)DWC3_GLOBALS_REGS_START,
 	},
 	{ },
 };
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index d56457c02996..46557cf52f4b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -84,6 +84,8 @@
 #define DWC3_OTG_REGS_START		0xcc00
 #define DWC3_OTG_REGS_END		0xccff
 
+#define DWC3_GLOBALS_REGS_START_FOR_RTK 0x8100
+
 /* Global Registers */
 #define DWC3_GSBUSCFG0		0xc100
 #define DWC3_GSBUSCFG1		0xc104
@@ -1118,6 +1120,8 @@ struct dwc3_scratchpad_array {
  * @wakeup_configured: set if the device is configured for remote wakeup.
  * @imod_interval: set the interrupt moderation interval in 250ns
  *			increments or 0 to disable.
+ * @global_regs_starting_offset: set the dwc3 global register start address
+ *	 and it is default at DWC3_GLOBALS_REGS_START (0xc100).
  * @max_cfg_eps: current max number of IN eps used across all USB configs.
  * @last_fifo_depth: last fifo depth used to determine next fifo ram start
  *		     address.
@@ -1334,6 +1338,7 @@ struct dwc3 {
 	unsigned		wakeup_configured:1;
 
 	u16			imod_interval;
+	u32			global_regs_starting_offset;
 
 	int			max_cfg_eps;
 	int			last_fifo_depth;
-- 
2.34.1

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

[Stanley Chang]

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(+)

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
 
-- 
2.34.1

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

[Krzysztof Kozlowski]

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

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

[Stanley Chang[昌育德]]

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.

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

[Krzysztof Kozlowski]

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

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

[Stanley Chang[昌育德]]

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.

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

[Krzysztof Kozlowski]

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

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

[Stanley Chang[昌育德]]

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

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

[Krzysztof Kozlowski]

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

Re: [PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[Thinh Nguyen]

On Tue, May 02, 2023, Stanley Chang wrote:
> 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.
> ---
>  drivers/usb/dwc3/core.c | 18 +++++++++++++++---
>  drivers/usb/dwc3/core.h |  5 +++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 0beaab932e7d..4f69b26d7dab 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/acpi.h>
>  #include <linux/pinctrl/consumer.h>
> @@ -1793,12 +1794,17 @@ static int dwc3_probe(struct platform_device *pdev)
>  	dwc->xhci_resources[0].flags = res->flags;
>  	dwc->xhci_resources[0].name = res->name;
>  
> +	dwc->global_regs_starting_offset = (u32)(uintptr_t)
> +		    of_device_get_match_data(dev);
> +	if (!dwc->global_regs_starting_offset)
> +		dwc->global_regs_starting_offset = DWC3_GLOBALS_REGS_START;
> +
>  	/*
>  	 * Request memory region but exclude xHCI regs,
>  	 * since it will be requested by the xhci-plat driver.
>  	 */
>  	dwc_res = *res;
> -	dwc_res.start += DWC3_GLOBALS_REGS_START;
> +	dwc_res.start += dwc->global_regs_starting_offset;

I think you're overcomplicating things here.

Can we just match using compatible string as mentioned before? I believe
I suggested to use that before but I think you had issue we getting it
because it's from the parent device?

Did you try this?

	dwc_res.start = DWC3_RTK_ABC_GLOBAL_OFFSET;

	if (dev->of_node) {
		struct device_node *parent = of_get_parent(dev->of_node);

		if (of_device_is_compatible(parent, "your-compatible"))
			dwc_res.start = DWC3_RTK_ABC_GLOBAL_OFFSET;

		of_node_put(parent);
	}

>  
>  	regs = devm_ioremap_resource(dev, &dwc_res);
>  	if (IS_ERR(regs))
> @@ -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 = "snps,dwc3-rtk-soc",
> +		.data = (void *)DWC3_GLOBALS_REGS_START_FOR_RTK,

Don't do this.

>  	},
>  	{
> -		.compatible = "synopsys,dwc3"
> +		.compatible = "synopsys,dwc3",
> +		.data = (void *)DWC3_GLOBALS_REGS_START,
>  	},
>  	{ },
>  };
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index d56457c02996..46557cf52f4b 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -84,6 +84,8 @@
>  #define DWC3_OTG_REGS_START		0xcc00
>  #define DWC3_OTG_REGS_END		0xccff
>  
> +#define DWC3_GLOBALS_REGS_START_FOR_RTK 0x8100
> +
>  /* Global Registers */
>  #define DWC3_GSBUSCFG0		0xc100
>  #define DWC3_GSBUSCFG1		0xc104
> @@ -1118,6 +1120,8 @@ struct dwc3_scratchpad_array {
>   * @wakeup_configured: set if the device is configured for remote wakeup.
>   * @imod_interval: set the interrupt moderation interval in 250ns
>   *			increments or 0 to disable.
> + * @global_regs_starting_offset: set the dwc3 global register start address
> + *	 and it is default at DWC3_GLOBALS_REGS_START (0xc100).
>   * @max_cfg_eps: current max number of IN eps used across all USB configs.
>   * @last_fifo_depth: last fifo depth used to determine next fifo ram start
>   *		     address.
> @@ -1334,6 +1338,7 @@ struct dwc3 {
>  	unsigned		wakeup_configured:1;
>  
>  	u16			imod_interval;
> +	u32			global_regs_starting_offset;
>  
>  	int			max_cfg_eps;
>  	int			last_fifo_depth;
> -- 
> 2.34.1
> 

Please note that this is very unique to Realtek, I'm not aware of any
other vendor that reconfigured the register offset that our IP
specified. So, it makes more sense to match using compatible string than
creating a separate property that you may be the only user that needs
it.

Thanks,
Thinh

Re: [PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[Thinh Nguyen]

Minor correction.

On Tue, May 02, 2023, Thinh Nguyen wrote:
> 
> Did you try this?
> 
> 	dwc_res.start = DWC3_RTK_ABC_GLOBAL_OFFSET;

Ignore the line above due to copy-paste error.

> 
> 	if (dev->of_node) {
> 		struct device_node *parent = of_get_parent(dev->of_node);
> 
> 		if (of_device_is_compatible(parent, "your-compatible"))
> 			dwc_res.start = DWC3_RTK_ABC_GLOBAL_OFFSET;
> 
> 		of_node_put(parent);
> 	}
> 

RE: [PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[Stanley Chang[昌育德]]

Hi Thinh,

> I think you're overcomplicating things here.
> 
> Can we just match using compatible string as mentioned before? I believe I
> suggested to use that before but I think you had issue we getting it because it's
> from the parent device?
> 
> Did you try this?
> 
>         dwc_res.start = DWC3_RTK_ABC_GLOBAL_OFFSET;
> 
>         if (dev->of_node) {
>                 struct device_node *parent =
> of_get_parent(dev->of_node);
> 
>                 if (of_device_is_compatible(parent, "your-compatible"))
>                         dwc_res.start =
> DWC3_RTK_ABC_GLOBAL_OFFSET;
> 
>                 of_node_put(parent);
>         }

This is a good idea. Thanks for your suggestion.
This patch works fine and it is simply. 
For the compatible name, I use that "realtek,rtd1xxx-dwc3". 
rtd1xxx is the name of SoCs, for rtd129x, rtd139x, rtd16xx, ... etc.
Do you have any concern?

New patch as follows
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 0beaab932e7d..cd4b69541776 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1800,6 +1800,17 @@ static int dwc3_probe(struct platform_device *pdev)
        dwc_res = *res;
        dwc_res.start += DWC3_GLOBALS_REGS_START;

+       if (dev->of_node) {
+               struct device_node *parent = of_get_parent(dev->of_node);
+
+               if (of_device_is_compatible(parent, "realtek,rtd1xxx-dwc3")) {
+                       dwc_res.start -= DWC3_GLOBALS_REGS_START;
+                       dwc_res.start += RTK_RTD1XXX_DWC3_GLOBALS_REGS_START;
+               }
+
+               of_node_put(parent);
+       }
+
        regs = devm_ioremap_resource(dev, &dwc_res);
        if (IS_ERR(regs))
                return PTR_ERR(regs);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index d56457c02996..db48aae211be 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -84,6 +84,8 @@
 #define DWC3_OTG_REGS_START            0xcc00
 #define DWC3_OTG_REGS_END              0xccff

+#define RTK_RTD1XXX_DWC3_GLOBALS_REGS_START    0x8100
+
 /* Global Registers */
 #define DWC3_GSBUSCFG0         0xc100
 #define DWC3_GSBUSCFG1         0xc104

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

[Stanley Chang[昌育德]]

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

Re: [PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[Thinh Nguyen]

On Wed, May 03, 2023, Stanley Chang[昌育德] wrote:
> Hi Thinh,
> 
> > I think you're overcomplicating things here.
> > 
> > Can we just match using compatible string as mentioned before? I believe I
> > suggested to use that before but I think you had issue we getting it because it's
> > from the parent device?
> > 
> > Did you try this?
> > 
> >         dwc_res.start = DWC3_RTK_ABC_GLOBAL_OFFSET;
> > 
> >         if (dev->of_node) {
> >                 struct device_node *parent =
> > of_get_parent(dev->of_node);
> > 
> >                 if (of_device_is_compatible(parent, "your-compatible"))
> >                         dwc_res.start =
> > DWC3_RTK_ABC_GLOBAL_OFFSET;
> > 
> >                 of_node_put(parent);
> >         }
> 
> This is a good idea. Thanks for your suggestion.
> This patch works fine and it is simply. 
> For the compatible name, I use that "realtek,rtd1xxx-dwc3". 
> rtd1xxx is the name of SoCs, for rtd129x, rtd139x, rtd16xx, ... etc.
> Do you have any concern?

I think it's fine.

> 
> New patch as follows
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 0beaab932e7d..cd4b69541776 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1800,6 +1800,17 @@ static int dwc3_probe(struct platform_device *pdev)
>         dwc_res = *res;
>         dwc_res.start += DWC3_GLOBALS_REGS_START;
> 
> +       if (dev->of_node) {
> +               struct device_node *parent = of_get_parent(dev->of_node);
> +
> +               if (of_device_is_compatible(parent, "realtek,rtd1xxx-dwc3")) {
> +                       dwc_res.start -= DWC3_GLOBALS_REGS_START;
> +                       dwc_res.start += RTK_RTD1XXX_DWC3_GLOBALS_REGS_START;
> +               }
> +
> +               of_node_put(parent);
> +       }
> +
>         regs = devm_ioremap_resource(dev, &dwc_res);
>         if (IS_ERR(regs))
>                 return PTR_ERR(regs);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index d56457c02996..db48aae211be 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -84,6 +84,8 @@
>  #define DWC3_OTG_REGS_START            0xcc00
>  #define DWC3_OTG_REGS_END              0xccff
> 
> +#define RTK_RTD1XXX_DWC3_GLOBALS_REGS_START    0x8100

Let's keep consistent with the DWC3_ prefix. Something like this:

#define DWC3_RTK_RTD1XXX_GLOBAL_REGS_START 0x8100

> +
>  /* Global Registers */
>  #define DWC3_GSBUSCFG0         0xc100
>  #define DWC3_GSBUSCFG1         0xc104
> 

Thanks,
Thinh

RE: [PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's global register start address

[Stanley Chang[昌育德]]

Hi Thinh,

> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> > d56457c02996..db48aae211be 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -84,6 +84,8 @@
> >  #define DWC3_OTG_REGS_START            0xcc00
> >  #define DWC3_OTG_REGS_END              0xccff
> >
> > +#define RTK_RTD1XXX_DWC3_GLOBALS_REGS_START    0x8100
> 
> Let's keep consistent with the DWC3_ prefix. Something like this:
> 
> #define DWC3_RTK_RTD1XXX_GLOBAL_REGS_START 0x8100

Okay. I will revise it.


Thanks,
Stanley