RE: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Monday, 24 July, 2023 4:55 PM
> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@xxxxxxxxx>;
> gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux-
> usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> Thinh.Nguyen@xxxxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB
> controller
> 
> On 24/07/2023 09:53, Ng, Adrian Ho Yin wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> >> Sent: Monday, 24 July, 2023 3:28 PM
> >> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@xxxxxxxxx>;
> >> gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> >> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux-
> >> usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> >> Thinh.Nguyen@xxxxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB
> >> controller
> >>
> >> On 24/07/2023 09:18, Ng, Adrian Ho Yin wrote:
> >>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> >>>> Sent: Monday, 24 July, 2023 3:05 PM
> >>>> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@xxxxxxxxx>;
> >>>> gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> >>>> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux-
> >>>> usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> >>>> Thinh.Nguyen@xxxxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> >>>> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB
> >>>> controller
> >>>>
> >>>> On 24/07/2023 08:36, adrian.ho.yin.ng@xxxxxxxxx wrote:
> >>>>> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@xxxxxxxxx>
> >>>>>
> >>>>> Existing binding intel,keembay-dwc3.yaml does not have the
> >>>>> required properties for Intel SoCFPGA devices.
> >>>>> Introduce new binding description for Intel SoCFPGA USB controller
> >>>>> which will be used for current and future SoCFPGA devices.
> >>>>>
> >>>>> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@xxxxxxxxx>
> >>>>> ---
> >>>>>  .../bindings/usb/intel,socfpga-dwc3.yaml      | 84
> +++++++++++++++++++
> >>>>>  1 file changed, 84 insertions(+)
> >>>>>  create mode 100644
> >>>>> Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> >>>>>
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> >>>>> b/Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..e36b087c2651
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/usb/intel,socfpga-
> dwc3.yam
> >>>>> +++ l
> >>>>> @@ -0,0 +1,84 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> >>>>> +1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/usb/intel,socfpga-dwc3.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: Intel SoCFPGA DWC3 USB controller
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Adrian Ng Ho Yin <adrian.ho.yin.ng@xxxxxxxxx>
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    items:
> >>>>> +      - enum:
> >>>>> +          - intel,agilex5-dwc3
> >>>>> +      - const: intel,socfpga-dwc3
> >>>>
> >>>> So you did not even wait for my answer? What happened here with
> >>>> this compatible? I asked you to change file name, not add intel,socfpga-
> dwc3.
> >>>> Again - why using different style for Agilex? Which style is correct?
> >>>>
> >>>
> >>> The intention is to use a common binding for Intel SoCFPGA products
> >>> that is
> >> using DWC3 controller.
> >>> This is done with reference to qcom,dwc3.yaml.
> >>
> >> Nope, your driver change does not match it at all. Your explanation
> >> does not make any sense.
> >>
> >> Don't answer only half of my questions. So third time - the last:
> >> since you add new style for Agilex, which style of Agilex compatibles is
> correct?
> 
> I still did not receive here answer. Which style, naming convention for agilex
> is correct for your platform?
> 
> Why this:
> git grep agilex | grep intel,
> 
> gives different compatibles than you start here? I assume Intel/Altera knows
> better the platform so will provide here some guidance. If unsure, please
> consult your colleagues.
> 
> 

Noted. Will consult Intel Linux group to get the correct naming convention and 
go through another round of internal review prior to address the issues/concerns 
raised before submitting the revised patches.

> >>
> >
> > My apologies.
> > In your opinion which is the proper practice?
> > 1. Create new binding for new products that is using the same controller.
> 
> What is "new binding"? What do you mean by that? New file, then not.
> 

Yes. New Binding means new file. Will not proceed with option 1.

> > 2. Create a common binding that will be used by products using the same
> controller?
> > Referring to the current bindings that are available the two options are
> being practiced at the moment.
> >
> > If option 1 is the proper practice the correct Agilex compatible is
> intel,agilex5-dwc3.
> > To rework the binding to cater for agilex5-dwc3 only. The compatible in
> glue driver will remain the same.
> >
> > If option 2 is the proper practice then the correct Agilex compatible is
> intel,socfpga-dwc3.
> > To update compatible in glue driver in V3.
> >
> 
> 
> Recommended practice is to use specific compatible for both: your device
> and as fallback for any future devices. In certain cases, option 2 is okay.
> 
> 

Noted. Thank you for the feedback. 

Thank You
Adrian  Ng




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux