Re: [PATCH net 2/4] net: dsa: mv88e6xxx: Give chips more time to activate their PPUs

[Tobias Waldekranz <tobias@waldekranz.com>]

On sön, dec 08, 2024 at 22:23, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> On lör, dec 07, 2024 at 16:38, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Fri, Dec 06, 2024 at 02:39:25PM +0100, Tobias Waldekranz wrote:
>>> On fre, dec 06, 2024 at 14:18, Andrew Lunn <andrew@lunn.ch> wrote:
>>> > On Fri, Dec 06, 2024 at 02:07:34PM +0100, Tobias Waldekranz wrote:
>>> >> In a daisy-chain of three 6393X devices, delays of up to 750ms are
>>> >> sometimes observed before completion of PPU initialization (Global 1,
>>> >> register 0, bit 15) is signaled. Therefore, allow chips more time
>>> >> before giving up.
>>> >>  static int mv88e6352_g1_wait_ppu_polling(struct mv88e6xxx_chip *chip)
>>> >>  {
>>> >>  	int bit = __bf_shf(MV88E6352_G1_STS_PPU_STATE);
>>> >> +	int err, i;
>>> >>  
>>> >> -	return mv88e6xxx_g1_wait_bit(chip, MV88E6XXX_G1_STS, bit, 1);
>>> >> +	for (i = 0; i < 20; i++) {
>>> >> +		err = _mv88e6xxx_wait_bit(chip, chip->info->global1_addr,
>>> >> +					  MV88E6XXX_G1_STS, bit, 1, NULL);
>>> >> +		if (err != -ETIMEDOUT)
>>> >> +			break;
>>> >> +	}
>>> >
>>> > The commit message does not indicate why it is necessary to swap to
>>> > _mv88e6xxx_wait_bit().
>>> 
>>> It is not strictly necessary, I just wanted to avoid flooding the logs
>>> with spurious timeout errors. Do you want me to update the message?
>>
>> Ah, the previous patch.
>>
>> I wounder if the simpler fix is just to increase the timeout? I don't
>
> It would certainly be simpler. To me, it just felt a bit dangerous to
> have a static 1s timeout buried that deep in the stack.
>
>> think we have any code specifically wanting a timeout, so changing the
>> timeout should have no real effect.
>
> I imagine some teardown scenario, in which we typically ignore return
> values. In that case, if we're trying to remove lots of objects from
> hardware that require waiting on busy bits (ATU/VTU), we could end up
> blocking for minutes rather than seconds.
>
> But it is definitely more of a gut feeling - I don't have a concrete
> example.

Andrew, have you had a chance to mull this over?

If you want to go with a global timeout then let's do that, but either
way I would really like to move this series forward.