From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933408AbcBZQfi (ORCPT ); Fri, 26 Feb 2016 11:35:38 -0500 Received: from hqemgate16.nvidia.com ([216.228.121.65]:10411 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754096AbcBZQfh convert rfc822-to-8bit (ORCPT ); Fri, 26 Feb 2016 11:35:37 -0500 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Fri, 26 Feb 2016 08:34:30 -0800 Subject: Re: [PATCH] mfd: Fix MACRO for commonly declared MFD cell attributes To: Lee Jones References: <1455821558-28493-1-git-send-email-rklein@nvidia.com> <20160219085019.GA3410@x1> <56C742C4.4080308@nvidia.com> CC: , Laxman Dewangan From: Rhyland Klein Message-ID: <56D07ED6.7030607@nvidia.com> Date: Fri, 26 Feb 2016 11:35:34 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56C742C4.4080308@nvidia.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: >> >>> MFD_ARRAY_SIZE() would not accurately return 0 if the passed >>> parameter was NULL. Fix this so that num_resources will >>> accurately be 0 in the case that _res is NULL. >>> >>> cc: Lee Jones >>> cc: Laxman Dewangan >>> Signed-off-by: Rhyland Klein >>> --- >>> include/linux/mfd/core.h | 15 +++++++++------ >>> 1 file changed, 9 insertions(+), 6 deletions(-) >>> >>> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h >>> index 1a5a87f3cd38..62136ccff1df 100644 >>> --- a/include/linux/mfd/core.h >>> +++ b/include/linux/mfd/core.h >>> @@ -18,11 +18,11 @@ >>> >>> #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)), \ >>> .of_compatible = (_compat), \ >>> @@ -31,16 +31,19 @@ >>> } >>> >>> #define OF_MFD_CELL(_name, _res, _pdata, _id, _compat) \ >>> - MFD_CELL_ALL(_name, _res, _pdata, _id, _compat, NULL) \ >>> + MFD_CELL_ALL(_name, MFD_ARRAY_SIZE((_res)), _res, \ >>> + _pdata, _id, _compat, NULL) \ >> >> I'm confused. Why would it be any different just by changing the call >> site of MFD_ARRAY_SIZE? > > It isn't different, but for MFD_CELL_NAME, it explicitly passes 0 > instead of using MFD_ARRAY_SIZE, as its the only place that doesn't > expect to have resources. > >> >> And what about .platform_data? > > This crashed for me (without the change) at : > > mfd_add_device(): > for (r = 0; r < cell->num_resources; r++) { > res[r].name = cell->resources[r].name; > res[r].flags = cell->resources[r].flags; > > where dereferencing cell->resources[0] when there are no resources. I > guess the platform_data could do the same, but I didn't run into it. > >> >> How about this change instead? >> >> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h >> index 1a5a87f..8440f42 100644 >> --- a/include/linux/mfd/core.h >> +++ b/include/linux/mfd/core.h >> @@ -16,7 +16,7 @@ >> >> #include >> >> -#define MFD_ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) >> +#define MFD_ARRAY_SIZE(arr) (arr ? (sizeof(arr) / sizeof((arr)[0])) : 0) >> >> #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. -rhyland -- nvpublic