[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <862e5225-2d8e-8b8f-fc6d-c9b48ac74bfc@gmail.com>
Date: Fri, 25 Aug 2023 08:26:55 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Lukasz Majewski <lukma@...x.de>, Tristram.Ha@...rochip.com
Cc: andrew@...n.ch, olteanv@...il.com, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, Woojung.Huh@...rochip.com,
pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH 2/2] net: dsa: microchip: Provide Module 4 KSZ9477 errata
(DS80000754C)
On 8/25/2023 1:39 AM, Lukasz Majewski wrote:
> Hi Tristram,
>
>>> +static int ksz9477_errata(struct dsa_switch *ds)
>>> +{
>>> + struct ksz_device *dev = ds->priv;
>>> + u16 val;
>>> + int p;
>>> +
>>> + /* KSZ9477 Errata DS80000754C
>>> + *
>>> + * Module 4: Energy Efficient Ethernet (EEE) feature select
>>> must be
>>> + * manually disabled
>>> + * The EEE feature is enabled by default, but it is not
>>> fully
>>> + * operational. It must be manually disabled through
>>> register
>>> + * controls. If not disabled, the PHY ports can
>>> auto-negotiate
>>> + * to enable EEE, and this feature can cause link drops
>>> when linked
>>> + * to another device supporting EEE.
>>> + *
>>> + * Only PHY ports (dsa user) [0-4] need to have the EEE
>>> advertisement
>>> + * bits cleared.
>>> + */
>>> +
>>> + for (p = 0; p < ds->num_ports; p++) {
>>> + if (!dsa_is_user_port(ds, p))
>>> + continue;
>>> +
>>> + ksz9477_port_mmd_read(dev, p, MMD_DEVICE_ID_EEE_ADV,
>>> + MMD_EEE_ADV, &val, 1);
>>> +
>>> + pr_err("%s: PORT: %d val: 0x%x pc: %d\n", __func__,
>>> p, val,
>>> + ds->num_ports);
>>> +
>>> + val &= ~(EEE_ADV_100MBIT | EEE_ADV_1GBIT);
>>> + ksz9477_port_mmd_write(dev, p,
>>> MMD_DEVICE_ID_EEE_ADV,
>>> + MMD_EEE_ADV, &val, 1);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> int ksz9477_setup(struct dsa_switch *ds)
>>> {
>>> struct ksz_device *dev = ds->priv;
>>> @@ -1157,7 +1195,7 @@ int ksz9477_setup(struct dsa_switch *ds)
>>> /* enable global MIB counter freeze function */
>>> ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE,
>>> true);
>>>
>>> - return 0;
>>> + return ksz9477_errata(ds);
>>> }
>>
>> I would prefer to execute the code in ksz9477_config_cpu_port(), as at
>> the end there is already a loop to do something to each port.
>
> Just some explanation of the taken approach:
>
> 1. I've followed already in-mainline code for ksz8795.c
> (ksz8_handle_global_errata(ds)) which is executed in ksz8_setup
> function.
>
> 2. I do believe, that separate "errata" function would be more
> readable, as KSZ9477 has many more erratas to be added.
>
>> The
>> check to disable EEE or not should be dev->info->internal_phy[port],
>> as one of the user ports can be RGMII or SGMII, which does not have a
>> PHY that can be accessed inside the switch.
>
> Yes, this would be better solution. Thanks for the suggestion.
>
>>
>> As the EEE register value is simply 6 it should be enough to just set
>> the register to zero. If so we do not need to add back those
>> ksz9477_port_mmd_setup functions and just use ksz_pwrite16() to write
>> to the MMD register.
>>
>
> IMHO adding functions to MMD modification would facilitate further
> development (for example LED setup).
We already have some KSZ9477 specific initialization done in the Micrel
PHY driver under drivers/net/phy/micrel.c, can we converge on the PHY
driver which has a reasonable amount of infrastructure for dealing with
workarounds, indirect or direct MMD accesses etc.?
--
Florian
Powered by blists - more mailing lists