> On 13/04/2023 04:53, Stanley Chang[昌育德] wrote:
> >
> >>
> >> On Tue, Apr 11, 2023 at 10:30 PM Stanley Chang
> >> <stanley_chang@xxxxxxxxxxx> wrote:
> >>>
> >>> Add a new 'snps,global-regs-starting-offset' DT to dwc3 core to
> >>> remap the global register start address
> >>>
> >>> The RTK DHC SoCs were designed the global register address offset at
> >>> 0x8100. The default address is at DWC3_GLOBALS_REGS_START (0xc100).
> >>> Therefore, add the property of device-tree to adjust this start address.
> >>>
> >>> Signed-off-by: Stanley Chang <stanley_chang@xxxxxxxxxxx>
> >>> ---
> >>> Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++
> >>> 1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> index be36956af53b..5cbf3b7ded04 100644
> >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >>> @@ -359,6 +359,13 @@ properties:
> >>> items:
> >>> enum: [1, 4, 8, 16, 32, 64, 128, 256]
> >>>
> >>> + snps,global-regs-starting-offset:
> >>> + description:
> >>> + value for remapping global register start address. For some dwc3
> >>> + controller, the dwc3 global register start address is not at
> >>> + default DWC3_GLOBALS_REGS_START (0xc100). This property is
> >> added to
> >>> + adjust the address.
> >>
> >> We already have 'reg' or using a specific compatible to handle
> >> differences. Use one of those, not a custom property. Generally,
> >> properties should be used for things that vary per board, not fixed in a given
> SoC.
> >>
> >> Rob
> >>
> >
> > The default offset is fixed by macro DWC3_GLOBALS_REGS_START, and it is
> not specified by reg.
>
> Are you saying that reg points to XHCI registers and the gap between them and
> DWC3_GLOBALS_REGS_START is different on some implementations of this IP?
Yes.
> > The dwc3/core is a general driver for every dwc3 IP of SoCs, and
> > vendor's definition and compatible should specify on its parent.
>
> Not entirely. It's how currently it is written, but not necessarily correct
> representation. The parent is only glue part which for some non-IP resources.
I agree.
I think this offset belongs to the IP resource.
But it is fixed value on dwc3/core driver.
Therefore, I had to add this patch to adjust it.
> > If we add a specific compatible to dwc3/core driver, then it will break this
> rule.
>
> What rule? The rule is to have specific compatibles, so now you are breaking it.
>
I just don't want dwc3/core to look like a specific Realtek driver.
If I add compatible to our platform, then apply this offset.
For example,
@@ -2046,6 +2046,9 @@ static const struct of_device_id of_dwc3_match[] = {
{
.compatible = "synopsys,dwc3"
},
+ {
+ .compatible = "realtek,dwc3"
+ },
{ },
};
Why not use a property of the device tree to specify this offset?
It will be more general. Other vendor IPs can also use this option if desired.
For example,
@@ -123,7 +123,7 @@ port0_dwc3: dwc3_u2drd@98020000 {
compatible = "synopsys,dwc3", "syscon";
reg = <0x98020000 0x9000>;
interrupts = <0 95 4>;
+ snps,global-regs-starting-offset = <0x8100>;
usb-phy = <&dwc3_u2drd_usb2phy>;
dr_mode = "host";
snps,dis_u2_susphy_quirk;
