[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Arnd Bergmann]

The CPPI-4.1 driver selects TI_CPPI41, which is a dmaengine
driver and that may not be available when CONFIG_DMADEVICES
is not set:

warning: (USB_TI_CPPI41_DMA) selects TI_CPPI41 which has unmet direct dependencies (DMADEVICES && ARCH_OMAP)

This adds an extra dependency to avoid generating warnings in randconfig
builds. Ideally we'd remove the 'select' statement, but that has the
potential to break defconfig files.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 411dd19c682d ("usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled")

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 1f2037bbeb0d..45c83baf675d 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
 
 config USB_TI_CPPI41_DMA
 	bool 'TI CPPI 4.1 (AM335x)'
-	depends on ARCH_OMAP
+	depends on ARCH_OMAP && DMADEVICES
 	select TI_CPPI41
 
 config USB_TUSB_OMAP_DMA

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Bin Liu]

Hi,

On Wed, Nov 18, 2015 at 10:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> The CPPI-4.1 driver selects TI_CPPI41, which is a dmaengine
> driver and that may not be available when CONFIG_DMADEVICES
> is not set:
>
> warning: (USB_TI_CPPI41_DMA) selects TI_CPPI41 which has unmet direct dependencies (DMADEVICES && ARCH_OMAP)
>
> This adds an extra dependency to avoid generating warnings in randconfig
> builds. Ideally we'd remove the 'select' statement, but that has the
> potential to break defconfig files.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 411dd19c682d ("usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled")
>
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index 1f2037bbeb0d..45c83baf675d 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>
>  config USB_TI_CPPI41_DMA
>         bool 'TI CPPI 4.1 (AM335x)'
> -       depends on ARCH_OMAP
> +       depends on ARCH_OMAP && DMADEVICES
>         select TI_CPPI41

I am not sure what the generic policy is, but instead of hiding
USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
perspective.

Thanks,
-Bin.

>
>  config USB_TUSB_OMAP_DMA
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Felipe Balbi]

Hi,

Bin Liu <binmlist@gmail.com> writes:
> On Wed, Nov 18, 2015 at 10:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> The CPPI-4.1 driver selects TI_CPPI41, which is a dmaengine
>> driver and that may not be available when CONFIG_DMADEVICES
>> is not set:
>>
>> warning: (USB_TI_CPPI41_DMA) selects TI_CPPI41 which has unmet direct dependencies (DMADEVICES && ARCH_OMAP)
>>
>> This adds an extra dependency to avoid generating warnings in randconfig
>> builds. Ideally we'd remove the 'select' statement, but that has the
>> potential to break defconfig files.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Fixes: 411dd19c682d ("usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled")
>>
>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>> index 1f2037bbeb0d..45c83baf675d 100644
>> --- a/drivers/usb/musb/Kconfig
>> +++ b/drivers/usb/musb/Kconfig
>> @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>>
>>  config USB_TI_CPPI41_DMA
>>         bool 'TI CPPI 4.1 (AM335x)'
>> -       depends on ARCH_OMAP
>> +       depends on ARCH_OMAP && DMADEVICES
>>         select TI_CPPI41
>
> I am not sure what the generic policy is, but instead of hiding
> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
> perspective.

that would mean "select DMADEVICES" and that's frowned upon.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151118/d0365a41/attachment.sig>

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Bin Liu]

Hi,

On Wed, Nov 18, 2015 at 1:27 PM, Felipe Balbi <balbi@ti.com> wrote:
>
> Hi,
>
> Bin Liu <binmlist@gmail.com> writes:
>> On Wed, Nov 18, 2015 at 10:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> The CPPI-4.1 driver selects TI_CPPI41, which is a dmaengine
>>> driver and that may not be available when CONFIG_DMADEVICES
>>> is not set:
>>>
>>> warning: (USB_TI_CPPI41_DMA) selects TI_CPPI41 which has unmet direct dependencies (DMADEVICES && ARCH_OMAP)
>>>
>>> This adds an extra dependency to avoid generating warnings in randconfig
>>> builds. Ideally we'd remove the 'select' statement, but that has the
>>> potential to break defconfig files.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> Fixes: 411dd19c682d ("usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled")
>>>
>>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>>> index 1f2037bbeb0d..45c83baf675d 100644
>>> --- a/drivers/usb/musb/Kconfig
>>> +++ b/drivers/usb/musb/Kconfig
>>> @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>>>
>>>  config USB_TI_CPPI41_DMA
>>>         bool 'TI CPPI 4.1 (AM335x)'
>>> -       depends on ARCH_OMAP
>>> +       depends on ARCH_OMAP && DMADEVICES
>>>         select TI_CPPI41
>>
>> I am not sure what the generic policy is, but instead of hiding
>> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
>> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
>> perspective.
>
> that would mean "select DMADEVICES" and that's frowned upon.

Currently 'select DMADEVICES' is not in there. Will adding it fix the
dependency warning in randconfig? Sorry for the question, but I don't
know enough about Kconfig to get the answer.

Regards,
-Bin.

>
> --
> balbi

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Arnd Bergmann]

On Wednesday 18 November 2015 12:29:27 Bin Liu wrote:
> > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> > index 1f2037bbeb0d..45c83baf675d 100644
> > --- a/drivers/usb/musb/Kconfig
> > +++ b/drivers/usb/musb/Kconfig
> > @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
> >
> >  config USB_TI_CPPI41_DMA
> >         bool 'TI CPPI 4.1 (AM335x)'
> > -       depends on ARCH_OMAP
> > +       depends on ARCH_OMAP && DMADEVICES
> >         select TI_CPPI41
> 
> I am not sure what the generic policy is, but instead of hiding
> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
> perspective.

General policy is that you should not 'select' a symbol that is
also user-visible, as that tends to cause dependency loops and
other problems when something is enabled without the user being
aware of that.

Ideally we should remove the 'select TI_CPPI41' here as well, but
what we could do instead is to make that a silent symbol and remove
the prompt so it always gets enabled implicitly when USB_TI_CPPI41_DMA
and DMADEVICES are both enabled.

	Arnd

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Felipe Balbi]

Hi,

Bin Liu <binmlist@gmail.com> writes:
>> Bin Liu <binmlist@gmail.com> writes:
>>> On Wed, Nov 18, 2015 at 10:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> The CPPI-4.1 driver selects TI_CPPI41, which is a dmaengine
>>>> driver and that may not be available when CONFIG_DMADEVICES
>>>> is not set:
>>>>
>>>> warning: (USB_TI_CPPI41_DMA) selects TI_CPPI41 which has unmet direct dependencies (DMADEVICES && ARCH_OMAP)
>>>>
>>>> This adds an extra dependency to avoid generating warnings in randconfig
>>>> builds. Ideally we'd remove the 'select' statement, but that has the
>>>> potential to break defconfig files.
>>>>
>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>> Fixes: 411dd19c682d ("usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled")
>>>>
>>>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>>>> index 1f2037bbeb0d..45c83baf675d 100644
>>>> --- a/drivers/usb/musb/Kconfig
>>>> +++ b/drivers/usb/musb/Kconfig
>>>> @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>>>>
>>>>  config USB_TI_CPPI41_DMA
>>>>         bool 'TI CPPI 4.1 (AM335x)'
>>>> -       depends on ARCH_OMAP
>>>> +       depends on ARCH_OMAP && DMADEVICES
>>>>         select TI_CPPI41
>>>
>>> I am not sure what the generic policy is, but instead of hiding
>>> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
>>> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
>>> perspective.
>>
>> that would mean "select DMADEVICES" and that's frowned upon.
>
> Currently 'select DMADEVICES' is not in there. Will adding it fix the
> dependency warning in randconfig? Sorry for the question, but I don't
> know enough about Kconfig to get the answer.

it certainly would, but we don't like to add "select XYZ" to Kconfig
because a select bypasses the dependency tree. Let me explain:

config A
       tristate "A"
       depends on B

config B
       tristate "B"


config C
       tristate "C"
       select A


C can select A without B being enabled.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151118/b9899acb/attachment.sig>

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Bin Liu]

Hi,

On Wed, Nov 18, 2015 at 2:27 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 18 November 2015 12:29:27 Bin Liu wrote:
>> > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>> > index 1f2037bbeb0d..45c83baf675d 100644
>> > --- a/drivers/usb/musb/Kconfig
>> > +++ b/drivers/usb/musb/Kconfig
>> > @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>> >
>> >  config USB_TI_CPPI41_DMA
>> >         bool 'TI CPPI 4.1 (AM335x)'
>> > -       depends on ARCH_OMAP
>> > +       depends on ARCH_OMAP && DMADEVICES
>> >         select TI_CPPI41
>>
>> I am not sure what the generic policy is, but instead of hiding
>> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
>> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
>> perspective.
>
> General policy is that you should not 'select' a symbol that is
> also user-visible, as that tends to cause dependency loops and
> other problems when something is enabled without the user being
> aware of that.

Understood. Thanks. I am okay with this patch.

>
> Ideally we should remove the 'select TI_CPPI41' here as well, but
> what we could do instead is to make that a silent symbol and remove
> the prompt so it always gets enabled implicitly when USB_TI_CPPI41_DMA
> and DMADEVICES are both enabled.

But what if DMADEVICES was disabled and USB_TI_CPPI41_DMA was enabled?
I would think I had CPPI fully enabled for MUSB, but it didn't because
TI_CPPI41 was disabled.

I would think this patch is the test option so far, we might have to
document somewhere that to dmaengine has to be enabled to use MUSB
CPPI, but I am not sure where the best place is to document...

Regards,
-Bin.

>
>         Arnd

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Felipe Balbi]

Hi,

Arnd Bergmann <arnd@arndb.de> writes:
> On Wednesday 18 November 2015 12:29:27 Bin Liu wrote:
>> > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>> > index 1f2037bbeb0d..45c83baf675d 100644
>> > --- a/drivers/usb/musb/Kconfig
>> > +++ b/drivers/usb/musb/Kconfig
>> > @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>> >
>> >  config USB_TI_CPPI41_DMA
>> >         bool 'TI CPPI 4.1 (AM335x)'
>> > -       depends on ARCH_OMAP
>> > +       depends on ARCH_OMAP && DMADEVICES
>> >         select TI_CPPI41
>> 
>> I am not sure what the generic policy is, but instead of hiding
>> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
>> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
>> perspective.
>
> General policy is that you should not 'select' a symbol that is
> also user-visible, as that tends to cause dependency loops and
> other problems when something is enabled without the user being
> aware of that.
>
> Ideally we should remove the 'select TI_CPPI41' here as well, but
> what we could do instead is to make that a silent symbol and remove
> the prompt so it always gets enabled implicitly when USB_TI_CPPI41_DMA
> and DMADEVICES are both enabled.

that should be perfect now that Tony L fixed this up so we can enable
all MUSB DMA Engines in a single zImage.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151118/b6237d3d/attachment-0001.sig>

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Bin Liu]

On Wed, Nov 18, 2015 at 2:38 PM, Felipe Balbi <balbi@ti.com> wrote:
>
> Hi,
>
> Bin Liu <binmlist@gmail.com> writes:
>>> Bin Liu <binmlist@gmail.com> writes:
>>>> On Wed, Nov 18, 2015 at 10:18 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>> The CPPI-4.1 driver selects TI_CPPI41, which is a dmaengine
>>>>> driver and that may not be available when CONFIG_DMADEVICES
>>>>> is not set:
>>>>>
>>>>> warning: (USB_TI_CPPI41_DMA) selects TI_CPPI41 which has unmet direct dependencies (DMADEVICES && ARCH_OMAP)
>>>>>
>>>>> This adds an extra dependency to avoid generating warnings in randconfig
>>>>> builds. Ideally we'd remove the 'select' statement, but that has the
>>>>> potential to break defconfig files.
>>>>>
>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>>> Fixes: 411dd19c682d ("usb: musb: Kconfig: Select the DMA driver if DMA mode of MUSB is enabled")
>>>>>
>>>>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>>>>> index 1f2037bbeb0d..45c83baf675d 100644
>>>>> --- a/drivers/usb/musb/Kconfig
>>>>> +++ b/drivers/usb/musb/Kconfig
>>>>> @@ -159,7 +159,7 @@ config USB_TI_CPPI_DMA
>>>>>
>>>>>  config USB_TI_CPPI41_DMA
>>>>>         bool 'TI CPPI 4.1 (AM335x)'
>>>>> -       depends on ARCH_OMAP
>>>>> +       depends on ARCH_OMAP && DMADEVICES
>>>>>         select TI_CPPI41
>>>>
>>>> I am not sure what the generic policy is, but instead of hiding
>>>> USB_TI_CPPI41_DMA if DMADEVICES is disabled, I'd like to enable
>>>> DMADEVICES if USB_TI_CPPI41_DMA is enabled, from user experience
>>>> perspective.
>>>
>>> that would mean "select DMADEVICES" and that's frowned upon.
>>
>> Currently 'select DMADEVICES' is not in there. Will adding it fix the
>> dependency warning in randconfig? Sorry for the question, but I don't
>> know enough about Kconfig to get the answer.
>
> it certainly would, but we don't like to add "select XYZ" to Kconfig
> because a select bypasses the dependency tree. Let me explain:
>
> config A
>        tristate "A"
>        depends on B
>
> config B
>        tristate "B"
>
>
> config C
>        tristate "C"
>        select A
>
>
> C can select A without B being enabled.

Thanks for the explanation, very clear.

>
> --
> balbi

[PATCH] usb: musb: USB_TI_CPPI41_DMA requires dmaengine support

[Arnd Bergmann]

On Wednesday 18 November 2015 14:39:10 Bin Liu wrote:
> > Ideally we should remove the 'select TI_CPPI41' here as well, but
> > what we could do instead is to make that a silent symbol and remove
> > the prompt so it always gets enabled implicitly when USB_TI_CPPI41_DMA
> > and DMADEVICES are both enabled.
> 
> But what if DMADEVICES was disabled and USB_TI_CPPI41_DMA was enabled?
> I would think I had CPPI fully enabled for MUSB, but it didn't because
> TI_CPPI41 was disabled.

That would cause a runtime failure, just like any other configuration
that does not enable all the hardware you want to use.
 
> I would think this patch is the test option so far, we might have to
> document somewhere that to dmaengine has to be enabled to use MUSB
> CPPI, but I am not sure where the best place is to document...

There are hundreds of device drivers that use dmaengines as a
backend, we don't normally document this, just like we don't
document the fact that you need to enable the right gpio, irqchip,
timer, clock etc drivers for your platform.

	Arnd