Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1213002 > unrolled thread

Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI Ulf Hansson <ulf.hansson@linaro.org> - 2015-08-25 14:10 +0200
    Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI Ulf Hansson <ulf.hansson@linaro.org> - 2015-08-27 13:40 +0200
      Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI Ulf Hansson <ulf.hansson@linaro.org> - 2015-08-27 14:40 +0200
        Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI Ulf Hansson <ulf.hansson@linaro.org> - 2015-08-27 16:20 +0200

#1213002 — Re: [PATCH] mmc: Kconfig: Add dependency on GPIOLIB for MMC_SDHCI

On 6 August 2015 at 07:39, Michal Simek <michal.simek@xilinx.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@xilinx.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> After RFC here https://lkml.org/lkml/2015/7/24/371
> adding dependency on MMC_SDHCI.
>
> ---
>  drivers/mmc/host/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index fd9a58e216a5..5519803c124b 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -38,6 +38,7 @@ config MMC_PXA
>  config MMC_SDHCI
>         tristate "Secure Digital Host Controller Interface support"
>         depends on HAS_DMA
> +       depends on GPIOLIB
>         help
>           This selects the generic Secure Digital Host Controller Interface.
>           It is used by manufacturers such as Texas Instruments(R), Ricoh(R)
> --
> 2.3.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1214484

On 25 August 2015 at 14:04, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 6 August 2015 at 07:39, Michal Simek <michal.simek@xilinx.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@xilinx.com>
>
> Thanks, applied for next!

kbuild test robot reports a warning for this one, so I am dropping it
from my next branch.

-----
warning: (AKEBONO) selects MMC_SDHCI which has unmet direct
dependencies (MMC && HAS_DMA && GPIOLIB)
https://lists.01.org/pipermail/kbuild-all/2015-August/011824.html
-----

Perhaps we should just live with this kind of configuration problems
as this patch is trying to fix?
Looking into drivers in general I don't think we normally specify all
the Kconfig dependencies to be able to meet a successfully probe().
The GPIOLIB is just one thing out of many.

Or you have a better idea? :-)

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214518

On 27 August 2015 at 13:43, Michal Simek <michal.simek@xilinx.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@linaro.org> wrote:
>>> On 6 August 2015 at 07:39, Michal Simek <michal.simek@xilinx.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@xilinx.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.

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.

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?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214593

On 27 August 2015 at 15:43, Michal Simek <michal.simek@xilinx.com> wrote:
> On 08/27/2015 02:30 PM, Ulf Hansson wrote:
>> On 27 August 2015 at 13:43, Michal Simek <michal.simek@xilinx.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@linaro.org> wrote:
>>>>> On 6 August 2015 at 07:39, Michal Simek <michal.simek@xilinx.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@xilinx.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web