netdev - Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add rgmii-mode to mt7531

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]

Date:   Mon, 9 May 2022 07:22:11 -0400
From:   Peter Geis <pgwipeout@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Frank Wunderlich <frank-w@...lic-files.de>,
        Heiko Stuebner <heiko@...ech.de>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Frank Wunderlich <linux@...web.de>,
        linux-mediatek@...ts.infradead.org, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Ungerer <gerg@...nel.org>,
        René van Dorst <opensource@...rst.com>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
Subject: Re: [PATCH v3 5/6] dt-bindings: net: dsa: make reset optional and add
 rgmii-mode to mt7531

On Mon, May 9, 2022 at 2:48 AM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>
> On 08/05/2022 19:04, Peter Geis wrote:
> > On Sun, May 8, 2022 at 8:12 AM Frank Wunderlich <frank-w@...lic-files.de> wrote:
> >>>
> >>> I think the issue is more for the description itself.
> >>>
> >>> Devicetree is only meant to describe the hardware and does in general don't
> >>> care how any firmware (Linux-kernel, *BSD, etc) handles it. So going with
> >>> "the kernel does it this way" is not a valid reason for a binding change ;-) .
>
> Exactly. The argument in commit msg was not matching the change, because
> driver implementation should not be (mostly) a reason for such changes.
>
> >>>
> >>> Instead in general want to reason that there are boards without this reset
> >>> facility and thus make it optional for those.
> >>
> >> if only the wording is the problem i try to rephrase it from hardware PoV.
> >>
> >> maybe something like this?
> >>
> >> https://github.com/frank-w/BPI-R2-4.14/commits/5.18-mt7531-mainline2/Documentation/devicetree/bindings/net/dsa/mediatek%2Cmt7530.yaml
>
> Looks ok.
>
> >>
> >> Another way is maybe increasing the delay after the reset (to give more time all
> >> come up again), but imho it is no good idea resetting the gmac/mdio-bus from the
> >> child device.
> >>
> >> have not looked into the gmac driver if this always  does the initial reset to
> >> have a "clean state". In this initial reset the switch will be resetted too
> >> and does not need an additional one which needs the gmac/mdio initialization
> >> to be done again.
> >
> > For clarification, the reset gpio line is purely to reset the phy.
> > If having the switch driver own the reset gpio instead of the gmac
> > breaks initialization that means there's a bug in the gmac driver
> > handling phy init.
> > In testing I've seen issues moving the reset line to the mdio node
> > with other phys and the stmmac gmac driver, so I do believe this is
> > the case.
>
> Yes, this seems reasonable, although Frank mentioned that reset is
> shared with gmac, so it resets some part of it as well?

No, the gpio-reset line is purely to reset the phy. The stmmac gmac
driver handles it because it seems initialization failures occur if
it's handled by the mdio drivers. I suspect this is due to a
difference between when the driver initializes the phy vs when the
driver triggers the reset.
They had tried to attach the gpio binding to both the gmac node and
the mdio node at the same time when only one can own it. Having it
owned by the switch driver on the mdio node leads to the same
initialization failures we see in other mdio drivers.

>
>
>
>
> Best regards,
> Krzysztof