All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Hongliang Yang" <hongliang.yang@cixtech.com>
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, olteanv@gmail.com
Subject: Re: [PATCH v7 1/2] dt-bindings: phy: Add CIX Sky1 USB3 PHY
Date: Wed, 11 Nov 2026 03:23:07 +0000	[thread overview]
Message-ID: <sashiko-outbox-208174@kernel.org> (raw)
In-Reply-To: <20261111030937.2954646-2-hongliang.yang@cixtech.com>

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] dt-bindings: phy: cix,sky1-usb3-phy uses #phy-cells = 0 for
  two-lane PHY
--

commit 2ddbb23ac4fa77b6667ad1996ba778aaeb0d7a6c
Author: Hongliang Yang <hongliang.yang@cixtech.com>

dt-bindings: phy: Add CIX Sky1 USB3 PHY

This commit adds the Device Tree binding schema for the CIX Sky1 USB3
SuperSpeed PHY.

> diff --git a/Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml b/Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml
> new file mode 100644
> index 0000000000000..205140bf84e7c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/cix,sky1-usb3-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CIX Sky1 USB3 PHY
> +
> +maintainers:
> +  - Hongliang Yang <hongliang.yang@cixtech.com>
> +
> +description: |
> +  The CIX Sky1 USB3 PHY provides the SuperSpeed lanes of the USB3 Type-A
> +  ports of the SoC.
> +
> +  Both lanes are programmed through one register block and share the
> +  same clocks and resets, so the node is a single PHY provider with
> +  #phy-cells = <0> that all consumers reference. The driver reference
> +  counts phy_init() and phy_exit(), so the block is set up once for all
> +  its consumers.
> +
> +properties:
> +  compatible:
> +    const: cix,sky1-usb3-phy
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 0

[Severity: Low]
As reported by Krzysztof Kozlowski on v6 [1], should #phy-cells be 1 so
that each USB3 controller consumer node can specify which of the two PHY
lanes it is wired to?

The hardware block contains two independent SuperSpeed lanes with separate
per-lane register sets and lane-disable controls serving two distinct USB3
controllers. Constraining #phy-cells to 0 omits which controller connects
to which lane from the Device Tree ABI, preventing per-lane control,
power management, or tuning without breaking the binding.

In addition, the description justifies #phy-cells = <0> by citing Linux
driver reference counting of phy_init() and phy_exit(), whereas bindings
should describe the hardware rather than driver implementation details.

[1] https://lore.kernel.org/r/20261105-kind-macaque-of-enhancement-ee0365@quoll

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111030937.2954646-1-hongliang.yang@cixtech.com?part=1

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Hongliang Yang" <hongliang.yang@cixtech.com>
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, olteanv@gmail.com
Subject: Re: [PATCH v7 1/2] dt-bindings: phy: Add CIX Sky1 USB3 PHY
Date: Wed, 11 Nov 2026 03:23:07 +0000	[thread overview]
Message-ID: <sashiko-outbox-208174@kernel.org> (raw)
In-Reply-To: <20261111030937.2954646-2-hongliang.yang@cixtech.com>

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] dt-bindings: phy: cix,sky1-usb3-phy uses #phy-cells = 0 for
  two-lane PHY
--

commit 2ddbb23ac4fa77b6667ad1996ba778aaeb0d7a6c
Author: Hongliang Yang <hongliang.yang@cixtech.com>

dt-bindings: phy: Add CIX Sky1 USB3 PHY

This commit adds the Device Tree binding schema for the CIX Sky1 USB3
SuperSpeed PHY.

> diff --git a/Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml b/Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml
> new file mode 100644
> index 0000000000000..205140bf84e7c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/cix,sky1-usb3-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CIX Sky1 USB3 PHY
> +
> +maintainers:
> +  - Hongliang Yang <hongliang.yang@cixtech.com>
> +
> +description: |
> +  The CIX Sky1 USB3 PHY provides the SuperSpeed lanes of the USB3 Type-A
> +  ports of the SoC.
> +
> +  Both lanes are programmed through one register block and share the
> +  same clocks and resets, so the node is a single PHY provider with
> +  #phy-cells = <0> that all consumers reference. The driver reference
> +  counts phy_init() and phy_exit(), so the block is set up once for all
> +  its consumers.
> +
> +properties:
> +  compatible:
> +    const: cix,sky1-usb3-phy
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#phy-cells":
> +    const: 0

[Severity: Low]
As reported by Krzysztof Kozlowski on v6 [1], should #phy-cells be 1 so
that each USB3 controller consumer node can specify which of the two PHY
lanes it is wired to?

The hardware block contains two independent SuperSpeed lanes with separate
per-lane register sets and lane-disable controls serving two distinct USB3
controllers. Constraining #phy-cells to 0 omits which controller connects
to which lane from the Device Tree ABI, preventing per-lane control,
power management, or tuning without breaking the binding.

In addition, the description justifies #phy-cells = <0> by citing Linux
driver reference counting of phy_init() and phy_exit(), whereas bindings
should describe the hardware rather than driver implementation details.

[1] https://lore.kernel.org/r/20261105-kind-macaque-of-enhancement-ee0365@quoll

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111030937.2954646-1-hongliang.yang@cixtech.com?part=1

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

  reply	other threads:[~2026-11-11  3:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11  3:09 [PATCH v7 0/2] phy: Add CIX Sky1 USB3 PHY support Hongliang Yang
2026-11-11  3:09 ` Hongliang Yang
2026-11-11  3:09 ` [PATCH v7 1/2] dt-bindings: phy: Add CIX Sky1 USB3 PHY Hongliang Yang
2026-11-11  3:09   ` Hongliang Yang
2026-11-11  3:23   ` sashiko-bot [this message]
2026-11-11  3:23     ` sashiko-bot
2026-11-11  7:53   ` Krzysztof Kozlowski
2026-11-11  7:53     ` Krzysztof Kozlowski
2026-11-11  8:27     ` Hongliang Yang
2026-11-11  8:27       ` Hongliang Yang
2026-11-11  3:09 ` [PATCH v7 2/2] phy: cix: Add CIX Sky1 USB3 PHY driver Hongliang Yang
2026-11-11  3:09   ` Hongliang Yang
2026-11-11  3:23   ` sashiko-bot
2026-11-11  3:23     ` sashiko-bot

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=sashiko-outbox-208174@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hongliang.yang@cixtech.com \
    --cc=linux-phy@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --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.