From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= <clement.leger@bootlin.com>
CC:     Julian Wiedmann <jwi@linux.ibm.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        Andrew Lunn <andrew@lunn.ch>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Denis Kirjanov <dkirjanov@suse.de>
Subject: Re: [PATCH net-next v3 1/4] dt-bindings: net: mscc,vsc7514-switch:
 convert txt bindings to yaml
Thread-Topic: [PATCH net-next v3 1/4] dt-bindings: net: mscc,vsc7514-switch:
 convert txt bindings to yaml
Thread-Index: AQHX4uryalRRqJ6wdEegqX4WYakZdqwWFj8AgAADCICAAAECgA==
Date:   Fri, 26 Nov 2021 18:04:31 +0000
Message-ID: <20211126180431.x23dcsipmwot5l6o@skbuf>
References: <20211126172739.329098-1-clement.leger@bootlin.com>
 <20211126172739.329098-2-clement.leger@bootlin.com>
 <20211126175004.6dsumhtepek4m36h@skbuf> <20211126190055.1911a142@fixe.home>
In-Reply-To: <20211126190055.1911a142@fixe.home>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?Q9ULkegf+UkAkyN6/8xWHIG0Bu9CgX7LLG1R76gNSXePZydAYchXJ1hQdZ?=
 =?iso-8859-1?Q?k+lDf+txG/4ufdOoxayLhf9xRFP3S4KHQiO8mhQ2R4JHnJq9rBwPCl+KGK?=
 =?iso-8859-1?Q?AEQeep1zvhK1eeuPdmEL9JUUq6ZN9n1pgSbWFtIW/U20tZGbVvGVWOxNKX?=
 =?iso-8859-1?Q?NjLg6VeAv5pk3PYHrBABsmTnFAZOliHtSiR12gV0vwEwzB6azeeP7x9E1V?=
 =?iso-8859-1?Q?43ZdwpQkVzRqI35bGFVDMJJbhOBHup3gn0qLh//fdaHjiG6egruZEjtw6t?=
 =?iso-8859-1?Q?qiKfuxyx6/Rju9TqZGPPdsqfqXv6ELeH9I+Bu2feou8/xR9jv+9rBEFMBv?=
 =?iso-8859-1?Q?0D+6XJBt9hmWHnmeb02Qs+NlkYjELDdsiBlfAg6QmIz3bzuc7attN0Z5dt?=
 =?iso-8859-1?Q?Fb9jQsIe5uTNUAW8lt1DyEUth/vtIElPtoEQZ/NeopGnL38kwmeqB3kzjz?=
 =?iso-8859-1?Q?28NkJ7Eeyt0WTUxEk7K1ZWd0t4Q7METUqPBl1t7qXUcAcLC7Ias7PX68Vg?=
 =?iso-8859-1?Q?lLDwmUIILcyZTKXZDF2vscKYSjl4zmL8x6fPMGy8NEm4HJQRrwqQdiSDXk?=
 =?iso-8859-1?Q?c2+hvAMCdXsb6hyCal7BJkoQITLgQwCRFqQMLIVgvCHgDHJiR6l1ogP1eC?=
 =?iso-8859-1?Q?ek90s4T3EtyTOfLfd/rhpsA9BDn8wg5Z31n4pWh0CcccaopM6XgEGRm3/C?=
 =?iso-8859-1?Q?eZf8ta3b7dSJszbyQ3tR/C7gBBcsOriQZsArhMPoT326a0mjHtAdoi3TA1?=
 =?iso-8859-1?Q?ZyNP7BFvcrBdc1JnXv+Eg8eDMXONZVqkwnBNIhbCY8IuZz4HjyTqOoo84H?=
 =?iso-8859-1?Q?JZklyXSnoPdkKoWbiNyDZicUBuiwQYBJW2shQnunxuwcVPAX4GZFUiL6K/?=
 =?iso-8859-1?Q?S08mBIaBhrRYN7owgzMhUMOWf+aH+btFxQiguXGS3XjbdfjJ6VQ0ju3gTf?=
 =?iso-8859-1?Q?DHUeIiK+4r+jdGmK0qQTAPnL5yQy6shsoJ/ELQe1z5/F3yoD+dIGvkpD3B?=
 =?iso-8859-1?Q?ZLUhotPhGwSTjfypWG5Olz7sQ12rtjDoQaVsTJfoEsfx5tvyNO61LWmjSY?=
 =?iso-8859-1?Q?ei0c1nsoFitDz83Td5Q9WuLh0Vki5vRe0IDznYZhrz+h/gC7sVfBNZbpMZ?=
 =?iso-8859-1?Q?ds0lQoAw0i1fqEuGxxi1O+Lm/MqT6Zym5GZpaQVuX1CGr8vvYwJSAwusz6?=
 =?iso-8859-1?Q?uLH0l/COmhFK7/kOJ4r4TnNuD4LjM6zf5Ie7SPN1H3fwIaAz6fY34ImQa6?=
 =?iso-8859-1?Q?sQrqAuxFyqrzOir30fc8tQxSofvC1hSGwaY/E/E3PndRumbdBVCS6xs63B?=
 =?iso-8859-1?Q?g3RgvxeR/ugM1V3cJQM5UDLAOEW115QJa+PDURgmZU1y7ABXfS8KIKZNwE?=
 =?iso-8859-1?Q?7S5Jpb85sMz6a3A/xxTp2XOyioZNU0TU0j0TcPLMUVqmN/mEYz9M/+vIgJ?=
 =?iso-8859-1?Q?pDFOxzFtwHwlJHuX/sv/Owpmn1cww2WUUwcHSROL51zDdFimbubnWs8Oq6?=
 =?iso-8859-1?Q?BkZgjOUKru6JPoqOjFdovOqZ8DsnbF5vznH+9xE37JWdQDIGs2+WJmh1pA?=
 =?iso-8859-1?Q?G/VeZZh77ryZBCttOgwzmcZ57Prvbuv6OLK7CHLqC6JMNH54tz8qaYwpHH?=
 =?iso-8859-1?Q?iTuF/FTGYWSOK9bRbqI7ZX/bNHjYDRy5lL/XQLAhz08iewOkTSEF8VAtdU?=
 =?iso-8859-1?Q?fAU846SciGKFdaM0zwY=3D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <F40E4C09DA066046AF27F4525BFF64FF@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9adacb12-1ad1-45e6-49e6-08d9b107328d
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Nov 2021 18:04:31.9854
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: YgZVDY+04BL8nvhsQW99bdz5SDhPCFW+2tTLmsVLY6im1JhhfBl6T+ne0056lbJJBKfLZM6hAYS6rSwAp0HUDw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6511
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Nov 26, 2021 at 07:00:55PM +0100, Cl=E9ment L=E9ger wrote:
> Le Fri, 26 Nov 2021 17:50:05 +0000,
> Vladimir Oltean <vladimir.oltean@nxp.com> a =E9crit :
>=20
> > On Fri, Nov 26, 2021 at 06:27:36PM +0100, Cl=E9ment L=E9ger wrote:
> > > +  ethernet-ports:
> > > +    type: object
> > > +
> > > +    properties:
> > > +      '#address-cells':
> > > +        const: 1
> > > +      '#size-cells':
> > > +        const: 0
> > > +
> > > +    additionalProperties: false
> > > +
> > > +    patternProperties:
> > > +      "^port@[0-9a-f]+$":
> > > +        type: object
> > > +        description: Ethernet ports handled by the switch
> > > +
> > > +        $ref: ethernet-controller.yaml#
> > > +
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          reg:
> > > +            description: Switch port number
> > > +
> > > +          phy-handle: true
> > > +
> > > +          phy-mode: true
> > > +
> > > +          fixed-link: true
> > > +
> > > +          mac-address: true
> > > +
> > > +        required:
> > > +          - reg
> > > +
> > > +        oneOf:
> > > +          - required:
> > > +              - phy-handle
> > > +              - phy-mode
> > > +          - required:
> > > +              - fixed-link =20
> >=20
> > Are you practically saying that a phy-mode would not be required with
> > fixed-link? Because it still is...
>=20
> I tried to get it right by looking at a binding you probably know
> (dsa.yaml), but none of them are using a oneOf property for these
> properties so I tried to guess what was really required or not. I will
> add the phy-mode property in the required field since it seems always
> needed:

So if it works without a phy-mode it is probably because of this in
ocelot_port_phylink_create():

	/* DT bindings of internal PHY ports are broken and don't
	 * specify a phy-mode
	 */
	if (phy_mode =3D=3D PHY_INTERFACE_MODE_NA)
		phy_mode =3D PHY_INTERFACE_MODE_INTERNAL;

but yeah, remove that and try out a fixed-link with no phy-mode, see
that you'll get an error.

>=20
> +        required:
> +          - reg
> +          - phy-mode
> +
> +        oneOf:
> +          - required:
> +              - phy-handle
> +          - required:
> +              - fixed-link =20

Looks good to me.

>=20
> Does it looks good to you ?
>=20
> Thanks,
>=20
> >=20
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> > > +  - interrupts
> > > +  - interrupt-names
> > > +  - ethernet-port =20
>=20
>=20
>=20
> --=20
> Cl=E9ment L=E9ger,
> Embedded Linux and Kernel engineer at Bootlin
> https://bootlin.com/=