linux-kernel - Re: [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add support for GPIO2 clock output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241211105100.GA4424@debian>
Date: Wed, 11 Dec 2024 11:51:00 +0100
From: Dimitri Fedrau <dima.fedrau@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Dimitri Fedrau <dimitri.fedrau@...bherr.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Andrew Davis <afd@...com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/2] dt-bindings: net: dp83822: Add support
 for GPIO2 clock output

Am Wed, Dec 11, 2024 at 10:43:40AM +0100 schrieb Krzysztof Kozlowski:
> On Wed, Dec 11, 2024 at 09:04:39AM +0100, Dimitri Fedrau wrote:
> > The GPIO2 pin on the DP83822 can be configured as clock output. Add
> > binding to support this feature.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@...bherr.com>
> > ---
> >  .../devicetree/bindings/net/ti,dp83822.yaml         |  7 +++++++
> >  include/dt-bindings/net/ti-dp83822.h                | 21 +++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ti,dp83822.yaml b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > index 784866ea392b2083e93d8dc9aaea93b70dc80934..4a4dc794f21162c6a61c3daeeffa08e666034679 100644
> > --- a/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > +++ b/Documentation/devicetree/bindings/net/ti,dp83822.yaml
> > @@ -96,6 +96,13 @@ properties:
> >        - master
> >        - slave
> >  
> > +  ti,gpio2-clk-out:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +       DP83822 PHY only.
> > +       Muxing option for GPIO2 pin. See dt-bindings/net/ti-dp83822.h for
> > +       applicable values. When omitted, the PHY's default will be left as is.
> 
> 1. Missing constraints, this looks like enum.
> 2. Missing explanation of values.
> 3. This should be most likely a string.
> 4. Extend your example with this. 
>
Ok, will fix it.

> > +
> >  required:
> >    - reg
> >  
> > diff --git a/include/dt-bindings/net/ti-dp83822.h b/include/dt-bindings/net/ti-dp83822.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..d569c90618b7bcae9ffe44eb041f7dae2e74e5d1
> > --- /dev/null
> > +++ b/include/dt-bindings/net/ti-dp83822.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> > +/*
> > + * Device Tree constants for the Texas Instruments DP83822 PHY
> > + *
> > + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
> > + *
> > + * Author: Dimitri Fedrau <dimitri.fedrau@...bherr.com>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_TI_DP83822_H
> > +#define _DT_BINDINGS_TI_DP83822_H
> > +
> > +/* IO_MUX_GPIO_CTRL - Clock source selection */
> > +#define DP83822_CLK_SRC_MAC_IF			0x0
> > +#define DP83822_CLK_SRC_XI			0x1
> > +#define DP83822_CLK_SRC_INT_REF			0x2
> > +#define DP83822_CLK_SRC_RMII_MASTER_MODE_REF	0x4
> > +#define DP83822_CLK_SRC_FREE_RUNNING		0x6
> > +#define DP83822_CLK_SRC_RECOVERED		0x7
> 
> These are not really bindings but some register values. Hex numbers
> indicate that. Don't store register values as bindings, because this
> is neither necessary nor helping.
>
Ok, got it. Have seen similar in <dt-bindings/net/ti-dp83867.h> or
<dt-bindings/net/ti-dp83869.h>, is it wrong there ?

Best regards,
Dimitri Fedrau