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
