diff mbox series

[v2,2/7] dt-bindings: soundwire: qcom: add 16-bit sample interval

Message ID 20230403132503.62090-3-krzysztof.kozlowski@linaro.org
State Superseded
Headers show
Series None | expand

Commit Message

Krzysztof Kozlowski April 3, 2023, 1:24 p.m. UTC
The port sample interval was always 16-bit, split into low and high
bytes.  This split was unnecessary, although harmless for older devices
because all of them used only lower byte (so values < 0xff).  With
support for Soundwire controller on Qualcomm SM8550 and its devices,
both bytes will be used, thus add a new 'qcom,ports-sinterval' property
to allow 16-bit sample intervals.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/soundwire/qcom,soundwire.yaml    | 22 +++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) April 4, 2023, 2:21 p.m. UTC | #1
On Mon, Apr 03, 2023 at 03:24:58PM +0200, Krzysztof Kozlowski wrote:
> The port sample interval was always 16-bit, split into low and high
> bytes.  This split was unnecessary, although harmless for older devices
> because all of them used only lower byte (so values < 0xff).  With
> support for Soundwire controller on Qualcomm SM8550 and its devices,
> both bytes will be used, thus add a new 'qcom,ports-sinterval' property
> to allow 16-bit sample intervals.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/soundwire/qcom,soundwire.yaml    | 22 +++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> index c283c594fb5c..883b8be9be1b 100644
> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> @@ -86,7 +86,7 @@ properties:
>    qcom,ports-sinterval-low:
>      $ref: /schemas/types.yaml#/definitions/uint8-array
>      description:
> -      Sample interval low of each data port.
> +      Sample interval (only lowest byte) of each data port.
>        Out ports followed by In ports. Used for Sample Interval calculation.
>        Value of 0xff indicates that this option is not implemented
>        or applicable for the respective data port.
> @@ -94,6 +94,19 @@ properties:
>      minItems: 3
>      maxItems: 16
>  
> +  qcom,ports-sinterval:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Sample interval of each data port.
> +      Out ports followed by In ports. Used for Sample Interval calculation.
> +      Value of 0xffff indicates that this option is not implemented
> +      or applicable for the respective data port.
> +      More info in MIPI Alliance SoundWire 1.0 Specifications.
> +    minItems: 3
> +    maxItems: 16
> +    items:
> +      maximum: 0xffff

Why not use uint16-array?

> +
>    qcom,ports-offset1:
>      $ref: /schemas/types.yaml#/definitions/uint8-array
>      description:
> @@ -219,10 +232,15 @@ required:
>    - '#size-cells'
>    - qcom,dout-ports
>    - qcom,din-ports
> -  - qcom,ports-sinterval-low
>    - qcom,ports-offset1
>    - qcom,ports-offset2
>  
> +oneOf:
> +  - required:
> +      - qcom,ports-sinterval-low
> +  - required:
> +      - qcom,ports-sinterval
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.34.1
>
Krzysztof Kozlowski April 4, 2023, 6:18 p.m. UTC | #2
On 04/04/2023 16:21, Rob Herring wrote:
>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> index c283c594fb5c..883b8be9be1b 100644
>> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> @@ -86,7 +86,7 @@ properties:
>>    qcom,ports-sinterval-low:
>>      $ref: /schemas/types.yaml#/definitions/uint8-array
>>      description:
>> -      Sample interval low of each data port.
>> +      Sample interval (only lowest byte) of each data port.
>>        Out ports followed by In ports. Used for Sample Interval calculation.
>>        Value of 0xff indicates that this option is not implemented
>>        or applicable for the respective data port.
>> @@ -94,6 +94,19 @@ properties:
>>      minItems: 3
>>      maxItems: 16
>>  
>> +  qcom,ports-sinterval:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      Sample interval of each data port.
>> +      Out ports followed by In ports. Used for Sample Interval calculation.
>> +      Value of 0xffff indicates that this option is not implemented
>> +      or applicable for the respective data port.
>> +      More info in MIPI Alliance SoundWire 1.0 Specifications.
>> +    minItems: 3
>> +    maxItems: 16
>> +    items:
>> +      maximum: 0xffff
> 
> Why not use uint16-array?

Because I am afraid it will grow in next version to 24 or 32 bits. I can
change easily maximum, but if I put here uint16-array, all DTS will have
/bytes 16/ annotation.


Best regards,
Krzysztof
Rob Herring (Arm) April 6, 2023, 3:59 p.m. UTC | #3
On Mon, 03 Apr 2023 15:24:58 +0200, Krzysztof Kozlowski wrote:
> The port sample interval was always 16-bit, split into low and high
> bytes.  This split was unnecessary, although harmless for older devices
> because all of them used only lower byte (so values < 0xff).  With
> support for Soundwire controller on Qualcomm SM8550 and its devices,
> both bytes will be used, thus add a new 'qcom,ports-sinterval' property
> to allow 16-bit sample intervals.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/soundwire/qcom,soundwire.yaml    | 22 +++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Srinivas Kandagatla April 12, 2023, 3:28 p.m. UTC | #4
On 03/04/2023 14:24, Krzysztof Kozlowski wrote:
> The port sample interval was always 16-bit, split into low and high
> bytes.  This split was unnecessary, although harmless for older devices
> because all of them used only lower byte (so values < 0xff).  With
> support for Soundwire controller on Qualcomm SM8550 and its devices,
> both bytes will be used, thus add a new 'qcom,ports-sinterval' property
> to allow 16-bit sample intervals.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   .../bindings/soundwire/qcom,soundwire.yaml    | 22 +++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> index c283c594fb5c..883b8be9be1b 100644
> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
> @@ -86,7 +86,7 @@ properties:
>     qcom,ports-sinterval-low:
>       $ref: /schemas/types.yaml#/definitions/uint8-array
>       description:
> -      Sample interval low of each data port.
> +      Sample interval (only lowest byte) of each data port.
>         Out ports followed by In ports. Used for Sample Interval calculation.
>         Value of 0xff indicates that this option is not implemented
>         or applicable for the respective data port.
> @@ -94,6 +94,19 @@ properties:
>       minItems: 3
>       maxItems: 16
>   
> +  qcom,ports-sinterval:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

Should this not be ref: /schemas/types.yaml#/definitions/uint16-array ?


--srini
> +    description:
> +      Sample interval of each data port.
> +      Out ports followed by In ports. Used for Sample Interval calculation.
> +      Value of 0xffff indicates that this option is not implemented
> +      or applicable for the respective data port.
> +      More info in MIPI Alliance SoundWire 1.0 Specifications.
> +    minItems: 3
> +    maxItems: 16
> +    items:
> +      maximum: 0xffff
> +
>     qcom,ports-offset1:
>       $ref: /schemas/types.yaml#/definitions/uint8-array
>       description:
> @@ -219,10 +232,15 @@ required:
>     - '#size-cells'
>     - qcom,dout-ports
>     - qcom,din-ports
> -  - qcom,ports-sinterval-low
>     - qcom,ports-offset1
>     - qcom,ports-offset2
>   
> +oneOf:
> +  - required:
> +      - qcom,ports-sinterval-low
> +  - required:
> +      - qcom,ports-sinterval
> +
>   additionalProperties: false
>   
>   examples:
Krzysztof Kozlowski April 12, 2023, 4:16 p.m. UTC | #5
On 12/04/2023 17:28, Srinivas Kandagatla wrote:
> 
> 
> On 03/04/2023 14:24, Krzysztof Kozlowski wrote:
>> The port sample interval was always 16-bit, split into low and high
>> bytes.  This split was unnecessary, although harmless for older devices
>> because all of them used only lower byte (so values < 0xff).  With
>> support for Soundwire controller on Qualcomm SM8550 and its devices,
>> both bytes will be used, thus add a new 'qcom,ports-sinterval' property
>> to allow 16-bit sample intervals.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../bindings/soundwire/qcom,soundwire.yaml    | 22 +++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> index c283c594fb5c..883b8be9be1b 100644
>> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>> @@ -86,7 +86,7 @@ properties:
>>     qcom,ports-sinterval-low:
>>       $ref: /schemas/types.yaml#/definitions/uint8-array
>>       description:
>> -      Sample interval low of each data port.
>> +      Sample interval (only lowest byte) of each data port.
>>         Out ports followed by In ports. Used for Sample Interval calculation.
>>         Value of 0xff indicates that this option is not implemented
>>         or applicable for the respective data port.
>> @@ -94,6 +94,19 @@ properties:
>>       minItems: 3
>>       maxItems: 16
>>   
>> +  qcom,ports-sinterval:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Should this not be ref: /schemas/types.yaml#/definitions/uint16-array ?

Same answer as for Rob:

Because I am afraid it will grow in next version to 24 or 32 bits. I can
change easily maximum, but if I put here uint16-array, all DTS will have
/bytes 16/ annotation.

Best regards,
Krzysztof
Srinivas Kandagatla April 13, 2023, 11:12 a.m. UTC | #6
On 12/04/2023 17:16, Krzysztof Kozlowski wrote:
> On 12/04/2023 17:28, Srinivas Kandagatla wrote:
>>
>>
>> On 03/04/2023 14:24, Krzysztof Kozlowski wrote:
>>> The port sample interval was always 16-bit, split into low and high
>>> bytes.  This split was unnecessary, although harmless for older devices
>>> because all of them used only lower byte (so values < 0xff).  With
>>> support for Soundwire controller on Qualcomm SM8550 and its devices,
>>> both bytes will be used, thus add a new 'qcom,ports-sinterval' property
>>> to allow 16-bit sample intervals.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>    .../bindings/soundwire/qcom,soundwire.yaml    | 22 +++++++++++++++++--
>>>    1 file changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>>> index c283c594fb5c..883b8be9be1b 100644
>>> --- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
>>> @@ -86,7 +86,7 @@ properties:
>>>      qcom,ports-sinterval-low:
>>>        $ref: /schemas/types.yaml#/definitions/uint8-array
>>>        description:
>>> -      Sample interval low of each data port.
>>> +      Sample interval (only lowest byte) of each data port.
>>>          Out ports followed by In ports. Used for Sample Interval calculation.
>>>          Value of 0xff indicates that this option is not implemented
>>>          or applicable for the respective data port.
>>> @@ -94,6 +94,19 @@ properties:
>>>        minItems: 3
>>>        maxItems: 16
>>>    
>>> +  qcom,ports-sinterval:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> Should this not be ref: /schemas/types.yaml#/definitions/uint16-array ?
> 
> Same answer as for Rob:
> 
> Because I am afraid it will grow in next version to 24 or 32 bits. I can
> change easily maximum, but if I put here uint16-array, all DTS will have
> /bytes 16/ annotation.
> 
As per MiPi Specs the sample Interval is an integer in the range 2 to 
65535. I don't see a value in making this u32, other than adding some 
confusion by deviating from specs.

--srini

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 17, 2023, 2:17 p.m. UTC | #7
On 13/04/2023 13:12, Srinivas Kandagatla wrote:

>>>> +  qcom,ports-sinterval:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>
>>> Should this not be ref: /schemas/types.yaml#/definitions/uint16-array ?
>>
>> Same answer as for Rob:
>>
>> Because I am afraid it will grow in next version to 24 or 32 bits. I can
>> change easily maximum, but if I put here uint16-array, all DTS will have
>> /bytes 16/ annotation.
>>
> As per MiPi Specs the sample Interval is an integer in the range 2 to 
> 65535. I don't see a value in making this u32, other than adding some 
> confusion by deviating from specs.

Hm, in such case I'll make it uint16.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
index c283c594fb5c..883b8be9be1b 100644
--- a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
+++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml
@@ -86,7 +86,7 @@  properties:
   qcom,ports-sinterval-low:
     $ref: /schemas/types.yaml#/definitions/uint8-array
     description:
-      Sample interval low of each data port.
+      Sample interval (only lowest byte) of each data port.
       Out ports followed by In ports. Used for Sample Interval calculation.
       Value of 0xff indicates that this option is not implemented
       or applicable for the respective data port.
@@ -94,6 +94,19 @@  properties:
     minItems: 3
     maxItems: 16
 
+  qcom,ports-sinterval:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Sample interval of each data port.
+      Out ports followed by In ports. Used for Sample Interval calculation.
+      Value of 0xffff indicates that this option is not implemented
+      or applicable for the respective data port.
+      More info in MIPI Alliance SoundWire 1.0 Specifications.
+    minItems: 3
+    maxItems: 16
+    items:
+      maximum: 0xffff
+
   qcom,ports-offset1:
     $ref: /schemas/types.yaml#/definitions/uint8-array
     description:
@@ -219,10 +232,15 @@  required:
   - '#size-cells'
   - qcom,dout-ports
   - qcom,din-ports
-  - qcom,ports-sinterval-low
   - qcom,ports-offset1
   - qcom,ports-offset2
 
+oneOf:
+  - required:
+      - qcom,ports-sinterval-low
+  - required:
+      - qcom,ports-sinterval
+
 additionalProperties: false
 
 examples: