All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, johan@kernel.org, rogerq@ti.com,
	Julian.FRIEDRICH@frequentis.com, upstream+omap@sigma-star.at
Subject: Re: [PATCH 2/2] dt-bindings: phy: ti,phy-usb3: document aspcie reference clock setting
Date: Wed, 5 Aug 2026 11:47:04 +0200	[thread overview]
Message-ID: <22ad4dab-dbf2-48f4-970f-ea9fc91f1e2b@kernel.org> (raw)
In-Reply-To: <5f10e213-2ab3-4309-a01e-5d0f098a5b35@sigma-star.at>

On 05/08/2026 10:42, David Oberhollenzer wrote:
> Hi,
> 
> On 8/5/26 9:39 AM, Krzysztof Kozlowski wrote:
>>
>> Not a valid naming, missing vendor prefix and syscon looks like
>> Linuxism, since you called it "system control module".
>>
> 
> I agree that this is a very vendor specific kluge and I suspected
> it might need a vendor prefix. However, I ultimately tried to stick
> to existing conventions.
> 
> There are 3 more syscon properties defined for this device:
>   - syscon-phy-power
>   - syscon-pllreset
>   - syscon-pcs

These, as the rest of the binding come, from 2015 or even earlier. A lot
changed during that time. It is a good idea to take recently reviewed
code as an example. It is a bad idea to take 10 yo code.

> 
> None of them have a vendor prefix, all of them use the
> description "system control module".
> 
> I take it, the existing properties already break best practices
> that were established later? Or is there another reason they
> lack a vendor prefix?

Just add vendor prefix to the new property.

> 
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    maxItems: 1
>>> +    items:
>>> +      items:
>>
>> Missing -
>>
> Likewise, I simply copied from the syscon blocks and made some
> minor changes. Hence also the identical description. So this is
> broken for the existing properties as well?

Hm, you are right, the syntax was actually correct. I missed maxItems.

Preferred syntax is for example in
Documentation/devicetree/bindings/crypto/intel,ixp4xx-crypto.yaml, but
your code was fine as well.



Best regards,
Krzysztof

WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org>
To: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, johan@kernel.org, rogerq@ti.com,
	Julian.FRIEDRICH@frequentis.com, upstream+omap@sigma-star.at
Subject: Re: [PATCH 2/2] dt-bindings: phy: ti,phy-usb3: document aspcie reference clock setting
Date: Wed, 5 Aug 2026 11:47:04 +0200	[thread overview]
Message-ID: <22ad4dab-dbf2-48f4-970f-ea9fc91f1e2b@kernel.org> (raw)
In-Reply-To: <5f10e213-2ab3-4309-a01e-5d0f098a5b35@sigma-star.at>

On 05/08/2026 10:42, David Oberhollenzer wrote:
> Hi,
> 
> On 8/5/26 9:39 AM, Krzysztof Kozlowski wrote:
>>
>> Not a valid naming, missing vendor prefix and syscon looks like
>> Linuxism, since you called it "system control module".
>>
> 
> I agree that this is a very vendor specific kluge and I suspected
> it might need a vendor prefix. However, I ultimately tried to stick
> to existing conventions.
> 
> There are 3 more syscon properties defined for this device:
>   - syscon-phy-power
>   - syscon-pllreset
>   - syscon-pcs

These, as the rest of the binding come, from 2015 or even earlier. A lot
changed during that time. It is a good idea to take recently reviewed
code as an example. It is a bad idea to take 10 yo code.

> 
> None of them have a vendor prefix, all of them use the
> description "system control module".
> 
> I take it, the existing properties already break best practices
> that were established later? Or is there another reason they
> lack a vendor prefix?

Just add vendor prefix to the new property.

> 
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    maxItems: 1
>>> +    items:
>>> +      items:
>>
>> Missing -
>>
> Likewise, I simply copied from the syscon blocks and made some
> minor changes. Hence also the identical description. So this is
> broken for the existing properties as well?

Hm, you are right, the syntax was actually correct. I missed maxItems.

Preferred syntax is for example in
Documentation/devicetree/bindings/crypto/intel,ixp4xx-crypto.yaml, but
your code was fine as well.



Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2026-08-05  9:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 11:18 [PATCH 0/2] phy: ti: ti-pipe3: allow configuring ACSPCIe TX/RX mode David Oberhollenzer
2026-08-04 11:18 ` David Oberhollenzer
2026-08-04 11:18 ` [PATCH 1/2] " David Oberhollenzer
2026-08-04 11:18   ` David Oberhollenzer
2026-08-04 11:27   ` sashiko-bot
2026-08-04 11:27     ` sashiko-bot
2026-08-05  7:26     ` David Oberhollenzer
2026-08-05  7:26       ` David Oberhollenzer
2026-08-04 11:18 ` [PATCH 2/2] dt-bindings: phy: ti,phy-usb3: document aspcie reference clock setting David Oberhollenzer
2026-08-04 11:18   ` David Oberhollenzer
2026-08-04 11:29   ` sashiko-bot
2026-08-04 11:29     ` sashiko-bot
2026-08-05  7:39   ` Krzysztof Kozlowski
2026-08-05  7:39     ` Krzysztof Kozlowski
2026-08-05  8:42     ` David Oberhollenzer
2026-08-05  8:42       ` David Oberhollenzer
2026-08-05  9:47       ` Krzysztof Kozlowski [this message]
2026-08-05  9:47         ` Krzysztof Kozlowski

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=22ad4dab-dbf2-48f4-970f-ea9fc91f1e2b@kernel.org \
    --to=krzk@kernel.org \
    --cc=Julian.FRIEDRICH@frequentis.com \
    --cc=conor+dt@kernel.org \
    --cc=david.oberhollenzer@sigma-star.at \
    --cc=devicetree@vger.kernel.org \
    --cc=johan@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=robh@kernel.org \
    --cc=rogerq@ti.com \
    --cc=upstream+omap@sigma-star.at \
    --cc=vkoul@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.