[PATCH v5 11/12] ASoC: dt-bindings: sound: cirrus: cs530x: add spi bus properties

Vitaly Rodionov posted 12 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v5 11/12] ASoC: dt-bindings: sound: cirrus: cs530x: add spi bus properties
Posted by Vitaly Rodionov 3 months, 2 weeks ago
This patch adds property for cs530x SPI control bus
with max frequency 24MHz.

Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
---
 Documentation/devicetree/bindings/sound/cirrus,cs530x.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs530x.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs530x.yaml
index 04ed197f91eb..b233bcd18d49 100644
--- a/Documentation/devicetree/bindings/sound/cirrus,cs530x.yaml
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs530x.yaml
@@ -30,6 +30,9 @@ properties:
   reg:
     maxItems: 1
 
+  spi-max-frequency:
+    maximum: 24000000
+
   '#sound-dai-cells':
     const: 1
 
-- 
2.43.0
Re: [PATCH v5 11/12] ASoC: dt-bindings: sound: cirrus: cs530x: add spi bus properties
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 22/10/2025 15:38, Vitaly Rodionov wrote:
> This patch adds property for cs530x SPI control bus
> with max frequency 24MHz.

Why?

You described what you did, we see that from the diff. Explain why you
are doing this, e.g. hardware is like that? Old hardware? New hardware?
Both? If only new, then why this is a separate patch?

Also, since I expect new version:
Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94


Best regards,
Krzysztof
Re: [PATCH v5 11/12] ASoC: dt-bindings: sound: cirrus: cs530x: add spi bus properties
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 22/10/2025 15:42, Krzysztof Kozlowski wrote:
> On 22/10/2025 15:38, Vitaly Rodionov wrote:
>> This patch adds property for cs530x SPI control bus
>> with max frequency 24MHz.
> 
> Why?
> 
> You described what you did, we see that from the diff. Explain why you
> are doing this, e.g. hardware is like that? Old hardware? New hardware?
> Both? If only new, then why this is a separate patch?
> 
> Also, since I expect new version:
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94
>

And after looking at next patch I understand why - you add SPI which was
not there, so this patch is not correct alone anyway. You need a ref to
spi-peripheral-props (see any other SPI device). My previous questions
still stay valid though - how this relates to existing devices and to
earlier patch.

Best regards,
Krzysztof
Re: [PATCH v5 11/12] ASoC: dt-bindings: sound: cirrus: cs530x: add spi bus properties
Posted by Vitaly Rodionov 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 03:50:09PM +0200, Krzysztof Kozlowski wrote:
> On 22/10/2025 15:42, Krzysztof Kozlowski wrote:
> > On 22/10/2025 15:38, Vitaly Rodionov wrote:
> >> This patch adds property for cs530x SPI control bus
> >> with max frequency 24MHz.
> > 
> > Why?
> > 
> > You described what you did, we see that from the diff. Explain why you
> > are doing this, e.g. hardware is like that? Old hardware? New hardware?
> > Both? If only new, then why this is a separate patch?
> > 
> > Also, since I expect new version:
> > Please do not use "This commit/patch/change", but imperative mood. See
> > longer explanation here:
> > https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94
> >
> 
> And after looking at next patch I understand why - you add SPI which was
> not there, so this patch is not correct alone anyway. You need a ref to
> spi-peripheral-props (see any other SPI device). My previous questions
> still stay valid though - how this relates to existing devices and to
> earlier patch.
> 
> Best regards,
> Krzysztof

Hi Krzysztof,

Thanks for pointing that out. You’re correct — the SPI part is added in the next patch,
so this one isn’t fully correct on its own. I’ll fix it by adding a reference to spi-peripheral-props
and will address your previous questions about its relation to existing devices and the earlier patch.

Thanks,
Vitaly