Re: [PATCH net-next v3 2/8] dt-bindings: phy: add the "fsl, lynx-28g" compatible

[Ioana Ciornei <ioana.ciornei@nxp.com>]

On Thu, Mar 10, 2022 at 05:47:31PM +0100, Krzysztof Kozlowski wrote:
> On 10/03/2022 15:51, Ioana Ciornei wrote:
> > Describe the "fsl,lynx-28g" compatible used by the Lynx 28G SerDes PHY
> > driver on Layerscape based SoCs.
> 
> The message is a bit misleading, because it suggests you add only
> compatible to existing bindings. Instead please look at the git log how
> people usually describe it in subject and message.

Sure, I can change the title and commit message.

> > +patternProperties:
> > +  '^phy@[0-9a-f]$':
> > +    type: object
> > +    properties:
> > +      reg:
> > +        description:
> > +          Number of the SerDes lane.
> > +        minimum: 0
> > +        maximum: 7
> > +
> > +      "#phy-cells":
> > +        const: 0
> 
> Why do you need all these children? You just enumerated them, without
> statuses, resources or any properties. This should be rather just index
> of lynx-28g phy.

I am just describing each lane of the SerDes block so that each ethernet
dts node references it directly.

Since I am new to the generic PHY infrastructure I was using the COMPHY
for the Marvell MVEBU SoCs (phy-mvebu-comphy.txt) as a loose example.
Each lane there is described as a different child node as well. The only
difference from the COMPHY is that Lynx 28G does not need #phy-cells =
<1> to reference the input port, we just use '#phy-cells = <0>' on each
lane.

What is wrong with this approach? Or better, is there an easier way to
do this?

> 
> > +
> > +    additionalProperties: false
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +      serdes_1: serdes_phy@1ea0000 {
> 
> node name just "phy"

Sure.

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