On Fri, Sep 25, 2015 at 6:10 PM, Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx> 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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
