| Message ID | 55DEF7FD.8020904@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? > 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, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/27/2015 04:12 PM, Ulf Hansson wrote: > 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. 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-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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