mbox series

[v2,0/2] Add support for Intel SocFPGA DWC3 USB controller

Message ID cover.1690179693.git.adrian.ho.yin.ng@intel.com
Headers show
Series Add support for Intel SocFPGA DWC3 USB controller | expand

Message

Ng, Adrian Ho Yin July 24, 2023, 6:36 a.m. UTC 
From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>

This patch set adds binding for Intel SoCFPGA DWC3 controller.
The new binding differs from the existing binding intel,keembay-dwc3.yaml
as the required properties are different.
This patch also adds compatible string for Intel Agilex5 in DWC3 simple glue driver.

---

V1-> V2:
- update commit title.
- update compatible string to match file name.
- update item description for clocks, reg and resets.
- removed defines for macro and update to use numbers.
- update node name to be generic.

Adrian Ng Ho Yin (2):
  dt-bindings: usb: Add Intel SoCFPGA USB controller
  usb: dwc3: of-simple: Add compatible string for Intel Agilex5 platform

 .../bindings/usb/intel,socfpga-dwc3.yaml      | 84 +++++++++++++++++++
 drivers/usb/dwc3/dwc3-of-simple.c             |  1 +
 2 files changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml

Comments

Krzysztof Kozlowski July 24, 2023, 7:27 a.m. UTC | #1 
On 24/07/2023 09:18, Ng, Adrian Ho Yin wrote:
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Monday, 24 July, 2023 3:05 PM
>> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>;
>> gregkh@linuxfoundation.org; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-
>> usb@vger.kernel.org; devicetree@vger.kernel.org;
>> Thinh.Nguyen@synopsys.com; p.zabel@pengutronix.de
>> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB controller
>>
>> On 24/07/2023 08:36, adrian.ho.yin.ng@intel.com wrote:
>>> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
>>>
>>> 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@intel.com>
>>> ---
>>>  .../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.yaml
>>> @@ -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@intel.com>
>>> +
>>> +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?


Best regards,
Krzysztof
Greg Kroah-Hartman July 25, 2023, 1:25 p.m. UTC | #2 
On Mon, Jul 24, 2023 at 02:36:58PM +0800, adrian.ho.yin.ng@intel.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> 
> 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@intel.com>
> ---
>  .../bindings/usb/intel,socfpga-dwc3.yaml      | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml

You don't seem to have followed the rules that your employer has for
kernel contributions, so we can't take this, sorry.  Please work with
the Intel Linux kernel group to resolve those issues first, and then
resubmit the patches properly.

thanks,

greg k-h
Ng, Adrian Ho Yin July 27, 2023, 2:09 a.m. UTC | #3 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Monday, 24 July, 2023 4:55 PM
> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>;
> gregkh@linuxfoundation.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-
> usb@vger.kernel.org; devicetree@vger.kernel.org;
> Thinh.Nguyen@synopsys.com; p.zabel@pengutronix.de
> 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@linaro.org>
> >> Sent: Monday, 24 July, 2023 3:28 PM
> >> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>;
> >> gregkh@linuxfoundation.org; robh+dt@kernel.org;
> >> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-
> >> usb@vger.kernel.org; devicetree@vger.kernel.org;
> >> Thinh.Nguyen@synopsys.com; p.zabel@pengutronix.de
> >> 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@linaro.org>
> >>>> Sent: Monday, 24 July, 2023 3:05 PM
> >>>> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>;
> >>>> gregkh@linuxfoundation.org; robh+dt@kernel.org;
> >>>> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-
> >>>> usb@vger.kernel.org; devicetree@vger.kernel.org;
> >>>> Thinh.Nguyen@synopsys.com; p.zabel@pengutronix.de
> >>>> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB
> >>>> controller
> >>>>
> >>>> On 24/07/2023 08:36, adrian.ho.yin.ng@intel.com wrote:
> >>>>> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> >>>>>
> >>>>> 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@intel.com>
> >>>>> ---
> >>>>>  .../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@intel.com>
> >>>>> +
> >>>>> +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
Ng, Adrian Ho Yin July 27, 2023, 2:11 a.m. UTC | #4 
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, 25 July, 2023 9:26 PM
> To: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; linux-usb@vger.kernel.org;
> devicetree@vger.kernel.org; Thinh.Nguyen@synopsys.com;
> p.zabel@pengutronix.de
> Subject: Re: [PATCH v2 1/2] dt-bindings: usb: Add Intel SoCFPGA USB
> controller
> 
> On Mon, Jul 24, 2023 at 02:36:58PM +0800, adrian.ho.yin.ng@intel.com wrote:
> > From: Adrian Ng Ho Yin <adrian.ho.yin.ng@intel.com>
> >
> > 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@intel.com>
> > ---
> >  .../bindings/usb/intel,socfpga-dwc3.yaml      | 84 +++++++++++++++++++
> >  1 file changed, 84 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/usb/intel,socfpga-dwc3.yaml
> 
> You don't seem to have followed the rules that your employer has for kernel
> contributions, so we can't take this, sorry.  Please work with the Intel Linux
> kernel group to resolve those issues first, and then resubmit the patches
> properly.
> 

Noted. Will work with the intel Linux group to resolve the issues/concerns prior to resubmitting the patches.
Thank You for the feedback and sorry for the inconveniences. 

Thank You.
Adrian Ng