Re: [PATCH net 0/4] net: dsa: mv88e6xxx: Amethyst (6393X) fixes

[Tobias Waldekranz <tobias@waldekranz.com>]

On mån, dec 09, 2024 at 09:23, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> Hi Tobias,
>
> On 07/12/2024 02:07, Tobias Waldekranz wrote:
>> This series provides a set of bug fixes discovered while bringing up a
>> new board using mv88e6393x chips.
>>
>> 1/4 adds logging of low-level I/O errors that where previously only
>> logged at a much higher layer, e.g. "probe failed" or "failed to add
>> VLAN", at which time the origin of the error was long gone. Not
>> exactly a bugfix, though still suitable for -net IMHO; but I'm also
>> happy to send it via net-next instead if that makes more sense.
>>
>> 2/4 fixes an issue I've never seen on any other board. At first I
>> assumed that there was some board-specific issue, but we've not been
>> able to find one. If you give the chip enough time, it will eventually
>> signal "PPU Polling" and everything else will work as
>> expected. Therefore I assume that all is in order, and that we simply
>> need to increase the timeout.
>>
>> 3/4 just broadens Chris' original fix to apply to all chips. Though I
>> have obviously not tested this on every supported device, I can't see
>> how this could possibly be chip specific. Was there some specific
>> reason for originally limiting the set of chips that this applied to?
>
> I think it was mainly because I didn't have a 88e639xx to test with 
> (much like you) so I kept the change isolated to the hardware I did have 
> access to.
>
> The original thread that kicked the original series off was 
> https://lore.kernel.org/netdev/72e8e25a-db0d-275f-e80e-0b74bf112832@alliedtelesis.co.nz/
>
> Since the only difference is the mode == MLO_AN_INBAND check I think 
> your change is reasonably safe.

Yeah exactly; and since that only applies when the user has explicitly
stated "the PHY will communicate the link information in-band", then I
don't see how forcing the link state could ever be the right thing to
do.

Thanks for providing the background!

>>
>> 4/4 can only be supported on the Amethyst, which can control the
>> ieee-multicast policy per-port, rather than via a global setting as
>> it's done on the older families.
>>
>> Tobias Waldekranz (4):
>>    net: dsa: mv88e6xxx: Improve I/O related error logging
>>    net: dsa: mv88e6xxx: Give chips more time to activate their PPUs
>>    net: dsa: mv88e6xxx: Never force link on in-band managed MACs
>>    net: dsa: mv88e6xxx: Limit rsvd2cpu policy to user ports on 6393X
>>
>>   drivers/net/dsa/mv88e6xxx/chip.c    | 92 ++++++++++++++++-------------
>>   drivers/net/dsa/mv88e6xxx/chip.h    |  6 +-
>>   drivers/net/dsa/mv88e6xxx/global1.c | 19 +++++-
>>   drivers/net/dsa/mv88e6xxx/port.c    | 48 +++++++--------
>>   drivers/net/dsa/mv88e6xxx/port.h    |  1 -
>>   5 files changed, 97 insertions(+), 69 deletions(-)
>>