Re: [PATCH 3/6] dt-bindings: sound: Add sound card bindings for Tesla FSD

Krzysztof Kozlowski
2022-10-22T16:49:23Z

On 21/10/2022 04:44, Padmanabhan Rajanbabu wrote:
>> On Fri, Oct 14, 2022 at 03:51:48PM +0530, Padmanabhan Rajanbabu wrote:
>>> Add dt-binding reference document to configure the DAI link for Tesla
>>> FSD sound card driver.
>>
>> The simple-card or graph-card bindings don't work for you?
> Thank you for reviewing the patch.
>
> The actual reason for having a custom sound card driver lies in the fact
> that the Samsung i2s cpu dai requires configuration of some Samsung
> specific properties like rfs, bfs, codec clock direction and root clock
> source. We do not have flexibility of configuring the same in simple card
> driver (sound/soc/generic/simple-card.c) or audio graph card driver
> (sound/soc/generic/audio-graph-card.c). The binding has been added to
> support the custom sound card driver.
>
> I understand from your query that the newly added card has device tree
> nodes and properties which are used in simple card as well, but having a
> different or new prefixes. I believe to address that, we can restructure
> the string names to generic ones.

You must use generic, existing properties where applicable.

> But I would like to understand, to reuse
> the simple card or audio graph card bindings, can we add secondary
> compatible strings in the simple card dt-binding for the custom sound card
> to probe ?

If you see other vendor compatibles there, then yes... But there aren't
any, right?

>>
>>>
>>> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
>>> ---
>>> .../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++++++++++++
>>> 1 file changed, 158 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
>>> b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
>>> new file mode 100644
>>> index 000000000000..4bd590f4ee27
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
>>> @@ -0,0 +1,158 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
>>> +2022 Samsung Electronics Co. Ltd.
>>> +%YAML 1.2
>>> +---
>>> +$id:
>>> +https://protect2.fireeye.com/v1/url?k=4ae54403-157e7d1c-4ae4cf4c-
>> 000b
>>> +abdfecba-9eb398ea304f8ae8&q=1&e=4935bed0-ce62-47dd-8faf-
>> 4750b01e22d3&
>>>
>> +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fsound%2Ftesla%2Cfsd-
>> card.ya
>>> +ml%23
>>> +$schema:
>>> +https://protect2.fireeye.com/v1/url?k=8c72226e-d3e91b71-8c73a921-
>> 000b
>>> +abdfecba-3ce999f6c052255b&q=1&e=4935bed0-ce62-47dd-8faf-
>> 4750b01e22d3&
>>> +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>>> +
>>> +title: Tesla FSD ASoC sound card driver
>>> +
>>> +maintainers:
>>> + - Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
>>> +
>>> +description: |
>>> + This binding describes the node properties and essential DT entries
>>> +of FSD
>>> + SoC sound card node
>>> +
>>> +select: false
>>
>> Never apply this schema. Why?
> Sorry about it, let me change the select property to true in the next
> patchset

No, just drop it. Look at other bindings or at example-schema

>>
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - tesla,fsd-sndcard
>>> +
>>> + model:
>>> + description: Describes the Name of the sound card
>>> + $ref: /schemas/types.yaml#/definitions/string
>>> +
>>> + widgets:
>>> + description: A list of DAPM widgets in the sound card. Each entry
> is a pair
>>> + of strings, the first being the widget name and the second being
> the
>>> + widget alias
>>> + $ref: /schemas/types.yaml#/definitions/string-array
>>> +
>>> + audio-routing:
>>> + description: A list of the connections between audio components.
> Each entry
>>> + is a pair of strings, the first being the connection's sink, the
> second
>>> + being the connection's source
>>> + $ref: /schemas/types.yaml#/definitions/string-array
>>> +
>>> + dai-tdm-slot-num:
>>> + description: Enables TDM mode and specifies the number of TDM slots
> to be
>>> + enabled
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [0, 1, 2, 3, 4, 5, 6, 7, 8]
>>
>> maximum: 8
> Okay
>>
>>> + default: 2
>>> +
>>> + dai-tdm-slot-width:
>>> + description: Specifies the slot width of each TDm slot enabled
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [8, 16, 24]
>>> + default: 16
>>
>> All the above have types defined. You should not be redefining the types.
> Okay
>>
>>> +
>>> + dai-link:
>>> + description: Holds the DAI link data between CPU, Codec and
> Platform
>>> + type: object
>>
>> additionalProperties: false
> okay
>>
>>> + properties:
>>> + link-name:
>>> + description: Specifies the name of the DAI link
>>> + $ref: /schemas/types.yaml#/definitions/string
>>> +
>>> + dai-format:
>>> + description: Specifies the serial data format of CPU DAI
>>> + $ref: /schemas/types.yaml#/definitions/string
>>> +
>>> + tesla,bitclock-master:
>>> + description: Specifies the phandle of bitclock master DAI
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> +
>>> + tesla,frame-master:
>>> + description: Specifies the phandle of frameclock master DAI
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>
>> These are common properties. Drop 'tesla'.
> Okay
>>
>>> +
>>> + cpu:
>>> + description: Holds the CPU DAI node and instance
>>> + type: object
>>
>> additionalProperties: false
> Okay
>>
>>> + properties:
>>> + sound-dai:
>>> + description: Specifies the phandle of CPU DAI node
>>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +
>>> + required:
>>> + - sound-dai
>>> +
>>> + codec:
>>> + description: Holds the Codec DAI node and instance
>>> + type: object
>>
>> additionalProperties: false
> Okay
>>
>>> + properties:
>>> + sound-dai:
>>> + description: Specifies the phandle of Codec DAI node
>>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>>
>> Already has a type. Need to define how many entries (maxItems: 1 ?).
> Okay. I'll update in the upcoming patch set
>>
>>> +
>>> + required:
>>> + - sound-dai
>>> +
>>> + required:
>>> + - link-name
>>> + - dai-format
>>> + - tesla,bitclock-master
>>> + - tesla,frame-master
>>> + - cpu
>>> +
>>> +dependencies:
>>> + dai-tdm-slot-width: [ 'dai-tdm-slot-num' ]
>>
>> This should be captured with tdm-slot.txt converted to schema.
> Okay
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - model
>>> + - dai-link
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + sound {
>>> + compatible = "tesla,fsd-sndcard";
>>> + status = "disabled";
>>
>> Why have a disabled example? Other than your example won't pass your
>> schema.
> Thanks for noticing, I'll double check and change the example keeping the
> status
> as enabled

No, just drop. Start with example-schema instead.

Best regards,
Krzysztof