netdev - Re: [PATCH net-next] net: phy: micrel: Add support for lan8842

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250722060954.ecaxrk7vq5ibuy55@DEN-DL-M31836.microchip.com>
Date: Tue, 22 Jul 2025 08:09:54 +0200
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <hkallweit1@...il.com>, <linux@...linux.org.uk>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<o.rempel@...gutronix.de>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] net: phy: micrel: Add support for lan8842

The 07/21/2025 16:22, Andrew Lunn wrote:

Hi Andrew,

> 
> > +static struct lan8842_hw_stat lan8842_hw_stats[] = {
> > +     { "phy_rx_correct_count", 2, 3, {88, 61, 60}},
> > +     { "phy_rx_crc_count", 2, 2, {63, 62}},
> > +     { "phy_tx_correct_count", 2, 3, {89, 85, 84}},
> > +     { "phy_tx_crc_count", 2, 2, {87, 86}},
> > +};
> 
> Hi Horatiu
> 
> Please could you look at using ethtool_phy_stats via the
> .get_phy_stats() phy driver op.

Yes, definetly I will update this in the next version.

> 
> > +static int lan8842_config_init(struct phy_device *phydev)
> > +{
> > +     int val;
> > +     int ret;
> > +
> > +     /* Reset the PHY */
> > +     val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);
> > +     if (val < 0)
> > +             return val;
> > +     val |= LAN8814_QSGMII_SOFT_RESET_BIT;
> > +     lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);
> 
> It looks like there are sufficient pairs of
> lanphy_read_page_reg()/lanphy_write_page_reg() you would be justified
> adding a lanphy_modify_page_reg().

I agree, I can do that.

> 
> > +}, {
> > +     .phy_id         = PHY_ID_LAN8842,
> > +     .phy_id_mask    = MICREL_PHY_ID_MASK,
> 
> I think you could use PHY_ID_MATCH_MODEL() here.  It would make it
> different to every other PHY, but maybe you could start with some
> cleanup patches, adding the _modify_ call etc?

Yes, I will use PHY_ID_MATCH_MODEL().
I was thinking to start with 2 cleanup patches, one to use
PHY_ID_MATCH_MODEL in the driver and one to introduce
lanphy_modify_page_reg and put it to use.
Do you have other suggestions?

> 
>         Andrew

-- 
/Horatiu