All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dimitri Fedrau <dima.fedrau@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Dimitri Fedrau <dimitri.fedrau@liebherr.com>,
	Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: phy: add support for NXPs TJA1145 CAN transceiver
Date: Thu, 31 Jul 2025 06:48:24 +0200	[thread overview]
Message-ID: <20250731044824.GA3815@legfed1> (raw)
In-Reply-To: <20250730-aromatic-optimistic-hyena-f1db1a@kuoka>

Hi Krzysztof,

Am Wed, Jul 30, 2025 at 09:35:12AM +0200 schrieb Krzysztof Kozlowski:
> On Mon, Jul 28, 2025 at 05:39:29PM +0200, Dimitri Fedrau wrote:
> > Adding documentation for NXPs TJA1145 CAN transceiver.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  .../devicetree/bindings/phy/nxp,tja1145-can.yaml   | 79 ++++++++++++++++++++++
> 
> Why isn't this in can directory with rest of CAN bindings?
> 
The directory contains CAN controller bindings. This is a CAN transceiver
binding. And the only CAN transceiver binding I could find so far is this:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
The driver uses the generic PHY framework as the driver for the tja1145
does.

> >  1 file changed, 79 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/nxp,tja1145-can.yaml b/Documentation/devicetree/bindings/phy/nxp,tja1145-can.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..10bf2bce1b35788b3284c42e544a56eda6d79947
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/nxp,tja1145-can.yaml
> 
> Filename should match compatible.
> 
Ok, will fix this.

> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/nxp,tja1145-can.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TJA1145 CAN transceiver
> > +
> > +maintainers:
> > +  - Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> 
> Missing ref to transceiver properties. Look at other CAN bindings.
>
There is only one transceiver property(max-bitrate) which I don't need
because the max-bitrate is known for the device. So why should I add it
to the DT ?
I'm only aware of CAN controller bindings making use of the transceiver
property which I think is because the PHYs they are supporting with this
are very simple which need no or little configuration to operate and the
max-bitrate property is needed to limit the bitrate.

> > +
> > +properties:
> > +  compatible:
> > +    const: nxp,tja1145
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  reg:
> > +    maxItems: 1
> 
> reg is the second property, also in "required:" block.
> 
Will fix this.

> > +
> > +  spi-max-frequency:
> > +    maximum: 4000000
> > +
> > +  spi-cpha: true
> > +
> > +  spi-cs-setup-delay-ns:
> > +    minimum: 50
> > +    default: 50
> > +
> > +  spi-cs-hold-delay-ns:
> > +    minimum: 50
> > +    default: 50
> > +
> > +  spi-cs-inactive-delay-ns:
> > +    minimum: 250
> > +    default: 250
> > +
> > +  vcc-supply:
> > +    description:
> > +      CAN transceiver supply voltage
> > +
> > +  vio-supply:
> > +    description:
> > +      Supply voltage for I/O level adaptor
> > +
> > +  vbat-supply:
> > +    description:
> > +      Battery supply voltage
> > +
> > +required:
> > +  - compatible
> > +  - "#phy-cells"
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        canphy@0 {
> 
> can-phy if something like this exist. If not, then probably can-transceiver
>
Will fix this.

> > +            compatible = "nxp,tja1145";
> > +            #phy-cells = <0>;
> > +            reg = <0>;
> 
> Please follow DTS coding style.
>
Ok.

Best regards,
Dimitri Fedrau

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

WARNING: multiple messages have this Message-ID (diff)
From: Dimitri Fedrau <dima.fedrau@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Dimitri Fedrau <dimitri.fedrau@liebherr.com>,
	Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: phy: add support for NXPs TJA1145 CAN transceiver
Date: Thu, 31 Jul 2025 06:48:24 +0200	[thread overview]
Message-ID: <20250731044824.GA3815@legfed1> (raw)
In-Reply-To: <20250730-aromatic-optimistic-hyena-f1db1a@kuoka>

Hi Krzysztof,

Am Wed, Jul 30, 2025 at 09:35:12AM +0200 schrieb Krzysztof Kozlowski:
> On Mon, Jul 28, 2025 at 05:39:29PM +0200, Dimitri Fedrau wrote:
> > Adding documentation for NXPs TJA1145 CAN transceiver.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  .../devicetree/bindings/phy/nxp,tja1145-can.yaml   | 79 ++++++++++++++++++++++
> 
> Why isn't this in can directory with rest of CAN bindings?
> 
The directory contains CAN controller bindings. This is a CAN transceiver
binding. And the only CAN transceiver binding I could find so far is this:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/devicetree/bindings/phy/ti,tcan104x-can.yaml
The driver uses the generic PHY framework as the driver for the tja1145
does.

> >  1 file changed, 79 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/nxp,tja1145-can.yaml b/Documentation/devicetree/bindings/phy/nxp,tja1145-can.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..10bf2bce1b35788b3284c42e544a56eda6d79947
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/nxp,tja1145-can.yaml
> 
> Filename should match compatible.
> 
Ok, will fix this.

> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/nxp,tja1145-can.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TJA1145 CAN transceiver
> > +
> > +maintainers:
> > +  - Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> 
> Missing ref to transceiver properties. Look at other CAN bindings.
>
There is only one transceiver property(max-bitrate) which I don't need
because the max-bitrate is known for the device. So why should I add it
to the DT ?
I'm only aware of CAN controller bindings making use of the transceiver
property which I think is because the PHYs they are supporting with this
are very simple which need no or little configuration to operate and the
max-bitrate property is needed to limit the bitrate.

> > +
> > +properties:
> > +  compatible:
> > +    const: nxp,tja1145
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  reg:
> > +    maxItems: 1
> 
> reg is the second property, also in "required:" block.
> 
Will fix this.

> > +
> > +  spi-max-frequency:
> > +    maximum: 4000000
> > +
> > +  spi-cpha: true
> > +
> > +  spi-cs-setup-delay-ns:
> > +    minimum: 50
> > +    default: 50
> > +
> > +  spi-cs-hold-delay-ns:
> > +    minimum: 50
> > +    default: 50
> > +
> > +  spi-cs-inactive-delay-ns:
> > +    minimum: 250
> > +    default: 250
> > +
> > +  vcc-supply:
> > +    description:
> > +      CAN transceiver supply voltage
> > +
> > +  vio-supply:
> > +    description:
> > +      Supply voltage for I/O level adaptor
> > +
> > +  vbat-supply:
> > +    description:
> > +      Battery supply voltage
> > +
> > +required:
> > +  - compatible
> > +  - "#phy-cells"
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        canphy@0 {
> 
> can-phy if something like this exist. If not, then probably can-transceiver
>
Will fix this.

> > +            compatible = "nxp,tja1145";
> > +            #phy-cells = <0>;
> > +            reg = <0>;
> 
> Please follow DTS coding style.
>
Ok.

Best regards,
Dimitri Fedrau

  reply	other threads:[~2025-07-31  4:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-28 15:39 [PATCH 0/2] phy: add basic support for NXPs TJA1145 CAN transceiver Dimitri Fedrau
2025-07-28 15:39 ` Dimitri Fedrau via B4 Relay
2025-07-28 15:39 ` Dimitri Fedrau via B4 Relay
2025-07-28 15:39 ` [PATCH 1/2] dt-bindings: phy: add " Dimitri Fedrau
2025-07-28 15:39   ` Dimitri Fedrau via B4 Relay
2025-07-28 15:39   ` Dimitri Fedrau via B4 Relay
2025-07-30  7:35   ` Krzysztof Kozlowski
2025-07-30  7:35     ` Krzysztof Kozlowski
2025-07-31  4:48     ` Dimitri Fedrau [this message]
2025-07-31  4:48       ` Dimitri Fedrau
2025-08-03  9:40       ` Krzysztof Kozlowski
2025-08-03  9:40         ` Krzysztof Kozlowski
2025-07-28 15:39 ` [PATCH 2/2] phy: add basic " Dimitri Fedrau
2025-07-28 15:39   ` Dimitri Fedrau via B4 Relay
2025-07-28 15:39   ` Dimitri Fedrau via B4 Relay

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=20250731044824.GA3815@legfed1 \
    --to=dima.fedrau@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dimitri.fedrau@liebherr.com \
    --cc=kishon@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=robh@kernel.org \
    --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.