Devicetree
 help / color / mirror / Atom feed
From: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"DavidS. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [net-next 1/3] dt-bindings: ptp: renesas,rcar-gen4-gptp: Add binding for R-Car Gen4
Date: Wed, 10 Jun 2026 10:53:54 +0200	[thread overview]
Message-ID: <20260610085354.GC2465390@ragnatech.se> (raw)
In-Reply-To: <6bd0229b-4895-48a2-9e36-0ea5296c7fb5@kernel.org>

Hi Krzysztof,

Thanks for your comments.

On 2026-06-10 08:54:06 +0200, Krzysztof Kozlowski wrote:
> On 09/06/2026 23:57, Niklas Söderlund wrote:
> > Add bindings for the R-Car Gen4 gPTP timer. The timer enables accurate
> > synchronization of the clock in the control system. The timer is
> > system-wide and used by different Ethernet devices on each Gen4 platform.
> 
> A nit, subject: drop second/last, redundant "binding for". The
> "dt-bindings" prefix is already stating that these are bindings.

Thanks, will fix.

> See also:
> https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Not sure I follow this one, L18 reads,

  "Few subsystems, like ASoC, media, regulators and SPI, expect reverse 
  order of the prefixes::"

But 'git log Documentation/devicetree/bindings/ptp/' shows all commits 
in that directory use the 'dt-bindings: ptp:' prefix.

> 
> 
> > 
> >   - On R-Car S4 it is shared between RSWITCH and RAVB.
> > 
> >   - On R-Car V4H it is shared between RTSN and RAVB.
> > 
> >   - On R-Car V4M it is only used by RAVB.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  .../bindings/ptp/renesas,rcar-gen4-gptp.yaml  | 64 +++++++++++++++++++
> >  MAINTAINERS                                   |  6 ++
> >  2 files changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> > new file mode 100644
> > index 000000000000..99e6e3ca73b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +# Copyright (C) 2026 Renesas Electronics Corp.
> > +# Copyright (C) 2026 Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/ptp/renesas,rcar-gen4-gptp.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas R-Car Gen4 gPTP timer
> > +
> > +maintainers:
> > +  - Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > +
> > +description:
> > +  The R-Car Gen4 gPTP timer enables accurate synchronization of the clock in
> > +  the control system. The timer is system-wide and used by different Ethernet
> > +  devices on each Gen4 platform.
> > +
> > +    - On R-Car S4 it is shared between RSWITCH and RAVB.
> > +    - On R-Car V4H it is shared between RTSN and RAVB.
> > +    - On R-Car V4M it is only used by RAVB.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - renesas,r8a779f0-gptp # S4-8
> > +          - renesas,r8a779g0-gptp # V4H
> > +          - renesas,r8a779h0-gptp # V4M
> > +      - const: renesas,rcar-gen4-gptp # Generic R-Car Gen4
> 
> Please drop comment or drop generic compatible and make it specific.
> Generic compatibles are discouraged, so don't advertise that. Look how
> other RECENT Renesas bindings do it.

Thanks, I will drop the 'Generic R-Car Gen4' comment. The reason is to 
reduce cruft in the driver. My current view is that there are no 
platform specific quirks needed, but experience show this sometimes 
happens.

Looking at RECENT Renesas bindings this seems to be the way.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - power-domains
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779g0-cpg-mssr.h>
> > +    #include <dt-bindings/power/r8a779g0-sysc.h>
> > +
> > +    gptp: gptp@e6449000 {
> 
> Drop unused label. Node name usually is "phc". Could be "ptp", but not
> "gptp". What is gptp in generic names?

Wops, thanks. Indeed there should be no label and the node named ptp.  
gptp is the name used in the R-Car documentation and I must have typed 
it from muscle memory, my bad.

> 
> >  L:	linux-iio@vger.kernel.org
> 
> 
> Best regards,
> Krzysztof

-- 
Kind Regards,
Niklas Söderlund

  reply	other threads:[~2026-06-10  8:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 21:57 [net-next 0/3] ptp: Add driver for R-Car Gen4 gPTP timer Niklas Söderlund
2026-06-09 21:57 ` [net-next 1/3] dt-bindings: ptp: renesas,rcar-gen4-gptp: Add binding for R-Car Gen4 Niklas Söderlund
2026-06-10  6:54   ` Krzysztof Kozlowski
2026-06-10  8:53     ` Niklas Söderlund [this message]
2026-06-10  9:01       ` Krzysztof Kozlowski
2026-06-10  9:14         ` Niklas Söderlund
2026-06-09 21:57 ` [net-next 2/3] ptp: Add driver " Niklas Söderlund
2026-06-09 21:57 ` [net-next 3/3] arm64: dts: renesas: r8a779g0: Add gPTP node Niklas Söderlund
2026-06-10  5:02 ` [net-next 0/3] ptp: Add driver for R-Car Gen4 gPTP timer Michael Dege
2026-06-10  7:14   ` Niklas Söderlund

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=20260610085354.GC2465390@ragnatech.se \
    --to=niklas.soderlund+renesas@ragnatech.se \
    --cc=andrew+netdev@lunn.ch \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=geert+renesas@glider.be \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=robh@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox