diff mbox series

[1/2] ASoC: tegra30: ahub: Use of_device_get_match_data

Message ID e568d621c9c05ee23732a6a6f9e3606a780b1707.1628971397.git.aakashhemadri123@gmail.com
State Not Applicable
Headers show
Series ASoC: tegra: Use of_device_get_match_data | expand

Commit Message

Aakash Hemadri Aug. 14, 2021, 8:12 p.m. UTC
Prefer `of_device_get_match_data` over `of_match_device`

Retrieve OF match data using `of_device_get_match_data`, this is cleaner
and better expresses intent.

Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
---
 sound/soc/tegra/tegra30_ahub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thierry Reding Aug. 16, 2021, 9:54 a.m. UTC | #1
On Sun, Aug 15, 2021 at 01:42:18AM +0530, Aakash Hemadri wrote:
> Prefer `of_device_get_match_data` over `of_match_device`
> 
> Retrieve OF match data using `of_device_get_match_data`, this is cleaner
> and better expresses intent.
> 
> Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
> ---
>  sound/soc/tegra/tegra30_ahub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Thierry Reding <treding@nvidia.com>
Bjorn Helgaas Aug. 16, 2021, 6:02 p.m. UTC | #2
On Sun, Aug 15, 2021 at 01:42:18AM +0530, Aakash Hemadri wrote:
> Prefer `of_device_get_match_data` over `of_match_device`
> 
> Retrieve OF match data using `of_device_get_match_data`, this is cleaner
> and better expresses intent.
> 
> Signed-off-by: Aakash Hemadri <aakashhemadri123@gmail.com>
> ---
>  sound/soc/tegra/tegra30_ahub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
> index b3e1df693381..0ac109b32329 100644
> --- a/sound/soc/tegra/tegra30_ahub.c
> +++ b/sound/soc/tegra/tegra30_ahub.c
> @@ -518,7 +518,7 @@ static int tegra30_ahub_probe(struct platform_device *pdev)
>  	void __iomem *regs_apbif, *regs_ahub;
>  	int ret = 0;
>  
> -	match = of_match_device(tegra30_ahub_of_match, &pdev->dev);
> +	match = of_device_get_match_data(&pdev->dev);

I think this is incorrect.

  const struct of_device_id *of_match_device(...)
  const void *of_device_get_match_data(...)

of_match_device() returns "struct of_device_id *", i.e., "match".

of_device_get_match_data() calls of_match_device() internally, then
returns "match->data".

>  	if (!match)
>  		return -EINVAL;
>  	soc_data = match->data;
> -- 
> 2.32.0
>
Mark Brown Aug. 16, 2021, 6:39 p.m. UTC | #3
On Sun, Aug 15, 2021 at 01:42:18AM +0530, Aakash Hemadri wrote:

> -	match = of_match_device(tegra30_ahub_of_match, &pdev->dev);
> +	match = of_device_get_match_data(&pdev->dev);
>  	if (!match)

Thierry, are you sure about your review here?  As others have been
pointing out of_device_get_match_data() returns match->data while
of_match_device() returns the device.
Aakash Hemadri Aug. 16, 2021, 7:46 p.m. UTC | #4
On 21/08/16 07:39PM, Mark Brown wrote:
> On Sun, Aug 15, 2021 at 01:42:18AM +0530, Aakash Hemadri wrote:
> 
> > -	match = of_match_device(tegra30_ahub_of_match, &pdev->dev);
> > +	match = of_device_get_match_data(&pdev->dev);
> >  	if (!match)
> 
> Thierry, are you sure about your review here?  As others have been
> pointing out of_device_get_match_data() returns match->data while
> of_match_device() returns the device.

Sorry for the confusion, and the glaring mistake.
Will fix and send v2.

Thanks,
Aakash Hemadri.
Mark Brown Aug. 16, 2021, 7:49 p.m. UTC | #5
On Tue, Aug 17, 2021 at 01:16:21AM +0530, Aakash Hemadri wrote:
> On 21/08/16 07:39PM, Mark Brown wrote:

> > Thierry, are you sure about your review here?  As others have been
> > pointing out of_device_get_match_data() returns match->data while
> > of_match_device() returns the device.

> Sorry for the confusion, and the glaring mistake.
> Will fix and send v2.

Since I applied the patches please send an incremental fix on top of
what's been applied rather than a v2.
Aakash Hemadri Aug. 16, 2021, 7:58 p.m. UTC | #6
On 21/08/16 08:49PM, Mark Brown wrote:
> Since I applied the patches please send an incremental fix on top of
> what's been applied rather than a v2.

Will do Mark!

Thank,
Aakash Hemadri
Thierry Reding Aug. 17, 2021, 1:52 p.m. UTC | #7
On Mon, Aug 16, 2021 at 07:39:06PM +0100, Mark Brown wrote:
> On Sun, Aug 15, 2021 at 01:42:18AM +0530, Aakash Hemadri wrote:
> 
> > -	match = of_match_device(tegra30_ahub_of_match, &pdev->dev);
> > +	match = of_device_get_match_data(&pdev->dev);
> >  	if (!match)
> 
> Thierry, are you sure about your review here?  As others have been
> pointing out of_device_get_match_data() returns match->data while
> of_match_device() returns the device.

Ugh... good catch. I (naively) assumed this was a mechanical conversion
like one of the many others that have been making the rounds and
evidently wasn't paying enough attention.

Thanks for spotting this, Bjorn!

Thierry
diff mbox series

Patch

diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index b3e1df693381..0ac109b32329 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -518,7 +518,7 @@  static int tegra30_ahub_probe(struct platform_device *pdev)
 	void __iomem *regs_apbif, *regs_ahub;
 	int ret = 0;
 
-	match = of_match_device(tegra30_ahub_of_match, &pdev->dev);
+	match = of_device_get_match_data(&pdev->dev);
 	if (!match)
 		return -EINVAL;
 	soc_data = match->data;