> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Monday, July 17, 2023 5:42 PM
> To: Li, Meng <Meng.Li@xxxxxxxxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx;
> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> dinguyen@xxxxxxxxxx; hminas@xxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] usb: dwc2: combine platform specific data for Intel Agilex
> and Stratix10
>
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe.
>
> On 17/07/2023 10:50, Meng Li wrote:
> > Intel Stratix10 is very the same with Agilex platform, the DWC2 IP on
> > the Stratix platform also does not support clock-gating. So, based on
> > commit 3d8d3504d233("usb: dwc2: Add platform specific data for Intel's
> > Agilex"), combine platform specific data for Intel Agilex and
> > Stratix10 together. In additional, in order to avoid breaking the old
> > device tree, keep compatible string "intel,socfpga-agilex-hsotg" unchanged.
> >
> > Signed-off-by: Meng Li <Meng.Li@xxxxxxxxxxxxx>
> > ---
> > Documentation/devicetree/bindings/usb/dwc2.yaml | 2 ++
>
> Bindings are always separate patch.
>
Got it.
> > arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 4 ++--
>
> As DTS is.
>
Got it.
> > drivers/usb/dwc2/params.c | 6 ++++--
> > 3 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml
> > b/Documentation/devicetree/bindings/usb/dwc2.yaml
> > index dc4988c0009c..c98ca98d5033 100644
> > --- a/Documentation/devicetree/bindings/usb/dwc2.yaml
> > +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
> > @@ -51,6 +51,7 @@ properties:
> > - amlogic,meson-gxbb-usb
> > - amlogic,meson-g12a-usb
> > - intel,socfpga-agilex-hsotg
> > + - intel,socfpga-hsotg
>
> Where is SoC specific compatible?
>
The socfpga is a SoC family, it includes Agilex ad Stratix10 SoCs.
In fact, we only need the compatible " intel,socfpga-hsotg " is enough.
But in order to avoid breaking the old device tree for agilex platform, I reserve the old compatible.
So, I think we don't need the Stratix10 compatible like "intel,socfpga-stratix10-hsotg "
> > - const: snps,dwc2
> > - const: amcc,dwc-otg
> > - const: apm,apm82181-dwc-otg
> > @@ -64,6 +65,7 @@ properties:
> > - const: snps,dwc2
> > - const: samsung,s3c6400-hsotg
> > - const: intel,socfpga-agilex-hsotg
> > + - const: intel,socfpga-hsotg
> >
> > reg:
> > maxItems: 1
> > diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> > b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> > index ea788a920eab..c5a51636f657 100644
> > --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> > +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>
> ...
>
> > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> > index 8eab5f38b110..6bb27a24e9e1 100644
> > --- a/drivers/usb/dwc2/params.c
> > +++ b/drivers/usb/dwc2/params.c
> > @@ -93,7 +93,7 @@ static void dwc2_set_s3c6400_params(struct
> dwc2_hsotg *hsotg)
> > p->phy_utmi_width = 8;
> > }
> >
> > -static void dwc2_set_socfpga_agilex_params(struct dwc2_hsotg *hsotg)
> > +static void dwc2_set_socfpga_params(struct dwc2_hsotg *hsotg)
>
> Why? Old name was ok...
>
Old name includes string "agilex" that represents only agilex SoC.
This patch is used to combine platform specific data for Intel Agilex and Stratix10, so create a common function name for socfpga family.
> > {
> > struct dwc2_core_params *p = &hsotg->params;
> >
> > @@ -266,7 +266,9 @@ const struct of_device_id dwc2_of_match_table[] = {
> > { .compatible = "st,stm32mp15-hsotg",
> > .data = dwc2_set_stm32mp15_hsotg_params },
> > { .compatible = "intel,socfpga-agilex-hsotg",
> > - .data = dwc2_set_socfpga_agilex_params },
> > + .data = dwc2_set_socfpga_params },
> > + { .compatible = "intel,socfpga-hsotg",
> > + .data = dwc2_set_socfpga_params },
>
> Aren't they compatible? Why do you need new entry for compatible devices?
>
In fact, the usb IP in Agilex and Sratix10 are the same. But it is not reasonable to use agilex compatible string "intel,socfpga-agilex-hsotg" in Stratix10 dts files.
So, I create a common function name and compatible string for socfpga family that includes Agilex and Stratix10 SoCs.
Thanks,
Limeng
> Best regards,
> Krzysztof
