diff mbox series

[v6,3/5] Asoc:qcom:lpass-cpu:Update dts property read API

Message ID 1600409084-29093-4-git-send-email-srivasam@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Qualcomm's lpass-hdmi ASoC driver to support audio over dp port | expand

Commit Message

Srinivasa Rao Mandadapu Sept. 18, 2020, 6:04 a.m. UTC
From: V Sujith Kumar Reddy <vsujithk@codeaurora.org>

Signed-off-by: Srinivasa Rao <srivasam@codeaurora.org>
Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
---
 sound/soc/qcom/lpass-cpu.c      | 2 +-
 sound/soc/qcom/lpass-platform.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Srinivas Kandagatla Sept. 21, 2020, 9:20 p.m. UTC | #1
On 18/09/2020 07:04, Srinivasa Rao Mandadapu wrote:
> From: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
> 
> Signed-off-by: Srinivasa Rao <srivasam@codeaurora.org>
> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
> ---
>   sound/soc/qcom/lpass-cpu.c      | 2 +-
>   sound/soc/qcom/lpass-platform.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index 1ee6d8b..5d84f63 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -575,7 +575,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
>   
>   	of_lpass_cpu_parse_dai_data(dev, drvdata);
>   
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");

Interesting!! this patch 
https://lore.kernel.org/alsa-devel/1597402388-14112-11-git-send-email-rohitkr@codeaurora.org/ 
just took it out and you add it back in!

Index is always preferred over name w.r.t device tree bindings, so lets 
stick with that for now!
Unless you have any strong reason to lookup resource by name?

--srini


>   
>   	drvdata->lpaif = devm_ioremap_resource(dev, res);
>   	if (IS_ERR((void const __force *)drvdata->lpaif)) {
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index df692ed..35aead1 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -638,7 +638,7 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev)
>   	struct lpass_variant *v = drvdata->variant;
>   	int ret;
>   
> -	drvdata->lpaif_irq = platform_get_irq(pdev, 0);
> +	drvdata->lpaif_irq = platform_get_irq_byname(pdev, "lpass-irq-lpaif");
>   	if (drvdata->lpaif_irq < 0)
>   		return -ENODEV;
>   
>
Mark Brown Sept. 22, 2020, 11:08 a.m. UTC | #2
On Mon, Sep 21, 2020 at 10:20:22PM +0100, Srinivas Kandagatla wrote:
> On 18/09/2020 07:04, Srinivasa Rao Mandadapu wrote:

> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");

> Index is always preferred over name w.r.t device tree bindings, so lets
> stick with that for now!

It is?  That's not usually the case...

> Unless you have any strong reason to lookup resource by name?

Looking things up by name tends to make the DT easier to read (since
things are named).
Srinivas Kandagatla Sept. 22, 2020, 11:22 a.m. UTC | #3
On 22/09/2020 12:08, Mark Brown wrote:
>> On 18/09/2020 07:04, Srinivasa Rao Mandadapu wrote:
>>> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");
>> Index is always preferred over name w.r.t device tree bindings, so lets
>> stick with that for now!
> It is?  That's not usually the case...
> 
>> Unless you have any strong reason to lookup resource by name?
> Looking things up by name tends to make the DT easier to read (since
> things are named).

I agree with you on this and I see the point, but Rob had a very 
different opinion about the reg-names bindings to start with.

This topic been discussed in the past with Rob in many instances ex: 
https://lore.kernel.org/linux-devicetree/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/

According to him, reg-names seems to be highly discouraged as it came 
along for the OMAP folks and was related to the hwmods stuff.


--srini
Mark Brown Sept. 22, 2020, 11:43 a.m. UTC | #4
On Tue, Sep 22, 2020 at 12:22:38PM +0100, Srinivas Kandagatla wrote:
> On 22/09/2020 12:08, Mark Brown wrote:

> I agree with you on this and I see the point, but Rob had a very different
> opinion about the reg-names bindings to start with.

> This topic been discussed in the past with Rob in many instances ex: https://lore.kernel.org/linux-devicetree/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/

> According to him, reg-names seems to be highly discouraged as it came along
> for the OMAP folks and was related to the hwmods stuff.

That's very much specific to reg, it's not true of the use of names in
general - Rob mentions cases like interrupts for example.
Rohit Kumar Sept. 22, 2020, 12:53 p.m. UTC | #5
On 9/22/2020 5:13 PM, Mark Brown wrote:
> On Tue, Sep 22, 2020 at 12:22:38PM +0100, Srinivas Kandagatla wrote:
>> On 22/09/2020 12:08, Mark Brown wrote:
>> I agree with you on this and I see the point, but Rob had a very different
>> opinion about the reg-names bindings to start with.
>> This topic been discussed in the past with Rob in many instances ex: https://lore.kernel.org/linux-devicetree/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/
>> According to him, reg-names seems to be highly discouraged as it came along
>> for the OMAP folks and was related to the hwmods stuff.
> That's very much specific to reg, it's not true of the use of names in
> general - Rob mentions cases like interrupts for example.

I see that patch to support hdmi adds another reg-name along with 
"lpass-lpaif".

So, platform_get_resource_byname() is better option.

+       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
"lpass-hdmiif");

Thanks,

Rohit
Srinivas Kandagatla Sept. 22, 2020, 1:57 p.m. UTC | #6
On 22/09/2020 13:53, Rohit Kumar wrote:
>> That's very much specific to reg, it's not true of the use of names in
>> general - Rob mentions cases like interrupts for example.
> 
Ofcourse using names suffix for clocks and interrupts has been justified!

I don't mind having dependency on reg-names as long as Rob is happy with 
DT Bindings!

--srini


> I see that patch to support hdmi adds another reg-name along with 
> "lpass-lpaif".
> 
> So, platform_get_resource_byname() is better option.
> 
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
> "lpass-hdmiif");
>
diff mbox series

Patch

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 1ee6d8b..5d84f63 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -575,7 +575,7 @@  int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 
 	of_lpass_cpu_parse_dai_data(dev, drvdata);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpass-lpaif");
 
 	drvdata->lpaif = devm_ioremap_resource(dev, res);
 	if (IS_ERR((void const __force *)drvdata->lpaif)) {
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index df692ed..35aead1 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -638,7 +638,7 @@  int asoc_qcom_lpass_platform_register(struct platform_device *pdev)
 	struct lpass_variant *v = drvdata->variant;
 	int ret;
 
-	drvdata->lpaif_irq = platform_get_irq(pdev, 0);
+	drvdata->lpaif_irq = platform_get_irq_byname(pdev, "lpass-irq-lpaif");
 	if (drvdata->lpaif_irq < 0)
 		return -ENODEV;