netdev - Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

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: <20200227185904.GQ25745@shell.armlinux.org.uk>
Date:   Thu, 27 Feb 2020 18:59:04 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        devicetree@...r.kernel.org, Jason Cooper <jason@...edaemon.net>,
        linux-arm-kernel@...ts.infradead.org,
        Mark Rutland <mark.rutland@....com>, netdev@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for
 marvell10g driver

On Thu, Feb 27, 2020 at 09:44:35AM -0800, Florian Fainelli wrote:
> On 2/27/20 1:52 AM, Russell King wrote:
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> > 
> > Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
> 
> We have been kicking the ball for way too long but there really ought to
> be a standardized binding to configure LED modes for a PHY. Something
> that we previously discussed here without making much progress because
> the LED maintainer was not involved:
> 
> http://patchwork.ozlabs.org/patch/1146609/
> http://patchwork.ozlabs.org/patch/1146610/
> http://patchwork.ozlabs.org/patch/1146611/
> http://patchwork.ozlabs.org/patch/1146612/
> 
> What you are proposing here is just a plain configuration interface via
> Device Tree, which is really borderline. It gets the job done, and it is
> extremely easy to maintain and use because people just stick in their
> register value in there, but boy, what a poor abstraction that is.
> 
> Maybe you can resume where Matthias left and improve upon his patch
> series, if nothing else for the binding and PHY layer integration?

That series is way too simplistic, and would not allow for a
usable configuration for a four-speed PHY such as this one.

The proposed binding in those patches makes the assumption that
the only time that a LED shall blink is when there is traffic.

LED configuration is highly PHY specific.

For the 88x3310, we have around 31 different conditions that the LED
can blink for, or be solid for, the blink rate, and the polarity -
each LED is controlled by 13 bits in total, and then there's the "dual"
modes for bi-color LEDs which cause other of the LED configuration
registers to be ignored.  In other words, it's rather complex.

We could choose to limit the complexity, but then that risks making
it useless for certain boards - such as the Macchiatobin board, where
the dual modes can't be used due to the way the LEDs are wired - see
the last patch, where I describe how the LEDs are configured to
behave, which is the sanest organisation I could come up with which
doesn't result in mixing up various modes.

In any case, I do not wish to add to my patch backlog right now.  Maybe
when the backlog is smaller, I'll consider it, but not before.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up