diff mbox

[1/1] mmc: host: enable OMAP DMA engine support for omap hosts by default

Message ID CAAwP0s2Efuuc7vsX3bZuwz175zUjnmsOV87OcZ06p_eoe4T3Ug@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas July 18, 2012, 8:49 a.m. UTC
On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:
> On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com> wrote:
>> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com> wrote:
>>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
>>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
>>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
>>>> >> <santosh.shilimkar@ti.com> wrote:
>>>> >> > Hi,
>>>> >> >
>>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
>>>> >> > <javier@dowhile0.org> wrote:
>>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely the DMA
>>>> >> >> engine API instead of the previous private DMA API implementation.
>>>> >> >>
>>>> >> >> So, if the kernel is built with support for any of these hosts but it
>>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails when trying
>>>> >> >> to obtain a DMA channel which leads to the following error on an OMAP3
>>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC support):
>>>> >> >>
>>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
>>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
>>>> >> >>
>>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP solves it.
>>>> >> >>
>>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
>>>> >> >> ---
>>>> >> > Considering, we are updating drivers to select the DMA engine, can you
>>>> >> > also include
>>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA engine.
>>>> >> >
>>>> >> > Regards
>>>> >> > Santosh
>>>> >>
>>>> >> Hi Santosh,
>>>> >>
>>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
>>>> >
>>>> > I don't think we should do this, the drivers should work with and without
>>>> > dma. This just needs to be added to the omap2plus_defconfig.
>>>> >
>>>> Well this was not decided based on any DMA CONFIG option before for
>>>> the subject drivers. It is already by default enabled if the DMA is supported
>>>> by the driver IP. There is a possibility to disable it from driver platform/dt
>>>> data so that still remains.
>>>
>>> I think it should rather be that if the driver is broken and does not work
>>> without DMA, it should have depends on CONFIG_DMA_OMAP.
>>>
>> I can confirm that omap MMC can't work without DMA; polled mode is not
>> supported / implemented.
>
> Same case for SPI driver as well. It uses DMA for everything except the cases
> where DMA doesn't make sense like 1 byte/2 byte etc. And its not configurable,
>
> At least considering this, it is better we do this per driver than enabling
> it at SOC config.
>
> Regards
> Santosh
> --

Hi Santosh,

And what about enabling it at the SoC config level but making the
drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
agree I can send something like this in two different patches (one for
the omap2plus_defconfig and another to make the drivers dependant on
the config option):


Best regards,
Javier

Comments

Santosh Shilimkar July 18, 2012, 9:11 a.m. UTC | #1
On Wed, Jul 18, 2012 at 2:19 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
>> On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com> wrote:
>>> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
>>>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
>>>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
>>>>> >> <santosh.shilimkar@ti.com> wrote:
>>>>> >> > Hi,
>>>>> >> >
>>>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
>>>>> >> > <javier@dowhile0.org> wrote:
>>>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely the DMA
>>>>> >> >> engine API instead of the previous private DMA API implementation.
>>>>> >> >>
>>>>> >> >> So, if the kernel is built with support for any of these hosts but it
>>>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails when trying
>>>>> >> >> to obtain a DMA channel which leads to the following error on an OMAP3
>>>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC support):
>>>>> >> >>
>>>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
>>>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
>>>>> >> >>
>>>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP solves it.
>>>>> >> >>
>>>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
>>>>> >> >> ---
>>>>> >> > Considering, we are updating drivers to select the DMA engine, can you
>>>>> >> > also include
>>>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA engine.
>>>>> >> >
>>>>> >> > Regards
>>>>> >> > Santosh
>>>>> >>
>>>>> >> Hi Santosh,
>>>>> >>
>>>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
>>>>> >
>>>>> > I don't think we should do this, the drivers should work with and without
>>>>> > dma. This just needs to be added to the omap2plus_defconfig.
>>>>> >
>>>>> Well this was not decided based on any DMA CONFIG option before for
>>>>> the subject drivers. It is already by default enabled if the DMA is supported
>>>>> by the driver IP. There is a possibility to disable it from driver platform/dt
>>>>> data so that still remains.
>>>>
>>>> I think it should rather be that if the driver is broken and does not work
>>>> without DMA, it should have depends on CONFIG_DMA_OMAP.
>>>>
>>> I can confirm that omap MMC can't work without DMA; polled mode is not
>>> supported / implemented.
>>
>> Same case for SPI driver as well. It uses DMA for everything except the cases
>> where DMA doesn't make sense like 1 byte/2 byte etc. And its not configurable,
>>
>> At least considering this, it is better we do this per driver than enabling
>> it at SOC config.
>>
>> Regards
>> Santosh
>> --
>
> Hi Santosh,
>
> And what about enabling it at the SoC config level but making the
> drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
> agree I can send something like this in two different patches (one for
> the omap2plus_defconfig and another to make the drivers dependant on
> the config option):
>
> diff --git a/arch/arm/configs/omap2plus_defconfig
> b/arch/arm/configs/omap2plus_defconfig
> index b152de7..e58edc3 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
>  CONFIG_RTC_CLASS=y
>  CONFIG_RTC_DRV_TWL92330=y
>  CONFIG_RTC_DRV_TWL4030=y
> +CONFIG_DMADEVICES=y
> +CONFIG_DMA_OMAP=y
>  CONFIG_EXT2_FS=y
>  CONFIG_EXT3_FS=y
>  # CONFIG_EXT3_FS_XATTR is not set
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index aa131b3..314c7bd 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -231,7 +231,7 @@ config MMC_SDHCI_S3C_DMA
>
>  config MMC_OMAP
>         tristate "TI OMAP Multimedia Card Interface support"
> -       depends on ARCH_OMAP
> +       depends on ARCH_OMAP && DMADEVICES && DMA_OMAP

May be. But since for sure a driver knows that it needs DMA
support to be enabled, I will just select it rather than depends
on.

Regards
santosh
Javier Martinez Canillas July 18, 2012, 9:16 a.m. UTC | #2
On Wed, Jul 18, 2012 at 11:11 AM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:
> On Wed, Jul 18, 2012 at 2:19 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
>> <santosh.shilimkar@ti.com> wrote:
>>> On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com> wrote:
>>>> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
>>>>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
>>>>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
>>>>>> >> <santosh.shilimkar@ti.com> wrote:
>>>>>> >> > Hi,
>>>>>> >> >
>>>>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
>>>>>> >> > <javier@dowhile0.org> wrote:
>>>>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely the DMA
>>>>>> >> >> engine API instead of the previous private DMA API implementation.
>>>>>> >> >>
>>>>>> >> >> So, if the kernel is built with support for any of these hosts but it
>>>>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails when trying
>>>>>> >> >> to obtain a DMA channel which leads to the following error on an OMAP3
>>>>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC support):
>>>>>> >> >>
>>>>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
>>>>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
>>>>>> >> >>
>>>>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP solves it.
>>>>>> >> >>
>>>>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
>>>>>> >> >> ---
>>>>>> >> > Considering, we are updating drivers to select the DMA engine, can you
>>>>>> >> > also include
>>>>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA engine.
>>>>>> >> >
>>>>>> >> > Regards
>>>>>> >> > Santosh
>>>>>> >>
>>>>>> >> Hi Santosh,
>>>>>> >>
>>>>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
>>>>>> >
>>>>>> > I don't think we should do this, the drivers should work with and without
>>>>>> > dma. This just needs to be added to the omap2plus_defconfig.
>>>>>> >
>>>>>> Well this was not decided based on any DMA CONFIG option before for
>>>>>> the subject drivers. It is already by default enabled if the DMA is supported
>>>>>> by the driver IP. There is a possibility to disable it from driver platform/dt
>>>>>> data so that still remains.
>>>>>
>>>>> I think it should rather be that if the driver is broken and does not work
>>>>> without DMA, it should have depends on CONFIG_DMA_OMAP.
>>>>>
>>>> I can confirm that omap MMC can't work without DMA; polled mode is not
>>>> supported / implemented.
>>>
>>> Same case for SPI driver as well. It uses DMA for everything except the cases
>>> where DMA doesn't make sense like 1 byte/2 byte etc. And its not configurable,
>>>
>>> At least considering this, it is better we do this per driver than enabling
>>> it at SOC config.
>>>
>>> Regards
>>> Santosh
>>> --
>>
>> Hi Santosh,
>>
>> And what about enabling it at the SoC config level but making the
>> drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
>> agree I can send something like this in two different patches (one for
>> the omap2plus_defconfig and another to make the drivers dependant on
>> the config option):
>>
>> diff --git a/arch/arm/configs/omap2plus_defconfig
>> b/arch/arm/configs/omap2plus_defconfig
>> index b152de7..e58edc3 100644
>> --- a/arch/arm/configs/omap2plus_defconfig
>> +++ b/arch/arm/configs/omap2plus_defconfig
>> @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
>>  CONFIG_RTC_CLASS=y
>>  CONFIG_RTC_DRV_TWL92330=y
>>  CONFIG_RTC_DRV_TWL4030=y
>> +CONFIG_DMADEVICES=y
>> +CONFIG_DMA_OMAP=y
>>  CONFIG_EXT2_FS=y
>>  CONFIG_EXT3_FS=y
>>  # CONFIG_EXT3_FS_XATTR is not set
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index aa131b3..314c7bd 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -231,7 +231,7 @@ config MMC_SDHCI_S3C_DMA
>>
>>  config MMC_OMAP
>>         tristate "TI OMAP Multimedia Card Interface support"
>> -       depends on ARCH_OMAP
>> +       depends on ARCH_OMAP && DMADEVICES && DMA_OMAP
>
> May be. But since for sure a driver knows that it needs DMA
> support to be enabled, I will just select it rather than depends
> on.
>
> Regards
> santosh

Yes I agree with you, I was just exploring other options :-)

Best regards,
Javier
Venkatraman S July 18, 2012, 9:38 a.m. UTC | #3
On Wed, Jul 18, 2012 at 2:46 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Wed, Jul 18, 2012 at 11:11 AM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
>> On Wed, Jul 18, 2012 at 2:19 PM, Javier Martinez Canillas
>> <martinez.javier@gmail.com> wrote:
>>> On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
>>> <santosh.shilimkar@ti.com> wrote:
>>>> On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com> wrote:
>>>>> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
>>>>>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
>>>>>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
>>>>>>> >> <santosh.shilimkar@ti.com> wrote:
>>>>>>> >> > Hi,
>>>>>>> >> >
>>>>>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
>>>>>>> >> > <javier@dowhile0.org> wrote:
>>>>>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely the DMA
>>>>>>> >> >> engine API instead of the previous private DMA API implementation.
>>>>>>> >> >>
>>>>>>> >> >> So, if the kernel is built with support for any of these hosts but it
>>>>>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails when trying
>>>>>>> >> >> to obtain a DMA channel which leads to the following error on an OMAP3
>>>>>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC support):
>>>>>>> >> >>
>>>>>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
>>>>>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
>>>>>>> >> >>
>>>>>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP solves it.
>>>>>>> >> >>
>>>>>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
>>>>>>> >> >> ---
>>>>>>> >> > Considering, we are updating drivers to select the DMA engine, can you
>>>>>>> >> > also include
>>>>>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA engine.
>>>>>>> >> >
>>>>>>> >> > Regards
>>>>>>> >> > Santosh
>>>>>>> >>
>>>>>>> >> Hi Santosh,
>>>>>>> >>
>>>>>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
>>>>>>> >
>>>>>>> > I don't think we should do this, the drivers should work with and without
>>>>>>> > dma. This just needs to be added to the omap2plus_defconfig.
>>>>>>> >
>>>>>>> Well this was not decided based on any DMA CONFIG option before for
>>>>>>> the subject drivers. It is already by default enabled if the DMA is supported
>>>>>>> by the driver IP. There is a possibility to disable it from driver platform/dt
>>>>>>> data so that still remains.
>>>>>>
>>>>>> I think it should rather be that if the driver is broken and does not work
>>>>>> without DMA, it should have depends on CONFIG_DMA_OMAP.
>>>>>>
>>>>> I can confirm that omap MMC can't work without DMA; polled mode is not
>>>>> supported / implemented.
>>>>
>>>> Same case for SPI driver as well. It uses DMA for everything except the cases
>>>> where DMA doesn't make sense like 1 byte/2 byte etc. And its not configurable,
>>>>
>>>> At least considering this, it is better we do this per driver than enabling
>>>> it at SOC config.
>>>>
>>>> Regards
>>>> Santosh
>>>> --
>>>
>>> Hi Santosh,
>>>
>>> And what about enabling it at the SoC config level but making the
>>> drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
>>> agree I can send something like this in two different patches (one for
>>> the omap2plus_defconfig and another to make the drivers dependant on
>>> the config option):
>>>
>>> diff --git a/arch/arm/configs/omap2plus_defconfig
>>> b/arch/arm/configs/omap2plus_defconfig
>>> index b152de7..e58edc3 100644
>>> --- a/arch/arm/configs/omap2plus_defconfig
>>> +++ b/arch/arm/configs/omap2plus_defconfig
>>> @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
>>>  CONFIG_RTC_CLASS=y
>>>  CONFIG_RTC_DRV_TWL92330=y
>>>  CONFIG_RTC_DRV_TWL4030=y
>>> +CONFIG_DMADEVICES=y
>>> +CONFIG_DMA_OMAP=y
>>>  CONFIG_EXT2_FS=y
>>>  CONFIG_EXT3_FS=y
>>>  # CONFIG_EXT3_FS_XATTR is not set
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index aa131b3..314c7bd 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -231,7 +231,7 @@ config MMC_SDHCI_S3C_DMA
>>>
>>>  config MMC_OMAP
>>>         tristate "TI OMAP Multimedia Card Interface support"
>>> -       depends on ARCH_OMAP
>>> +       depends on ARCH_OMAP && DMADEVICES && DMA_OMAP
>>
>> May be. But since for sure a driver knows that it needs DMA
>> support to be enabled, I will just select it rather than depends
>> on.
>>
>> Regards
>> santosh
>
> Yes I agree with you, I was just exploring other options :-)
>
For MMC atleast, there's already a patch in mmc-next to do a "Depends On"
http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commit;h=7c68046c99f0b96d965c31cf42814b9a0f15ad46
Santosh Shilimkar July 18, 2012, 9:44 a.m. UTC | #4
On Wed, Jul 18, 2012 at 3:08 PM, S, Venkatraman <svenkatr@ti.com> wrote:
> On Wed, Jul 18, 2012 at 2:46 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:

[...]

>>>>
>>>> And what about enabling it at the SoC config level but making the
>>>> drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
>>>> agree I can send something like this in two different patches (one for
>>>> the omap2plus_defconfig and another to make the drivers dependant on
>>>> the config option):
>>>>
>>>> diff --git a/arch/arm/configs/omap2plus_defconfig
>>>> b/arch/arm/configs/omap2plus_defconfig
>>>> index b152de7..e58edc3 100644
>>>> --- a/arch/arm/configs/omap2plus_defconfig
>>>> +++ b/arch/arm/configs/omap2plus_defconfig
>>>> @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
>>>>  CONFIG_RTC_CLASS=y
>>>>  CONFIG_RTC_DRV_TWL92330=y
>>>>  CONFIG_RTC_DRV_TWL4030=y
>>>> +CONFIG_DMADEVICES=y
>>>> +CONFIG_DMA_OMAP=y
>>>>  CONFIG_EXT2_FS=y
>>>>  CONFIG_EXT3_FS=y
>>>>  # CONFIG_EXT3_FS_XATTR is not set
>>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>>> index aa131b3..314c7bd 100644
>>>> --- a/drivers/mmc/host/Kconfig
>>>> +++ b/drivers/mmc/host/Kconfig
>>>> @@ -231,7 +231,7 @@ config MMC_SDHCI_S3C_DMA
>>>>
>>>>  config MMC_OMAP
>>>>         tristate "TI OMAP Multimedia Card Interface support"
>>>> -       depends on ARCH_OMAP
>>>> +       depends on ARCH_OMAP && DMADEVICES && DMA_OMAP
>>>
>>> May be. But since for sure a driver knows that it needs DMA
>>> support to be enabled, I will just select it rather than depends
>>> on.
>>>
>>> Regards
>>> santosh
>>
>> Yes I agree with you, I was just exploring other options :-)
>>
> For MMC atleast, there's already a patch in mmc-next to do a "Depends On"
> http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commit;h=7c68046c99f0b96d965c31cf42814b9a0f15ad46

Change log assumes says .

"The patch simply make them depend on DMA_OMAP since DMA_OMAP
will select DMA_ENGINE automatically"

This won't be true if the DMA selection are not done
at ARCH_OMAP level as discussed in this thread.

Having said that, I think Russell and Tony need to
take call on how this needs to be handled.

Regards
Santosh
Tony Lindgren July 19, 2012, 11:32 a.m. UTC | #5
* Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 02:49]:
> 
> "The patch simply make them depend on DMA_OMAP since DMA_OMAP
> will select DMA_ENGINE automatically"
> 
> This won't be true if the DMA selection are not done
> at ARCH_OMAP level as discussed in this thread.
> 
> Having said that, I think Russell and Tony need to
> take call on how this needs to be handled.

As the DMA channels can run out, drivers should also work
without DMA. And building everything as modules should be
possible for the distro kernels.

So I'd rather not have either select or depends and have
the drivers fixed.

Tony
Peter Meerwald-Stadler Aug. 23, 2012, 9 p.m. UTC | #6
On Wed, 18 Jul 2012, Javier Martinez Canillas wrote:

> On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
> > On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com> wrote:
> >> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com> wrote:
> >>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
> >>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com> wrote:
> >>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
> >>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
> >>>> >> <santosh.shilimkar@ti.com> wrote:
> >>>> >> > Hi,
> >>>> >> >
> >>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
> >>>> >> > <javier@dowhile0.org> wrote:
> >>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely the DMA
> >>>> >> >> engine API instead of the previous private DMA API implementation.
> >>>> >> >>
> >>>> >> >> So, if the kernel is built with support for any of these hosts but it
> >>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails when trying
> >>>> >> >> to obtain a DMA channel which leads to the following error on an OMAP3
> >>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC support):
> >>>> >> >>
> >>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA engine channel 48
> >>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX DMA engine channel 62
> >>>> >> >>
> >>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP solves it.
> >>>> >> >>
> >>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
> >>>> >> >> ---
> >>>> >> > Considering, we are updating drivers to select the DMA engine, can you
> >>>> >> > also include
> >>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA engine.
> >>>> >> >
> >>>> >> > Regards
> >>>> >> > Santosh
> >>>> >>
> >>>> >> Hi Santosh,
> >>>> >>
> >>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
> >>>> >
> >>>> > I don't think we should do this, the drivers should work with and without
> >>>> > dma. This just needs to be added to the omap2plus_defconfig.
> >>>> >
> >>>> Well this was not decided based on any DMA CONFIG option before for
> >>>> the subject drivers. It is already by default enabled if the DMA is supported
> >>>> by the driver IP. There is a possibility to disable it from driver platform/dt
> >>>> data so that still remains.
> >>>
> >>> I think it should rather be that if the driver is broken and does not work
> >>> without DMA, it should have depends on CONFIG_DMA_OMAP.
> >>>
> >> I can confirm that omap MMC can't work without DMA; polled mode is not
> >> supported / implemented.
> >
> > Same case for SPI driver as well. It uses DMA for everything except the cases
> > where DMA doesn't make sense like 1 byte/2 byte etc. And its not configurable,
> >
> > At least considering this, it is better we do this per driver than enabling
> > it at SOC config.
> >
> > Regards
> > Santosh
> > --
> 
> Hi Santosh,
> 
> And what about enabling it at the SoC config level but making the
> drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
> agree I can send something like this in two different patches (one for
> the omap2plus_defconfig and another to make the drivers dependant on
> the config option):
> 
> diff --git a/arch/arm/configs/omap2plus_defconfig
> b/arch/arm/configs/omap2plus_defconfig
> index b152de7..e58edc3 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
>  CONFIG_RTC_CLASS=y
>  CONFIG_RTC_DRV_TWL92330=y
>  CONFIG_RTC_DRV_TWL4030=y
> +CONFIG_DMADEVICES=y
> +CONFIG_DMA_OMAP=y
>  CONFIG_EXT2_FS=y
>  CONFIG_EXT3_FS=y
>  # CONFIG_EXT3_FS_XATTR is not set

above has been merged, 89269ef1f0abc72c551198123e19cd4edfd43cf4
but I am missing the patches below in mainline (3.6-rc3) -- what happened?

as Javier pointed out in https://patchwork.kernel.org/patch/1203391/, 
MMC is broken support e.g. on beagleboard unless DMA_OMAP is defined

I suggest to take below patches and help to avoid some extra gray hair for 
people looking for a fix for non-booting beagleboards

thanks, p.

> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index aa131b3..314c7bd 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -231,7 +231,7 @@ config MMC_SDHCI_S3C_DMA
> 
>  config MMC_OMAP
>  	tristate "TI OMAP Multimedia Card Interface support"
> -	depends on ARCH_OMAP
> +	depends on ARCH_OMAP && DMADEVICES && DMA_OMAP
>  	select TPS65010 if MACH_OMAP_H2
>  	help
>  	  This selects the TI OMAP Multimedia card Interface.
> @@ -242,7 +242,8 @@ config MMC_OMAP
> 
>  config MMC_OMAP_HS
>  	tristate "TI OMAP High Speed Multimedia Card Interface support"
> -	depends on SOC_OMAP2430 || ARCH_OMAP3 || ARCH_OMAP4
> +	depends on (SOC_OMAP2430 || ARCH_OMAP3 || ARCH_OMAP4) && \
> +		    DMADEVICES && DMA_OMAP
>  	help
>  	  This selects the TI OMAP High Speed Multimedia card Interface.
>  	  If you have an OMAP2430 or OMAP3 board or OMAP4 board with a
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index cd2fe35..1c23242 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -237,7 +237,7 @@ config SPI_OMAP_UWIRE
> 
>  config SPI_OMAP24XX
>  	tristate "McSPI driver for OMAP"
> -	depends on ARCH_OMAP2PLUS
> +	depends on ARCH_OMAP2PLUS && DMADEVICES && DMA_OMAP
>  	help
>  	  SPI master controller for OMAP24XX and later Multichannel SPI
>  	  (McSPI) modules.
Santosh Shilimkar Aug. 24, 2012, 7:10 a.m. UTC | #7
On Fri, Aug 24, 2012 at 2:30 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>
> On Wed, 18 Jul 2012, Javier Martinez Canillas wrote:
>
> > On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
> > <santosh.shilimkar@ti.com> wrote:
> > > On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com>
> > > wrote:
> > >> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com>
> > >> wrote:
> > >>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
> > >>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com>
> > >>>> wrote:
> > >>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
> > >>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
> > >>>> >> <santosh.shilimkar@ti.com> wrote:
> > >>>> >> > Hi,
> > >>>> >> >
> > >>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
> > >>>> >> > <javier@dowhile0.org> wrote:
> > >>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely
> > >>>> >> >> the DMA
> > >>>> >> >> engine API instead of the previous private DMA API
> > >>>> >> >> implementation.
> > >>>> >> >>
> > >>>> >> >> So, if the kernel is built with support for any of these
> > >>>> >> >> hosts but it
> > >>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails
> > >>>> >> >> when trying
> > >>>> >> >> to obtain a DMA channel which leads to the following error on
> > >>>> >> >> an OMAP3
> > >>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC
> > >>>> >> >> support):
> > >>>> >> >>
> > >>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA
> > >>>> >> >> engine channel 48
> > >>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX
> > >>>> >> >> DMA engine channel 62
> > >>>> >> >>
> > >>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP
> > >>>> >> >> solves it.
> > >>>> >> >>
> > >>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
> > >>>> >> >> ---
> > >>>> >> > Considering, we are updating drivers to select the DMA engine,
> > >>>> >> > can you
> > >>>> >> > also include
> > >>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA
> > >>>> >> > engine.
> > >>>> >> >
> > >>>> >> > Regards
> > >>>> >> > Santosh
> > >>>> >>
> > >>>> >> Hi Santosh,
> > >>>> >>
> > >>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
> > >>>> >
> > >>>> > I don't think we should do this, the drivers should work with and
> > >>>> > without
> > >>>> > dma. This just needs to be added to the omap2plus_defconfig.
> > >>>> >
> > >>>> Well this was not decided based on any DMA CONFIG option before for
> > >>>> the subject drivers. It is already by default enabled if the DMA is
> > >>>> supported
> > >>>> by the driver IP. There is a possibility to disable it from driver
> > >>>> platform/dt
> > >>>> data so that still remains.
> > >>>
> > >>> I think it should rather be that if the driver is broken and does
> > >>> not work
> > >>> without DMA, it should have depends on CONFIG_DMA_OMAP.
> > >>>
> > >> I can confirm that omap MMC can't work without DMA; polled mode is
> > >> not
> > >> supported / implemented.
> > >
> > > Same case for SPI driver as well. It uses DMA for everything except
> > > the cases
> > > where DMA doesn't make sense like 1 byte/2 byte etc. And its not
> > > configurable,
> > >
> > > At least considering this, it is better we do this per driver than
> > > enabling
> > > it at SOC config.
> > >
> > > Regards
> > > Santosh
> > > --
> >
> > Hi Santosh,
> >
> > And what about enabling it at the SoC config level but making the
> > drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
> > agree I can send something like this in two different patches (one for
> > the omap2plus_defconfig and another to make the drivers dependant on
> > the config option):
> >
> > diff --git a/arch/arm/configs/omap2plus_defconfig
> > b/arch/arm/configs/omap2plus_defconfig
> > index b152de7..e58edc3 100644
> > --- a/arch/arm/configs/omap2plus_defconfig
> > +++ b/arch/arm/configs/omap2plus_defconfig
> > @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
> >  CONFIG_RTC_CLASS=y
> >  CONFIG_RTC_DRV_TWL92330=y
> >  CONFIG_RTC_DRV_TWL4030=y
> > +CONFIG_DMADEVICES=y
> > +CONFIG_DMA_OMAP=y
> >  CONFIG_EXT2_FS=y
> >  CONFIG_EXT3_FS=y
> >  # CONFIG_EXT3_FS_XATTR is not set
>
> above has been merged, 89269ef1f0abc72c551198123e19cd4edfd43cf4
> but I am missing the patches below in mainline (3.6-rc3) -- what happened?
>
> as Javier pointed out in https://patchwork.kernel.org/patch/1203391/,
> MMC is broken support e.g. on beagleboard unless DMA_OMAP is defined
>
> I suggest to take below patches and help to avoid some extra gray hair for
> people looking for a fix for non-booting beagleboards
>
May be I am missing something, but why you would need below patch
for beagle with "89269ef1f0abc72c551198123e19cd4edfd43cf4" commit
enabling the DMA_OMAP for all OMAP boards.

Regards
Santosh
Peter Meerwald-Stadler Aug. 24, 2012, 7:51 a.m. UTC | #8
On Fri, 24 Aug 2012, Shilimkar, Santosh wrote:

> On Fri, Aug 24, 2012 at 2:30 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
> >
> > On Wed, 18 Jul 2012, Javier Martinez Canillas wrote:
> >
> > > On Wed, Jul 18, 2012 at 10:36 AM, Shilimkar, Santosh
> > > <santosh.shilimkar@ti.com> wrote:
> > > > On Wed, Jul 18, 2012 at 1:14 PM, S, Venkatraman <svenkatr@ti.com>
> > > > wrote:
> > > >> On Wed, Jul 18, 2012 at 12:40 PM, Tony Lindgren <tony@atomide.com>
> > > >> wrote:
> > > >>> * Shilimkar, Santosh <santosh.shilimkar@ti.com> [120718 00:09]:
> > > >>>> On Wed, Jul 18, 2012 at 12:29 PM, Tony Lindgren <tony@atomide.com>
> > > >>>> wrote:
> > > >>>> > * Javier Martinez Canillas <javier@dowhile0.org> [120716 23:56]:
> > > >>>> >> On Tue, Jul 17, 2012 at 8:45 AM, Shilimkar, Santosh
> > > >>>> >> <santosh.shilimkar@ti.com> wrote:
> > > >>>> >> > Hi,
> > > >>>> >> >
> > > >>>> >> > On Tue, Jul 17, 2012 at 6:00 AM, Javier Martinez Canillas
> > > >>>> >> > <javier@dowhile0.org> wrote:
> > > >>>> >> >> The OMAP MMC and OMAP High Speed MMC hosts now use entirely
> > > >>>> >> >> the DMA
> > > >>>> >> >> engine API instead of the previous private DMA API
> > > >>>> >> >> implementation.
> > > >>>> >> >>
> > > >>>> >> >> So, if the kernel is built with support for any of these
> > > >>>> >> >> hosts but it
> > > >>>> >> >> doesn't support DMA devices nor OMAP DMA support, it fails
> > > >>>> >> >> when trying
> > > >>>> >> >> to obtain a DMA channel which leads to the following error on
> > > >>>> >> >> an OMAP3
> > > >>>> >> >> IGEPv2 Rev.C board (and probably on most OMAP boards with MMC
> > > >>>> >> >> support):
> > > >>>> >> >>
> > > >>>> >> >> [ 2.199981] omap_hsmmc omap_hsmmc.1: unable to obtain RX DMA
> > > >>>> >> >> engine channel 48
> > > >>>> >> >> [    2.215087] omap_hsmmc omap_hsmmc.0: unable to obtain RX
> > > >>>> >> >> DMA engine channel 62
> > > >>>> >> >>
> > > >>>> >> >> selecting automatically CONFIG_DMADEVICES and CONFIG_DMA_OMAP
> > > >>>> >> >> solves it.
> > > >>>> >> >>
> > > >>>> >> >> Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
> > > >>>> >> >> ---
> > > >>>> >> > Considering, we are updating drivers to select the DMA engine,
> > > >>>> >> > can you
> > > >>>> >> > also include
> > > >>>> >> > "drivers/spi/spi-omap2-mcspi.c" which is also updated for DMA
> > > >>>> >> > engine.
> > > >>>> >> >
> > > >>>> >> > Regards
> > > >>>> >> > Santosh
> > > >>>> >>
> > > >>>> >> Hi Santosh,
> > > >>>> >>
> > > >>>> >> Ok, I'll send a v2 now which includes spi-omap2-mcspi then.
> > > >>>> >
> > > >>>> > I don't think we should do this, the drivers should work with and
> > > >>>> > without
> > > >>>> > dma. This just needs to be added to the omap2plus_defconfig.
> > > >>>> >
> > > >>>> Well this was not decided based on any DMA CONFIG option before for
> > > >>>> the subject drivers. It is already by default enabled if the DMA is
> > > >>>> supported
> > > >>>> by the driver IP. There is a possibility to disable it from driver
> > > >>>> platform/dt
> > > >>>> data so that still remains.
> > > >>>
> > > >>> I think it should rather be that if the driver is broken and does
> > > >>> not work
> > > >>> without DMA, it should have depends on CONFIG_DMA_OMAP.
> > > >>>
> > > >> I can confirm that omap MMC can't work without DMA; polled mode is
> > > >> not
> > > >> supported / implemented.
> > > >
> > > > Same case for SPI driver as well. It uses DMA for everything except
> > > > the cases
> > > > where DMA doesn't make sense like 1 byte/2 byte etc. And its not
> > > > configurable,
> > > >
> > > > At least considering this, it is better we do this per driver than
> > > > enabling
> > > > it at SOC config.
> > > >
> > > > Regards
> > > > Santosh
> > > > --
> > >
> > > Hi Santosh,
> > >
> > > And what about enabling it at the SoC config level but making the
> > > drivers dependant on CONFIG_DMADEVICES and CONFIG_DMA_OMAP? If you
> > > agree I can send something like this in two different patches (one for
> > > the omap2plus_defconfig and another to make the drivers dependant on
> > > the config option):
> > >
> > > diff --git a/arch/arm/configs/omap2plus_defconfig
> > > b/arch/arm/configs/omap2plus_defconfig
> > > index b152de7..e58edc3 100644
> > > --- a/arch/arm/configs/omap2plus_defconfig
> > > +++ b/arch/arm/configs/omap2plus_defconfig
> > > @@ -193,6 +193,8 @@ CONFIG_MMC_OMAP_HS=y
> > >  CONFIG_RTC_CLASS=y
> > >  CONFIG_RTC_DRV_TWL92330=y
> > >  CONFIG_RTC_DRV_TWL4030=y
> > > +CONFIG_DMADEVICES=y
> > > +CONFIG_DMA_OMAP=y
> > >  CONFIG_EXT2_FS=y
> > >  CONFIG_EXT3_FS=y
> > >  # CONFIG_EXT3_FS_XATTR is not set
> >
> > above has been merged, 89269ef1f0abc72c551198123e19cd4edfd43cf4
> > but I am missing the patches below in mainline (3.6-rc3) -- what happened?
> >
> > as Javier pointed out in https://patchwork.kernel.org/patch/1203391/,
> > MMC is broken support e.g. on beagleboard unless DMA_OMAP is defined
> >
> > I suggest to take below patches and help to avoid some extra gray hair for
> > people looking for a fix for non-booting beagleboards
> >
> May be I am missing something, but why you would need below patch
> for beagle with "89269ef1f0abc72c551198123e19cd4edfd43cf4" commit
> enabling the DMA_OMAP for all OMAP boards.

the commit just sets CONFIG_DMA_OMAP=y and CONFIG_DMADEVICES=y in 
omap2plus_defconfig; this does not help people updating the kernel while 
keeping the config, nor does it help people in configuring the kernel

there is a dependency (at least for beagleboard) between MMC_OMAP_HS and 
DMA_OMAP, and I think this should be made explicit

regards, p.
Russell King - ARM Linux Aug. 24, 2012, 9:42 a.m. UTC | #9
On Fri, Aug 24, 2012 at 09:51:15AM +0200, Peter Meerwald wrote:
> the commit just sets CONFIG_DMA_OMAP=y and CONFIG_DMADEVICES=y in 
> omap2plus_defconfig; this does not help people updating the kernel while 
> keeping the config, nor does it help people in configuring the kernel
> 
> there is a dependency (at least for beagleboard) between MMC_OMAP_HS and 
> DMA_OMAP, and I think this should be made explicit

Well, this is where stuff starts to get really yucky, because that
means if you have DMA_OMAP as a module, you have to have MMC_OMAP_HS
as a module too.  Or vice versa.  Which is a real pain for further
development of DMA_OMAP.

Whatever, the solution here is NOT to add select statements to the
Kconfig to force DMA engine support and DMA_OMAP to 'y' for OMAP.
The best solution is for MMC_OMAP_HS to depend on DMA_OMAP, but that
will just mean that you'll end up with MMC_OMAP_HS disabled in your
config witout DMA engine support.  Another less desirable solution
is to have MMC_OMAP_HS select DMA engine and DMA_OMAP.
Santosh Shilimkar Aug. 24, 2012, 10:21 a.m. UTC | #10
On Fri, Aug 24, 2012 at 3:12 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>
> On Fri, Aug 24, 2012 at 09:51:15AM +0200, Peter Meerwald wrote:
> > the commit just sets CONFIG_DMA_OMAP=y and CONFIG_DMADEVICES=y in
> > omap2plus_defconfig; this does not help people updating the kernel while
> > keeping the config, nor does it help people in configuring the kernel
> >
> > there is a dependency (at least for beagleboard) between MMC_OMAP_HS and
> > DMA_OMAP, and I think this should be made explicit
>
> Well, this is where stuff starts to get really yucky, because that
> means if you have DMA_OMAP as a module, you have to have MMC_OMAP_HS
> as a module too.  Or vice versa.  Which is a real pain for further
> development of DMA_OMAP.
>
> Whatever, the solution here is NOT to add select statements to the
> Kconfig to force DMA engine support and DMA_OMAP to 'y' for OMAP.
> The best solution is for MMC_OMAP_HS to depend on DMA_OMAP, but that
> will just mean that you'll end up with MMC_OMAP_HS disabled in your
> config witout DMA engine support.  Another less desirable solution
> is to have MMC_OMAP_HS select DMA engine and DMA_OMAP.

Part of the patch [1] does the last part.
MMC_OMAP_HS select DMA engine and DMA_OMAP.

Regards
Santosh

[1] https://patchwork.kernel.org/patch/1203391/
Russell King - ARM Linux Aug. 24, 2012, 10:39 a.m. UTC | #11
On Fri, Aug 24, 2012 at 03:51:26PM +0530, Shilimkar, Santosh wrote:
> On Fri, Aug 24, 2012 at 3:12 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> >
> > On Fri, Aug 24, 2012 at 09:51:15AM +0200, Peter Meerwald wrote:
> > > the commit just sets CONFIG_DMA_OMAP=y and CONFIG_DMADEVICES=y in
> > > omap2plus_defconfig; this does not help people updating the kernel while
> > > keeping the config, nor does it help people in configuring the kernel
> > >
> > > there is a dependency (at least for beagleboard) between MMC_OMAP_HS and
> > > DMA_OMAP, and I think this should be made explicit
> >
> > Well, this is where stuff starts to get really yucky, because that
> > means if you have DMA_OMAP as a module, you have to have MMC_OMAP_HS
> > as a module too.  Or vice versa.  Which is a real pain for further
> > development of DMA_OMAP.
> >
> > Whatever, the solution here is NOT to add select statements to the
> > Kconfig to force DMA engine support and DMA_OMAP to 'y' for OMAP.
> > The best solution is for MMC_OMAP_HS to depend on DMA_OMAP, but that
> > will just mean that you'll end up with MMC_OMAP_HS disabled in your
> > config witout DMA engine support.  Another less desirable solution
> > is to have MMC_OMAP_HS select DMA engine and DMA_OMAP.
> 
> Part of the patch [1] does the last part.
> MMC_OMAP_HS select DMA engine and DMA_OMAP.
> 
> Regards
> Santosh
> 
> [1] https://patchwork.kernel.org/patch/1203391/

Well, I never saw that patch.  When I say "I'm going to be away for most
of July, and I won't be reading email, and I won't catch up with email
when I'm back" and when I get back I explicitly ask for stuff which needs
my attention sending, that's hardly surprising...

But anyway, as I said above, the "select" option is less desirable because
it forces stuff.  And if you've seen the kind of crap that you have to go
through to figure out why the hell you can't disable an option, you'll
understand why I consider that solution revolting.

Take, for instance, a list of dependencies spits out by menuconfig that
fills your entire screen, and you have to work out by reading carefully
through 2K of characters which combination of options is forcing the one
you want to turn off back on.  That is why "select" used inappropriately
is pure evil incarnate.
Santosh Shilimkar Aug. 24, 2012, 10:45 a.m. UTC | #12
On Fri, Aug 24, 2012 at 4:09 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Aug 24, 2012 at 03:51:26PM +0530, Shilimkar, Santosh wrote:
>> On Fri, Aug 24, 2012 at 3:12 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> >
>> > On Fri, Aug 24, 2012 at 09:51:15AM +0200, Peter Meerwald wrote:
>> > > the commit just sets CONFIG_DMA_OMAP=y and CONFIG_DMADEVICES=y in
>> > > omap2plus_defconfig; this does not help people updating the kernel while
>> > > keeping the config, nor does it help people in configuring the kernel
>> > >
>> > > there is a dependency (at least for beagleboard) between MMC_OMAP_HS and
>> > > DMA_OMAP, and I think this should be made explicit
>> >
>> > Well, this is where stuff starts to get really yucky, because that
>> > means if you have DMA_OMAP as a module, you have to have MMC_OMAP_HS
>> > as a module too.  Or vice versa.  Which is a real pain for further
>> > development of DMA_OMAP.
>> >
>> > Whatever, the solution here is NOT to add select statements to the
>> > Kconfig to force DMA engine support and DMA_OMAP to 'y' for OMAP.
>> > The best solution is for MMC_OMAP_HS to depend on DMA_OMAP, but that
>> > will just mean that you'll end up with MMC_OMAP_HS disabled in your
>> > config witout DMA engine support.  Another less desirable solution
>> > is to have MMC_OMAP_HS select DMA engine and DMA_OMAP.
>>
>> Part of the patch [1] does the last part.
>> MMC_OMAP_HS select DMA engine and DMA_OMAP.
>>
>> Regards
>> Santosh
>>
>> [1] https://patchwork.kernel.org/patch/1203391/
>
> Well, I never saw that patch.  When I say "I'm going to be away for most
> of July, and I won't be reading email, and I won't catch up with email
> when I'm back" and when I get back I explicitly ask for stuff which needs
> my attention sending, that's hardly surprising...
>
> But anyway, as I said above, the "select" option is less desirable because
> it forces stuff.  And if you've seen the kind of crap that you have to go
> through to figure out why the hell you can't disable an option, you'll
> understand why I consider that solution revolting.
>
> Take, for instance, a list of dependencies spits out by menuconfig that
> fills your entire screen, and you have to work out by reading carefully
> through 2K of characters which combination of options is forcing the one
> you want to turn off back on.  That is why "select" used inappropriately
> is pure evil incarnate.
I fully agree on the select and it's undesirable side effects.
Since the polling mode isn't supported yet on OMAP MMC drivers, there
was no choice. As per the previous discussion on [1], MMC and SPI driver
folks are looking at supporting polling mode support which can remove
the force select needed for OMAP_DMA.

Regards
santosh

Regards
Santosh
Peter Meerwald-Stadler Aug. 24, 2012, 12:10 p.m. UTC | #13
> >> > > the commit just sets CONFIG_DMA_OMAP=y and CONFIG_DMADEVICES=y in
> >> > > omap2plus_defconfig; this does not help people updating the kernel while
> >> > > keeping the config, nor does it help people in configuring the kernel
> >> > >
> >> > > there is a dependency (at least for beagleboard) between MMC_OMAP_HS and
> >> > > DMA_OMAP, and I think this should be made explicit
> >> >
> >> > Well, this is where stuff starts to get really yucky, because that
> >> > means if you have DMA_OMAP as a module, you have to have MMC_OMAP_HS
> >> > as a module too.  Or vice versa.  Which is a real pain for further
> >> > development of DMA_OMAP.
> >> >
> >> > Whatever, the solution here is NOT to add select statements to the
> >> > Kconfig to force DMA engine support and DMA_OMAP to 'y' for OMAP.
> >> > The best solution is for MMC_OMAP_HS to depend on DMA_OMAP, but that
> >> > will just mean that you'll end up with MMC_OMAP_HS disabled in your
> >> > config witout DMA engine support.  Another less desirable solution
> >> > is to have MMC_OMAP_HS select DMA engine and DMA_OMAP.
> >>
> >> Part of the patch [1] does the last part.
> >> MMC_OMAP_HS select DMA engine and DMA_OMAP.
> >>
> >> Regards
> >> Santosh
> >>
> >> [1] https://patchwork.kernel.org/patch/1203391/
> >
> > Well, I never saw that patch.  When I say "I'm going to be away for most
> > of July, and I won't be reading email, and I won't catch up with email
> > when I'm back" and when I get back I explicitly ask for stuff which needs
> > my attention sending, that's hardly surprising...
> >
> > But anyway, as I said above, the "select" option is less desirable because
> > it forces stuff.  And if you've seen the kind of crap that you have to go
> > through to figure out why the hell you can't disable an option, you'll
> > understand why I consider that solution revolting.
> >
> > Take, for instance, a list of dependencies spits out by menuconfig that
> > fills your entire screen, and you have to work out by reading carefully
> > through 2K of characters which combination of options is forcing the one
> > you want to turn off back on.  That is why "select" used inappropriately
> > is pure evil incarnate.
> I fully agree on the select and it's undesirable side effects.
> Since the polling mode isn't supported yet on OMAP MMC drivers, there
> was no choice. As per the previous discussion on [1], MMC and SPI driver
> folks are looking at supporting polling mode support which can remove
> the force select needed for OMAP_DMA.

with Linux 3.6-rc3:

CONFIG_DMA_OMAP=m
CONFIG_DMA_ENGINE=y
CONFIG_DMA_VIRTUAL_CHANNELS=m
CONFIG_MMC_OMAP_HS=y
CONFIG_SPI_OMAP24XX=y
CONFIG_MTD_NAND_OMAP2=y

  LD      init/built-in.o
drivers/built-in.o: In function `omap2_mcspi_setup':
/home/pmeerw/linux-pmeerw/drivers/spi/spi-omap2-mcspi.c:859: undefined reference to `omap_dma_filter_fn'
drivers/built-in.o: In function `omap_nand_probe':
/home/pmeerw/linux-pmeerw/drivers/mtd/nand/omap2.c:1371: undefined reference to `omap_dma_filter_fn'
drivers/built-in.o: In function `omap_hsmmc_probe':
/home/pmeerw/linux-pmeerw/drivers/mmc/host/omap_hsmmc.c:2039: undefined reference to `omap_dma_filter_fn'
make: *** [vmlinux] Error 1

in addition to SPI and MMC, there seems to be also a NAND dependency

regards, p.
Russell King - ARM Linux Aug. 25, 2012, 7:57 a.m. UTC | #14
On Fri, Aug 24, 2012 at 02:10:38PM +0200, Peter Meerwald wrote:
> with Linux 3.6-rc3:
> 
> CONFIG_DMA_OMAP=m
> CONFIG_DMA_ENGINE=y
> CONFIG_DMA_VIRTUAL_CHANNELS=m
> CONFIG_MMC_OMAP_HS=y
> CONFIG_SPI_OMAP24XX=y
> CONFIG_MTD_NAND_OMAP2=y
> 
>   LD      init/built-in.o
> drivers/built-in.o: In function `omap2_mcspi_setup':
> /home/pmeerw/linux-pmeerw/drivers/spi/spi-omap2-mcspi.c:859: undefined reference to `omap_dma_filter_fn'
> drivers/built-in.o: In function `omap_nand_probe':
> /home/pmeerw/linux-pmeerw/drivers/mtd/nand/omap2.c:1371: undefined reference to `omap_dma_filter_fn'
> drivers/built-in.o: In function `omap_hsmmc_probe':
> /home/pmeerw/linux-pmeerw/drivers/mmc/host/omap_hsmmc.c:2039: undefined reference to `omap_dma_filter_fn'
> make: *** [vmlinux] Error 1
> 
> in addition to SPI and MMC, there seems to be also a NAND dependency

Yes.  Unlike the PL08x driver, OMAP has no way to pass the filter function
into its drivers, and I didn't want the pain of fiddling around with DT
crap to try and work out how to do this.  So what you have here is
something that I got as far as I could, and TI then wanted it pushed into
mainline.

I have no solution for the above at present - the long term solution is
to sort out how to deal with DMA engine with DT etc.  At the moment,
there has been discussion about this but no solution.
diff mbox

Patch

diff --git a/arch/arm/configs/omap2plus_defconfig
b/arch/arm/configs/omap2plus_defconfig
index b152de7..e58edc3 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -193,6 +193,8 @@  CONFIG_MMC_OMAP_HS=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_TWL92330=y
 CONFIG_RTC_DRV_TWL4030=y
+CONFIG_DMADEVICES=y
+CONFIG_DMA_OMAP=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 # CONFIG_EXT3_FS_XATTR is not set
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index aa131b3..314c7bd 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -231,7 +231,7 @@  config MMC_SDHCI_S3C_DMA

 config MMC_OMAP
 	tristate "TI OMAP Multimedia Card Interface support"
-	depends on ARCH_OMAP
+	depends on ARCH_OMAP && DMADEVICES && DMA_OMAP
 	select TPS65010 if MACH_OMAP_H2
 	help
 	  This selects the TI OMAP Multimedia card Interface.
@@ -242,7 +242,8 @@  config MMC_OMAP

 config MMC_OMAP_HS
 	tristate "TI OMAP High Speed Multimedia Card Interface support"
-	depends on SOC_OMAP2430 || ARCH_OMAP3 || ARCH_OMAP4
+	depends on (SOC_OMAP2430 || ARCH_OMAP3 || ARCH_OMAP4) && \
+		    DMADEVICES && DMA_OMAP
 	help
 	  This selects the TI OMAP High Speed Multimedia card Interface.
 	  If you have an OMAP2430 or OMAP3 board or OMAP4 board with a
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index cd2fe35..1c23242 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -237,7 +237,7 @@  config SPI_OMAP_UWIRE

 config SPI_OMAP24XX
 	tristate "McSPI driver for OMAP"
-	depends on ARCH_OMAP2PLUS
+	depends on ARCH_OMAP2PLUS && DMADEVICES && DMA_OMAP
 	help
 	  SPI master controller for OMAP24XX and later Multichannel SPI
 	  (McSPI) modules.