Re: [PATCH v2] irqchip/ls-scfg-msi: Use device_get_match_data()

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Fri, Oct 20, 2023 at 08:02:56AM -0500, Rob Herring wrote:
> Use preferred device_get_match_data() instead of of_match_device() to
> get the driver match data in a single step without the unnecessary
> intermediate match pointer. With this, adjust the includes to
> explicitly include the correct headers. That also serves as
> preparation to remove implicit includes within the DT headers.
> of_platform.h currently includes platform_device.h among others.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

> v2:
>  - Expand commit message
> ---
>  drivers/irqchip/irq-ls-scfg-msi.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c b/drivers/irqchip/irq-ls-scfg-msi.c
> index f31a262fe438..15cf80b46322 100644
> --- a/drivers/irqchip/irq-ls-scfg-msi.c
> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
> @@ -17,7 +17,8 @@
>  #include <linux/irqdomain.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_pci.h>
> -#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/spinlock.h>
>  
>  #define MSI_IRQS_PER_MSIR	32
> @@ -334,20 +335,17 @@ MODULE_DEVICE_TABLE(of, ls_scfg_msi_id);
>  
>  static int ls_scfg_msi_probe(struct platform_device *pdev)
>  {
> -	const struct of_device_id *match;
>  	struct ls_scfg_msi *msi_data;
>  	struct resource *res;
>  	int i, ret;
>  
> -	match = of_match_device(ls_scfg_msi_id, &pdev->dev);
> -	if (!match)
> -		return -ENODEV;
> -
>  	msi_data = devm_kzalloc(&pdev->dev, sizeof(*msi_data), GFP_KERNEL);
>  	if (!msi_data)
>  		return -ENOMEM;
>  
> -	msi_data->cfg = (struct ls_scfg_msi_cfg *) match->data;
> +	msi_data->cfg = (struct ls_scfg_msi_cfg *)device_get_match_data(&pdev->dev);

I wanted to suggest to drop the unnecessary cast of the void pointer.
But the driver keeps msi_data->cfg as a non-const pointer, and that
causes a warning that is otherwise suppressed by this cast. So eliminating
the type cast and keeping "cfg" as const is definitely a separate change.

> +	if (!msi_data->cfg)
> +		return -ENODEV;
>  
>  	msi_data->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
>  	if (IS_ERR(msi_data->regs)) {
> -- 
> 2.42.0
>