netdev - Re: [RFC PATCH net-next 5/6] microchip: lan865x: add driver support for Microchip's LAN865X MACPHY

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8c0b32c8-ac9f-2510-f8f2-96594ae2abb8@microchip.com>
Date: Wed, 20 Sep 2023 12:53:35 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <andrew@...n.ch>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <robh+dt@...nel.org>,
	<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>, <corbet@....net>,
	<Steen.Hegelund@...rochip.com>, <rdunlap@...radead.org>, <horms@...nel.org>,
	<casper.casan@...il.com>, <netdev@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-doc@...r.kernel.org>, <Horatiu.Vultur@...rochip.com>,
	<Woojung.Huh@...rochip.com>, <Nicolas.Ferre@...rochip.com>,
	<UNGLinuxDriver@...rochip.com>, <Thorsten.Kummermehr@...rochip.com>,
	<Ciprian.Regus@...log.com>, <jtm@...ingdog.com>
Subject: Re: [RFC PATCH net-next 5/6] microchip: lan865x: add driver support
 for Microchip's LAN865X MACPHY

Hi Andrew,

On 19/09/23 6:20 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> Sure, I can move this part to oa_tc6 lib. If I understand you correctly
>> you are talking about the Standard Capabilities Register (0x0002)
>> defined in the OPEN Alliance 10BASE-T1x MAC-PHY Serial Interface spec
>> right? If so, the 9th bit of this register tells about Indirect PHY
>> Register access Capability. Did you mean this bit? If so, this bit
>> describes the below,
>>
>> IPRAC - Indirect PHY Register Access Capability. Indicates if PHY
>> registers are indirectly accessible through the MDIO/MDC registers MDIOACCn.
> 
> Yes. If the core relies on any functionality which is optional in the
> standard, it should check if the capability bit is set, and do a
> dev_erro() and return -ENODEV if a device does not actually have
> it. That makes it clear the core needs extending to support a device.
Ok, will do that.
> 
> If you are only using mandatory parts of the spec, then no test is
> needed.
Ok.
> 
>>> I would expect to see a call to phy_ethtool_ksettings_set()
>>> here. phylib should be able to do some of the validation.
>> Ah ok, doing the below will make the life easier.
>> .set_link_ksettings   = phy_ethtool_set_link_ksettings,
> 
> Please do some testing and check that phy_ethtool_set_link_ksettings
> doe actually reject all invalid setting. I cannot guarantee it does,
> and if it does not, it might actually be a PHY driver bug.
Yes sure.
> 
>>>> +static int lan865x_net_open(struct net_device *netdev)
>>>> +{
>>>> +     struct lan865x_priv *priv = netdev_priv(netdev);
>>>> +     int ret;
>>>> +
>>>> +     if (!is_valid_ether_addr(netdev->dev_addr)) {
>>>> +             if (netif_msg_ifup(priv))
>>>> +                     netdev_err(netdev, "Invalid MAC address %pm", netdev->dev_addr);
>>>> +             return -EADDRNOTAVAIL;
>>>
>>> Using a random MAC address is the normal workaround for not having a
>>> valid MAC address via OTP flash etc.
>> Ah ok, you mean to use eth_hw_addr_random(netdev) instead of returning
>> error.
> 
> Yes. And this is generally done earlier than open, as part of
> probe. You want to avoid surprising userspace when the MAC address
> suddenly changes at open time.
Ah ok ok, will move that to probe.

Best Regards,
Parthiban V
> 
>           Andrew