linux-kernel - Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC

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: <CAPDyKFq8aAQzpUJt2oZuP=ofO1GakZUtezDzFW=bRy4OD7P9Vw@mail.gmail.com>
Date:	Thu, 27 Aug 2015 16:12:44 +0200
From:	Ulf Hansson <ulf.hansson@...aro.org>
To:	Michal Simek <michal.simek@...inx.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Michal Simek <monstr@...str.eu>,
	Scott Branden <sbranden@...adcom.com>,
	Weijun Yang <Weijun.Yang@....com>,
	Andrew Bresticker <abrestic@...omium.org>,
	Jean Delvare <jdelvare@...e.de>,
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	linux-mmc <linux-mmc@...r.kernel.org>,
	Alim Akhtar <alim.akhtar@...sung.com>,
	addy ke <addy.ke@...k-chips.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Chaotian Jing <chaotian.jing@...iatek.com>,
	Kevin Hao <haokexin@...il.com>,
	Vincent Yang <vincent.yang.fujitsu@...il.com>
Subject: Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI

On 27 August 2015 at 15:43, Michal Simek <michal.simek@...inx.com> wrote:
> On 08/27/2015 02:30 PM, Ulf Hansson wrote:
>> On 27 August 2015 at 13:43, Michal Simek <michal.simek@...inx.com> wrote:
>>> Hi Ulf,
>>>
>>> On 08/27/2015 01:32 PM, Ulf Hansson wrote:
>>>> On 25 August 2015 at 14:04, Ulf Hansson <ulf.hansson@...aro.org> wrote:
>>>>> On 6 August 2015 at 07:39, Michal Simek <michal.simek@...inx.com> 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@...inx.com>
>>>>>
>>>>> 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.

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.

Do you want to send another patch?

Kind regards
Uffe
--
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/