Re: [PATCH] mfd: Fix MACRO for commonly declared MFD cell attributes

From: Lee Jones
Date: Wed Mar 02 2016 - 08:08:37 EST


On Mon, 29 Feb 2016, Laxman Dewangan wrote:

>
> On Friday 26 February 2016 10:05 PM, Rhyland Klein wrote:
> >On 2/19/2016 11:28 AM, Rhyland Klein wrote:
> >>On 2/19/2016 3:50 AM, Lee Jones wrote:
> >>>On Thu, 18 Feb 2016, Rhyland Klein wrote:
> >>>
> >>>> #define MFD_ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> >>>>-#define MFD_CELL_ALL(_name, _res, _pdata, _id, _compat, _match) \
> >>>>+#define MFD_CELL_ALL(_name, _nres, _res, _pdata, _id, _compat, _match) \
> >>>> { \
> >>>> .name = (_name), \
> >>>> .resources = (_res), \
> >>>>- .num_resources = MFD_ARRAY_SIZE((_res)), \
> >>>>+ .num_resources = (_nres), \
> >>>> .platform_data = (_pdata), \
> >>>> .pdata_size = MFD_ARRAY_SIZE((_pdata)), \
> The pdata_size initialization is also not correct. This is not the
> array count but the size
> of the platform data which is passed. Should be sizeof((_pdata))
>
>
>
> >> #define MFD_CELL_ALL(_name, _res, _pdata, _id, _compat, _match) \
> >> { \
> >>
> >>That was my first thought too. However, I see this when I try to compile
> >>that:
> >>
> >>In file included from drivers/mfd/max77620.c:18:0:
> >>include/linux/mfd/core.h:19:34: warning: the address of âgpio_resourcesâ
> >>will always evaluate as âtrueâ [-Waddress]
> >> #define MFD_ARRAY_SIZE(arr) (arr ? (sizeof(arr) / sizeof((arr)[0])) : 0)
> >>
> >>7 different times. This patch was the only way I seemed to be able to
> >>WAR around compile time warnings.
> >>
> >>-rhyland
> >>
> >Did you not see warnings like this when you compiled the kernel? Did you
> >find a different approach than what I proposed above to deal with it?
> >I'd like to get this in soon so that when the max77620 drivers are all
> >in and using it, it should be functional.
> >
> I think the following change also crash in runtime:
>
> /***
> commit e60a946f05db2cac857025da6ffb72df48d3be54
> Author: Lee Jones <lee.jones@xxxxxxxxxx>
>
> mfd: ab8500: Provide a small example using new MFD cell MACROs
>
> ***/
>
> Should we have something MFD_CELL_RES, MFD_CELL_RES_PDATA,
> MFD_CELL_PDATA, for more common user and not to pass the NULL here.

I'll have a re-think about this.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog