netdev - Re: [PATCH net-next 4/5] net: tn40xx: enable driver to support TN4010 cards with AQR105 PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e632b37f-e39c-48a8-a18e-72f23a3e3a08@gmx.net>
Date: Sat, 14 Sep 2024 10:52:12 +0200
From: Hans-Frieder Vogt <hfdevel@....net>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com,
 linux@...linux.org.uk, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com
Subject: Re: [PATCH net-next 4/5] net: tn40xx: enable driver to support TN4010
 cards with AQR105 PHY

On 13.09.2024 09.24, FUJITA Tomonori wrote:
> On Wed, 11 Sep 2024 21:30:54 +0200
> Hans-Frieder Vogt <hfdevel@....net> wrote:
>
>> Prepare the tn40xx driver to load for Tehuti TN9510 cards
>> and set bit 3 in the TN40_REG_MDIO_CMD_STAT register, because otherwise the
>> AQR105 PHY will not be found. The function of bit 3 is unclear, but may have
>> something to do with the length of the preamble in MDIO communication.
>>
>> Signed-off-by: Hans-Frieder Vogt <hfdevel@....net>
>> ---
>>   drivers/net/ethernet/tehuti/tn40.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/tehuti/tn40.c b/drivers/net/ethernet/tehuti/tn40.c
>> index 259bdac24cf2..4e6f2f781ffc 100644
>> --- a/drivers/net/ethernet/tehuti/tn40.c
>> +++ b/drivers/net/ethernet/tehuti/tn40.c
>> @@ -1760,6 +1760,9 @@ static int tn40_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>   		goto err_unset_drvdata;
>>   	}
>>
>> +	/* essential for identification of some PHYs is bit 3 set */
>> +	ret = tn40_read_reg(priv, TN40_REG_MDIO_CMD_STAT);
>> +	tn40_write_reg(priv, TN40_REG_MDIO_CMD_STAT, ret | 0x8);
>>   	ret = tn40_mdiobus_init(priv);
> How about setting the speed of mdio 1MHZ by calling
> tn40_mdio_set_speed() like the vendor driver?
it is not about setting the speed here, but ensuring that bit 3 is set.
But I agree, with tn40_mdio_set_speed()
the intended effect is also achieved, and will make the code more readable.
And my fear to overwrite other bits in this register is also not
justified, because the initial
value of the register is consistently 0x3ec0.
>
> The following works for my TN9510 card.
>
> diff --git a/drivers/net/ethernet/tehuti/tn40_mdio.c b/drivers/net/ethernet/tehuti/tn40_mdio.c
> index bbd95fabbea0..e8b8dea250f2 100644
> --- a/drivers/net/ethernet/tehuti/tn40_mdio.c
> +++ b/drivers/net/ethernet/tehuti/tn40_mdio.c
> @@ -183,6 +183,7 @@ int tn40_mdiobus_init(struct tn40_priv *priv)
>   			ret);
>   	}
>
> +	tn40_mdio_set_speed(priv, TN40_MDIO_SPEED_1MHZ);
>   	ret = devm_mdiobus_register(&pdev->dev, bus);
>   	if (ret) {
>   		dev_err(&pdev->dev, "failed to register mdiobus %d %u %u\n",
Yes, of course it can be moved here.
Will change it in the next version of the patch.

Thanks!
Hans