Re: [PATCH] mmc: pwrseq_simple: Fix incorrect handling of GPIO bitmap

[Marek Szyprowski]

Hi again,

On 2018-09-26 10:14, Marek Szyprowski wrote:
> On 2018-09-25 21:24, Janusz Krzysztofik wrote:
>> Commit b9762bebc633 ("gpiolib: Pass bitmaps, not integer arrays, to
>> get/set array") changed the way GPIO values are passed to
>> gpiod_get/set_array_value() and friends. The updated code of
>> mmc_pwrseq_simple_set_gpios_value() incorrectly uses the 'value'
>> argument as a bitmap of GPIO values and assigns it directly to the
>> 'values' bitmap variable passed to gpiod_set_array_value_cansleep()
>> instead of filling that bitmap with bits equal to the 'value' argument.
>> As a result, boot hanging caused by incorrectly handled MMC device
>> has been observed.
>>
>> As a side effect of that incorrect interpreation of the 'value'
>> argument, wrong assumption is taken about the 'values' bitmap size
>> never exceding the number of bits of the 'value' argument type.
>>
>> Fix it.
>>
>> Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx>
>> ---
>> Hi,
>>
>> I hope I've finally identified the root cause of the boot hang reported
>> by Marek Szyprowski. I've reviewed the code of other divers updated for
>> the modified GPIO API and found no more issues of that kind.
>>
>> Marek, can you please test this fix on top of next-20180920 with the fix
>> "gpiolib: Fix missing updates of bitmap index" also applied?
>
> Yes, I've just did such test (next-20180920 + "gpiolib: Fix missing
> updates of bitmap index" + "mmc: pwrseq_simple: Fix incorrect handling
> of GPIO bitmap") and sadly it doesn't fix the boot hang.
>
> With some more debugs in mmc_pwrseq_simple I've noticed that
> gpiod_set_array_value_cansleep() never ends and busyloops somewhere.
> I'm checking this now.

I busyloops inside the internal do { } while loop (lines 3163-3201) in
gpiod_set_array_value_complex(). 'i' is never incremented.

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland