Re: [PATCH v2] net: mv88e6xxx: fix missing SMI address initialization

Stefan Roese
2024-03-05T10:27:43Z

Hi Marek,

On 3/4/24 15:48, Marek Behún wrote:
> Hi Stefan,
>
> On Wed, 14 Feb 2024 08:50:16 +0100
> Stefan Roese <sr@denx.de> wrote:
>
>> Hi Ramon,
>>
>> On 1/9/24 10:37, Stefan Roese wrote:
>>> On 12/18/23 17:09, Marek Behún wrote:
>>>> On Wed, 6 Dec 2023 15:35:56 +0100
>>>> Marek Mojík <marek.mojik@nic.cz> wrote:
>>>>
>>>>> The mv88e6xxx driver does not currently initialize the smi_addr
>>>>> field, but
>>>>> instead keeps the default zero value. This leads to driver being
>>>>> unusable
>>>>> on devices where the switch is not on address zero of the mdio bus. Fix
>>>>> this problem by reading the SMI address from device tree.
>>>>>
>>>>> Signed-off-by: Marek Mojík <marek.mojik@nic.cz>
>>>>> ---
>>>>>   drivers/net/mv88e6xxx.c | 8 ++++++++
>>>>>   1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c
>>>>> index c073f81e72..8fbbc1cacc 100644
>>>>> --- a/drivers/net/mv88e6xxx.c
>>>>> +++ b/drivers/net/mv88e6xxx.c
>>>>> @@ -745,6 +745,7 @@ static int mv88e6xxx_probe(struct udevice *dev)
>>>>>   {
>>>>>       struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
>>>>>       struct mv88e6xxx_priv *priv = dev_get_priv(dev);
>>>>> +    fdt_addr_t smi_addr;
>>>>>       int val, ret;
>>>>>       if (ofnode_valid(dev_ofnode(dev)) &&
>>>>> @@ -753,6 +754,13 @@ static int mv88e6xxx_probe(struct udevice *dev)
>>>>>           return -ENODEV;
>>>>>       }
>>>>> +    smi_addr = dev_read_addr(dev);
>>>>> +    if (smi_addr == FDT_ADDR_T_NONE) {
>>>>> +        dev_err(dev, "Missing SMI address\n");
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +    priv->smi_addr = smi_addr;
>>>>> +
>>>>>       /* probe internal mdio bus */
>>>>>       ret = mv88e6xxx_probe_mdio(dev);
>>>>>       if (ret)
>>>>
>>>> Reviewed-by: Marek Behún <kabel@kernel.org>
>>>
>>> Reviewed-by: Stefan Roese <sr@denx.de>
>>>
>>> Ramon, this patch was assigned to me as an Marvell MVEBU patch
>>> depends on it. I've assigned it to you now. Or if it's okay that
>>> I pull it with the Marvell tree, just let me know.
>>
>> Any updates on this?
>>
>> Thanks,
>> Stefan
>
> Stefan, it seems that Ramon is unavailable for this.
> Can we take this? We've been using it on Omnia and it works correctly.

Yes, I also thought about pulling this one in. As a matter of fact,
I'm currently busy with creating a pull request for Tom.

Thanks,
Stefan