linux-kernel - Re: [PATCH net-next v14 2/5] net: ti: icssm-prueth: Adds ICSSM Ethernet driver

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: <1b892cde-bcdc-4a4e-83b7-35cc13eef8f4@ti.com>
Date: Mon, 1 Sep 2025 20:18:50 +0530
From: "Anwar, Md Danish" <a0501179@...com>
To: Parvathi Pudi <parvathi@...thit.com>, <danishanwar@...com>,
        <rogerq@...nel.org>, <andrew+netdev@...n.ch>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <ssantosh@...nel.org>, <richardcochran@...il.com>, <m-malladi@...com>,
        <s.hauer@...gutronix.de>, <afd@...com>, <jacob.e.keller@...el.com>,
        <horms@...nel.org>, <johan@...nel.org>, <m-karicheri2@...com>,
        <s-anna@...com>, <glaroque@...libre.com>, <saikrishnag@...vell.com>,
        <kory.maincent@...tlin.com>, <diogo.ivo@...mens.com>,
        <javier.carrasco.cruz@...il.com>, <basharath@...thit.com>
CC: <linux-arm-kernel@...ts.infradead.org>, <netdev@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <vadim.fedorenko@...ux.dev>, <alok.a.tiwari@...cle.com>,
        <bastien.curutchet@...tlin.com>, <pratheesh@...com>, <prajith@...com>,
        <vigneshr@...com>, <praneeth@...com>, <srk@...com>, <rogerq@...com>,
        <krishna@...thit.com>, <pmohan@...thit.com>, <mohan@...thit.com>
Subject: Re: [PATCH net-next v14 2/5] net: ti: icssm-prueth: Adds ICSSM
 Ethernet driver

Hi Parvathi,

On 8/22/2025 6:55 PM, Parvathi Pudi wrote:
> From: Roger Quadros <rogerq@...com>
> 
> Updates Kernel configuration to enable PRUETH driver and its dependencies
> along with makefile changes to add the new PRUETH driver.
> 
> Changes includes init and deinit of ICSSM PRU Ethernet driver including
> net dev registration and firmware loading for DUAL-MAC mode running on
> PRU-ICSS2 instance.
> 
> Changes also includes link handling, PRU booting, default firmware loading
> and PRU stopping using existing remoteproc driver APIs.
> 
> Signed-off-by: Roger Quadros <rogerq@...com>
> Signed-off-by: Andrew F. Davis <afd@...com>
> Signed-off-by: Basharath Hussain Khaja <basharath@...thit.com>
> Signed-off-by: Parvathi Pudi <parvathi@...thit.com>

[ ... ]

> +	/* get mac address from DT and set private and netdev addr */
> +	ret = of_get_ethdev_address(eth_node, ndev);
> +	if (!is_valid_ether_addr(ndev->dev_addr)) {
> +		eth_hw_addr_random(ndev);
> +		dev_warn(prueth->dev, "port %d: using random MAC addr: %pM\n",
> +			 port, ndev->dev_addr);
> +	}
> +	ether_addr_copy(emac->mac_addr, ndev->dev_addr);
> +
> +	/* connect PHY */
> +	emac->phydev = of_phy_get_and_connect(ndev, eth_node,
> +					      icssm_emac_adjust_link);
> +	if (!emac->phydev) {
> +		dev_dbg(prueth->dev, "PHY connection failed\n");
> +		ret = -EPROBE_DEFER;
> +		goto free;
> +	}
> +

Why are you returning EPROBE_DEFER here? If phy connection fails, you
should just return and fail the probe. That's what ICSSG driver does.

In drivers/net/ethernet/ti/icssg/icssg_prueth.c

 404   │     ndev->phydev = of_phy_connect(emac->ndev, emac->phy_node,
 405   │                       &emac_adjust_link, 0,
 406   │                       emac->phy_if);
 407   │     if (!ndev->phydev) {
 408   │         dev_err(prueth->dev, "couldn't connect to phy %s\n",
 409   │             emac->phy_node->full_name);
 410   │         return -ENODEV;
 411   │     }


Before phy connect you do `dev_warn(prueth->dev, "port %d: using random
MAC addr: %pM\n"`

If device is using random mac address, this will be printed, your phy
connect fails, you try probe again, print comes again, phy fails again
and so on ...

This results in system getting spammed with continuos prints of "using
random MAC addr"

I suggest if phy fails, let the probe fail don't do EPROBE_DEFER.

Saw this issue on few boards which has issue with ICSSG phy.

> +	/* remove unsupported modes */
> +	phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
-- 
Thanks and Regards,
Md Danish Anwar