diff mbox

Build warning in drivers/dma/mmp_tdma.c

Message ID CAPcyv4i1i+3ia5qcYA-LR4TFq1MOCXTHLhW2u5maq6Us9kyATg@mail.gmail.com (mailing list archive)
State Changes Requested
Delegated to: Dan Williams
Headers show

Commit Message

Dan Williams Nov. 28, 2013, 10:58 p.m. UTC 
My build warning test is failing on this driver please fix:

drivers/dma/mmp_tdma.c:236:8: warning: 'tdcr' may be used
uninitialized in this function [-Wuninitialized]

It's valid as mmp_tdma_control as the direction is specified in
mmp_tdma_control() and may not be one of the two tests in that branch.



I'm also carrying patch to add a missing dependency:

commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
Author: Dan Williams <dan.j.williams@intel.com>
Date:   Thu Nov 28 12:27:38 2013 -0800

    dma: mmp_dma depends on CPU_MMP2

    It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y

    Signed-off-by: Dan Williams <dan.j.williams@intel.com>


Is there a different ARCH type that this driver should be depending on?

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Zhangfei Gao Dec. 2, 2013, 6:42 a.m. UTC | #1 
Thanks Dan

On Fri, Nov 29, 2013 at 6:58 AM, Dan Williams <dan.j.williams@intel.com> wrote:

> I'm also carrying patch to add a missing dependency:
>
> commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
> Author: Dan Williams <dan.j.williams@intel.com>
> Date:   Thu Nov 28 12:27:38 2013 -0800
>
>     dma: mmp_dma depends on CPU_MMP2
>
>     It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y
>
>     Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 446687cc2334..dad83634cb65 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -287,7 +287,7 @@ config DMA_SA11X0
>
>  config MMP_TDMA
>         bool "MMP Two-Channel DMA support"
> -       depends on ARCH_MMP
> +       depends on ARCH_MMP && CPU_MMP2

Hi, Qiao

Do you have comments?
Does this work on PXA910?

Does sram also use on pxa910?
arch/arm/mach-mmp/Makefile
obj-$(CONFIG_CPU_MMP2)          += mmp2.o sram.o

Zhangfei
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qiao Zhou Dec. 2, 2013, 7:34 a.m. UTC | #2 
On 12/02/2013 02:42 PM, Zhangfei Gao wrote:
> Thanks Dan
>
> On Fri, Nov 29, 2013 at 6:58 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>
>> I'm also carrying patch to add a missing dependency:
>>
>> commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
>> Author: Dan Williams <dan.j.williams@intel.com>
>> Date:   Thu Nov 28 12:27:38 2013 -0800
>>
>>      dma: mmp_dma depends on CPU_MMP2
>>
>>      It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y
>>
>>      Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index 446687cc2334..dad83634cb65 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -287,7 +287,7 @@ config DMA_SA11X0
>>
>>   config MMP_TDMA
>>          bool "MMP Two-Channel DMA support"
>> -       depends on ARCH_MMP
>> +       depends on ARCH_MMP && CPU_MMP2
>
> Hi, Qiao
>
> Do you have comments?
> Does this work on PXA910?
MMP_TDMA should not depend on CPU_MMP2. PXA910, and other chip set also 
use mmp_tdma.
>
> Does sram also use on pxa910?
> arch/arm/mach-mmp/Makefile
> obj-$(CONFIG_CPU_MMP2)          += mmp2.o sram.o
yes, PXA910 also uses sram.
>
> Zhangfei
>
Zhangfei Gao Dec. 2, 2013, 7:42 a.m. UTC | #3 
Thanks Qiao for confirmation

On Mon, Dec 2, 2013 at 3:34 PM, Qiao Zhou <zhouqiao@marvell.com> wrote:
> On 12/02/2013 02:42 PM, Zhangfei Gao wrote:
>>
>> Thanks Dan
>>
>> On Fri, Nov 29, 2013 at 6:58 AM, Dan Williams <dan.j.williams@intel.com>
>> wrote:
>>
>>> I'm also carrying patch to add a missing dependency:
>>>
>>> commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
>>> Author: Dan Williams <dan.j.williams@intel.com>
>>> Date:   Thu Nov 28 12:27:38 2013 -0800
>>>
>>>      dma: mmp_dma depends on CPU_MMP2
>>>
>>>      It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y
>>>
>>>      Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>>
>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>> index 446687cc2334..dad83634cb65 100644
>>> --- a/drivers/dma/Kconfig
>>> +++ b/drivers/dma/Kconfig
>>> @@ -287,7 +287,7 @@ config DMA_SA11X0
>>>
>>>   config MMP_TDMA
>>>          bool "MMP Two-Channel DMA support"
>>> -       depends on ARCH_MMP
>>> +       depends on ARCH_MMP && CPU_MMP2
>>
>>
>> Hi, Qiao
>>
>> Do you have comments?
>> Does this work on PXA910?
>
> MMP_TDMA should not depend on CPU_MMP2. PXA910, and other chip set also use
> mmp_tdma.
>
>>
>> Does sram also use on pxa910?
>> arch/arm/mach-mmp/Makefile
>> obj-$(CONFIG_CPU_MMP2)          += mmp2.o sram.o
>
> yes, PXA910 also uses sram.

Then we may need clarify the dependence about sram.o to solve Dan's question?

Thanks
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams Dec. 2, 2013, 8:05 p.m. UTC | #4 
On Sun, Dec 1, 2013 at 11:34 PM, Qiao Zhou <zhouqiao@marvell.com> wrote:
> On 12/02/2013 02:42 PM, Zhangfei Gao wrote:
>>
>> Thanks Dan
>>
>> On Fri, Nov 29, 2013 at 6:58 AM, Dan Williams <dan.j.williams@intel.com>
>> wrote:
>>
>>> I'm also carrying patch to add a missing dependency:
>>>
>>> commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
>>> Author: Dan Williams <dan.j.williams@intel.com>
>>> Date:   Thu Nov 28 12:27:38 2013 -0800
>>>
>>>      dma: mmp_dma depends on CPU_MMP2
>>>
>>>      It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y
>>>
>>>      Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>>
>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>> index 446687cc2334..dad83634cb65 100644
>>> --- a/drivers/dma/Kconfig
>>> +++ b/drivers/dma/Kconfig
>>> @@ -287,7 +287,7 @@ config DMA_SA11X0
>>>
>>>   config MMP_TDMA
>>>          bool "MMP Two-Channel DMA support"
>>> -       depends on ARCH_MMP
>>> +       depends on ARCH_MMP && CPU_MMP2
>>
>>
>> Hi, Qiao
>>
>> Do you have comments?
>> Does this work on PXA910?
>
> MMP_TDMA should not depend on CPU_MMP2. PXA910, and other chip set also use
> mmp_tdma.

It does depend on CPU_MMP2, as I can generate the following build
failure with the attached config.

drivers/built-in.o: In function `mmp_tdma_free_descriptor':
drivers/dma/mmp_tdma.c:329: undefined reference to `sram_get_gpool'
drivers/built-in.o: In function `mmp_tdma_alloc_descriptor':
drivers/dma/mmp_tdma.c:379: undefined reference to `sram_get_gpool'

--
Dan
Zhangfei Gao Dec. 3, 2013, 2:13 a.m. UTC | #5 
On Tue, Dec 3, 2013 at 4:05 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Sun, Dec 1, 2013 at 11:34 PM, Qiao Zhou <zhouqiao@marvell.com> wrote:

>>>> commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
>>>> Author: Dan Williams <dan.j.williams@intel.com>
>>>> Date:   Thu Nov 28 12:27:38 2013 -0800
>>>>
>>>>      dma: mmp_dma depends on CPU_MMP2
>>>>
>>>>      It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y
>>>>
>>>>      Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>>>
>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>> index 446687cc2334..dad83634cb65 100644
>>>> --- a/drivers/dma/Kconfig
>>>> +++ b/drivers/dma/Kconfig
>>>> @@ -287,7 +287,7 @@ config DMA_SA11X0
>>>>
>>>>   config MMP_TDMA
>>>>          bool "MMP Two-Channel DMA support"
>>>> -       depends on ARCH_MMP
>>>> +       depends on ARCH_MMP && CPU_MMP2
>>>
>>>
>>> Hi, Qiao
>>>
>>> Do you have comments?
>>> Does this work on PXA910?
>>
>> MMP_TDMA should not depend on CPU_MMP2. PXA910, and other chip set also use
>> mmp_tdma.
>
> It does depend on CPU_MMP2, as I can generate the following build
> failure with the attached config.
>
> drivers/built-in.o: In function `mmp_tdma_free_descriptor':
> drivers/dma/mmp_tdma.c:329: undefined reference to `sram_get_gpool'
> drivers/built-in.o: In function `mmp_tdma_alloc_descriptor':
> drivers/dma/mmp_tdma.c:379: undefined reference to `sram_get_gpool'
>

How about modifying arch/arm/mach-mmp/Makefile
Like:
-obj-y                          += common.o devices.o time.o
+obj-y                          += common.o devices.o time.o sram.o
-obj-$(CONFIG_CPU_MMP2)         += mmp2.o sram.o
+obj-$(CONFIG_CPU_MMP2)         += mmp2.o

As Qiao mentioned sram is used both on mmp2 and pxa910.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Haojian Zhuang Dec. 3, 2013, 2:31 a.m. UTC | #6 
On 12/03/2013 10:13 AM, Zhangfei Gao wrote:
> On Tue, Dec 3, 2013 at 4:05 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Sun, Dec 1, 2013 at 11:34 PM, Qiao Zhou <zhouqiao@marvell.com> wrote:
>
>>>>> commit 1a4ee91dacedde3e82d8ce6eeace7f16884474f9
>>>>> Author: Dan Williams <dan.j.williams@intel.com>
>>>>> Date:   Thu Nov 28 12:27:38 2013 -0800
>>>>>
>>>>>       dma: mmp_dma depends on CPU_MMP2
>>>>>
>>>>>       It calls sram_get_gpool() which is only defined if CONFIG_CPU_MMP2=y
>>>>>
>>>>>       Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>>>>
>>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>> index 446687cc2334..dad83634cb65 100644
>>>>> --- a/drivers/dma/Kconfig
>>>>> +++ b/drivers/dma/Kconfig
>>>>> @@ -287,7 +287,7 @@ config DMA_SA11X0
>>>>>
>>>>>    config MMP_TDMA
>>>>>           bool "MMP Two-Channel DMA support"
>>>>> -       depends on ARCH_MMP
>>>>> +       depends on ARCH_MMP && CPU_MMP2
>>>>
>>>>
>>>> Hi, Qiao
>>>>
>>>> Do you have comments?
>>>> Does this work on PXA910?
>>>
>>> MMP_TDMA should not depend on CPU_MMP2. PXA910, and other chip set also use
>>> mmp_tdma.
>>
>> It does depend on CPU_MMP2, as I can generate the following build
>> failure with the attached config.
>>
>> drivers/built-in.o: In function `mmp_tdma_free_descriptor':
>> drivers/dma/mmp_tdma.c:329: undefined reference to `sram_get_gpool'
>> drivers/built-in.o: In function `mmp_tdma_alloc_descriptor':
>> drivers/dma/mmp_tdma.c:379: undefined reference to `sram_get_gpool'
>>
>
> How about modifying arch/arm/mach-mmp/Makefile
> Like:
> -obj-y                          += common.o devices.o time.o
> +obj-y                          += common.o devices.o time.o sram.o
> -obj-$(CONFIG_CPU_MMP2)         += mmp2.o sram.o
> +obj-$(CONFIG_CPU_MMP2)         += mmp2.o
>
> As Qiao mentioned sram is used both on mmp2 and pxa910.
>
> Thanks
>

I prefer to add CONFIG_MMP_SRAM instead, since sram isn't available in 
pxa168.

Regards
Haojian
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" 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/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 446687cc2334..dad83634cb65 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -287,7 +287,7 @@  config DMA_SA11X0

 config MMP_TDMA
        bool "MMP Two-Channel DMA support"
-       depends on ARCH_MMP
+       depends on ARCH_MMP && CPU_MMP2
        select DMA_ENGINE
        help
          Support the MMP Two-Channel DMA engine.