Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Date:   Fri, 13 Jan 2023 22:48:53 +0100
From:   Michael Walle <michael@walle.cc>
To:     Rob Herring <robh@kernel.org>
Cc:     "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Xu Liang <lxu@maxlinear.com>, Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v3 2/4] dt-bindings: net: phy: add MaxLinear
 GPY2xx bindings
In-Reply-To: <CAL_JsqKMo6op93WQHpuVdBV6tOPYa9Pyu4geRQmOeQAicEsLWg@mail.gmail.com>
References: <20230109123013.3094144-1-michael@walle.cc>
 <20230109123013.3094144-3-michael@walle.cc>
 <20230111202639.GA1236027-robh@kernel.org>
 <73f8aad30e0d5c3badbd62030e545ef6@walle.cc>
 <CAL_JsqKMo6op93WQHpuVdBV6tOPYa9Pyu4geRQmOeQAicEsLWg@mail.gmail.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <da96809052c936581b5e6b258e1b2133@walle.cc>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Am 2023-01-13 17:38, schrieb Rob Herring:
> On Wed, Jan 11, 2023 at 4:30 PM Michael Walle <michael@walle.cc> wrote:
>> 
>> Am 2023-01-11 21:26, schrieb Rob Herring:
>> > 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.
>> 
>> Nobody said, that the interrupt line will stay asserted. The broken
>> behavior is that of the PHY doesn't respond *immediately* with a
>> deassertion of the interrupt line after the its internal status
>> register is cleared. Instead there is a random delay of up to 2ms.
> 
> With only "keep the interrupt line asserted even after the interrupt
> status register is cleared", I assume that means forever, not some
> delay.

Fair enough. I'll send a doc patch.

>> There is already a workaround to avoid an interrupt storm by delaying
>> the ISR until the line is actually cleared. *But* if this line is
>> a shared one. The line is blocked by these 2ms and important
>> interrupts (like PTP timestaming) of other devices on this line
>> will get delayed. Therefore, the only viabale option is to disable
>> the interrupts handling in the broken PHY altogether. I.e. the line
>> will never be asserted by the broken PHY.
> 
> Okay, that makes more sense.
> 
> So really, this is just an 'is shared interrupt' flag. If not shared,
> then there's no reason to not use the interrupt?

Correct.

> Assuming all
> interrupts are described in DT, we already have that information. It's
> just hard and inefficient to get it. You have to parse all interrupts
> with the same parent and check for the same cells. If we're going to
> add something more explicit, we should consider something common. It's
> not the first time shared interrupts have come up, and we probably
> have some properties already. For something common, I'd probably make
> this a flag in interrupt cells rather than a property. That would
> handle cases with multiple interrupts better.

What kind of flag do you have in mind? Could you give an example?

-michael