Re: [PATCH] mips: dts: ralink: Clarify usage of MT7621 ethernet phy arguments

[Sergio Paracuellos]

Hi Liviu,

On Wed, May 10, 2023 at 7:56 PM Liviu Dudau <liviu@xxxxxxxxxxx> wrote:
>
> Hi Arınç,
>
> On Wed, May 10, 2023 at 02:59:44PM +0200, Arınç ÜNAL wrote:
> > On 9.05.2023 22:00, Liviu Dudau wrote:
> > > The device tree uses numbers as arguments to the phys property that are
> > > confusing for newcomers. Define names for the values and use them in the
> > > device tree.
> > >
> > > Signed-off-by: Liviu Dudau <liviu@xxxxxxxxxxx>
> >
> > You should document this on
> > instead of
> > doing this. Under the phys property, add 'description:' and explain this.
>
> There is already some sort of explanation under
> Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml, so I'm
> not sure what I'm improving by adding new text in the /pci/ section.
>
> Maybe I haven't explained properly in the commit message, this is meant to
> give a name to the 1 and 0 values used in the device tree, not to clarify
> any perceived missing documentation.

What is that useful for if the bindings are well documented? The
description in the
'Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml' file
for the '#phy-cells'
property is already telling you what that cell is used for. It is
obvious that zero means not dual ported and one means dual ported.
So for me there is no need to add anything extra but in case you want
to clarify anything you should modify binding documentation not the
device tree file at all.

Thanks,
    Sergio Paracuellos
>
> >
> > Arınç
>
> Best regards,
> Liviu
>
> --
> Everyone who uses computers frequently has had, from time to time,
> a mad desire to attack the precocious abacus with an axe.
>                                   -- John D. Clark, Ignition!