Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1233613

Re: [PATCH 4/6] extcon: arizona: Add device binding for second jack detect pin on GPIO5

Show all headers | View raw

On Fri, Sep 25, 2015 at 6:10 PM, Charles Keepax
<ckeepax@opensource.wolfsonmicro.com> wrote:
> On Fri, Sep 25, 2015 at 09:54:26AM +0900, Chanwoo Choi wrote:
>> Hi Charles,
>>
>> I have one comment.
>> I think current extcon-arizona.c has the many platform data
>> so, extcon-arizona.c use the too much if statement to support each feature
>> for different wolfsonmicro codec. I think it cause the complicated code.
>>
>> For example,
>> You may use 'struct of_device_id' as following. You used already this method
>> on drivers/mfd/arizona-core.c. If you separate the function of each wmXXXX arizona,
>> it makes improved readability for extcon-arizona.c and some user will use extcon-arizora
>> more easily.
>>
>> struct arizona_extcon_data {
>>       void (*init)(...);
>>       void (*irq_handler)(...);
>>       ...
>> };
>>
>> struct arizona_extcon_data wm8994_data {
>>       .init = wm8994_extcon_init,
>>       .irq_handler = wm8994_extcon_irq_handler;
>>       ...
>> };
>>
>> static const struct of_device_id arizona_extcon_dt_match[] = {
>>       {
>>               .compatible = "wm8994-arizona-extcon",
>>               .data = (void *)wm8994_data;
>>       }, {
>>               .compatible = "wmXXXX-arizona-extcon",
>>               .data = (void *)wmXXXX_data;
>>       },
>> };
>>
>> I expect that you will revise the arizona-extcon.c driver on next time.
>
> I will have an investigation to see what ideas I can come up with
> here and send through some patches.
>
> But I am slightly worried you think the gains will be better than
> they actually will be. Most of the platform data is just that
> platform data, and needs to be configured based on the system not
> based on the CODEC.

I agree. As you mentioned, platform data will be configured based on the system.
But, I mean that there are many platform datas for extcon-arizona in
"struct arizona_pdata" (in include/linux/mfd/arizona/pdata.h).

There are four sound code using the extcon-arizona driver as
following: I check the following list
in drivers/mfd/arizona-core.c.
- wm5102
- wm5110
- wm8997
- wm8998

If upper four sound codec include the same headset detection IP
(extcon-arizona.c)
without any difference about detection IP in sound codec, you don't
need to revise
the extcon-arizona.c driver.

But, If some field of  "struct arizona_pdata" (in
include/linux/mfd/arizona/pdata.h)
are used for only specific sound codec (wmXXX series), I think we need
to revise it.

I'm positive about your patches of extcon-arizona. Just I think that
you can make it
more easier and improved readability.

Thanks,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH 4/6] extcon: arizona: Add device binding for second jack  detect pin on GPIO5 Chanwoo Choi <cw00.choi@samsung.com> - 2015-09-25 03:00 +0200
  Re: [PATCH 4/6] extcon: arizona: Add device binding for second  jack detect pin on GPIO5 Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2015-09-25 11:40 +0200
    Re: [PATCH 4/6] extcon: arizona: Add device binding for second jack  detect pin on GPIO5 Chanwoo Choi <cwchoi00@gmail.com> - 2015-09-27 07:00 +0200

csiph-web