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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGTfZH13KbQqjSLaussyqYBTHWAdcGpyq4099TF=zf76xXpSEg@mail.gmail.com>
Date:	Sun, 27 Sep 2015 13:50:57 +0900
From:	Chanwoo Choi <cwchoi00@...il.com>
To:	Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
Cc:	Lee Jones <lee.jones@...aro.org>, Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"myungjoo.ham@...sung.com" <myungjoo.ham@...sung.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	devicetree <devicetree@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/6] extcon: arizona: Add device binding for second jack
 detect pin on GPIO5

On Fri, Sep 25, 2015 at 6:10 PM, Charles Keepax
<ckeepax@...nsource.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/