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

[Lee Jones <lee.jones@linaro.org>]

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@linaro.org>
> 
>     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