netdev - Re: [PATCH net-next v3 2/4] dt-bindings: net: phy: add MaxLinear GPY2xx bindings

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:   Wed, 11 Jan 2023 14:26:39 -0600
From:   Rob Herring <robh@...nel.org>
To:     Michael Walle <michael@...le.cc>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Xu Liang <lxu@...linear.com>, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/4] dt-bindings: net: phy: add MaxLinear
 GPY2xx bindings

On Mon, Jan 09, 2023 at 01:30:11PM +0100, Michael Walle wrote:
> Add the device tree bindings for the MaxLinear GPY2xx PHYs, which
> essentially adds just one flag: maxlinear,use-broken-interrupts.
> 
> One might argue, that if interrupts are broken, just don't use
> the interrupt property in the first place. But it needs to be more
> nuanced. First, this interrupt line is also used to wake up systems by
> WoL, which has nothing to do with the (broken) PHY interrupt handling.

I don't understand how this is useful. If the interrupt line is asserted 
after the 1st interrupt, how is it ever deasserted later on to be 
useful. 

In any case, you could use 'wakeup-source' if that's the functionality 
you need. Then just ignore the interrupt if 'wakeup-source' is not 
present.

> Second and more importantly, there are devicetrees which have this
> property set. Thus, within the driver we have to switch off interrupt
> handling by default as a workaround. But OTOH, a systems designer who
> knows the hardware and knows there are no shared interrupts for example,
> can use this new property as a hint to the driver that it can enable the
> interrupt nonetheless.

Pretty sure I said this already, but this schema has no effect. Add an 
extra property to the example and see. No error despite your 
'unevaluatedProperties: false'. Or drop 'interrupts-extended' and no 
dependency error... 

You won't get errors as there's no defined way to decide when to apply 
this because it is based on node name or compatible unless you do a 
custom select, but I don't see what you would key off of here...

The real answer here is add a compatible. But I'm tired of pointing this 
out to the networking maintainers every damn time. Ethernet PHYs are not 
special.

Rob