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
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Fri, 04 Mar 2022 13:46:44 +0100
From:   Michael Walle <michael@walle.cc>
To:     Horatiu Vultur <horatiu.vultur@microchip.com>
Cc:     Lars Povlsen <lars.povlsen@microchip.com>,
        Steen Hegelund <Steen.Hegelund@microchip.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        UNGLinuxDriver@microchip.com, linux-arm-kernel@lists.infradead.org,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        Colin Foster <colin.foster@in-advantage.com>
Subject: Re: [PATCH v1 5/5] pinctrl: microchip-sgpio: wait until output is
 actually set
In-Reply-To: <20220304120911.i5rngplg5l6gnnyy@soft-dev3-1.localhost>
References: <20220224161021.2197263-1-michael@walle.cc>
 <20220224161021.2197263-6-michael@walle.cc>
 <20220225092427.jjilv3qo52crsmuw@soft-dev3-1.localhost>
 <2f8a215c67269d639290515931d10b78@walle.cc>
 <20220304120911.i5rngplg5l6gnnyy@soft-dev3-1.localhost>
User-Agent: Roundcube Webmail/1.4.12
Message-ID: <40ccf0647d7ec0487f71f662eec80528@walle.cc>
Precedence: bulk

Hi Horatiu,

Am 2022-03-04 13:09, schrieb Horatiu Vultur:
> The 02/25/2022 12:29, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Hi Horatiu,
>> 
>> Am 2022-02-25 10:24, schrieb Horatiu Vultur:
>> > The 02/24/2022 17:10, Michael Walle wrote:
>> > > Right now, when a gpio value is set, the actual hardware pin gets set
>> > > asynchronously. When linux write the output register, it takes some
>> > > time
>> > > until it is actually propagated to the output shift registers. If that
>> > > output port is connected to an I2C mux for example, the linux driver
>> > > assumes the I2C bus is already switched although it is not.
>> > >
>> > > Fortunately, there is a single shot mode with a feedback: you can
>> > > trigger the single shot and the hardware will clear that bit once it
>> > > has
>> > > finished the clocking and strobed the load signal of the shift
>> > > registers. This can take a considerable amount of time though.
>> > > Measuremens have shown that it takes up to a whole burst cycle gap
>> > > which
>> > > is about 50ms on the largest setting. Therefore, we have to mark the
>> > > output bank as sleepable. To avoid unnecessary waiting, just trigger
>> > > the
>> > > single shot if the value was actually changed.
>> >
>> > I have tested this patch series on our lan9668 board and it worked
>> > fine. Thanks!
>> 
>> Thanks for testing!
>> 
>> > I just have few questions:
>> > 1. What about other boards/chips that have this sgpio, do they have
>> > also
>> >    the same issue? Because from what I recall on sparx5 they don't have
>> >    this issue. I have seen it only on lan9668.
>> 
>> Unfortunatly, I don't have any knowledge what IP core is used in
>> which SoC. I assumed the lan9668 used the same as the sparx5. If
>> that is not the case, we need a new compatible. Do you know if it
>> the same?
> 
> From what I see, it is the same IP.

Good to know.

>> On the sparx5 are there any peripheral who you would actually
>> notice that the timing is off?
> 
> There are some SFP connected, similar to lan966x. So I don't understand
> why that issue is not seen there.

Is there an I2C mux, too? Or just the SFP signals connected to
the SGPIO? What I was seeing is that during probing of the SFPs
the SFPs EEPROM is read and when the I2C mux is controlled by the
SGPIO it will switch too late - or even worse, in the middle of
a transaction. I would speculate the timing isn't that critical
with signals just connected directly to the SFP.

In any case, I think it is pretty clear that it cannot work the
way it is right now, no? See the very next paragraph...

>> That being said, I'd assume all the serial gpio controller has
>> this "flaw". Simply because a register write won't block until the
>> value is shifted out to the shift register and actualy loaded by
>> strobing the load signal. It just depends on your burst setting
>> (even with bursts off, and clocking all the time) on how large
>> the delay is. So you might or might not notice it on a board.

.. here

-michael