diff mbox

[v2,5/7] ASoC: qdsp6: Add depends on OF

Message ID 20180614111138.8923-6-niklas.cassel@linaro.org (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Niklas Cassel June 14, 2018, 11:11 a.m. UTC
of_platform_device_destroy is only defined when building
with CONFIG_OF=y. Add a depends on OF.

sound/soc/qcom/qdsp6/q6afe.o: In function `q6afe_remove':
q6afe.c:(.text+0x363): undefined reference to `of_platform_device_destroy'
sound/soc/qcom/qdsp6/q6adm.o: In function `q6adm_remove':
q6adm.c:(.text+0x216): undefined reference to `of_platform_device_destroy'
sound/soc/qcom/qdsp6/q6asm.o: In function `q6asm_remove':
q6asm.c:(.text+0x723): undefined reference to `of_platform_device_destroy'

Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 sound/soc/qcom/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Brown June 18, 2018, 11:06 a.m. UTC | #1
On Thu, Jun 14, 2018 at 01:11:36PM +0200, Niklas Cassel wrote:
> of_platform_device_destroy is only defined when building
> with CONFIG_OF=y. Add a depends on OF.

Is it sensible that of_platform_device_destroy() is only defined when
building with CONFIG_OF=y?
Niklas Cassel June 18, 2018, 12:39 p.m. UTC | #2
On Mon, Jun 18, 2018 at 12:06:42PM +0100, Mark Brown wrote:
> On Thu, Jun 14, 2018 at 01:11:36PM +0200, Niklas Cassel wrote:
> > of_platform_device_destroy is only defined when building
> > with CONFIG_OF=y. Add a depends on OF.
> 
> Is it sensible that of_platform_device_destroy() is only defined when
> building with CONFIG_OF=y?

I'm redirecting that question to the device tree maintainers.

There are a few of_* functions in include/linux/of_platform.h
that are only defined when CONFIG_OF=y:

of_platform_device_create()
of_platform_device_destroy()
of_platform_bus_probe()
of_device_alloc()

Rob, Frank, do you want me to create static inline dummy versions of these?

Regards,
Niklas
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring June 18, 2018, 2:48 p.m. UTC | #3
On Mon, Jun 18, 2018 at 6:39 AM, Niklas Cassel <niklas.cassel@linaro.org> wrote:
> On Mon, Jun 18, 2018 at 12:06:42PM +0100, Mark Brown wrote:
>> On Thu, Jun 14, 2018 at 01:11:36PM +0200, Niklas Cassel wrote:
>> > of_platform_device_destroy is only defined when building
>> > with CONFIG_OF=y. Add a depends on OF.
>>
>> Is it sensible that of_platform_device_destroy() is only defined when
>> building with CONFIG_OF=y?
>
> I'm redirecting that question to the device tree maintainers.
>
> There are a few of_* functions in include/linux/of_platform.h
> that are only defined when CONFIG_OF=y:
>
> of_platform_device_create()
> of_platform_device_destroy()
> of_platform_bus_probe()
> of_device_alloc()
>
> Rob, Frank, do you want me to create static inline dummy versions of these?

No, because generally you should not be using these functions
directly. Yes, there are some users, but if you look at the tree,
there are few or isolated (PowerPC) users. Using
of_platform_populate/of_platform_depopulate is preferred.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Niklas Cassel June 18, 2018, 8:08 p.m. UTC | #4
On Mon, Jun 18, 2018 at 08:48:32AM -0600, Rob Herring wrote:
> On Mon, Jun 18, 2018 at 6:39 AM, Niklas Cassel <niklas.cassel@linaro.org> wrote:
> > On Mon, Jun 18, 2018 at 12:06:42PM +0100, Mark Brown wrote:
> >> On Thu, Jun 14, 2018 at 01:11:36PM +0200, Niklas Cassel wrote:
> >> > of_platform_device_destroy is only defined when building
> >> > with CONFIG_OF=y. Add a depends on OF.
> >>
> >> Is it sensible that of_platform_device_destroy() is only defined when
> >> building with CONFIG_OF=y?
> >
> > I'm redirecting that question to the device tree maintainers.
> >
> > There are a few of_* functions in include/linux/of_platform.h
> > that are only defined when CONFIG_OF=y:
> >
> > of_platform_device_create()
> > of_platform_device_destroy()
> > of_platform_bus_probe()
> > of_device_alloc()
> >
> > Rob, Frank, do you want me to create static inline dummy versions of these?
> 
> No, because generally you should not be using these functions
> directly. Yes, there are some users, but if you look at the tree,
> there are few or isolated (PowerPC) users. Using
> of_platform_populate/of_platform_depopulate is preferred.

of_platform_device_destroy() is also used by sound/soc/qcom/qdsp6/*
which is why I suggested this patch:
https://marc.info/?l=alsa-devel&m=152932497413567
that adds "depends on OF" for SND_SOC_QDSP6 in sound/soc/qcom/Kconfig.

Or do you think that a better solution would be to modify
sound/soc/qcom/qdsp6/* so that it instead uses
of_platform_populate()/of_platform_depopulate()?
(Or perhaps this isn't even an option,
I'm not at all familiar with the QDSP6 code).

Regards,
Niklas
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring June 18, 2018, 8:53 p.m. UTC | #5
On Mon, Jun 18, 2018 at 2:08 PM, Niklas Cassel <niklas.cassel@linaro.org> wrote:
> On Mon, Jun 18, 2018 at 08:48:32AM -0600, Rob Herring wrote:
>> On Mon, Jun 18, 2018 at 6:39 AM, Niklas Cassel <niklas.cassel@linaro.org> wrote:
>> > On Mon, Jun 18, 2018 at 12:06:42PM +0100, Mark Brown wrote:
>> >> On Thu, Jun 14, 2018 at 01:11:36PM +0200, Niklas Cassel wrote:
>> >> > of_platform_device_destroy is only defined when building
>> >> > with CONFIG_OF=y. Add a depends on OF.
>> >>
>> >> Is it sensible that of_platform_device_destroy() is only defined when
>> >> building with CONFIG_OF=y?
>> >
>> > I'm redirecting that question to the device tree maintainers.
>> >
>> > There are a few of_* functions in include/linux/of_platform.h
>> > that are only defined when CONFIG_OF=y:
>> >
>> > of_platform_device_create()
>> > of_platform_device_destroy()
>> > of_platform_bus_probe()
>> > of_device_alloc()
>> >
>> > Rob, Frank, do you want me to create static inline dummy versions of these?
>>
>> No, because generally you should not be using these functions
>> directly. Yes, there are some users, but if you look at the tree,
>> there are few or isolated (PowerPC) users. Using
>> of_platform_populate/of_platform_depopulate is preferred.
>
> of_platform_device_destroy() is also used by sound/soc/qcom/qdsp6/*
> which is why I suggested this patch:
> https://marc.info/?l=alsa-devel&m=152932497413567
> that adds "depends on OF" for SND_SOC_QDSP6 in sound/soc/qcom/Kconfig.
>
> Or do you think that a better solution would be to modify
> sound/soc/qcom/qdsp6/* so that it instead uses
> of_platform_populate()/of_platform_depopulate()?

Yes, that is preferred. However, that won't work here because the
child nodes don't have compatible strings. Maybe we should add them as
this all just went in. That would also allow DT based module
autoloading to work (which I don't think would currently). Really, as
is, of_platform_device_create isn't needed here and you could just use
platform_device_register_simple instead. The child driver would have
to get the DT node pointer from the parent device instead.

But if you want to add empty functions for just
of_platform_device_{create,destroy}, I guess that is fine.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas Kandagatla June 19, 2018, 9:36 a.m. UTC | #6
On 18/06/18 21:53, Rob Herring wrote:
> On Mon, Jun 18, 2018 at 2:08 PM, Niklas Cassel <niklas.cassel@linaro.org> wrote:
>> On Mon, Jun 18, 2018 at 08:48:32AM -0600, Rob Herring wrote:
>>> On Mon, Jun 18, 2018 at 6:39 AM, Niklas Cassel <niklas.cassel@linaro.org> wrote:
>>>> On Mon, Jun 18, 2018 at 12:06:42PM +0100, Mark Brown wrote:
>>>>> On Thu, Jun 14, 2018 at 01:11:36PM +0200, Niklas Cassel wrote:
>>>>>> of_platform_device_destroy is only defined when building
>>>>>> with CONFIG_OF=y. Add a depends on OF.
>>>>>
>>>>> Is it sensible that of_platform_device_destroy() is only defined when
>>>>> building with CONFIG_OF=y?
>>>>
>>>> I'm redirecting that question to the device tree maintainers.
>>>>
>>>> There are a few of_* functions in include/linux/of_platform.h
>>>> that are only defined when CONFIG_OF=y:
>>>>
>>>> of_platform_device_create()
>>>> of_platform_device_destroy()
>>>> of_platform_bus_probe()
>>>> of_device_alloc()
>>>>
>>>> Rob, Frank, do you want me to create static inline dummy versions of these?
>>>
>>> No, because generally you should not be using these functions
>>> directly. Yes, there are some users, but if you look at the tree,
>>> there are few or isolated (PowerPC) users. Using
>>> of_platform_populate/of_platform_depopulate is preferred.
>>
>> of_platform_device_destroy() is also used by sound/soc/qcom/qdsp6/*
>> which is why I suggested this patch:
>> https://marc.info/?l=alsa-devel&m=152932497413567
>> that adds "depends on OF" for SND_SOC_QDSP6 in sound/soc/qcom/Kconfig.
>>
>> Or do you think that a better solution would be to modify
>> sound/soc/qcom/qdsp6/* so that it instead uses
>> of_platform_populate()/of_platform_depopulate()?
> 
> Yes, that is preferred. However, that won't work here because the
> child nodes don't have compatible strings. Maybe we should add them as
> this all just went in. That would also allow DT based module
> autoloading to work (which I don't think would currently). Really, as
> is, of_platform_device_create isn't needed here and you could just use
> platform_device_register_simple instead. The child driver would have
> to get the DT node pointer from the parent device instead.
> 
compatible was the reason why I started using 
of_platform_device_{create,destroy} apis. As you said we could also do 
it with simple platform device too.

I think adding compatible seems to be better option here along with 
of_platform_populate()/of_platform_depopulate(), I will try this!

thanks,
srini

> But if you want to add empty functions for just
> of_platform_device_{create,destroy}, I guess that is fine.
> 
> Rob
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 87838fa27997..3989ec856441 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -67,7 +67,7 @@  config SND_SOC_QDSP6_ASM_DAI
 
 config SND_SOC_QDSP6
 	tristate "SoC ALSA audio driver for QDSP6"
-	depends on QCOM_APR && HAS_DMA
+	depends on QCOM_APR && HAS_DMA && OF
 	select SND_SOC_QDSP6_COMMON
 	select SND_SOC_QDSP6_CORE
 	select SND_SOC_QDSP6_AFE