linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
@ 2013-04-08 10:12 Hector Palacios
  2013-04-08 12:48 ` Shawn Guo
  0 siblings, 1 reply; 10+ messages in thread
From: Hector Palacios @ 2013-04-08 10:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: maxime.ripard, marex, shawn.guo, fabio.estevam, hector.palacios

MicroSD card sockets don't usually have card detect line. This pin
is actually not needed for the MMC to work and it is more of a
platform design decission to have it.
The card detect pin already has a configuration entry of its own:
'mmc0_cd_cfg' so we complete the iomux configuration here and let
platforms to include it or not depending on whether the card detect
line is routed to the SD socket.

Signed-off-by: Hector Palacios <hector.palacios@digi.com>
---

Hello,

All imx28 based platforms except 'bluegiga,apx4devkit' and 
'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc configuration
so please check whether this patch would break these platforms.

 arch/arm/boot/dts/imx28.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 9b18e81..4222e4e 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -449,7 +449,6 @@
 						0x2060 /* MX28_PAD_SSP0_DATA6__SSP0_D6 */
 						0x2070 /* MX28_PAD_SSP0_DATA7__SSP0_D7 */
 						0x2080 /* MX28_PAD_SSP0_CMD__SSP0_CMD */
-						0x2090 /* MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT */
 						0x20a0 /* MX28_PAD_SSP0_SCK__SSP0_SCK */
 					>;
 					fsl,drive-strength = <1>;
@@ -465,7 +464,6 @@
 						0x2020 /* MX28_PAD_SSP0_DATA2__SSP0_D2 */
 						0x2030 /* MX28_PAD_SSP0_DATA3__SSP0_D3 */
 						0x2080 /* MX28_PAD_SSP0_CMD__SSP0_CMD */
-						0x2090 /* MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT */
 						0x20a0 /* MX28_PAD_SSP0_SCK__SSP0_SCK */
 					>;
 					fsl,drive-strength = <1>;
@@ -477,6 +475,8 @@
 					fsl,pinmux-ids = <
 						0x2090 /* MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT */
 					>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
 					fsl,pull-up = <0>;
 				};
 
-- 
1.8.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-08 10:12 [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config Hector Palacios
@ 2013-04-08 12:48 ` Shawn Guo
  2013-04-08 13:58   ` Hector Palacios
  0 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2013-04-08 12:48 UTC (permalink / raw)
  To: Hector Palacios; +Cc: linux-kernel, maxime.ripard, marex, fabio.estevam

On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> MicroSD card sockets don't usually have card detect line. This pin
> is actually not needed for the MMC to work and it is more of a
> platform design decission to have it.
> The card detect pin already has a configuration entry of its own:
> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
> platforms to include it or not depending on whether the card detect
> line is routed to the SD socket.
> 
Sounds sensible.

> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> ---
> 
> Hello,
> 
> All imx28 based platforms except 'bluegiga,apx4devkit' and 
> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc configuration
> so please check whether this patch would break these platforms.
> 
I just tested the patch on imx28-evk and card-detection still works.  So
patches applied, thanks.

Shawn

>  arch/arm/boot/dts/imx28.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 9b18e81..4222e4e 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -449,7 +449,6 @@
>  						0x2060 /* MX28_PAD_SSP0_DATA6__SSP0_D6 */
>  						0x2070 /* MX28_PAD_SSP0_DATA7__SSP0_D7 */
>  						0x2080 /* MX28_PAD_SSP0_CMD__SSP0_CMD */
> -						0x2090 /* MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT */
>  						0x20a0 /* MX28_PAD_SSP0_SCK__SSP0_SCK */
>  					>;
>  					fsl,drive-strength = <1>;
> @@ -465,7 +464,6 @@
>  						0x2020 /* MX28_PAD_SSP0_DATA2__SSP0_D2 */
>  						0x2030 /* MX28_PAD_SSP0_DATA3__SSP0_D3 */
>  						0x2080 /* MX28_PAD_SSP0_CMD__SSP0_CMD */
> -						0x2090 /* MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT */
>  						0x20a0 /* MX28_PAD_SSP0_SCK__SSP0_SCK */
>  					>;
>  					fsl,drive-strength = <1>;
> @@ -477,6 +475,8 @@
>  					fsl,pinmux-ids = <
>  						0x2090 /* MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT */
>  					>;
> +					fsl,drive-strength = <1>;
> +					fsl,voltage = <1>;
>  					fsl,pull-up = <0>;
>  				};
>  
> -- 
> 1.8.2
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-08 12:48 ` Shawn Guo
@ 2013-04-08 13:58   ` Hector Palacios
  2013-04-08 14:50     ` Shawn Guo
  0 siblings, 1 reply; 10+ messages in thread
From: Hector Palacios @ 2013-04-08 13:58 UTC (permalink / raw)
  To: Shawn Guo; +Cc: linux-kernel, maxime.ripard, marex, fabio.estevam

On 04/08/2013 02:48 PM, Shawn Guo wrote:
> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
>> MicroSD card sockets don't usually have card detect line. This pin
>> is actually not needed for the MMC to work and it is more of a
>> platform design decission to have it.
>> The card detect pin already has a configuration entry of its own:
>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
>> platforms to include it or not depending on whether the card detect
>> line is routed to the SD socket.
>>
> Sounds sensible.
>
>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>> ---
>>
>> Hello,
>>
>> All imx28 based platforms except 'bluegiga,apx4devkit' and
>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc configuration
>> so please check whether this patch would break these platforms.
>>
> I just tested the patch on imx28-evk and card-detection still works.  So
> patches applied, thanks.

The EVK and most platforms will work because they are using 'mmc0_cd_cfg' so actually 
this patch does not change anything on them.
Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1' however are not 
referencing 'mmc0_cd_cfg' so after applying this patch they will have unconfigured CD 
line and they may break.
The driver will call get_cd() upon probing, which returns the status of the CD line.
Please check these two platforms before applying.

In fact I was looking now at how to skip the calling of get_cd() hook when you specify 
'non-removable' in the Device Tree. According to the bindings document:

   - non-removable: non-removable slot (like eMMC); *assume always present.*

This property is not handled in mxs-mmc.c but even if I add the code to handle it and 
set MMC_CAP_NONREMOVABLE flag, the get_cd() is called at least once and returning with 
the status of CD line. I think this is wrong because it should assume the card is 
present. The CD line may not be connected at all and may have any value.

Regards,
-- 
Héctor Palacios

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-08 13:58   ` Hector Palacios
@ 2013-04-08 14:50     ` Shawn Guo
  2013-04-08 16:28       ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2013-04-08 14:50 UTC (permalink / raw)
  To: Hector Palacios; +Cc: linux-kernel, maxime.ripard, marex, fabio.estevam

On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> On 04/08/2013 02:48 PM, Shawn Guo wrote:
> >On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> >>MicroSD card sockets don't usually have card detect line. This pin
> >>is actually not needed for the MMC to work and it is more of a
> >>platform design decission to have it.
> >>The card detect pin already has a configuration entry of its own:
> >>'mmc0_cd_cfg' so we complete the iomux configuration here and let
> >>platforms to include it or not depending on whether the card detect
> >>line is routed to the SD socket.
> >>
> >Sounds sensible.
> >
> >>Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> >>---
> >>
> >>Hello,
> >>
> >>All imx28 based platforms except 'bluegiga,apx4devkit' and
> >>'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc configuration
> >>so please check whether this patch would break these platforms.
> >>
> >I just tested the patch on imx28-evk and card-detection still works.  So
> >patches applied, thanks.
> 
> The EVK and most platforms will work because they are using
> 'mmc0_cd_cfg' so actually this patch does not change anything on
> them.
> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> however are not referencing 'mmc0_cd_cfg' so after applying this
> patch they will have unconfigured CD line and they may break.

Ah, yes.  I thought that any board that has CD support has to reference
'mmc0_cd_cfg'.  That's not necessarily true.

> The driver will call get_cd() upon probing, which returns the status of the CD line.
> Please check these two platforms before applying.

Ok, let's wait for people owning the boards to confirm.

Shawn


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-08 14:50     ` Shawn Guo
@ 2013-04-08 16:28       ` Marek Vasut
  2013-04-09  7:18         ` Hector Palacios
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2013-04-08 16:28 UTC (permalink / raw)
  To: Shawn Guo; +Cc: Hector Palacios, linux-kernel, maxime.ripard, fabio.estevam

Dear Shawn Guo,

> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> > On 04/08/2013 02:48 PM, Shawn Guo wrote:
> > >On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> > >>MicroSD card sockets don't usually have card detect line. This pin
> > >>is actually not needed for the MMC to work and it is more of a
> > >>platform design decission to have it.
> > >>The card detect pin already has a configuration entry of its own:
> > >>'mmc0_cd_cfg' so we complete the iomux configuration here and let
> > >>platforms to include it or not depending on whether the card detect
> > >>line is routed to the SD socket.
> > >
> > >Sounds sensible.
> > >
> > >>Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> > >>---
> > >>
> > >>Hello,
> > >>
> > >>All imx28 based platforms except 'bluegiga,apx4devkit' and
> > >>'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
> > >>configuration so please check whether this patch would break these
> > >>platforms.
> > >
> > >I just tested the patch on imx28-evk and card-detection still works.  So
> > >patches applied, thanks.
> > 
> > The EVK and most platforms will work because they are using
> > 'mmc0_cd_cfg' so actually this patch does not change anything on
> > them.
> > Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> > however are not referencing 'mmc0_cd_cfg' so after applying this
> > patch they will have unconfigured CD line and they may break.
> 
> Ah, yes.  I thought that any board that has CD support has to reference
> 'mmc0_cd_cfg'.  That's not necessarily true.
> 
> > The driver will call get_cd() upon probing, which returns the status of
> > the CD line. Please check these two platforms before applying.
> 
> Ok, let's wait for people owning the boards to confirm.

Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before on the 
olinuxino -- the slot is there, it's just the CD line that's missing.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-08 16:28       ` Marek Vasut
@ 2013-04-09  7:18         ` Hector Palacios
  2013-04-09  8:15           ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Hector Palacios @ 2013-04-09  7:18 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Shawn Guo, linux-kernel, maxime.ripard, fabio.estevam, linux-mmc

Dear Marek Vasut,

On 04/08/2013 06:28 PM, Marek Vasut wrote:
> Dear Shawn Guo,
>
>> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
>>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
>>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
>>>>> MicroSD card sockets don't usually have card detect line. This pin
>>>>> is actually not needed for the MMC to work and it is more of a
>>>>> platform design decission to have it.
>>>>> The card detect pin already has a configuration entry of its own:
>>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
>>>>> platforms to include it or not depending on whether the card detect
>>>>> line is routed to the SD socket.
>>>>
>>>> Sounds sensible.
>>>>
>>>>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>>>>> ---
>>>>>
>>>>> Hello,
>>>>>
>>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
>>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
>>>>> configuration so please check whether this patch would break these
>>>>> platforms.
>>>>
>>>> I just tested the patch on imx28-evk and card-detection still works.  So
>>>> patches applied, thanks.
>>>
>>> The EVK and most platforms will work because they are using
>>> 'mmc0_cd_cfg' so actually this patch does not change anything on
>>> them.
>>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
>>> however are not referencing 'mmc0_cd_cfg' so after applying this
>>> patch they will have unconfigured CD line and they may break.
>>
>> Ah, yes.  I thought that any board that has CD support has to reference
>> 'mmc0_cd_cfg'.  That's not necessarily true.
>>
>>> The driver will call get_cd() upon probing, which returns the status of
>>> the CD line. Please check these two platforms before applying.
>>
>> Ok, let's wait for people owning the boards to confirm.
>
> Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before on the
> olinuxino -- the slot is there, it's just the CD line that's missing.

I'm not sure of what you mean. The mxs-mmc.c driver already sets the 
MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform does not even 
route the CD line because the microSD socket does not have it.
So what I have done is modify the driver to parse the property 'non-removable' from 
the device tree in order to set the MMC_CAP_NONREMOVABLE flag:

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 206fe49..2a3b9c9 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -679,15 +682,20 @@ static int mxs_mmc_probe(struct platform_device *pdev)
         /* set mmc core parameters */
         mmc->ops = &mxs_mmc_ops;
         mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
-                   MMC_CAP_SDIO_IRQ | MMC_CAP_NEEDS_POLL;
+                   MMC_CAP_SDIO_IRQ;

         of_property_read_u32(np, "bus-width", &bus_width);
         if (bus_width == 4)
                 mmc->caps |= MMC_CAP_4_BIT_DATA;
         else if (bus_width == 8)
                 mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
-       host->wp_gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);

+       if (of_find_property(np, "non-removable", NULL))
+               mmc->caps |= MMC_CAP_NONREMOVABLE;
+       else
+               mmc->caps |= MMC_CAP_NEEDS_POLL;
+
+       host->wp_gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
         if (flags & OF_GPIO_ACTIVE_LOW)
                 host->wp_inverted = 1;


In theory, and according to the mmc bindings doc, this flag must assume the card is 
always present. However the MMC core function mmc_rescan() will still call at least 
once the get_cd() hook, which in the mxs-mmc.c returns the value of the CD line. In my 
platform (where the CD line is not connected) this is returning 0, so I needed to do 
the following:

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 206fe49..2a3b9c9 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -95,6 +95,9 @@ static int mxs_mmc_get_cd(struct mmc_host *mmc)
         struct mxs_mmc_host *host = mmc_priv(mmc);
         struct mxs_ssp *ssp = &host->ssp;

+       if (mmc->caps & MMC_CAP_NONREMOVABLE)
+               return 1;
+
         return !(readl(ssp->base + HW_SSP_STATUS(ssp)) &
                  BM_SSP_STATUS_CARD_DETECT);
  }

to return a 1 when the driver should assume that the card is there. I wonder however 
if this should be a global mmc patch instead, and have mmc_rescan() not call the host 
get_cd() hook at all if MMC_CAP_NONREMOVABLE is set.

Regards,
-- 
Héctor Palacios

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-09  7:18         ` Hector Palacios
@ 2013-04-09  8:15           ` Marek Vasut
  2013-04-09  9:00             ` Hector Palacios
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2013-04-09  8:15 UTC (permalink / raw)
  To: Hector Palacios
  Cc: Shawn Guo, linux-kernel, maxime.ripard, fabio.estevam, linux-mmc

Dear Hector Palacios,

> Dear Marek Vasut,
> 
> On 04/08/2013 06:28 PM, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> >> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> >>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
> >>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> >>>>> MicroSD card sockets don't usually have card detect line. This pin
> >>>>> is actually not needed for the MMC to work and it is more of a
> >>>>> platform design decission to have it.
> >>>>> The card detect pin already has a configuration entry of its own:
> >>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
> >>>>> platforms to include it or not depending on whether the card detect
> >>>>> line is routed to the SD socket.
> >>>> 
> >>>> Sounds sensible.
> >>>> 
> >>>>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> >>>>> ---
> >>>>> 
> >>>>> Hello,
> >>>>> 
> >>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
> >>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
> >>>>> configuration so please check whether this patch would break these
> >>>>> platforms.
> >>>> 
> >>>> I just tested the patch on imx28-evk and card-detection still works. 
> >>>> So patches applied, thanks.
> >>> 
> >>> The EVK and most platforms will work because they are using
> >>> 'mmc0_cd_cfg' so actually this patch does not change anything on
> >>> them.
> >>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> >>> however are not referencing 'mmc0_cd_cfg' so after applying this
> >>> patch they will have unconfigured CD line and they may break.
> >> 
> >> Ah, yes.  I thought that any board that has CD support has to reference
> >> 'mmc0_cd_cfg'.  That's not necessarily true.
> >> 
> >>> The driver will call get_cd() upon probing, which returns the status of
> >>> the CD line. Please check these two platforms before applying.
> >> 
> >> Ok, let's wait for people owning the boards to confirm.
> > 
> > Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before
> > on the olinuxino -- the slot is there, it's just the CD line that's
> > missing.
> 
> I'm not sure of what you mean. The mxs-mmc.c driver already sets the
> MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform
> does not even route the CD line because the microSD socket does not have
> it.
> So what I have done is modify the driver to parse the property
> 'non-removable' from the device tree in order to set the
> MMC_CAP_NONREMOVABLE flag:

Yes, I get it. I have two remarks still:
1) The card is removable (you can pull it out from olinuxino's slot)
2) Why is the NEEDS_POLL set by default ?
3) Does the NEEDS_POLL not solve the issue with missing CD line?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-09  8:15           ` Marek Vasut
@ 2013-04-09  9:00             ` Hector Palacios
  2013-04-09 10:51               ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Hector Palacios @ 2013-04-09  9:00 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Shawn Guo, linux-kernel, maxime.ripard, fabio.estevam, linux-mmc

Dear Marek Vasut,

On 04/09/2013 10:15 AM, Marek Vasut wrote:
> Dear Hector Palacios,
>
>> Dear Marek Vasut,
>>
>> On 04/08/2013 06:28 PM, Marek Vasut wrote:
>>> Dear Shawn Guo,
>>>
>>>> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
>>>>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
>>>>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
>>>>>>> MicroSD card sockets don't usually have card detect line. This pin
>>>>>>> is actually not needed for the MMC to work and it is more of a
>>>>>>> platform design decission to have it.
>>>>>>> The card detect pin already has a configuration entry of its own:
>>>>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
>>>>>>> platforms to include it or not depending on whether the card detect
>>>>>>> line is routed to the SD socket.
>>>>>>
>>>>>> Sounds sensible.
>>>>>>
>>>>>>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
>>>>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
>>>>>>> configuration so please check whether this patch would break these
>>>>>>> platforms.
>>>>>>
>>>>>> I just tested the patch on imx28-evk and card-detection still works.
>>>>>> So patches applied, thanks.
>>>>>
>>>>> The EVK and most platforms will work because they are using
>>>>> 'mmc0_cd_cfg' so actually this patch does not change anything on
>>>>> them.
>>>>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
>>>>> however are not referencing 'mmc0_cd_cfg' so after applying this
>>>>> patch they will have unconfigured CD line and they may break.
>>>>
>>>> Ah, yes.  I thought that any board that has CD support has to reference
>>>> 'mmc0_cd_cfg'.  That's not necessarily true.
>>>>
>>>>> The driver will call get_cd() upon probing, which returns the status of
>>>>> the CD line. Please check these two platforms before applying.
>>>>
>>>> Ok, let's wait for people owning the boards to confirm.
>>>
>>> Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before
>>> on the olinuxino -- the slot is there, it's just the CD line that's
>>> missing.
>>
>> I'm not sure of what you mean. The mxs-mmc.c driver already sets the
>> MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform
>> does not even route the CD line because the microSD socket does not have
>> it.
>> So what I have done is modify the driver to parse the property
>> 'non-removable' from the device tree in order to set the
>> MMC_CAP_NONREMOVABLE flag:
>
> Yes, I get it. I have two remarks still:
> 1) The card is removable (you can pull it out from olinuxino's slot)

True, I misunderstood the use of 'non-removable'. So I guess I could use 'broken-cd' 
property instead, right?

> 2) Why is the NEEDS_POLL set by default ?

Because the CD line cannot cause an interrupt in this controller.

> 3) Does the NEEDS_POLL not solve the issue with missing CD line?

No. CD polling relies on the status register. The field CARD_DETECT in HW_SSP_STATUS 
register directly reflects the state of the SSP_DETECT input pin. If the pin is not 
connected it can have any value, so I guess we need a custom flag on the driver:

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 206fe49..ec0874b 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -72,6 +72,7 @@ struct mxs_mmc_host {
         int                             sdio_irq_en;
         int                             wp_gpio;
         bool                            wp_inverted;
+       bool                            cd_broken;
  };

  static int mxs_mmc_get_ro(struct mmc_host *mmc)
@@ -95,6 +96,9 @@ static int mxs_mmc_get_cd(struct mmc_host *mmc)
         struct mxs_mmc_host *host = mmc_priv(mmc);
         struct mxs_ssp *ssp = &host->ssp;

+       if (host->cd_broken)
+               return 1;
+
         return !(readl(ssp->base + HW_SSP_STATUS(ssp)) &
                  BM_SSP_STATUS_CARD_DETECT);
  }
@@ -686,8 +690,11 @@ static int mxs_mmc_probe(struct platform_device *pdev)
                 mmc->caps |= MMC_CAP_4_BIT_DATA;
         else if (bus_width == 8)
                 mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
-       host->wp_gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);

+       if (of_find_property(np, "broken-cd", NULL))
+               host->cd_broken = 1;
+
+       host->wp_gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
         if (flags & OF_GPIO_ACTIVE_LOW)
                 host->wp_inverted = 1;

Thank you for your comments.
-- 
Héctor Palacios

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-09  9:00             ` Hector Palacios
@ 2013-04-09 10:51               ` Marek Vasut
  2013-04-10  8:56                 ` Hector Palacios
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2013-04-09 10:51 UTC (permalink / raw)
  To: Hector Palacios
  Cc: Shawn Guo, linux-kernel, maxime.ripard, fabio.estevam, linux-mmc

Hi Hector,

> Dear Marek Vasut,
> 
> On 04/09/2013 10:15 AM, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> >> Dear Marek Vasut,
> >> 
> >> On 04/08/2013 06:28 PM, Marek Vasut wrote:
> >>> Dear Shawn Guo,
> >>> 
> >>>> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> >>>>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
> >>>>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> >>>>>>> MicroSD card sockets don't usually have card detect line. This pin
> >>>>>>> is actually not needed for the MMC to work and it is more of a
> >>>>>>> platform design decission to have it.
> >>>>>>> The card detect pin already has a configuration entry of its own:
> >>>>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
> >>>>>>> platforms to include it or not depending on whether the card detect
> >>>>>>> line is routed to the SD socket.
> >>>>>> 
> >>>>>> Sounds sensible.
> >>>>>> 
> >>>>>>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> >>>>>>> ---
> >>>>>>> 
> >>>>>>> Hello,
> >>>>>>> 
> >>>>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
> >>>>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
> >>>>>>> configuration so please check whether this patch would break these
> >>>>>>> platforms.
> >>>>>> 
> >>>>>> I just tested the patch on imx28-evk and card-detection still works.
> >>>>>> So patches applied, thanks.
> >>>>> 
> >>>>> The EVK and most platforms will work because they are using
> >>>>> 'mmc0_cd_cfg' so actually this patch does not change anything on
> >>>>> them.
> >>>>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> >>>>> however are not referencing 'mmc0_cd_cfg' so after applying this
> >>>>> patch they will have unconfigured CD line and they may break.
> >>>> 
> >>>> Ah, yes.  I thought that any board that has CD support has to
> >>>> reference 'mmc0_cd_cfg'.  That's not necessarily true.
> >>>> 
> >>>>> The driver will call get_cd() upon probing, which returns the status
> >>>>> of the CD line. Please check these two platforms before applying.
> >>>> 
> >>>> Ok, let's wait for people owning the boards to confirm.
> >>> 
> >>> Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before
> >>> on the olinuxino -- the slot is there, it's just the CD line that's
> >>> missing.
> >> 
> >> I'm not sure of what you mean. The mxs-mmc.c driver already sets the
> >> MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform
> >> does not even route the CD line because the microSD socket does not have
> >> it.
> >> So what I have done is modify the driver to parse the property
> >> 'non-removable' from the device tree in order to set the
> > 
> >> MMC_CAP_NONREMOVABLE flag:
> > Yes, I get it. I have two remarks still:
> > 1) The card is removable (you can pull it out from olinuxino's slot)
> 
> True, I misunderstood the use of 'non-removable'. So I guess I could use
> 'broken-cd' property instead, right?

Yep, seems ok.

> > 2) Why is the NEEDS_POLL set by default ?
> 
> Because the CD line cannot cause an interrupt in this controller.

Ok, understood.

NOTE: Here is an idea -- we can solve this the same way it's solved with the MXC 
SDHCI, configure the CD pin(s) as GPIO(s) and then they'll be able to generate 
interrupt. On the other hand, this shall definitelly be done in a separate 
patch.

> > 3) Does the NEEDS_POLL not solve the issue with missing CD line?
> 
> No. CD polling relies on the status register. The field CARD_DETECT in
> HW_SSP_STATUS register directly reflects the state of the SSP_DETECT input
> pin. If the pin is not connected it can have any value, so I guess we need
> a custom flag on the driver:

Understood!

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config
  2013-04-09 10:51               ` Marek Vasut
@ 2013-04-10  8:56                 ` Hector Palacios
  0 siblings, 0 replies; 10+ messages in thread
From: Hector Palacios @ 2013-04-10  8:56 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Shawn Guo, linux-kernel, maxime.ripard, fabio.estevam, linux-mmc

On 04/09/2013 12:51 PM, Marek Vasut wrote:
> Hi Hector,
>
>> Dear Marek Vasut,
>>
>> On 04/09/2013 10:15 AM, Marek Vasut wrote:
>>> Dear Hector Palacios,
>>>
>>>> Dear Marek Vasut,
>>>>
>>>> On 04/08/2013 06:28 PM, Marek Vasut wrote:
>>>>> Dear Shawn Guo,
>>>>>
>>>>>> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
>>>>>>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
>>>>>>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
>>>>>>>>> MicroSD card sockets don't usually have card detect line. This pin
>>>>>>>>> is actually not needed for the MMC to work and it is more of a
>>>>>>>>> platform design decission to have it.
>>>>>>>>> The card detect pin already has a configuration entry of its own:
>>>>>>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
>>>>>>>>> platforms to include it or not depending on whether the card detect
>>>>>>>>> line is routed to the SD socket.
>>>>>>>>
>>>>>>>> Sounds sensible.
>>>>>>>>
>>>>>>>>> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
>>>>>>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
>>>>>>>>> configuration so please check whether this patch would break these
>>>>>>>>> platforms.
>>>>>>>>
>>>>>>>> I just tested the patch on imx28-evk and card-detection still works.
>>>>>>>> So patches applied, thanks.
>>>>>>>
>>>>>>> The EVK and most platforms will work because they are using
>>>>>>> 'mmc0_cd_cfg' so actually this patch does not change anything on
>>>>>>> them.
>>>>>>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
>>>>>>> however are not referencing 'mmc0_cd_cfg' so after applying this
>>>>>>> patch they will have unconfigured CD line and they may break.
>>>>>>
>>>>>> Ah, yes.  I thought that any board that has CD support has to
>>>>>> reference 'mmc0_cd_cfg'.  That's not necessarily true.
>>>>>>
>>>>>>> The driver will call get_cd() upon probing, which returns the status
>>>>>>> of the CD line. Please check these two platforms before applying.
>>>>>>
>>>>>> Ok, let's wait for people owning the boards to confirm.
>>>>>
>>>>> Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before
>>>>> on the olinuxino -- the slot is there, it's just the CD line that's
>>>>> missing.
>>>>
>>>> I'm not sure of what you mean. The mxs-mmc.c driver already sets the
>>>> MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform
>>>> does not even route the CD line because the microSD socket does not have
>>>> it.
>>>> So what I have done is modify the driver to parse the property
>>>> 'non-removable' from the device tree in order to set the
>>>
>>>> MMC_CAP_NONREMOVABLE flag:
>>> Yes, I get it. I have two remarks still:
>>> 1) The card is removable (you can pull it out from olinuxino's slot)
>>
>> True, I misunderstood the use of 'non-removable'. So I guess I could use
>> 'broken-cd' property instead, right?
>
> Yep, seems ok.
>
>>> 2) Why is the NEEDS_POLL set by default ?
>>
>> Because the CD line cannot cause an interrupt in this controller.
>
> Ok, understood.
>
> NOTE: Here is an idea -- we can solve this the same way it's solved with the MXC
> SDHCI, configure the CD pin(s) as GPIO(s) and then they'll be able to generate
> interrupt. On the other hand, this shall definitelly be done in a separate
> patch.
>
>>> 3) Does the NEEDS_POLL not solve the issue with missing CD line?
>>
>> No. CD polling relies on the status register. The field CARD_DETECT in
>> HW_SSP_STATUS register directly reflects the state of the SSP_DETECT input
>> pin. If the pin is not connected it can have any value, so I guess we need
>> a custom flag on the driver:

Patch available in this other thread (pending resubmission):
http://comments.gmane.org/gmane.linux.kernel.mmc/19982

With this, card detection works via polling even if you don't use the Card Detect line.

Regards,
-- 
Héctor Palacios

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-04-10  8:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-08 10:12 [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config Hector Palacios
2013-04-08 12:48 ` Shawn Guo
2013-04-08 13:58   ` Hector Palacios
2013-04-08 14:50     ` Shawn Guo
2013-04-08 16:28       ` Marek Vasut
2013-04-09  7:18         ` Hector Palacios
2013-04-09  8:15           ` Marek Vasut
2013-04-09  9:00             ` Hector Palacios
2013-04-09 10:51               ` Marek Vasut
2013-04-10  8:56                 ` Hector Palacios

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).