netdev - Re: [PATCH net-next v2 1/3] net: phy: remove calls to genphy_config

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: <cc12c859-2572-02f9-3303-6a8bffad0a96@gmail.com>
Date:   Fri, 16 Aug 2019 14:58:27 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        David Miller <davem@...emloft.net>,
        Kevin Hilman <khilman@...libre.com>,
        Vivien Didelot <vivien.didelot@...il.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "open list:ARM/Amlogic Meson..." <linux-amlogic@...ts.infradead.org>
Subject: Re: [PATCH net-next v2 1/3] net: phy: remove calls to
 genphy_config_init

On 8/16/19 1:31 PM, Heiner Kallweit wrote:
> Supported PHY features are either auto-detected or explicitly set.
> In both cases calling genphy_config_init isn't needed. All that
> genphy_config_init does is removing features that are set as
> supported but can't be auto-detected. Basically it duplicates the
> code in genphy_read_abilities. Therefore remove such calls from
> all PHY drivers.
> 
> v2:
> - remove call also from new adin PHY driver
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>

Looks good, just one question below:

> +static int dummy_config_init(struct phy_device *phydev)
> +{
> +	return 0;
> +}
> +
>  static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
>  	{ TI_DP83848C_PHY_ID, 0xfffffff0 },
>  	{ NS_DP83848C_PHY_ID, 0xfffffff0 },
> @@ -113,13 +113,13 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
>  
>  static struct phy_driver dp83848_driver[] = {
>  	DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY",
> -			   genphy_config_init),
> +			   dummy_config_init),
>  	DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY",
> -			   genphy_config_init),
> +			   dummy_config_init),
>  	DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY",
>  			   dp83848_config_init),
>  	DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY",
> -			   genphy_config_init),
> +			   dummy_config_init),

drv->config_init is an optional callback so you could just either pass
NULL as an argument to the macro, or simply remove that parameter?
-- 
Florian