linux-kernel - Re: [PATCH v2 3/6] extcon-gpio: Add support for active-low presence detect pins

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <52310984.30809@roeck-us.net>
Date:	Wed, 11 Sep 2013 17:23:32 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Chanwoo Choi <cw00.choi@...sung.com>
CC:	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	Rob Herring <rob.herring@...xeda.com>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Stephen Warren <swarren@...dotorg.org>,
	Ian Campbell <ian.campbell@...rix.com>,
	MyungJoo Ham <myungjoo.ham@...sung.com>,
	Grant Likely <grant.likely@...aro.org>
Subject: Re: [PATCH v2 3/6] extcon-gpio: Add support for active-low presence
 detect pins

On 09/11/2013 04:57 PM, Chanwoo Choi wrote:
> Hi Guenter,
>
> As I mentioned on previous reply, I modify patch name as following:
> extcon-gpio: Add support for active-low presence detect pins
> ->
> extcon: gpio: Add support for active-low presence detect pins
>
> I prefer you could add patch description. Also, you should test

As mentioned before, I sent out v2 of this patch before I read your comments
to the previous patch.

> on extcon latest branch to protect merge conflict. This patch
> has conflict issue on extcon-linus branch. I fix up and applied it.
>
I'll make sure to so that in the future.

> And I think you might use tab size is 4 characters. you should change tab size
> from 4 characters to 8 characters. You can check linux kernel coding sytle on

Thanks for the reminder, but the problem is a different one. See below.

> "/kernel/Documentation/CodingStyle - chapter 1: Indentation".
>
> On 09/11/2013 11:25 AM, Guenter Roeck wrote:
>>
>> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
>> ---
>> v2: Document gpio_active_low variable in gpio_extcon_platform_data
>>      Rewrite active-low logic to be easier to read.
>>
>>   drivers/extcon/extcon-gpio.c       |    4 ++++
>>   include/linux/extcon/extcon-gpio.h |    5 +++++
>>   2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
>> index 862743b..8491f86 100644
>> --- a/drivers/extcon/extcon-gpio.c
>> +++ b/drivers/extcon/extcon-gpio.c
>> @@ -34,6 +34,7 @@
>>   struct gpio_extcon_data {
>>   	struct extcon_dev edev;
>>   	unsigned gpio;
>> +	bool gpio_active_low;
>>   	const char *state_on;
>>   	const char *state_off;
>>   	int irq;
>> @@ -49,6 +50,8 @@ static void gpio_extcon_work(struct work_struct *work)
>>   			     work);
>>
>>   	state = gpio_get_value(data->gpio);
>> +	if (data->gpio_active_low)
>> +		state = !state;
>>   	extcon_set_state(&data->edev, state);
>>   }
>>
>> @@ -96,6 +99,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
>>
>>   	extcon_data->edev.name = pdata->name;
>>   	extcon_data->gpio = pdata->gpio;
>> +	extcon_data->gpio_active_low = pdata->gpio_active_low;
>>   	extcon_data->state_on = pdata->state_on;
>>   	extcon_data->state_off = pdata->state_off;
>>   	if (pdata->state_on && pdata->state_off)
>> diff --git a/include/linux/extcon/extcon-gpio.h b/include/linux/extcon/extcon-gpio.h
>> index 2d8307f..51ef15c 100644
>> --- a/include/linux/extcon/extcon-gpio.h
>> +++ b/include/linux/extcon/extcon-gpio.h
>> @@ -27,6 +27,10 @@
>>    * struct gpio_extcon_platform_data - A simple GPIO-controlled extcon device.
>>    * @name	The name of this GPIO extcon device.
>>    * @gpio	Corresponding GPIO.
>> + * @gpio_active_low:
>> + * 		Boolean describing whether gpio active state is 1 or 0
>> + * 		If true, low state of gpio means active.
>> + * 		If false, high state of gpio means active.
>
> I think you may use tab size is 4 characters.
>
No, that isn't it. When I cut-and-pasted your proposed text I was not careful enough
and did not remove a space after the '*'. No idea how it got there, but it isn't the
tab setting.

Usually I _do_ run my patches through checkpatch.pl to avoid problems like that.
Sorry, my bad; looks like I was shooting too quickly last night and didn't do that.

Guenter

--
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/