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 <[email protected]>
>>> Signed-off-by: Hans de Goede <[email protected]>
>>> ---
>>> 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
