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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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.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?

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.


>>
> 
> 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.

> 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.


Best regards,
Krzysztof