linux-phy.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v2 6/8] net: ethernet: renesas: rswitch: Add R-Car Gen4 gPTP support
       [not found] ` <20220921084745.3355107-7-yoshihiro.shimoda.uh@renesas.com>
@ 2022-09-21 14:34   ` Richard Cochran
  2022-09-22  0:10     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 21+ messages in thread
From: Richard Cochran @ 2022-09-21 14:34 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: kishon, vkoul, davem, edumazet, kuba, pabeni, robh+dt,
	krzysztof.kozlowski+dt, geert+renesas, andrew, linux-phy, netdev,
	devicetree, linux-renesas-soc

On Wed, Sep 21, 2022 at 05:47:43PM +0900, Yoshihiro Shimoda wrote:

> +static int rcar_gen4_ptp_gettime(struct ptp_clock_info *ptp,
> +				 struct timespec64 *ts)
> +{
> +	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
> +
> +	ts->tv_nsec = ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t0);
> +	ts->tv_sec = ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t1) |
> +		     ((s64)ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t2) << 32);

No locking here ...

> +	return 0;
> +}
> +
> +static int rcar_gen4_ptp_settime(struct ptp_clock_info *ptp,
> +				 const struct timespec64 *ts)
> +{
> +	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
> +
> +	iowrite32(1, ptp_priv->addr + ptp_priv->offs->disable);
> +	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t2);
> +	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t1);
> +	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t0);
> +	iowrite32(1, ptp_priv->addr + ptp_priv->offs->enable);
> +	iowrite32(ts->tv_sec >> 32, ptp_priv->addr + ptp_priv->offs->config_t2);
> +	iowrite32(ts->tv_sec, ptp_priv->addr + ptp_priv->offs->config_t1);
> +	iowrite32(ts->tv_nsec, ptp_priv->addr + ptp_priv->offs->config_t0);

... or here?

You need to protect multiple register access against concurrent callers.

Thanks,
Richard

> +	return 0;
> +}

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
       [not found] <20220921084745.3355107-1-yoshihiro.shimoda.uh@renesas.com>
       [not found] ` <20220921084745.3355107-7-yoshihiro.shimoda.uh@renesas.com>
@ 2022-09-21 14:40 ` Jakub Kicinski
  2022-09-22  0:46   ` Yoshihiro Shimoda
       [not found] ` <20220921084745.3355107-3-yoshihiro.shimoda.uh@renesas.com>
       [not found] ` <20220921084745.3355107-5-yoshihiro.shimoda.uh@renesas.com>
  3 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2022-09-21 14:40 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: kishon, vkoul, davem, edumazet, pabeni, richardcochran, robh+dt,
	krzysztof.kozlowski+dt, geert+renesas, andrew, linux-phy, netdev,
	devicetree, linux-renesas-soc

On Wed, 21 Sep 2022 17:47:37 +0900 Yoshihiro Shimoda wrote:
> Subject: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support

I think you may be slightly confused about the use of the treewide
prefix. Perhaps Geert or one of the upstream-savvy contractors could
help you navigate targeting the correct trees?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 6/8] net: ethernet: renesas: rswitch: Add R-Car Gen4 gPTP support
  2022-09-21 14:34   ` [PATCH v2 6/8] net: ethernet: renesas: rswitch: Add R-Car Gen4 gPTP support Richard Cochran
@ 2022-09-22  0:10     ` Yoshihiro Shimoda
  0 siblings, 0 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-22  0:10 UTC (permalink / raw)
  To: Richard Cochran
  Cc: kishon@ti.com, vkoul@kernel.org, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, andrew@lunn.ch,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

Hi Richard,

Thank you for your review!

> From: Richard Cochran, Sent: Wednesday, September 21, 2022 11:35 PM
> 
> On Wed, Sep 21, 2022 at 05:47:43PM +0900, Yoshihiro Shimoda wrote:
> 
> > +static int rcar_gen4_ptp_gettime(struct ptp_clock_info *ptp,
> > +				 struct timespec64 *ts)
> > +{
> > +	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
> > +
> > +	ts->tv_nsec = ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t0);
> > +	ts->tv_sec = ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t1) |
> > +		     ((s64)ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t2) << 32);
> 
> No locking here ...
> 
> > +	return 0;
> > +}
> > +
> > +static int rcar_gen4_ptp_settime(struct ptp_clock_info *ptp,
> > +				 const struct timespec64 *ts)
> > +{
> > +	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
> > +
> > +	iowrite32(1, ptp_priv->addr + ptp_priv->offs->disable);
> > +	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t2);
> > +	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t1);
> > +	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t0);
> > +	iowrite32(1, ptp_priv->addr + ptp_priv->offs->enable);
> > +	iowrite32(ts->tv_sec >> 32, ptp_priv->addr + ptp_priv->offs->config_t2);
> > +	iowrite32(ts->tv_sec, ptp_priv->addr + ptp_priv->offs->config_t1);
> > +	iowrite32(ts->tv_nsec, ptp_priv->addr + ptp_priv->offs->config_t0);
> 
> ... or here?
> 
> You need to protect multiple register access against concurrent callers.

I understood it. I'll add such protections.

Best regards,
Yoshihiro Shimoda

> Thanks,
> Richard
> 
> > +	return 0;
> > +}

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-21 14:40 ` [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support Jakub Kicinski
@ 2022-09-22  0:46   ` Yoshihiro Shimoda
  2022-09-22  1:06     ` Jakub Kicinski
  2022-09-24  6:57     ` Vinod Koul
  0 siblings, 2 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-22  0:46 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: kishon@ti.com, vkoul@kernel.org, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, richardcochran@gmail.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, andrew@lunn.ch,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

Hi Jakub,

Thank you for your comment!

> From: Jakub Kicinski, Sent: Wednesday, September 21, 2022 11:40 PM
> 
> On Wed, 21 Sep 2022 17:47:37 +0900 Yoshihiro Shimoda wrote:
> > Subject: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
> 
> I think you may be slightly confused about the use of the treewide
> prefix. Perhaps Geert or one of the upstream-savvy contractors could
> help you navigate targeting the correct trees?

I thought we have 2 types about the use of the treewide:
1) Completely depends on multiple subsystems and/or
   change multiple subsystems in a patch.
2) Convenient for review.

This patch series type is the 2) above. However, should I use
treewide for the 1) only?

Best regards,
Yoshihiro Shimoda


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-22  0:46   ` Yoshihiro Shimoda
@ 2022-09-22  1:06     ` Jakub Kicinski
  2022-09-22  1:18       ` Andrew Lunn
  2022-09-22  1:20       ` Yoshihiro Shimoda
  2022-09-24  6:57     ` Vinod Koul
  1 sibling, 2 replies; 21+ messages in thread
From: Jakub Kicinski @ 2022-09-22  1:06 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: kishon@ti.com, vkoul@kernel.org, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, richardcochran@gmail.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, andrew@lunn.ch,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

On Thu, 22 Sep 2022 00:46:34 +0000 Yoshihiro Shimoda wrote:
> I thought we have 2 types about the use of the treewide:
> 1) Completely depends on multiple subsystems and/or
>    change multiple subsystems in a patch.
> 2) Convenient for review.
> 
> This patch series type is the 2) above. However, should I use
> treewide for the 1) only?

I thought "treewide" means you're changing something across the tree.
If you want to get a new platform reviewed I'd just post the patches
as RFC without any prefix in the subject. But I could be wrong.

My main point (which I did a pretty poor job of actually making)
was that for the networking driver to be merged it needs to get
posted separately.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-22  1:06     ` Jakub Kicinski
@ 2022-09-22  1:18       ` Andrew Lunn
  2022-09-22  1:31         ` Yoshihiro Shimoda
  2022-09-22  1:20       ` Yoshihiro Shimoda
  1 sibling, 1 reply; 21+ messages in thread
From: Andrew Lunn @ 2022-09-22  1:18 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Yoshihiro Shimoda, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

On Wed, Sep 21, 2022 at 06:06:40PM -0700, Jakub Kicinski wrote:
> On Thu, 22 Sep 2022 00:46:34 +0000 Yoshihiro Shimoda wrote:
> > I thought we have 2 types about the use of the treewide:
> > 1) Completely depends on multiple subsystems and/or
> >    change multiple subsystems in a patch.
> > 2) Convenient for review.
> > 
> > This patch series type is the 2) above. However, should I use
> > treewide for the 1) only?
> 
> I thought "treewide" means you're changing something across the tree.
> If you want to get a new platform reviewed I'd just post the patches
> as RFC without any prefix in the subject. But I could be wrong.
> 
> My main point (which I did a pretty poor job of actually making)
> was that for the networking driver to be merged it needs to get
> posted separately.

Expanding on that...

You have a clock patch, which should go via the clock subsystem Maintainer.
You have a PHY path, which should go via the generic PHY subsystem Maintainer.
You have an Ethernet driver and binding patch, which can go via netdev,
Cc: the device tree list.
And a patch to add the needed nodes to .dts files which can go via the
renesas Maintainer.

At an early RFC stage, posting them all at once can be useful, to help
see all the bits and pieces. But by the time you have code ready for
merging, it should really go via easu subsystem Maintainer.

All these patches should then meet up in next, and work. If any are
missing, the driver should return -ENODEV or similar.

If there are any compile time dependencies in these patches, then we
need to handle them differently. But at a very quick glance, i don't
see any.

	 Andrew

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-22  1:06     ` Jakub Kicinski
  2022-09-22  1:18       ` Andrew Lunn
@ 2022-09-22  1:20       ` Yoshihiro Shimoda
  1 sibling, 0 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-22  1:20 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: kishon@ti.com, vkoul@kernel.org, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, richardcochran@gmail.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, andrew@lunn.ch,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

Hi Jakub,

Thank you for your reply!

> From: Jakub Kicinski, Sent: Thursday, September 22, 2022 10:07 AM
> 
> On Thu, 22 Sep 2022 00:46:34 +0000 Yoshihiro Shimoda wrote:
> > I thought we have 2 types about the use of the treewide:
> > 1) Completely depends on multiple subsystems and/or
> >    change multiple subsystems in a patch.
> > 2) Convenient for review.
> >
> > This patch series type is the 2) above. However, should I use
> > treewide for the 1) only?
> 
> I thought "treewide" means you're changing something across the tree.
> If you want to get a new platform reviewed I'd just post the patches
> as RFC without any prefix in the subject. But I could be wrong.

Using RFC is reasonable, I think. I'll use this at next time. Thanks!

> My main point (which I did a pretty poor job of actually making)
> was that for the networking driver to be merged it needs to get
> posted separately.

I'll separate patches for the networking driver on v3 patch.

Best regards,
Yoshihiro Shimoda


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-22  1:18       ` Andrew Lunn
@ 2022-09-22  1:31         ` Yoshihiro Shimoda
  0 siblings, 0 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-22  1:31 UTC (permalink / raw)
  To: Andrew Lunn, Jakub Kicinski
  Cc: kishon@ti.com, vkoul@kernel.org, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, richardcochran@gmail.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

Hi Andrew,

> From: Andrew Lunn, Sent: Thursday, September 22, 2022 10:19 AM
> 
> On Wed, Sep 21, 2022 at 06:06:40PM -0700, Jakub Kicinski wrote:
> > On Thu, 22 Sep 2022 00:46:34 +0000 Yoshihiro Shimoda wrote:
> > > I thought we have 2 types about the use of the treewide:
> > > 1) Completely depends on multiple subsystems and/or
> > >    change multiple subsystems in a patch.
> > > 2) Convenient for review.
> > >
> > > This patch series type is the 2) above. However, should I use
> > > treewide for the 1) only?
> >
> > I thought "treewide" means you're changing something across the tree.
> > If you want to get a new platform reviewed I'd just post the patches
> > as RFC without any prefix in the subject. But I could be wrong.
> >
> > My main point (which I did a pretty poor job of actually making)
> > was that for the networking driver to be merged it needs to get
> > posted separately.
> 
> Expanding on that...
> 
> You have a clock patch, which should go via the clock subsystem Maintainer.
> You have a PHY path, which should go via the generic PHY subsystem Maintainer.
> You have an Ethernet driver and binding patch, which can go via netdev,
> Cc: the device tree list.
> And a patch to add the needed nodes to .dts files which can go via the
> renesas Maintainer.
> 
> At an early RFC stage, posting them all at once can be useful, to help
> see all the bits and pieces. But by the time you have code ready for
> merging, it should really go via easu subsystem Maintainer.

Thank you very much for the detailed explanation. I completely understood it.

> All these patches should then meet up in next, and work. If any are
> missing, the driver should return -ENODEV or similar.

Yes, I did test such things.

> If there are any compile time dependencies in these patches, then we
> need to handle them differently. But at a very quick glance, i don't
> see any.

You're correct. This patch series doesn't depend in compile time.

Best regards,
Yoshihiro Shimoda

> 	 Andrew

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/8] dt-bindings: phy: renesas: Document Renesas Ethernet SERDES
       [not found] ` <20220921084745.3355107-3-yoshihiro.shimoda.uh@renesas.com>
@ 2022-09-22  7:28   ` Krzysztof Kozlowski
  2022-09-22  7:39     ` Yoshihiro Shimoda
  0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-22  7:28 UTC (permalink / raw)
  To: Yoshihiro Shimoda, kishon, vkoul, davem, edumazet, kuba, pabeni,
	richardcochran, robh+dt, krzysztof.kozlowski+dt, geert+renesas
  Cc: andrew, linux-phy, netdev, devicetree, linux-renesas-soc

On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> Document Renesas Etherent SERDES for R-Car S4-8 (r8a779f0).
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  .../bindings/phy/renesas,ether-serdes.yaml    | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> new file mode 100644
> index 000000000000..04d650244a6a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml

Filename based on compatible, so renesas,r8a779f0-ether-serdes.yaml

> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/renesas,ether-serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Ethernet SERDES
> +
> +maintainers:
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +properties:
> +  compatible:
> +    const: renesas,r8a779f0-ether-serdes
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  '#phy-cells':
> +    description: Port number of SERDES.
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - resets
> +  - power-domains
> +  - '#phy-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> +    #include <dt-bindings/power/r8a779f0-sysc.h>
> +
> +    ethernet@e6880000 {

Hm, isn't this a phy?

> +            compatible = "renesas,r8a779f0-ether-serdes";
> +            reg = <0xe6444000 0xc00>;
> +            clocks = <&cpg CPG_MOD 1506>;
> +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> +            resets = <&cpg 1506>;
> +            #phy-cells = <1>;
> +    };

Best regards,
Krzysztof


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
       [not found] ` <20220921084745.3355107-5-yoshihiro.shimoda.uh@renesas.com>
@ 2022-09-22  7:37   ` Krzysztof Kozlowski
  2022-09-22 12:13     ` Andrew Lunn
  2022-09-26  6:10     ` Yoshihiro Shimoda
  0 siblings, 2 replies; 21+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-22  7:37 UTC (permalink / raw)
  To: Yoshihiro Shimoda, kishon, vkoul, davem, edumazet, kuba, pabeni,
	richardcochran, robh+dt, krzysztof.kozlowski+dt, geert+renesas
  Cc: andrew, linux-phy, netdev, devicetree, linux-renesas-soc

On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> Document Renesas Etherent Switch for R-Car S4-8 (r8a779f0).
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  .../bindings/net/renesas,etherswitch.yaml     | 286 ++++++++++++++++++
>  1 file changed, 286 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> new file mode 100644
> index 000000000000..988d14f5c54e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml

Isn't dsa directory for this?

> @@ -0,0 +1,286 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/renesas,etherswitch.yaml#

Filename: renesas,r8a779f0-ether-switch.yaml

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Ethernet Switch
> +
> +maintainers:
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +properties:
> +  compatible:
> +    const: renesas,r8a779f0-ether-switch
> +
> +  reg:
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: base
> +      - const: secure_base
> +
> +  interrupts:
> +    maxItems: 47
> +
> +  interrupt-names:
> +    items:
> +      - const: mfwd_error
> +      - const: race_error
> +      - const: coma_error
> +      - const: gwca0_error
> +      - const: gwca1_error
> +      - const: etha0_error
> +      - const: etha1_error
> +      - const: etha2_error
> +      - const: gptp0_status
> +      - const: gptp1_status
> +      - const: mfwd_status
> +      - const: race_status
> +      - const: coma_status
> +      - const: gwca0_status
> +      - const: gwca1_status
> +      - const: etha0_status
> +      - const: etha1_status
> +      - const: etha2_status
> +      - const: rmac0_status
> +      - const: rmac1_status
> +      - const: rmac2_status
> +      - const: gwca0_rxtx0
> +      - const: gwca0_rxtx1
> +      - const: gwca0_rxtx2
> +      - const: gwca0_rxtx3
> +      - const: gwca0_rxtx4
> +      - const: gwca0_rxtx5
> +      - const: gwca0_rxtx6
> +      - const: gwca0_rxtx7
> +      - const: gwca1_rxtx0
> +      - const: gwca1_rxtx1
> +      - const: gwca1_rxtx2
> +      - const: gwca1_rxtx3
> +      - const: gwca1_rxtx4
> +      - const: gwca1_rxtx5
> +      - const: gwca1_rxtx6
> +      - const: gwca1_rxtx7
> +      - const: gwca0_rxts0
> +      - const: gwca0_rxts1
> +      - const: gwca1_rxts0
> +      - const: gwca1_rxts1
> +      - const: rmac0_mdio
> +      - const: rmac1_mdio
> +      - const: rmac2_mdio
> +      - const: rmac0_phy
> +      - const: rmac1_phy
> +      - const: rmac2_phy
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: fck
> +      - const: tsn
> +
> +  resets:
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: rswitch2
> +      - const: tsn
> +
> +  iommus:
> +    maxItems: 16
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  ethernet-ports:
> +    type: object
> +
> +    properties:
> +      '#address-cells':
> +        description: Port number of ETHA (TSNA).
> +        const: 1

Blank line

> +      '#size-cells':
> +        const: 0
> +
> +    additionalProperties: false

Don't put it between properties. For nested object usually this is
before properties:

> +
> +    patternProperties:
> +      "^port@[0-9a-f]+$":
> +        type: object
> +

Skip blank line.

> +        $ref: "/schemas/net/ethernet-controller.yaml#"

No need for quotes.

> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            description:
> +              Port number of ETHA (TSNA).
> +
> +          phy-handle:
> +            description:
> +              Phandle of an Ethernet PHY.

Why do you need to mention this property? Isn't it coming from
ethernet-controller.yaml?

> +
> +          phy-mode:
> +            description:
> +              This specifies the interface used by the Ethernet PHY.
> +            enum:
> +              - mii
> +              - sgmii
> +              - usxgmii
> +
> +          phys:
> +            maxItems: 1
> +            description:
> +              Phandle of an Ethernet SERDES.

This is getting confusing. You have now:
- phy-handle
- phy
- phy-device
- phys
in one schema... although lan966x serdes seems to do the same. :/

> +
> +          mdio:
> +            $ref: "/schemas/net/mdio.yaml#"

No need for quotes. Are you sure this is property of each port? I don't
know the net/ethernet bindings that good, so I need to ask sometimes
basic questions. Other bindings seem to do it differently a bit.

> +            unevaluatedProperties: false
> +
> +        required:
> +          - phy-handle
> +          - phy-mode
> +          - phys
> +          - mdio
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - power-domains
> +  - ethernet-ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/r8a779f0-sysc.h>
> +
> +    ethernet@e6880000 {
> +            compatible = "renesas,r8a779f0-ether-switch";

Wrong indentation. Use 4 spaces.

> +            reg = <0xe6880000 0x20000>, <0xe68c0000 0x20000>;
> +            reg-names = "base", "secure_base";
> +            interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH>,
Best regards,
Krzysztof


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 2/8] dt-bindings: phy: renesas: Document Renesas Ethernet SERDES
  2022-09-22  7:28   ` [PATCH v2 2/8] dt-bindings: phy: renesas: Document Renesas Ethernet SERDES Krzysztof Kozlowski
@ 2022-09-22  7:39     ` Yoshihiro Shimoda
  2022-09-22  7:56       ` Geert Uytterhoeven
  0 siblings, 1 reply; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-22  7:39 UTC (permalink / raw)
  To: Krzysztof Kozlowski, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be
  Cc: andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

Hi Krzysztof,

Thank you for your review!

> From: Krzysztof Kozlowski, Sent: Thursday, September 22, 2022 4:29 PM
> 
> On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> > Document Renesas Etherent SERDES for R-Car S4-8 (r8a779f0).
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  .../bindings/phy/renesas,ether-serdes.yaml    | 54 +++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > new file mode 100644
> > index 000000000000..04d650244a6a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> 
> Filename based on compatible, so renesas,r8a779f0-ether-serdes.yaml

I got it. I'll rename the file.

> > @@ -0,0 +1,54 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
<snip>
> > +
> > +title: Renesas Ethernet SERDES
> > +
> > +maintainers:
> > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: renesas,r8a779f0-ether-serdes
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  '#phy-cells':
> > +    description: Port number of SERDES.
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - resets
> > +  - power-domains
> > +  - '#phy-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> > +    #include <dt-bindings/power/r8a779f0-sysc.h>
> > +
> > +    ethernet@e6880000 {
> 
> Hm, isn't this a phy?

Oops. I copied and pasted this from other patch...
I'll fix this as "serdes@e6444000".

Best regards,
Yoshihiro Shimoda

> > +            compatible = "renesas,r8a779f0-ether-serdes";
> > +            reg = <0xe6444000 0xc00>;
> > +            clocks = <&cpg CPG_MOD 1506>;
> > +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> > +            resets = <&cpg 1506>;
> > +            #phy-cells = <1>;
> > +    };
> 
> Best regards,
> Krzysztof


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/8] dt-bindings: phy: renesas: Document Renesas Ethernet SERDES
  2022-09-22  7:39     ` Yoshihiro Shimoda
@ 2022-09-22  7:56       ` Geert Uytterhoeven
  2022-09-22  8:20         ` Yoshihiro Shimoda
  0 siblings, 1 reply; 21+ messages in thread
From: Geert Uytterhoeven @ 2022-09-22  7:56 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: Krzysztof Kozlowski, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be,
	andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

Hi Shimoda-san,

On Thu, Sep 22, 2022 at 9:39 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Krzysztof Kozlowski, Sent: Thursday, September 22, 2022 4:29 PM
> > On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> > > Document Renesas Etherent SERDES for R-Car S4-8 (r8a779f0).
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > >  .../bindings/phy/renesas,ether-serdes.yaml    | 54 +++++++++++++++++++
> > >  1 file changed, 54 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > > new file mode 100644
> > > index 000000000000..04d650244a6a
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> >
> > Filename based on compatible, so renesas,r8a779f0-ether-serdes.yaml
>
> I got it. I'll rename the file.

Is this serdes present on other R-Car Gen4 SoCs, or is it (so far) only
found on R-Car S4-8?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 2/8] dt-bindings: phy: renesas: Document Renesas Ethernet SERDES
  2022-09-22  7:56       ` Geert Uytterhoeven
@ 2022-09-22  8:20         ` Yoshihiro Shimoda
  0 siblings, 0 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-22  8:20 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Krzysztof Kozlowski, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be,
	andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, September 22, 2022 4:56 PM
> 
> Hi Shimoda-san,
> 
> On Thu, Sep 22, 2022 at 9:39 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Krzysztof Kozlowski, Sent: Thursday, September 22, 2022 4:29 PM
> > > On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> > > > Document Renesas Etherent SERDES for R-Car S4-8 (r8a779f0).
> > > >
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > ---
> > > >  .../bindings/phy/renesas,ether-serdes.yaml    | 54 +++++++++++++++++++
> > > >  1 file changed, 54 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > > b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > > > new file mode 100644
> > > > index 000000000000..04d650244a6a
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/renesas,ether-serdes.yaml
> > >
> > > Filename based on compatible, so renesas,r8a779f0-ether-serdes.yaml
> >
> > I got it. I'll rename the file.
> 
> Is this serdes present on other R-Car Gen4 SoCs, or is it (so far) only
> found on R-Car S4-8?

So far it's only found on R-Car S4 series.

Best regards,
Yoshihiro Shimoda

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
  2022-09-22  7:37   ` [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch Krzysztof Kozlowski
@ 2022-09-22 12:13     ` Andrew Lunn
  2022-09-26  6:10     ` Yoshihiro Shimoda
  1 sibling, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2022-09-22 12:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yoshihiro Shimoda, kishon, vkoul, davem, edumazet, kuba, pabeni,
	richardcochran, robh+dt, krzysztof.kozlowski+dt, geert+renesas,
	linux-phy, netdev, devicetree, linux-renesas-soc

On Thu, Sep 22, 2022 at 09:37:28AM +0200, Krzysztof Kozlowski wrote:
> On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> > Document Renesas Etherent Switch for R-Car S4-8 (r8a779f0).
> > 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  .../bindings/net/renesas,etherswitch.yaml     | 286 ++++++++++++++++++
> >  1 file changed, 286 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> > new file mode 100644
> > index 000000000000..988d14f5c54e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> 
> Isn't dsa directory for this?

This is not a DSA driver, but a pure switchdev driver. It is similar
to prestera, lan966x, sparx5, which put there binding in net.

   Andrew

   

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-22  0:46   ` Yoshihiro Shimoda
  2022-09-22  1:06     ` Jakub Kicinski
@ 2022-09-24  6:57     ` Vinod Koul
  2022-09-26  0:21       ` Yoshihiro Shimoda
  1 sibling, 1 reply; 21+ messages in thread
From: Vinod Koul @ 2022-09-24  6:57 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: Jakub Kicinski, kishon@ti.com, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, richardcochran@gmail.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, andrew@lunn.ch,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

On 22-09-22, 00:46, Yoshihiro Shimoda wrote:
> Hi Jakub,
> 
> Thank you for your comment!
> 
> > From: Jakub Kicinski, Sent: Wednesday, September 21, 2022 11:40 PM
> > 
> > On Wed, 21 Sep 2022 17:47:37 +0900 Yoshihiro Shimoda wrote:
> > > Subject: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
> > 
> > I think you may be slightly confused about the use of the treewide
> > prefix. Perhaps Geert or one of the upstream-savvy contractors could
> > help you navigate targeting the correct trees?
> 
> I thought we have 2 types about the use of the treewide:
> 1) Completely depends on multiple subsystems and/or
>    change multiple subsystems in a patch.
> 2) Convenient for review.
> 
> This patch series type is the 2) above. However, should I use
> treewide for the 1) only?

No, How is it convenient for review.. I would like to see a series just
for phy... I dont need to see the whole other things...

Maybe Convenient for you to toss the pile to upstream reviewers, surely
not for us!

-- 
~Vinod

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
  2022-09-24  6:57     ` Vinod Koul
@ 2022-09-26  0:21       ` Yoshihiro Shimoda
  0 siblings, 0 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-26  0:21 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Jakub Kicinski, kishon@ti.com, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, richardcochran@gmail.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	geert+renesas@glider.be, andrew@lunn.ch,
	linux-phy@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org

Hi Vinod,

> From: Vinod Koul, Sent: Saturday, September 24, 2022 3:57 PM
> 
> On 22-09-22, 00:46, Yoshihiro Shimoda wrote:
> > Hi Jakub,
> >
> > Thank you for your comment!
> >
> > > From: Jakub Kicinski, Sent: Wednesday, September 21, 2022 11:40 PM
> > >
> > > On Wed, 21 Sep 2022 17:47:37 +0900 Yoshihiro Shimoda wrote:
> > > > Subject: [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support
> > >
> > > I think you may be slightly confused about the use of the treewide
> > > prefix. Perhaps Geert or one of the upstream-savvy contractors could
> > > help you navigate targeting the correct trees?
> >
> > I thought we have 2 types about the use of the treewide:
> > 1) Completely depends on multiple subsystems and/or
> >    change multiple subsystems in a patch.
> > 2) Convenient for review.
> >
> > This patch series type is the 2) above. However, should I use
> > treewide for the 1) only?
> 
> No, How is it convenient for review.. I would like to see a series just
> for phy... I dont need to see the whole other things...
> 
> Maybe Convenient for you to toss the pile to upstream reviewers, surely
> not for us!

Thank you very much for your comments. I understood it.
I will not submit this type of treewide from now on.

Best regards,
Yoshihiro Shimoda


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
  2022-09-22  7:37   ` [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch Krzysztof Kozlowski
  2022-09-22 12:13     ` Andrew Lunn
@ 2022-09-26  6:10     ` Yoshihiro Shimoda
  2022-09-26  6:50       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-26  6:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be
  Cc: andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

Hi Krzysztof,

> From: Krzysztof Kozlowski, Sent: Thursday, September 22, 2022 4:37 PM
> 
> On 21/09/2022 10:47, Yoshihiro Shimoda wrote:
> > Document Renesas Etherent Switch for R-Car S4-8 (r8a779f0).
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  .../bindings/net/renesas,etherswitch.yaml     | 286 ++++++++++++++++++
> >  1 file changed, 286 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> > new file mode 100644
> > index 000000000000..988d14f5c54e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml
> 
> Isn't dsa directory for this?

As Andrew mentioned, this is not a DSA driver.

> > @@ -0,0 +1,286 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/renesas,etherswitch.yaml#
> 
> Filename: renesas,r8a779f0-ether-switch.yaml

I'll rename this file.

> > +$schema:
<snip>
> > +
> > +title: Renesas Ethernet Switch
> > +
> > +maintainers:
> > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: renesas,r8a779f0-ether-switch
> > +
> > +  reg:
> > +    maxItems: 2
> > +
> > +  reg-names:
> > +    items:
> > +      - const: base
> > +      - const: secure_base
> > +
> > +  interrupts:
> > +    maxItems: 47
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: mfwd_error
> > +      - const: race_error
> > +      - const: coma_error
> > +      - const: gwca0_error
> > +      - const: gwca1_error
> > +      - const: etha0_error
> > +      - const: etha1_error
> > +      - const: etha2_error
> > +      - const: gptp0_status
> > +      - const: gptp1_status
> > +      - const: mfwd_status
> > +      - const: race_status
> > +      - const: coma_status
> > +      - const: gwca0_status
> > +      - const: gwca1_status
> > +      - const: etha0_status
> > +      - const: etha1_status
> > +      - const: etha2_status
> > +      - const: rmac0_status
> > +      - const: rmac1_status
> > +      - const: rmac2_status
> > +      - const: gwca0_rxtx0
> > +      - const: gwca0_rxtx1
> > +      - const: gwca0_rxtx2
> > +      - const: gwca0_rxtx3
> > +      - const: gwca0_rxtx4
> > +      - const: gwca0_rxtx5
> > +      - const: gwca0_rxtx6
> > +      - const: gwca0_rxtx7
> > +      - const: gwca1_rxtx0
> > +      - const: gwca1_rxtx1
> > +      - const: gwca1_rxtx2
> > +      - const: gwca1_rxtx3
> > +      - const: gwca1_rxtx4
> > +      - const: gwca1_rxtx5
> > +      - const: gwca1_rxtx6
> > +      - const: gwca1_rxtx7
> > +      - const: gwca0_rxts0
> > +      - const: gwca0_rxts1
> > +      - const: gwca1_rxts0
> > +      - const: gwca1_rxts1
> > +      - const: rmac0_mdio
> > +      - const: rmac1_mdio
> > +      - const: rmac2_mdio
> > +      - const: rmac0_phy
> > +      - const: rmac1_phy
> > +      - const: rmac2_phy
> > +
> > +  clocks:
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      - const: fck
> > +      - const: tsn
> > +
> > +  resets:
> > +    maxItems: 2
> > +
> > +  reset-names:
> > +    items:
> > +      - const: rswitch2
> > +      - const: tsn
> > +
> > +  iommus:
> > +    maxItems: 16
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  ethernet-ports:
> > +    type: object
> > +
> > +    properties:
> > +      '#address-cells':
> > +        description: Port number of ETHA (TSNA).
> > +        const: 1
> 
> Blank line

I'll add a blank line here.

> > +      '#size-cells':
> > +        const: 0
> > +
> > +    additionalProperties: false
> 
> Don't put it between properties. For nested object usually this is
> before properties:

I'll drop it.

> > +
> > +    patternProperties:
> > +      "^port@[0-9a-f]+$":
> > +        type: object
> > +
> 
> Skip blank line.

I got it.

> > +        $ref: "/schemas/net/ethernet-controller.yaml#"
> 
> No need for quotes.

I'll drop the quotes.

> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            description:
> > +              Port number of ETHA (TSNA).
> > +
> > +          phy-handle:
> > +            description:
> > +              Phandle of an Ethernet PHY.
> 
> Why do you need to mention this property? Isn't it coming from
> ethernet-controller.yaml?

Indeed. I'll drop the description.

> > +
> > +          phy-mode:
> > +            description:
> > +              This specifies the interface used by the Ethernet PHY.
> > +            enum:
> > +              - mii
> > +              - sgmii
> > +              - usxgmii
> > +
> > +          phys:
> > +            maxItems: 1
> > +            description:
> > +              Phandle of an Ethernet SERDES.
> 
> This is getting confusing. You have now:
> - phy-handle
> - phy
> - phy-device
> - phys
> in one schema... although lan966x serdes seems to do the same. :/

Yes... I found the following documents have "phy" and "phy-handle" by using
git grep -l -w "phys" `git grep -l phy-handle Documentation/devicetree/bindings/`:
Documentation/devicetree/bindings/net/cdns,macb.yaml
Documentation/devicetree/bindings/net/cpsw.txt
Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
Documentation/devicetree/bindings/phy/phy-bindings.txt

And I'm interesting that the phy-bindings.txt said the following:
-----
phys : the phandle for the PHY device (used by the PHY subsystem; not to be
       confused with the Ethernet specific 'phy' and 'phy-handle' properties,
       see Documentation/devicetree/bindings/net/ethernet.txt for these)
-----

> > +
> > +          mdio:
> > +            $ref: "/schemas/net/mdio.yaml#"
> 
> No need for quotes.

I got it.

> Are you sure this is property of each port? I don't
> know the net/ethernet bindings that good, so I need to ask sometimes
> basic questions. Other bindings seem to do it differently a bit.

Yes, each port has mdio bus.

> > +            unevaluatedProperties: false
> > +
> > +        required:
> > +          - phy-handle
> > +          - phy-mode
> > +          - phys
> > +          - mdio
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - clock-names
> > +  - resets
> > +  - power-domains
> > +  - ethernet-ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/power/r8a779f0-sysc.h>
> > +
> > +    ethernet@e6880000 {
> > +            compatible = "renesas,r8a779f0-ether-switch";
> 
> Wrong indentation. Use 4 spaces.

I'll fix it.

Best regards,
Yoshihiro Shimoda

> > +            reg = <0xe6880000 0x20000>, <0xe68c0000 0x20000>;
> > +            reg-names = "base", "secure_base";
> > +            interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH>,
> Best regards,
> Krzysztof


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
  2022-09-26  6:10     ` Yoshihiro Shimoda
@ 2022-09-26  6:50       ` Krzysztof Kozlowski
  2022-09-26  9:14         ` Yoshihiro Shimoda
  0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-26  6:50 UTC (permalink / raw)
  To: Yoshihiro Shimoda, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be
  Cc: andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

On 26/09/2022 08:10, Yoshihiro Shimoda wrote:

> I'll add a blank line here.
> 
>>> +      '#size-cells':
>>> +        const: 0
>>> +
>>> +    additionalProperties: false
>>
>> Don't put it between properties. For nested object usually this is
>> before properties:
> 
> I'll drop it.

Don't drop, but instead put it before "properties" for this nested object.

> 
>>> +
>>> +    patternProperties:
>>> +      "^port@[0-9a-f]+$":
>>> +        type: object
>>> +
>>
>> Skip blank line.
> 
> I got it.
> 
>>> +        $ref: "/schemas/net/ethernet-controller.yaml#"
>>
>> No need for quotes.
> 
> I'll drop the quotes.
> 
>>> +        unevaluatedProperties: false
>>> +
>>> +        properties:
>>> +          reg:
>>> +            description:
>>> +              Port number of ETHA (TSNA).
>>> +
>>> +          phy-handle:
>>> +            description:
>>> +              Phandle of an Ethernet PHY.
>>
>> Why do you need to mention this property? Isn't it coming from
>> ethernet-controller.yaml?
> 
> Indeed. I'll drop the description.
> 
>>> +
>>> +          phy-mode:
>>> +            description:
>>> +              This specifies the interface used by the Ethernet PHY.
>>> +            enum:
>>> +              - mii
>>> +              - sgmii
>>> +              - usxgmii
>>> +
>>> +          phys:
>>> +            maxItems: 1
>>> +            description:
>>> +              Phandle of an Ethernet SERDES.
>>
>> This is getting confusing. You have now:
>> - phy-handle
>> - phy
>> - phy-device
>> - phys
>> in one schema... although lan966x serdes seems to do the same. :/
> 
> Yes... I found the following documents have "phy" and "phy-handle" by using
> git grep -l -w "phys" `git grep -l phy-handle Documentation/devicetree/bindings/`:
> Documentation/devicetree/bindings/net/cdns,macb.yaml
> Documentation/devicetree/bindings/net/cpsw.txt
> Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
> Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
> Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> And I'm interesting that the phy-bindings.txt said the following:
> -----
> phys : the phandle for the PHY device (used by the PHY subsystem; not to be
>        confused with the Ethernet specific 'phy' and 'phy-handle' properties,
>        see Documentation/devicetree/bindings/net/ethernet.txt for these)
> -----

Indeed, seems ok.

> 

Best regards,
Krzysztof


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
  2022-09-26  6:50       ` Krzysztof Kozlowski
@ 2022-09-26  9:14         ` Yoshihiro Shimoda
  2022-09-26  9:25           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-26  9:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be
  Cc: andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

> From: Krzysztof Kozlowski, Sent: Monday, September 26, 2022 3:50 PM
> 
> On 26/09/2022 08:10, Yoshihiro Shimoda wrote:
> 
> > I'll add a blank line here.
> >
> >>> +      '#size-cells':
> >>> +        const: 0
> >>> +
> >>> +    additionalProperties: false
> >>
> >> Don't put it between properties. For nested object usually this is
> >> before properties:
> >
> > I'll drop it.
> 
> Don't drop, but instead put it before "properties" for this nested object.

Oh, I got it. Thanks!
I'll put this before "properties:" like below:
-----
  ethernet-ports:
    type: object

    additionalProperties: false

    properties:
      '#address-cells':
        description: Port number of ETHA (TSNA).
        const: 1

      '#size-cells':
        const: 0
-----

Best regards,
Yoshihiro Shimoda

> >
> >>> +
> >>> +    patternProperties:
> >>> +      "^port@[0-9a-f]+$":
> >>> +        type: object
> >>> +
> >>
> >> Skip blank line.
> >
> > I got it.
> >
> >>> +        $ref: "/schemas/net/ethernet-controller.yaml#"
> >>
> >> No need for quotes.
> >
> > I'll drop the quotes.
> >
> >>> +        unevaluatedProperties: false
> >>> +
> >>> +        properties:
> >>> +          reg:
> >>> +            description:
> >>> +              Port number of ETHA (TSNA).
> >>> +
> >>> +          phy-handle:
> >>> +            description:
> >>> +              Phandle of an Ethernet PHY.
> >>
> >> Why do you need to mention this property? Isn't it coming from
> >> ethernet-controller.yaml?
> >
> > Indeed. I'll drop the description.
> >
> >>> +
> >>> +          phy-mode:
> >>> +            description:
> >>> +              This specifies the interface used by the Ethernet PHY.
> >>> +            enum:
> >>> +              - mii
> >>> +              - sgmii
> >>> +              - usxgmii
> >>> +
> >>> +          phys:
> >>> +            maxItems: 1
> >>> +            description:
> >>> +              Phandle of an Ethernet SERDES.
> >>
> >> This is getting confusing. You have now:
> >> - phy-handle
> >> - phy
> >> - phy-device
> >> - phys
> >> in one schema... although lan966x serdes seems to do the same. :/
> >
> > Yes... I found the following documents have "phy" and "phy-handle" by using
> > git grep -l -w "phys" `git grep -l phy-handle Documentation/devicetree/bindings/`:
> > Documentation/devicetree/bindings/net/cdns,macb.yaml
> > Documentation/devicetree/bindings/net/cpsw.txt
> > Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
> > Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
> > Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> > Documentation/devicetree/bindings/phy/phy-bindings.txt
> >
> > And I'm interesting that the phy-bindings.txt said the following:
> > -----
> > phys : the phandle for the PHY device (used by the PHY subsystem; not to be
> >        confused with the Ethernet specific 'phy' and 'phy-handle' properties,
> >        see Documentation/devicetree/bindings/net/ethernet.txt for these)
> > -----
> 
> Indeed, seems ok.
> 
> >
> 
> Best regards,
> Krzysztof

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
  2022-09-26  9:14         ` Yoshihiro Shimoda
@ 2022-09-26  9:25           ` Krzysztof Kozlowski
  2022-09-26 10:20             ` Yoshihiro Shimoda
  0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-26  9:25 UTC (permalink / raw)
  To: Yoshihiro Shimoda, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be
  Cc: andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

On 26/09/2022 11:14, Yoshihiro Shimoda wrote:

>>
>> Don't drop, but instead put it before "properties" for this nested object.
> 
> Oh, I got it. Thanks!
> I'll put this before "properties:" like below:
> -----
>   ethernet-ports:
>     type: object
> 

Without blank line here.

>     additionalProperties: false
>>     properties:

This is ok.

>       '#address-cells':
>         description: Port number of ETHA (TSNA).
>         const: 1
> 
>       '#size-cells':
>         const: 0
> -----

Best regards,
Krzysztof


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch
  2022-09-26  9:25           ` Krzysztof Kozlowski
@ 2022-09-26 10:20             ` Yoshihiro Shimoda
  0 siblings, 0 replies; 21+ messages in thread
From: Yoshihiro Shimoda @ 2022-09-26 10:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski, kishon@ti.com, vkoul@kernel.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, richardcochran@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, geert+renesas@glider.be
  Cc: andrew@lunn.ch, linux-phy@lists.infradead.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org

> From: Krzysztof Kozlowski, Sent: Monday, September 26, 2022 6:25 PM
> 
> On 26/09/2022 11:14, Yoshihiro Shimoda wrote:
> 
> >>
> >> Don't drop, but instead put it before "properties" for this nested object.
> >
> > Oh, I got it. Thanks!
> > I'll put this before "properties:" like below:
> > -----
> >   ethernet-ports:
> >     type: object
> >
> 
> Without blank line here.

I got it. Thank you very much for your support!

Best regards,
Yoshihiro Shimoda

> >     additionalProperties: false
> >>     properties:
> 
> This is ok.
> 
> >       '#address-cells':
> >         description: Port number of ETHA (TSNA).
> >         const: 1
> >
> >       '#size-cells':
> >         const: 0
> > -----
> 
> Best regards,
> Krzysztof

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-09-26 10:22 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20220921084745.3355107-1-yoshihiro.shimoda.uh@renesas.com>
     [not found] ` <20220921084745.3355107-7-yoshihiro.shimoda.uh@renesas.com>
2022-09-21 14:34   ` [PATCH v2 6/8] net: ethernet: renesas: rswitch: Add R-Car Gen4 gPTP support Richard Cochran
2022-09-22  0:10     ` Yoshihiro Shimoda
2022-09-21 14:40 ` [PATCH v2 0/8] treewide: Add R-Car S4-8 Ethernet Switch support Jakub Kicinski
2022-09-22  0:46   ` Yoshihiro Shimoda
2022-09-22  1:06     ` Jakub Kicinski
2022-09-22  1:18       ` Andrew Lunn
2022-09-22  1:31         ` Yoshihiro Shimoda
2022-09-22  1:20       ` Yoshihiro Shimoda
2022-09-24  6:57     ` Vinod Koul
2022-09-26  0:21       ` Yoshihiro Shimoda
     [not found] ` <20220921084745.3355107-3-yoshihiro.shimoda.uh@renesas.com>
2022-09-22  7:28   ` [PATCH v2 2/8] dt-bindings: phy: renesas: Document Renesas Ethernet SERDES Krzysztof Kozlowski
2022-09-22  7:39     ` Yoshihiro Shimoda
2022-09-22  7:56       ` Geert Uytterhoeven
2022-09-22  8:20         ` Yoshihiro Shimoda
     [not found] ` <20220921084745.3355107-5-yoshihiro.shimoda.uh@renesas.com>
2022-09-22  7:37   ` [PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch Krzysztof Kozlowski
2022-09-22 12:13     ` Andrew Lunn
2022-09-26  6:10     ` Yoshihiro Shimoda
2022-09-26  6:50       ` Krzysztof Kozlowski
2022-09-26  9:14         ` Yoshihiro Shimoda
2022-09-26  9:25           ` Krzysztof Kozlowski
2022-09-26 10:20             ` Yoshihiro Shimoda

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).