[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[Tuomas Tynkkynen]

Commit 8620f384098b ("dm: sunxi: Linksprite_pcDuino3: Correct polarity
of MMC card detect") claims that the Pcduino3 device tree has an
incorrect polarity for the card detect pin, but the actual problem is
that unlike the Linux MMC driver, the U-Boot driver isn't respecting the
cd-invert property (see Documentation/devicetree/bindings/mmc/mmc.txt)
which tells that the card detect signal level should be inverted.

Fix this properly by adding support for the cd-inverted property while
reverting the original commit at the same time. While at it, I noticed
the driver always enables pullups for the card detect line, which is not
right if the card detect GPIO is active-high, so fix that as well.

Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
---
 arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
 drivers/mmc/sunxi_mmc.c             | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
index 37b1e0ee9b..1a8b39be1d 100644
--- a/arch/arm/dts/sun7i-a20-pcduino3.dts
+++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
@@ -164,7 +164,7 @@
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
 	vmmc-supply = <&reg_vcc3v3>;
 	bus-width = <4>;
-	cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */
+	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
 	cd-inverted;
 	status = "okay";
 };
diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
index 4edb4be46c..7cc7303570 100644
--- a/drivers/mmc/sunxi_mmc.c
+++ b/drivers/mmc/sunxi_mmc.c
@@ -30,6 +30,7 @@ struct sunxi_mmc_priv {
 	uint32_t *mclkreg;
 	unsigned fatal_err;
 	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
+	int cd_inverted;
 	struct sunxi_mmc *reg;
 	struct mmc_config cfg;
 };
@@ -545,7 +546,7 @@ static int sunxi_mmc_getcd(struct udevice *dev)
 	struct sunxi_mmc_priv *priv = dev_get_priv(dev);
 
 	if (dm_gpio_is_valid(&priv->cd_gpio))
-		return dm_gpio_get_value(&priv->cd_gpio);
+		return dm_gpio_get_value(&priv->cd_gpio) ^ priv->cd_inverted;
 
 	return 1;
 }
@@ -606,8 +607,15 @@ static int sunxi_mmc_probe(struct udevice *dev)
 	if (!gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio,
 				  GPIOD_IS_IN)) {
 		int cd_pin = gpio_get_number(&priv->cd_gpio);
+		int cd_state = priv->cd_gpio.flags & GPIOD_ACTIVE_LOW ? 0 : 1;
 
-		sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_UP);
+		priv->cd_inverted = dev_read_bool(dev, "cd-inverted");
+		cd_state ^= priv->cd_inverted;
+
+		if (cd_state)
+			sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_DOWN);
+		else
+			sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_UP);
 	}
 
 	upriv->mmc = &plat->mmc;
-- 
2.15.0

[U-Boot] [PATCH 2/2] ARM: sunxi: Enable DM MMC+SATA for the PcDuino3 Nano board

[Tuomas Tynkkynen]

After the previous commit which adds support for the cd-inverted
property to the sunxi MMC driver, driver-model MMC works fine on this
board.

Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
---
 configs/Linksprite_pcDuino3_Nano_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/Linksprite_pcDuino3_Nano_defconfig b/configs/Linksprite_pcDuino3_Nano_defconfig
index 13538fafd1..44ce8b3ead 100644
--- a/configs/Linksprite_pcDuino3_Nano_defconfig
+++ b/configs/Linksprite_pcDuino3_Nano_defconfig
@@ -17,9 +17,11 @@ CONFIG_SPL_I2C_SUPPORT=y
 # CONFIG_SPL_ISO_PARTITION is not set
 # CONFIG_SPL_EFI_PARTITION is not set
 CONFIG_SCSI_AHCI=y
+CONFIG_DM_MMC=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_RGMII=y
 CONFIG_SUN7I_GMAC=y
 CONFIG_SCSI=y
+CONFIG_DM_SCSI=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE=y
-- 
2.15.0

[U-Boot] [PATCH 2/2] ARM: sunxi: Enable DM MMC+SATA for the PcDuino3 Nano board

[Maxime Ripard]

On Mon, Dec 18, 2017 at 10:09:42PM +0200, Tuomas Tynkkynen wrote:
> After the previous commit which adds support for the cd-inverted
> property to the sunxi MMC driver, driver-model MMC works fine on this
> board.
> 
> Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> ---
>  configs/Linksprite_pcDuino3_Nano_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configs/Linksprite_pcDuino3_Nano_defconfig b/configs/Linksprite_pcDuino3_Nano_defconfig
> index 13538fafd1..44ce8b3ead 100644
> --- a/configs/Linksprite_pcDuino3_Nano_defconfig
> +++ b/configs/Linksprite_pcDuino3_Nano_defconfig
> @@ -17,9 +17,11 @@ CONFIG_SPL_I2C_SUPPORT=y
>  # CONFIG_SPL_ISO_PARTITION is not set
>  # CONFIG_SPL_EFI_PARTITION is not set
>  CONFIG_SCSI_AHCI=y
> +CONFIG_DM_MMC=y
>  CONFIG_ETH_DESIGNWARE=y
>  CONFIG_RGMII=y
>  CONFIG_SUN7I_GMAC=y
>  CONFIG_SCSI=y
> +CONFIG_DM_SCSI=y

Those should not be in a single defconfig, but enabled for all the
Allwinner boards (that have SATA and MMC).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171220/235eb17f/attachment.sig>

[U-Boot] [PATCH 2/2] ARM: sunxi: Enable DM MMC+SATA for the PcDuino3 Nano board

[Tuomas Tynkkynen]

On Wed, 2017-12-20 at 13:26 +0100, Maxime Ripard wrote:
> On Mon, Dec 18, 2017 at 10:09:42PM +0200, Tuomas Tynkkynen wrote:
> > After the previous commit which adds support for the cd-inverted
> > property to the sunxi MMC driver, driver-model MMC works fine on
> > this
> > board.
> > 
> > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> > ---
> >  configs/Linksprite_pcDuino3_Nano_defconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/configs/Linksprite_pcDuino3_Nano_defconfig
> > b/configs/Linksprite_pcDuino3_Nano_defconfig
> > index 13538fafd1..44ce8b3ead 100644
> > --- a/configs/Linksprite_pcDuino3_Nano_defconfig
> > +++ b/configs/Linksprite_pcDuino3_Nano_defconfig
> > @@ -17,9 +17,11 @@ CONFIG_SPL_I2C_SUPPORT=y
> >  # CONFIG_SPL_ISO_PARTITION is not set
> >  # CONFIG_SPL_EFI_PARTITION is not set
> >  CONFIG_SCSI_AHCI=y
> > +CONFIG_DM_MMC=y
> >  CONFIG_ETH_DESIGNWARE=y
> >  CONFIG_RGMII=y
> >  CONFIG_SUN7I_GMAC=y
> >  CONFIG_SCSI=y
> > +CONFIG_DM_SCSI=y
> 
> Those should not be in a single defconfig, but enabled for all the
> Allwinner boards (that have SATA and MMC).
> 
> Maxime
> 

Makes sense. I don't have any other boards to test with so I just
imitated Simon's change for the non-Nano pcduino3.

I will try to see if this could be imply'd in Kconfig.

[U-Boot] [PATCH 2/2] ARM: sunxi: Enable DM MMC+SATA for the PcDuino3 Nano board

[Maxime Ripard]

On Wed, Dec 20, 2017 at 03:37:13PM +0200, Tuomas Tynkkynen wrote:
> On Wed, 2017-12-20 at 13:26 +0100, Maxime Ripard wrote:
> > On Mon, Dec 18, 2017 at 10:09:42PM +0200, Tuomas Tynkkynen wrote:
> > > After the previous commit which adds support for the cd-inverted
> > > property to the sunxi MMC driver, driver-model MMC works fine on
> > > this
> > > board.
> > > 
> > > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> > > ---
> > >  configs/Linksprite_pcDuino3_Nano_defconfig | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/configs/Linksprite_pcDuino3_Nano_defconfig
> > > b/configs/Linksprite_pcDuino3_Nano_defconfig
> > > index 13538fafd1..44ce8b3ead 100644
> > > --- a/configs/Linksprite_pcDuino3_Nano_defconfig
> > > +++ b/configs/Linksprite_pcDuino3_Nano_defconfig
> > > @@ -17,9 +17,11 @@ CONFIG_SPL_I2C_SUPPORT=y
> > >  # CONFIG_SPL_ISO_PARTITION is not set
> > >  # CONFIG_SPL_EFI_PARTITION is not set
> > >  CONFIG_SCSI_AHCI=y
> > > +CONFIG_DM_MMC=y
> > >  CONFIG_ETH_DESIGNWARE=y
> > >  CONFIG_RGMII=y
> > >  CONFIG_SUN7I_GMAC=y
> > >  CONFIG_SCSI=y
> > > +CONFIG_DM_SCSI=y
> > 
> > Those should not be in a single defconfig, but enabled for all the
> > Allwinner boards (that have SATA and MMC).
> > 
> > Maxime
> > 
> 
> Makes sense. I don't have any other boards to test with so I just
> imitated Simon's change for the non-Nano pcduino3.
> 
> I will try to see if this could be imply'd in Kconfig.

You can also use a default y if ARCH_SUNXI. That's actually a better
option, since imply still have a few glitches that generate an error
when the situation is valid.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171221/a9c82649/attachment.sig>

[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[Maxime Ripard]

On Mon, Dec 18, 2017 at 10:09:41PM +0200, Tuomas Tynkkynen wrote:
> Commit 8620f384098b ("dm: sunxi: Linksprite_pcDuino3: Correct polarity
> of MMC card detect") claims that the Pcduino3 device tree has an
> incorrect polarity for the card detect pin, but the actual problem is
> that unlike the Linux MMC driver, the U-Boot driver isn't respecting the
> cd-invert property (see Documentation/devicetree/bindings/mmc/mmc.txt)
> which tells that the card detect signal level should be inverted.
> 
> Fix this properly by adding support for the cd-inverted property while
> reverting the original commit at the same time. While at it, I noticed
> the driver always enables pullups for the card detect line, which is not
> right if the card detect GPIO is active-high, so fix that as well.
> 
> Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> ---
>  arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
>  drivers/mmc/sunxi_mmc.c             | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
> index 37b1e0ee9b..1a8b39be1d 100644
> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
> @@ -164,7 +164,7 @@
>  	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
>  	vmmc-supply = <&reg_vcc3v3>;
>  	bus-width = <4>;
> -	cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */
> +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>  	cd-inverted;
>  	status = "okay";
>  };
> diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> index 4edb4be46c..7cc7303570 100644
> --- a/drivers/mmc/sunxi_mmc.c
> +++ b/drivers/mmc/sunxi_mmc.c
> @@ -30,6 +30,7 @@ struct sunxi_mmc_priv {
>  	uint32_t *mclkreg;
>  	unsigned fatal_err;
>  	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
> +	int cd_inverted;
>  	struct sunxi_mmc *reg;
>  	struct mmc_config cfg;
>  };
> @@ -545,7 +546,7 @@ static int sunxi_mmc_getcd(struct udevice *dev)
>  	struct sunxi_mmc_priv *priv = dev_get_priv(dev);
>  
>  	if (dm_gpio_is_valid(&priv->cd_gpio))
> -		return dm_gpio_get_value(&priv->cd_gpio);
> +		return dm_gpio_get_value(&priv->cd_gpio) ^ priv->cd_inverted;
>  
>  	return 1;
>  }
> @@ -606,8 +607,15 @@ static int sunxi_mmc_probe(struct udevice *dev)
>  	if (!gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio,
>  				  GPIOD_IS_IN)) {
>  		int cd_pin = gpio_get_number(&priv->cd_gpio);
> +		int cd_state = priv->cd_gpio.flags & GPIOD_ACTIVE_LOW ? 0 : 1;

Isn't that change itself enough to get what you wanted?

You really shouldn't be doing anything else.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171220/19e4c77d/attachment.sig>

[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[Tuomas Tynkkynen]

On Wed, 2017-12-20 at 13:26 +0100, Maxime Ripard wrote:
> On Mon, Dec 18, 2017 at 10:09:41PM +0200, Tuomas Tynkkynen wrote:
> > Commit 8620f384098b ("dm: sunxi: Linksprite_pcDuino3: Correct
> > polarity
> > of MMC card detect") claims that the Pcduino3 device tree has an
> > incorrect polarity for the card detect pin, but the actual problem
> > is
> > that unlike the Linux MMC driver, the U-Boot driver isn't
> > respecting the
> > cd-invert property (see
> > Documentation/devicetree/bindings/mmc/mmc.txt)
> > which tells that the card detect signal level should be inverted.
> > 
> > Fix this properly by adding support for the cd-inverted property
> > while
> > reverting the original commit at the same time. While at it, I
> > noticed
> > the driver always enables pullups for the card detect line, which
> > is not
> > right if the card detect GPIO is active-high, so fix that as well.
> > 
> > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> > ---
> >  arch/arm/dts/sun7i-a20-pcduino3.dts |  2 +-
> >  drivers/mmc/sunxi_mmc.c             | 12 ++++++++++--
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts
> > b/arch/arm/dts/sun7i-a20-pcduino3.dts
> > index 37b1e0ee9b..1a8b39be1d 100644
> > --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
> > +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
> > @@ -164,7 +164,7 @@
> >  	pinctrl-0 = <&mmc0_pins_a>,
> > <&mmc0_cd_pin_reference_design>;
> >  	vmmc-supply = <&reg_vcc3v3>;
> >  	bus-width = <4>;
> > -	cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */
> > +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> >  	cd-inverted;
> >  	status = "okay";
> >  };
> > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> > index 4edb4be46c..7cc7303570 100644
> > --- a/drivers/mmc/sunxi_mmc.c
> > +++ b/drivers/mmc/sunxi_mmc.c
> > @@ -30,6 +30,7 @@ struct sunxi_mmc_priv {
> >  	uint32_t *mclkreg;
> >  	unsigned fatal_err;
> >  	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
> > +	int cd_inverted;
> >  	struct sunxi_mmc *reg;
> >  	struct mmc_config cfg;
> >  };
> > @@ -545,7 +546,7 @@ static int sunxi_mmc_getcd(struct udevice *dev)
> >  	struct sunxi_mmc_priv *priv = dev_get_priv(dev);
> >  
> >  	if (dm_gpio_is_valid(&priv->cd_gpio))
> > -		return dm_gpio_get_value(&priv->cd_gpio);
> > +		return dm_gpio_get_value(&priv->cd_gpio) ^ priv-
> > >cd_inverted;
> >  
> >  	return 1;
> >  }
> > @@ -606,8 +607,15 @@ static int sunxi_mmc_probe(struct udevice
> > *dev)
> >  	if (!gpio_request_by_name(dev, "cd-gpios", 0, &priv-
> > >cd_gpio,
> >  				  GPIOD_IS_IN)) {
> >  		int cd_pin = gpio_get_number(&priv->cd_gpio);
> > +		int cd_state = priv->cd_gpio.flags &
> > GPIOD_ACTIVE_LOW ? 0 : 1;
> 
> Isn't that change itself enough to get what you wanted?
> 
> You really shouldn't be doing anything else.

Was that the correct part you meant to quote? The cd_state
was only for the somewhat pull-up change... Sorry about that,
here's the minimal (probably whitespace-damaged) change:

diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
index 37b1e0ee9b..1a8b39be1d 100644
--- a/arch/arm/dts/sun7i-a20-pcduino3.dts
+++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
@@ -164,7 +164,7 @@
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
 	vmmc-supply = <&reg_vcc3v3>;
 	bus-width = <4>;
-	cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */
+	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
 	cd-inverted;
 	status = "okay";
 };
diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
index 4edb4be46c..a974543148 100644
--- a/drivers/mmc/sunxi_mmc.c
+++ b/drivers/mmc/sunxi_mmc.c
@@ -30,6 +30,7 @@ struct sunxi_mmc_priv {
 	uint32_t *mclkreg;
 	unsigned fatal_err;
 	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
+	int cd_inverted;
 	struct sunxi_mmc *reg;
 	struct mmc_config cfg;
 };
@@ -545,7 +546,7 @@ static int sunxi_mmc_getcd(struct udevice *dev)
 	struct sunxi_mmc_priv *priv = dev_get_priv(dev);
 
 	if (dm_gpio_is_valid(&priv->cd_gpio))
-		return dm_gpio_get_value(&priv->cd_gpio);
+		return dm_gpio_get_value(&priv->cd_gpio) ^ priv->cd_inverted;
 
 	return 1;
 }
@@ -607,6 +608,8 @@ static int sunxi_mmc_probe(struct udevice *dev)
 				  GPIOD_IS_IN)) {
 		int cd_pin = gpio_get_number(&priv->cd_gpio);
 
+		priv->cd_inverted = dev_read_bool(dev, "cd-inverted");
+
 		sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_UP);
 	}
 
So: if the DT specifies GPIO_ACTIVE_LOW, dm_gpio_get_value()
does its own inversion of the GPIO level. Then, on top of that
we do another inversion if the "cd-inverted" property was specified.
This matches what the Linux MMC driver does.

Does that make sense?

Thanks.

[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[Maxime Ripard]

Hi Tuomas,

On Wed, Dec 20, 2017 at 03:34:47PM +0200, Tuomas Tynkkynen wrote:
> > > @@ -606,8 +607,15 @@ static int sunxi_mmc_probe(struct udevice
> > > *dev)
> > >  	if (!gpio_request_by_name(dev, "cd-gpios", 0, &priv-
> > > >cd_gpio,
> > >  				  GPIOD_IS_IN)) {
> > >  		int cd_pin = gpio_get_number(&priv->cd_gpio);
> > > +		int cd_state = priv->cd_gpio.flags &
> > > GPIOD_ACTIVE_LOW ? 0 : 1;
> > 
> > Isn't that change itself enough to get what you wanted?
> > 
> > You really shouldn't be doing anything else.
> 
> Was that the correct part you meant to quote? The cd_state
> was only for the somewhat pull-up change... Sorry about that,
> here's the minimal (probably whitespace-damaged) change:

Sorry if it wasn't really clear, I actually wanted to say that
cd-inverted is deprecated, and you should just teach about the gpio
flags like you did.

The rest seems superfluous, especially since I'd expect the
combination of our GPIO flags and cd-inverted in our DTs to be buggy
in most cases.

> diff --git a/arch/arm/dts/sun7i-a20-pcduino3.dts b/arch/arm/dts/sun7i-a20-pcduino3.dts
> index 37b1e0ee9b..1a8b39be1d 100644
> --- a/arch/arm/dts/sun7i-a20-pcduino3.dts
> +++ b/arch/arm/dts/sun7i-a20-pcduino3.dts
> @@ -164,7 +164,7 @@
>  	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_reference_design>;
>  	vmmc-supply = <&reg_vcc3v3>;
>  	bus-width = <4>;
> -	cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */
> +	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>  	cd-inverted;
>  	status = "okay";
>  };
> diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> index 4edb4be46c..a974543148 100644
> --- a/drivers/mmc/sunxi_mmc.c
> +++ b/drivers/mmc/sunxi_mmc.c
> @@ -30,6 +30,7 @@ struct sunxi_mmc_priv {
>  	uint32_t *mclkreg;
>  	unsigned fatal_err;
>  	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
> +	int cd_inverted;
>  	struct sunxi_mmc *reg;
>  	struct mmc_config cfg;
>  };
> @@ -545,7 +546,7 @@ static int sunxi_mmc_getcd(struct udevice *dev)
>  	struct sunxi_mmc_priv *priv = dev_get_priv(dev);
>  
>  	if (dm_gpio_is_valid(&priv->cd_gpio))
> -		return dm_gpio_get_value(&priv->cd_gpio);
> +		return dm_gpio_get_value(&priv->cd_gpio) ^ priv->cd_inverted;
>  
>  	return 1;
>  }
> @@ -607,6 +608,8 @@ static int sunxi_mmc_probe(struct udevice *dev)
>  				  GPIOD_IS_IN)) {
>  		int cd_pin = gpio_get_number(&priv->cd_gpio);
>  
> +		priv->cd_inverted = dev_read_bool(dev, "cd-inverted");
> +
>  		sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_UP);
>  	}
>  
> So: if the DT specifies GPIO_ACTIVE_LOW, dm_gpio_get_value()
> does its own inversion of the GPIO level. Then, on top of that
> we do another inversion if the "cd-inverted" property was specified.
> This matches what the Linux MMC driver does.

Hmmm, right.

I guess in your particular case then, you'd be better off droping the
cd-inverted property. Like I said, it's really legacy these days. That
of course doesn't prevent the rest of the patch to go in.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171221/14fca879/attachment.sig>

[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[Tuomas Tynkkynen]

On Thu, 2017-12-21 at 14:09 +0100, Maxime Ripard wrote:

> >  		sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_UP);
> >  	}
> >  
> > So: if the DT specifies GPIO_ACTIVE_LOW, dm_gpio_get_value()
> > does its own inversion of the GPIO level. Then, on top of that
> > we do another inversion if the "cd-inverted" property was
> > specified.
> > This matches what the Linux MMC driver does.
> 
> Hmmm, right.
> 
> I guess in your particular case then, you'd be better off droping the
> cd-inverted property. Like I said, it's really legacy these days.
> That
> of course doesn't prevent the rest of the patch to go in.

Yes, getting rid of the cd-inverted properties from the DTs should make
card detection work without any code changes. So the plan of action
will be:

- Drop cd-inverted from U-Boot's sun7i-a20-pcduino3.dts
- Flip the GPIO_ACTIVE_* around + drop cd-inverted from Linux DTs
- Re-sync DTs from Linux to U-Boot
- Finally switch all sunxi boards to DM_MMC in U-Boot

[U-Boot] [PATCH 1/2] mmc: sunxi: Support cd-inverted DT property

[Maxime Ripard]

On Thu, Dec 21, 2017 at 04:09:46PM +0200, Tuomas Tynkkynen wrote:
> On Thu, 2017-12-21 at 14:09 +0100, Maxime Ripard wrote:
> 
> > >  		sunxi_gpio_set_pull(cd_pin, SUNXI_GPIO_PULL_UP);
> > >  	}
> > >  
> > > So: if the DT specifies GPIO_ACTIVE_LOW, dm_gpio_get_value()
> > > does its own inversion of the GPIO level. Then, on top of that
> > > we do another inversion if the "cd-inverted" property was
> > > specified.
> > > This matches what the Linux MMC driver does.
> > 
> > Hmmm, right.
> > 
> > I guess in your particular case then, you'd be better off droping the
> > cd-inverted property. Like I said, it's really legacy these days.
> > That
> > of course doesn't prevent the rest of the patch to go in.
> 
> Yes, getting rid of the cd-inverted properties from the DTs should make
> card detection work without any code changes. So the plan of action
> will be:
> 
> - Drop cd-inverted from U-Boot's sun7i-a20-pcduino3.dts
> - Flip the GPIO_ACTIVE_* around + drop cd-inverted from Linux DTs
> - Re-sync DTs from Linux to U-Boot
> - Finally switch all sunxi boards to DM_MMC in U-Boot

Looks good to me :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171221/395bf245/attachment.sig>