Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI
From: Michal Simek
Date: Thu Aug 27 2015 - 10:26:45 EST
On 08/27/2015 04:12 PM, Ulf Hansson wrote:
> On 27 August 2015 at 15:43, Michal Simek <michal.simek@xxxxxxxxxx> wrote:
>> On 08/27/2015 02:30 PM, Ulf Hansson wrote:
>>> On 27 August 2015 at 13:43, Michal Simek <michal.simek@xxxxxxxxxx> wrote:
>>>> Hi Ulf,
>>>>
>>>> On 08/27/2015 01:32 PM, Ulf Hansson wrote:
>>>>> On 25 August 2015 at 14:04, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>>>> On 6 August 2015 at 07:39, Michal Simek <michal.simek@xxxxxxxxxx> wrote:
>>>>>>> Add GPIOLIB dependency for MMC_SDHCI.
>>>>>>>
>>>>>>> Problem was observed after adding the patch
>>>>>>> "mmc: sdhci-of-arasan: Call OF parsing for MMC"
>>>>>>> (sha1: 16b23787fc709fe60c5d2bd05927b1a3da33d4e9) which calls
>>>>>>> mmc_of_parse() -> mmc_gpiod_request_cd() (slot-gpio.c) which
>>>>>>> calls devm_gpiod_get_index() which returns -ENOSYS.
>>>>>>>
>>>>>>> Error log:
>>>>>>> sdhci-arasan ff160000.sdhci: parsing dt failed (4294967258)
>>>>>>> sdhci-arasan: probe of ff160000.sdhci failed with error -38
>>>>>>>
>>>>>>> Signed-off-by: Michal Simek <michal.simek@xxxxxxxxxx>
>>>>>>
>>>>>> Thanks, applied for next!
>>>>>
>>>>> kbuild test robot reports a warning for this one, so I am dropping it
>>>>> from my next branch.
>>>>
>>>> I think is just better to fix the problem there instead of dropping this
>>>> patch which fix GPIO dependency.
>>>>
>>>> Fix is quite easy.
>>>> diff --git a/arch/powerpc/platforms/44x/Kconfig
>>>> b/arch/powerpc/platforms/44x/Kconfig
>>>> index 5538e57c36c1..874f07c7d0b8 100644
>>>> --- a/arch/powerpc/platforms/44x/Kconfig
>>>> +++ b/arch/powerpc/platforms/44x/Kconfig
>>>> @@ -219,6 +219,7 @@ config AKEBONO
>>>> select USB_EHCI_HCD_PLATFORM if USB_EHCI_HCD
>>>> select MMC_SDHCI
>>>> select MMC_SDHCI_PLTFM
>>>> + select GPIOLIB
>>>> select ATA
>>>> select SATA_AHCI_PLATFORM
>>>> help
>>>>
>>>> But the question is if we should keep these ancient targets in the tree.
>>>>
>>>> I am happy to send this patch but it should go via PPC tree. Or are you
>>>> happy to apply it to your tree?
>>>
>>> It's getting really late for 4.3 so I would rather postpone this to
>>> the next release cycle.
>>
>> No problem at all. :-)
>>
>>>
>>> As I stated in my earlier reply, do we really want to add the GPIOLIB
>>> dependency to the Kconfig file for SDHCI?
>>> I assume we have lots of other Kconfig dependencies, then these should
>>> also to be added for the same reasons. I doubt this is the right thing
>>> to do.
>>
>> Is it the right solution not to list them if they are there?
>
> Well, I don't think there are *one* answer to this.
>
> Still, the reason to why we have API implementing stubs when used is
> to manage these cases.
>
>>
>>> How about if the mmc core instead treat GPIOs as optional from an API
>>> point of view and thus it won't cause ->probe() to fail. Is that a way
>>> forward for you?
>>
>> In my test case I am not using GPIO at all and probe is failing. If this
>> is fixed because it is probably common setting for others we don't need
>> to list this dependency.
>> But really for my case and I am not using gpio at all probe just failed
>> which is incorrect and should be fixed.
>>
>
> Thanks for clarifying. In this regard I think it's pretty obvious that
> we need to make GPIO optional.
> Else we will force the footprint to increase for the kernel image,
> even when not needed.
right. Footprint is valid argument.
>
> Typically checking for -ENOSYS from the response from the GPIOLIB
> would do the trick. That's actually what we do already for GPIOs in
> mmc pwrseq simple case.
I didn't parse the gpio code to be 100% sure that this is enough. Also
if this will cover all cases which we can have. Depends on gpio error
code consistency.
> Do you want to send another patch?
I would love to but I am quite occupied right now but I am happy to test
on our SoC when we have right fix for it.
Thanks,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/