2023-05-02 05:11:37

by Stanley Chang[昌育德]

[permalink] [raw]
Subject: [PATCH v4 1/2] usb: dwc3: core: add support for RTK SoC custom's 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 <[email protected]>
---
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


2023-05-02 22:40:05

by Thinh Nguyen

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

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 <[email protected]>
> ---
> 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

2023-05-02 22:40:57

by Thinh Nguyen

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

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);
> }
>

2023-05-03 03:16:55

by Stanley Chang[昌育德]

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

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

2023-05-03 22:38:28

by Thinh Nguyen

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

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

2023-05-04 03:34:02

by Stanley Chang[昌育德]

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

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