lkml.org 
[lkml]   [2017]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] extcon: int3496: Set the id pin to direction-input if necessary
On 2017년 03월 13일 19:51, Hans de Goede wrote:
> Hi,
>
> On 13-03-17 11:30, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2017년 03월 13일 17:40, Hans de Goede wrote:
>>> With the new more strict ACPI gpio code the dsdt's IoRestriction
>>> flags are honored on gpiod_get, but in some dsdt's it is wrong,
>>> so explicitly call gpiod_direction_input on the id gpio if
>>> necessary.
>>>
>>> This fixes the following errors when the int3496 code is used
>>> together with the new more strict ACPI gpio code:
>>>
>>> [ 2382.484415] gpio gpiochip1: (INT33FF:01): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
>>> [ 2382.484425] gpio gpiochip1: (INT33FF:01): unable to lock HW IRQ 3 for IRQ
>>> [ 2382.484429] genirq: Failed to request resources for INT3496:00 (irq 174) on irqchip chv-gpio
>>> [ 2382.484518] intel-int3496 INT3496:00: can't request IRQ for USB ID GPIO: -22
>>> [ 2382.500359] intel-int3496: probe of INT3496:00 failed with error -22
>>>
>>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> -Warn about firmware bug when the dsdt's IoRestriction does not allow input
>>> ---
>>> drivers/extcon/extcon-intel-int3496.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/extcon/extcon-intel-int3496.c b/drivers/extcon/extcon-intel-int3496.c
>>> index b8ac947..18801eb 100644
>>> --- a/drivers/extcon/extcon-intel-int3496.c
>>> +++ b/drivers/extcon/extcon-intel-int3496.c
>>> @@ -113,6 +113,10 @@ static int int3496_probe(struct platform_device *pdev)
>>> dev_err(dev, "can't request USB ID GPIO: %d\n", ret);
>>> return ret;
>>> }
>>
>> Need to add one blank line at here.
>
> I grouped it together with the earlier error check since it is still
> checking the returned gpiodesc is valid, but if you prefer to add a line
> that is fine by me.

If you want to group them, you should add it as following:

} else if (gpiod_get_direction(data->gpio_usb_id) != GPIOF_DIR_IN) {

>
>>
>>> + if (gpiod_get_direction(data->gpio_usb_id) != GPIOF_DIR_IN) {
>>> + dev_warn(dev, "firmware bug USB ID GPIO not in input mode, fixing\n");
>>
>> The length of warning comment is over 80 char.
>
> Which is allowed, try running checkpatch.pl on the patch. log messages may
> cross the 80 char limit, to avoid splitting them over multiple lines which
> would make grepping for them hard.
>
>> We need to reduce the length of comment.
>
> Nope, not needed, as said this is allowed.
>
>> I modify the comment as following: If you ok, I'll apply it.
>> "ID pin isn't in input mode due to firmware bug"
>
> I prefer my original text which better describes what is happening
> and as said before, the text going over the 80 char limit is allowed.

Right. The over the 80 char is possible. But, I prefer to write
the comment under the 80 char. But if you want to use more detailed
comment, I think that you can make your sentence more formal.

[snip]

--
Best Regards,
Chanwoo Choi
Samsung Electronics

\
 
 \ /
  Last update: 2017-03-13 12:05    [W:0.047 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site