> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@xxxxxxxxxx] > Sent: 22 October 2022 10:19 PM > To: Padmanabhan Rajanbabu <p.rajanbabu@xxxxxxxxxxx>; 'Rob Herring' > <robh@xxxxxxxxxx> > Cc: lgirdwood@xxxxxxxxx; broonie@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; s.nawrocki@xxxxxxxxxxx; > perex@xxxxxxxx; tiwai@xxxxxxxx; pankaj.dubey@xxxxxxxxxxx; > alim.akhtar@xxxxxxxxxxx; rcsekar@xxxxxxxxxxx; > aswani.reddy@xxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-samsung- > soc@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 3/6] dt-bindings: sound: Add sound card bindings for > Tesla FSD > > 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. Okay > > > 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? Yes you are right, we don't see other vendor compatibles. But, am I allowed to add such secondary compatibles so that we can extend the simple card and its utilities to a large extent? If no, then I believe we will need a separate binding to extend the generic properties. > > >> > >>> > >>> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@xxxxxxxxxxx> > >>> --- > >>> .../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@xxxxxxxxxxx> > >>> + > >>> +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
