linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Andrei.Simion@microchip.com, claudiu.beznea@tuxon.dev,
	lgirdwood@gmail.com, broonie@kernel.org, perex@perex.cz,
	tiwai@suse.com, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, Nicolas.Ferre@microchip.com,
	alexandre.belloni@bootlin.com
Cc: alsa-devel@alsa-project.org, linux-sound@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] ASoC: dt-bindings: microchip,sama7g5-i2smcc: Add 'sound-name-prefix' property
Date: Mon, 9 Sep 2024 19:51:20 +0200	[thread overview]
Message-ID: <7659bae7-93f6-492d-a54f-21abc611c2f2@kernel.org> (raw)
In-Reply-To: <d4b445ec-dc9c-4313-8ef2-e5c086f21449@microchip.com>

On 09/09/2024 14:28, Andrei.Simion@microchip.com wrote:
>>>      minItems: 1
>>>
>>> +  sound-name-prefix:
>>> +    pattern: "^I2SMCC[0-9]$"
>>
>> This does not look correct. Name/prefix can be anything matching real
>> hardware, why are you restricting it? How can you predict all names?
>>
> Based on the datasheet, the SoC(s) have the following naming conventions:
>   - sama7g5: I2SMCC0 and I2SMCC1
>   - sam9x60/sam9x75: I2SMCC
> 
> To accommodate these variations, I propose using a more relaxed pattern: "^I2SMCC(0-9)?$".
> This pattern allows for both the fixed prefix and an optional single digit at the end.
> What are your thoughts on this approach?

I understand this does not differ per board, because it is component of
the SoC, yet still I do not see any value in enforcing name.

> 
> 
>>> +    $ref: /schemas/types.yaml#/definitions/string
>>> +    description:
>>> +      Unique prefixes for the sink/source names of the component, ensuring
>>> +      distinct identification among multiple instances.
>>
>> You are duplicating property definitions. This is not needed at all.
>> Maybe your schema misses $ref to common schema.
>>
> 
> I understand the concern about duplicating property definitions.
> In the current file, I have referenced `dai-common` as shown here:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/sound/microchip%2Csama7g5-i2smcc.yaml#L74C1-L75C27
> 
> Could you please confirm if this reference is correctly implemented,

Yeah, the dai-common $ref is correct, so you should not need it. Do you
see any warning?

> or suggest any adjustments needed to align with the common schema?

I claim nothing has to be done and entire patch is redundant or not much
helpful. Your commit msg did not explain *why* this is needed and what
problem you are fixing, so what I can say? I don't know why should be
aligned because I do not understand the problem being fixed.

Best regards,
Krzysztof



  parent reply	other threads:[~2024-09-09 17:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-09  8:35 [PATCH 0/2] Adjust Stream Name and DT Bindings Updates Andrei Simion
2024-09-09  8:35 ` [PATCH 1/2] ASoC: atmel: mchp-i2s-mcc: Remove interface name from stream_name Andrei Simion
2024-09-09  8:35 ` [PATCH 2/2] ASoC: dt-bindings: microchip,sama7g5-i2smcc: Add 'sound-name-prefix' property Andrei Simion
2024-09-09  8:38   ` Krzysztof Kozlowski
     [not found]     ` <d4b445ec-dc9c-4313-8ef2-e5c086f21449@microchip.com>
2024-09-09 17:51       ` Krzysztof Kozlowski [this message]
2024-09-10  7:17     ` Krzysztof Kozlowski
2024-09-09 21:01 ` (subset) [PATCH 0/2] Adjust Stream Name and DT Bindings Updates Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7659bae7-93f6-492d-a54f-21abc611c2f2@kernel.org \
    --to=krzk@kernel.org \
    --cc=Andrei.Simion@microchip.com \
    --cc=Nicolas.Ferre@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=conor+dt@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=robh@kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).