Re: [PATCH v2 17/21] dt-bindings: serial: document support for SA8255p

[Krzysztof Kozlowski]

On 04/09/2024 14:54, Nikunj Kela wrote:
> 
> On 9/3/2024 11:36 PM, Krzysztof Kozlowski wrote:
>> On Tue, Sep 03, 2024 at 03:02:36PM -0700, Nikunj Kela wrote:
>>> Add compatibles representing UART support on SA8255p.
>>>
>>> Clocks and interconnects are being configured in the firmware VM
>>> on SA8255p platform, therefore making them optional.
>>>
>>> CC: Praveen Talari <quic_ptalari@xxxxxxxxxxx>
>>> Signed-off-by: Nikunj Kela <quic_nkela@xxxxxxxxxxx>
>>> ---
>>>  .../serial/qcom,serial-geni-qcom.yaml         | 53 ++++++++++++++++---
>>>  1 file changed, 47 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>>> index dd33794b3534..b63c984684f3 100644
>>> --- a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>>> @@ -10,14 +10,13 @@ maintainers:
>>>    - Andy Gross <agross@xxxxxxxxxx>
>>>    - Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>>>  
>>> -allOf:
>>> -  - $ref: /schemas/serial/serial.yaml#
>>> -
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - qcom,geni-uart
>>>        - qcom,geni-debug-uart
>>> +      - qcom,sa8255p-geni-uart
>>> +      - qcom,sa8255p-geni-debug-uart
>> Why devices are not compatible? What changed in programming model?
> 
> The cover-letter explains what is changed for devices in this platform.
> I will add the description in this patch too.

Many of us do not read cover letters. They don't really matter,
especially that serial tree will not include it. Each commit must stand
on its own.

> 
> 
>>
>>>  
>>>    clocks:
>>>      maxItems: 1
>>> @@ -51,18 +50,49 @@ properties:
>>>        - const: sleep
>>>  
>>>    power-domains:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  power-domain-names:
>> This does not match power-domains anymore.
> 
> Single power domain doesn't need to use power-domain-names binding as it
> is not needed however for multiple(in this case 2), you need to provide
> names. I will add this property to if block and only keep maxItems here.

The xxx and xxx-names properties always go in sync. Otherwise we do not
really know what is the power domain for other variants.

You are allowed to be unspecific about power domain (so maxItems: 1) if
it is obvious. You now made it non-obvious, so above flexibility does
not apply anymore.

Best regards,
Krzysztof