netdev - Re: [PATCH net-next v2 01/10] net: freescale: ucc_geth: Drop support for the "interface" DT property

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] [day] [month] [year] [list]

Message-ID: <20241115121914.GL1062410@kernel.org>
Date: Fri, 15 Nov 2024 12:19:14 +0000
From: Simon Horman <horms@...nel.org>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: davem@...emloft.net, Andrew Lunn <andrew@...n.ch>,
	Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Russell King <linux@...linux.org.uk>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
	Herve Codina <herve.codina@...tlin.com>,
	Uwe Kleine-König <u.kleine-koenig@...libre.com>,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH net-next v2 01/10] net: freescale: ucc_geth: Drop support
 for the "interface" DT property

On Thu, Nov 14, 2024 at 04:35:52PM +0100, Maxime Chevallier wrote:
> In april 2007, ucc_geth was converted to phylib with :
> 
> commit 728de4c927a3 ("ucc_geth: migrate ucc_geth to phylib").
> 
> In that commit, the device-tree property "interface", that could be used to
> retrieve the PHY interface mode was deprecated.
> 
> DTS files that still used that property were converted along the way, in
> the following commit, also dating from april 2007 :
> 
> commit 0fd8c47cccb1 ("[POWERPC] Replace undocumented interface properties in dts files")
> 
> 17 years later, there's no users of that property left and I hope it's
> safe to say we can remove support from that in the ucc_geth driver,
> making the probe() function a bit simpler.
> 
> Should there be any users that have a DT that was generated when 2.6.21 was
> cutting-edge, print an error message with hints on how to convert the
> devicetree if the 'interface' property is found.
> 
> With that property gone, we can greatly simplify the parsing of the
> phy-interface-mode from the devicetree by using of_get_phy_mode(),
> allowing the removal of the open-coded parsing in the driver.
> 
> Reviewed-by: Andrew Lunn <andrew@...n.ch>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> ---
> V2: No changes
> 
>  drivers/net/ethernet/freescale/ucc_geth.c | 63 +++++------------------
>  1 file changed, 12 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c

...

> @@ -3627,18 +3588,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
>  	/* Find the TBI PHY node.  If it's not there, we don't support SGMII */
>  	ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
>  
> -	/* get the phy interface type, or default to MII */
> -	prop = of_get_property(np, "phy-connection-type", NULL);
> -	if (!prop) {
> -		/* handle interface property present in old trees */
> -		prop = of_get_property(ug_info->phy_node, "interface", NULL);
> -		if (prop != NULL) {
> -			phy_interface = enet_to_phy_interface[*prop];
> -			max_speed = enet_to_speed[*prop];
> -		} else
> -			phy_interface = PHY_INTERFACE_MODE_MII;
> -	} else {
> -		phy_interface = to_phy_interface((const char *)prop);
> +	prop = of_get_property(ug_info->phy_node, "interface", NULL);
> +	if (prop) {
> +		dev_err(&ofdev->dev,
> +			"Device-tree property 'interface' is no longer supported. Please use 'phy-connection-type' instead.");
> +		goto err_put_tbi;

Hi Maxime,

This goto will result in err being returned by this function.
But here err is 0. Should it be set to an error value instead?

Flagged by Smatch.

> +	}
> +
> +	err = of_get_phy_mode(np, &phy_interface);
> +	if (err) {
> +		dev_err(&ofdev->dev, "Invalid phy-connection-type");
> +		goto err_put_tbi;
>  	}
>  
>  	/* get speed, or derive from PHY interface */
> @@ -3746,6 +3706,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
>  err_deregister_fixed_link:
>  	if (of_phy_is_fixed_link(np))
>  		of_phy_deregister_fixed_link(np);
> +err_put_tbi:
>  	of_node_put(ug_info->tbi_node);
>  	of_node_put(ug_info->phy_node);
>  	return err;
> -- 
> 2.47.0
> 
>