linux-kernel - Re: [PATCH v3 1/4] ASoC: dt-bindings: lpc32xx: Add lpc32xx i2s DT binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 14:14:49 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Vladimir Zapolskiy <vz@...ia.com>,
 Russell King <linux@...linux.org.uk>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, "J.M.B. Downing"
 <jonathan.downing@...tel.com>, Arnd Bergmann <arnd@...db.de>,
 Chancel Liu <chancel.liu@....com>, Michael Ellerman <mpe@...erman.id.au>,
 linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 alsa-devel@...a-project.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v3 1/4] ASoC: dt-bindings: lpc32xx: Add lpc32xx i2s DT
 binding

On 17/06/2024 11:33, Piotr Wojtaszczyk wrote:
> On Sat, Jun 15, 2024 at 12:01 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>> Do not attach (thread) your patchsets to some other threads (unrelated
>> or older versions). This buries them deep in the mailbox and might
>> interfere with applying entire sets.
> 
> I'm sorry about that, it won't happen again.
> 
>>> +  dma-vc-names:
>>
>> Missing vendor prefix... but I don't really get what's the point of this
>> property.
> 
> Is "nxp,lpc3xxx-dma-vc-names" acceptable?

No, because it does not help me to understand:
" what's the point of this property."

> 
>>
>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>> +    description: |
>>> +      names of virtual pl08x dma channels for tx and rx
>>> +      directions in this order.
>>> +    minItems: 2
>>> +    maxItems: 2
>>
>> What part of hardware or board configuration this represents?
>>
>> It wasn't here and nothing in changelog explained it.
> 
> That's information which DMA signal and mux setting an I2S interface uses.
> It's a name (bus_id field) of platform data entry from phy3250.c in
> [PATCH v3 3/4].

platform entries from driver do not seem related at all to hardware
description. You know encode driver model into bindings, so obviously no-go.

> It's used by snd_soc_dai_init_dma_data() in [PATCH v3 4/4] to give the
> dmaengine a
> hint which dma config to use. The LPC32xx doesn't have yet a dmamux driver like

and if I change driver platform data to foo and bar, does the DTS work? No.

> lpc18xx-dmamux.c therefore it still uses platform data entries for
> pl08x dma channels
> and 'SND_DMAENGINE_PCM_FLAG_NO_DT | SND_DMAENGINE_PCM_FLAG_COMPAT'
> flags in the devm_snd_dmaengine_pcm_register().
> Typically instead of this platform data you would use regular 'dma'
> and 'dma-names' if it had
> proper dmamux driver like lpc18xx-dmamux.c

Exactly. Use these.

> 
>>
>> Drop.
>>
>>
>>> +
>>> +  "#sound-dai-cells":
>>> +    const: 0
>>> +
> 
> The "dai-common.yam" doesn't declare a default value for this so

Where is my comment to which you refer to? Please do not drop context
from replies. I have no clue what you want to discuss here.

> isn't it required? It's declared in others yaml files like:
> Documentation/devicetree/bindings/sound/qcom,q6apm.yaml

Best regards,
Krzysztof