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 <and...@lunn.ch>
> Signed-off-by: Maxime Chevallier <maxime.chevall...@bootlin.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
>
>
