lkml.org 
[lkml]   [2016]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V6 2/8] mfd: max77620: add core driver for MAX77620/MAX20024

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.

\
 
 \ /
  Last update: 2016-02-01 10:21    [W:0.056 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site