From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752938AbcBAJEG (ORCPT <rfc822;w@1wt.eu>);
	Mon, 1 Feb 2016 04:04:06 -0500
Received: from hqemgate15.nvidia.com ([216.228.121.64]:3686 "EHLO
	hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751692AbcBAJEB (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 1 Feb 2016 04:04:01 -0500
X-PGP-Universal: processed;
	by hqnvupgp07.nvidia.com on Mon, 01 Feb 2016 01:04:14 -0800
Message-ID: <56AF1CDE.7090703@nvidia.com>
Date: Mon, 1 Feb 2016 14:22:46 +0530
From: Laxman Dewangan <ldewangan@nvidia.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
MIME-Version: 1.0
To: Lee Jones <lee.jones@linaro.org>
CC: <robh+dt@kernel.org>, <pawel.moll@arm.com>, <mark.rutland@arm.com>,
        <ijc+devicetree@hellion.org.uk>, <galak@codeaurora.org>,
        <linus.walleij@linaro.org>, <gnurou@gmail.com>, <broonie@kernel.org>,
        <a.zummo@towertech.it>, <alexandre.belloni@free-electrons.com>,
        <lgirdwood@gmail.com>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
        <rtc-linux@googlegroups.com>, <swarren@nvidia.com>,
        <treding@nvidia.com>, <k.kozlowski@samsung.com>,
        Chaitanya Bandi <bandik@nvidia.com>,
        "Mallikarjun Kasoju" <mkasoju@nvidia.com>
Subject: Re: [PATCH V6 2/8] mfd: max77620: add core driver for MAX77620/MAX20024
References: <1453988274-28052-1-git-send-email-ldewangan@nvidia.com> <1453988274-28052-3-git-send-email-ldewangan@nvidia.com> <20160129090632.GS3368@x1> <56AB462A.8090307@nvidia.com> <20160201085950.GA3368@x1>
In-Reply-To: <20160201085950.GA3368@x1>
X-Originating-IP: [10.19.65.30]
X-ClientProxiedBy: DRUKMAIL101.nvidia.com (10.25.59.19) To
 bgmail102.nvidia.com (10.25.59.11)
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On Monday 01 February 2016 02:29 PM, Lee Jones wrote:
> On Fri, 29 Jan 2016, Laxman Dewangan wrote:
>
>> Thanks Lee for review.
>> I will take care of most of stuff on next version of patch.
>>
>> However, I have some query form your comment.
>> On Friday 29 January 2016 02:36 PM, Lee Jones wrote:
>>> On Thu, 28 Jan 2016, Laxman Dewangan wrote:
>>>
>>>
>>>> +	}
>>>> +
>>>> +#define MAX20024_SUB_MODULE_NO_RES(_name, _id)			\
>>>> +	[_id] = {						\
>>>> +		.name = "max20024-"#_name,			\
>>>> +		.id = _id,					\
>>>> +	}
>>> I don't want people hand-rolling this stuff.  If it's useful to you,
>>> it's useful to others, so great a generic implementation that lives in
>>> the kernel headers directory.
>> yaah, generic implementation possible. I can put the new defines in
>> the mfd/core.h.
>>
>> This will be similar to
>> +/* Define mfd cells with name and resource */
>> +#define DEFINE_MFD_CELL_NAME_RESOURCE(_name, _res)             \
>> +       {                                                       \
>> +               .name = (_name),                                \
>> +               .num_resources = ARRAY_SIZE((res)),             \
>> +               .resources = (_res),                            \
>> +       }
>> +
>> +/* Define mfd cells with name */
>> +#define DEFINE_MFD_CELL_NAME(_name)                            \
>> +       {                                                       \
>> +               .name = (_name),                                \
>> +       }
>> +
>>
>> This will be separate patch and should be applied before this series.
>> Does it look fine?
> Hmm... Actually, I have my own ideas of how this should look.  How do
> you feel about me submitting my own patch.  I'll keep you on Cc, so
> you can review and make use of it in your set.

Sure, I am fine with this. Please send the patch and CC me so that I can 
make my patch on top of it and void my mfd/core.h patch.
Thanks for taking care.


>>>> +static const struct i2c_device_id max77620_id[] = {
>>>> +	{"max77620", MAX77620},
>>>> +	{"max20024", MAX20024},
>>>> +	{},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(i2c, max77620_id);
>>>> +
>>>> +static const struct of_device_id max77620_of_match[] = {
>>>> +	{
>>>> +		.compatible = "maxim,max77620",
>>>> +		.data = &max77620_cells,
>>>> +	}, {
>>>> +		.compatible = "maxim,max20024",
>>>> +		.data = &max20024_cells,
>>>> +	}, {
>>>> +	},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, max77620_of_match);
>>> This is not acceptable.  EITHER use DT OR MFD methods of registering
>>> devices, do not mix the two.
>> You mean I need to either provide the i2c_device_id table or the
>> of_device_id table, not both?
>> Do I need to protect it by CONFIG_OF?
>>
>> This only support the DT method of registration. So do I need to
>> remove i2c_device_id?
> No, I mean I don't want you providing platform data via an MFD cell
> and passing it through the OF .data attribute.

This is not platform data, this is chip specific data.
However, In patch V7, I removed this and use the id_table for the chip 
data to optimized the chip ID.
This is not required.