[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAL_JsqL0v+a5rijNdEqD7A=DbLeWWSHA2vOmYMv4hVN6nL97fA@mail.gmail.com>
Date: Mon, 16 Oct 2023 08:33:10 -0500
From: Rob Herring <robh@...nel.org>
To: Alexander Stein <alexander.stein@...tq-group.com>
Cc: Wei Fang <wei.fang@....com>, Shenwei Wang <shenwei.wang@....com>,
Clark Wang <xiaoning.wang@....com>, "David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
NXP Linux Team <linux-imx@....com>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] net: fec: Fix device_get_match_data usage
On Mon, Oct 16, 2023 at 7:24 AM Alexander Stein
<alexander.stein@...tq-group.com> wrote:
>
> device_get_match_data() returns an entry of fec_devtype, an array of
> struct platform_device_id. But the desired struct fec_devinfo information
> is stored in platform_device_id.driver_data. Thus directly storing
> device_get_match_data() result in dev_info is wrong.
> Instead, similar to before the change, update the pdev->id_entry if
> device_get_match_data() returned non-NULL.
>
> Fixes: b0377116decd ("net: ethernet: Use device_get_match_data()")
> Signed-off-by: Alexander Stein <alexander.stein@...tq-group.com>
> ---
> Admittedly I am not a fan of adding a additional struct platform_device_id
> pointer. But as long a this driver supports non-DT probes it can be non-NULL.
Besides Coldfire, none of the non-DT platform id's are still needed.
I'm not sure we need an entry for Coldfire if it's the only one and
there is no driver data. Maybe for module autoloading? I don't
remember offhand.
Regardless, fec_dt_ids should be updated to use fec_foo_info structs
directly instead of the indirection with the platform_device_id
structs.
> drivers/net/ethernet/freescale/fec_main.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 5eb756871a963..dc7c3ef5ba9de 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -4297,6 +4297,7 @@ fec_probe(struct platform_device *pdev)
> char irq_name[8];
> int irq_cnt;
> const struct fec_devinfo *dev_info;
> + const struct platform_device_id *plat_dev_id;
>
> fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
>
> @@ -4311,9 +4312,10 @@ fec_probe(struct platform_device *pdev)
> /* setup board info structure */
> fep = netdev_priv(ndev);
>
> - dev_info = device_get_match_data(&pdev->dev);
> - if (!dev_info)
> - dev_info = (const struct fec_devinfo *)pdev->id_entry->driver_data;
I don't know why I kept this line. Probably because I originally used
of_device_get_match_data instead. It's redundant because
device_get_match_data() will do platform id matching too.
> + plat_dev_id = device_get_match_data(&pdev->dev);
> + if (plat_dev_id)
> + pdev->id_entry = plat_dev_id;
> + dev_info = (const struct fec_devinfo *)pdev->id_entry->driver_data;
> if (dev_info)
> fep->quirks = dev_info->quirks;
>
> --
> 2.34.1
>
Powered by blists - more mailing lists