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: <8e9fecd4-f9bc-46d0-b7bf-fbf7ac83cc80@lunn.ch>
Date: Tue, 19 Sep 2023 14:50:43 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Parthiban.Veerasooran@...rochip.com
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
Subject: Re: [RFC PATCH net-next 5/6] microchip: lan865x: add driver support
 for Microchip's LAN865X MACPHY

> 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.

If you are only using mandatory parts of the spec, then no test is
needed.

> > 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.

> >> +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.

	 Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ