[U-Boot] [PATCH 1/3] gpio: omap_gpio: Add xlate function to support GPIO_ACTIVE_LOW

[U-Boot] [PATCH 1/3] gpio: omap_gpio: Add xlate function to support GPIO_ACTIVE_LOW

[Adam Ford]

The gpio driver doesn't current support knowing whether or not
GPIO is active low or high.  It simply returns the value.  The
side effect of this, is that the MMC routines which check
the status card detect or write protect must use a u-boot specific
dts modification to enable inverting the pin values when
GPIO_ACTIVE_LOW is used.  This allows the invert option to be removed

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c
index 555eba2662..0ecd2f374a 100644
--- a/drivers/gpio/omap_gpio.c
+++ b/drivers/gpio/omap_gpio.c
@@ -24,6 +24,7 @@
 #include <asm/io.h>
 #include <linux/errno.h>
 #include <malloc.h>
+#include <dt-bindings/gpio/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -275,12 +276,25 @@ static int omap_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_INPUT;
 }
 
+static int omap_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
+			   struct ofnode_phandle_args *args)
+{
+	desc->offset = args->args[0];
+
+	if (args->args[1] & GPIO_ACTIVE_LOW)
+		desc->flags = GPIOD_ACTIVE_LOW;
+	else
+		desc->flags = 0;
+	return 0;
+}
+
 static const struct dm_gpio_ops gpio_omap_ops = {
 	.direction_input	= omap_gpio_direction_input,
 	.direction_output	= omap_gpio_direction_output,
 	.get_value		= omap_gpio_get_value,
 	.set_value		= omap_gpio_set_value,
 	.get_function		= omap_gpio_get_function,
+	.xlate                  = omap_gpio_xlate,
 };
 
 static int omap_gpio_probe(struct udevice *dev)
-- 
2.17.1

[U-Boot] [PATCH 2/3] mmc: omap_mmc: Remove invert references with gpio xlate available

[Adam Ford]

With omap_gpio now translating GPIO_ACTIVE_LOW, any boards
using the 'invert' option will no longer need to do this.  This
patch removes the support for 'invert' from the MMC driver.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 8ab56d247d..e9786ec5bb 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -83,7 +83,6 @@ struct omap_hsmmc_data {
 #if CONFIG_IS_ENABLED(DM_MMC)
 	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
 	struct gpio_desc wp_gpio;	/* Write Protect GPIO */
-	bool cd_inverted;
 #else
 	int cd_gpio;
 	int wp_gpio;
@@ -1377,8 +1376,6 @@ static int omap_hsmmc_getcd(struct udevice *dev)
 	if (value < 0)
 		return 1;
 
-	if (priv->cd_inverted)
-		return !value;
 	return value;
 }
 
@@ -1860,10 +1857,6 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev)
 	}
 #endif
 
-#ifdef OMAP_HSMMC_USE_GPIO
-	plat->cd_inverted = fdtdec_get_bool(fdt, node, "cd-inverted");
-#endif
-
 	return 0;
 }
 #endif
@@ -1892,9 +1885,6 @@ static int omap_hsmmc_probe(struct udevice *dev)
 	priv->base_addr = plat->base_addr;
 	priv->controller_flags = plat->controller_flags;
 	priv->hw_rev = plat->hw_rev;
-#ifdef OMAP_HSMMC_USE_GPIO
-	priv->cd_inverted = plat->cd_inverted;
-#endif
 
 #ifdef CONFIG_BLK
 	mmc = plat->mmc;
-- 
2.17.1

[U-Boot] [PATCH 2/3] mmc: omap_mmc: Remove invert references with gpio xlate available

[Jean-Jacques Hiblot]

Adam,


On 05/09/2018 11:35, Adam Ford wrote:
> With omap_gpio now translating GPIO_ACTIVE_LOW, any boards
> using the 'invert' option will no longer need to do this.  This
> patch removes the support for 'invert' from the MMC driver.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index 8ab56d247d..e9786ec5bb 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -83,7 +83,6 @@ struct omap_hsmmc_data {
>   #if CONFIG_IS_ENABLED(DM_MMC)
>   	struct gpio_desc cd_gpio;	/* Change Detect GPIO */
>   	struct gpio_desc wp_gpio;	/* Write Protect GPIO */
> -	bool cd_inverted;

>   #else
>   	int cd_gpio;
>   	int wp_gpio;
> @@ -1377,8 +1376,6 @@ static int omap_hsmmc_getcd(struct udevice *dev)
>   	if (value < 0)
>   		return 1;
>   
> -	if (priv->cd_inverted)
> -		return !value;
>   	return value;
>   }
>   
> @@ -1860,10 +1857,6 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev)
>   	}
>   #endif
>   
> -#ifdef OMAP_HSMMC_USE_GPIO
> -	plat->cd_inverted = fdtdec_get_bool(fdt, node, "cd-inverted");
> -#endif
> -
>   	return 0;
>   }
>   #endif
> @@ -1892,9 +1885,6 @@ static int omap_hsmmc_probe(struct udevice *dev)
>   	priv->base_addr = plat->base_addr;
>   	priv->controller_flags = plat->controller_flags;
>   	priv->hw_rev = plat->hw_rev;
> -#ifdef OMAP_HSMMC_USE_GPIO
> -	priv->cd_inverted = plat->cd_inverted;
Could you remove it also from struct omap_hsmmc_plat since it is not 
used anymore ?

JJ
> -#endif
>   
>   #ifdef CONFIG_BLK
>   	mmc = plat->mmc;

[U-Boot] [PATCH 3/3] ARM: DTS: various omap3: Remove cd-inverted from u-boot.dtsi

[Adam Ford]

With the omap_mmc driver no longer supporting cd-inverted, this
patch removes all these references since they are not needed.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm/dts/am3517-evm-u-boot.dtsi b/arch/arm/dts/am3517-evm-u-boot.dtsi
index c02beaad77..59df819f9d 100644
--- a/arch/arm/dts/am3517-evm-u-boot.dtsi
+++ b/arch/arm/dts/am3517-evm-u-boot.dtsi
@@ -10,10 +10,6 @@
 	};
 };
 
-&mmc1 {
-	cd-inverted;
-};
-
 &uart1 {
 	reg-shift = <2>;
 };
diff --git a/arch/arm/dts/omap3-beagle-u-boot.dtsi b/arch/arm/dts/omap3-beagle-u-boot.dtsi
index 094f9557b7..41beaf0900 100644
--- a/arch/arm/dts/omap3-beagle-u-boot.dtsi
+++ b/arch/arm/dts/omap3-beagle-u-boot.dtsi
@@ -11,10 +11,6 @@
 	};
 };
 
-&mmc1 {
-	cd-inverted;
-};
-
 &uart1 {
 	reg-shift = <2>;
 };
diff --git a/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi b/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
index 094f9557b7..41beaf0900 100644
--- a/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
+++ b/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
@@ -11,10 +11,6 @@
 	};
 };
 
-&mmc1 {
-	cd-inverted;
-};
-
 &uart1 {
 	reg-shift = <2>;
 };
diff --git a/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi b/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
index 094f9557b7..41beaf0900 100644
--- a/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
+++ b/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
@@ -11,10 +11,6 @@
 	};
 };
 
-&mmc1 {
-	cd-inverted;
-};
-
 &uart1 {
 	reg-shift = <2>;
 };
diff --git a/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi b/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
index b09ce0efb5..de411316d8 100644
--- a/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
+++ b/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
@@ -11,10 +11,6 @@
 	};
 };
 
-&mmc1 {
-	cd-inverted;
-};
-
 &uart1 {
 	reg-shift = <2>;
 };
diff --git a/arch/arm/dts/omap3-evm-u-boot.dtsi b/arch/arm/dts/omap3-evm-u-boot.dtsi
index b09ce0efb5..de411316d8 100644
--- a/arch/arm/dts/omap3-evm-u-boot.dtsi
+++ b/arch/arm/dts/omap3-evm-u-boot.dtsi
@@ -11,10 +11,6 @@
 	};
 };
 
-&mmc1 {
-	cd-inverted;
-};
-
 &uart1 {
 	reg-shift = <2>;
 };
-- 
2.17.1

[U-Boot] [PATCH 3/3] ARM: DTS: various omap3: Remove cd-inverted from u-boot.dtsi

[Jean-Jacques Hiblot]

Hi Adam,


On 05/09/2018 11:35, Adam Ford wrote:
> With the omap_mmc driver no longer supporting cd-inverted, this
> patch removes all these references since they are not needed.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/arch/arm/dts/am3517-evm-u-boot.dtsi b/arch/arm/dts/am3517-evm-u-boot.dtsi
> index c02beaad77..59df819f9d 100644
> --- a/arch/arm/dts/am3517-evm-u-boot.dtsi
> +++ b/arch/arm/dts/am3517-evm-u-boot.dtsi
> @@ -10,10 +10,6 @@
>   	};
>   };
>   
> -&mmc1 {
> -	cd-inverted;
> -};
> -
>   &uart1 {
>   	reg-shift = <2>;
>   };
> diff --git a/arch/arm/dts/omap3-beagle-u-boot.dtsi b/arch/arm/dts/omap3-beagle-u-boot.dtsi
> index 094f9557b7..41beaf0900 100644
> --- a/arch/arm/dts/omap3-beagle-u-boot.dtsi
> +++ b/arch/arm/dts/omap3-beagle-u-boot.dtsi
> @@ -11,10 +11,6 @@
>   	};
>   };
>   
> -&mmc1 {
> -	cd-inverted;
> -};
> -
>   &uart1 {
>   	reg-shift = <2>;
>   };
> diff --git a/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi b/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
> index 094f9557b7..41beaf0900 100644
> --- a/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
> +++ b/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
> @@ -11,10 +11,6 @@
>   	};
>   };
>   
> -&mmc1 {
> -	cd-inverted;
> -};
> -
>   &uart1 {
>   	reg-shift = <2>;
>   };
> diff --git a/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi b/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
> index 094f9557b7..41beaf0900 100644
> --- a/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
> +++ b/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
> @@ -11,10 +11,6 @@
>   	};
>   };
>   
> -&mmc1 {
> -	cd-inverted;
> -};
> -
>   &uart1 {
>   	reg-shift = <2>;
>   };
> diff --git a/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi b/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
> index b09ce0efb5..de411316d8 100644
> --- a/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
> +++ b/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
> @@ -11,10 +11,6 @@
>   	};
>   };
>   
> -&mmc1 {
> -	cd-inverted;
> -};
> -
>   &uart1 {
>   	reg-shift = <2>;
>   };
> diff --git a/arch/arm/dts/omap3-evm-u-boot.dtsi b/arch/arm/dts/omap3-evm-u-boot.dtsi
> index b09ce0efb5..de411316d8 100644
> --- a/arch/arm/dts/omap3-evm-u-boot.dtsi
> +++ b/arch/arm/dts/omap3-evm-u-boot.dtsi
> @@ -11,10 +11,6 @@
>   	};
>   };
>   
> -&mmc1 {
> -	cd-inverted;
> -};
> -
>   &uart1 {
>   	reg-shift = <2>;
>   };

Shouldn't  logicpd-torpedo-37xx-devkit-u-boot.dtsi be updated ?

JJ

[U-Boot] [PATCH 3/3] ARM: DTS: various omap3: Remove cd-inverted from u-boot.dtsi

[Adam Ford]

On Wed, Sep 5, 2018 at 7:24 AM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>
> Hi Adam,
>
>
> On 05/09/2018 11:35, Adam Ford wrote:
> > With the omap_mmc driver no longer supporting cd-inverted, this
> > patch removes all these references since they are not needed.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/arch/arm/dts/am3517-evm-u-boot.dtsi b/arch/arm/dts/am3517-evm-u-boot.dtsi
> > index c02beaad77..59df819f9d 100644
> > --- a/arch/arm/dts/am3517-evm-u-boot.dtsi
> > +++ b/arch/arm/dts/am3517-evm-u-boot.dtsi
> > @@ -10,10 +10,6 @@
> >       };
> >   };
> >
> > -&mmc1 {
> > -     cd-inverted;
> > -};
> > -
> >   &uart1 {
> >       reg-shift = <2>;
> >   };
> > diff --git a/arch/arm/dts/omap3-beagle-u-boot.dtsi b/arch/arm/dts/omap3-beagle-u-boot.dtsi
> > index 094f9557b7..41beaf0900 100644
> > --- a/arch/arm/dts/omap3-beagle-u-boot.dtsi
> > +++ b/arch/arm/dts/omap3-beagle-u-boot.dtsi
> > @@ -11,10 +11,6 @@
> >       };
> >   };
> >
> > -&mmc1 {
> > -     cd-inverted;
> > -};
> > -
> >   &uart1 {
> >       reg-shift = <2>;
> >   };
> > diff --git a/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi b/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
> > index 094f9557b7..41beaf0900 100644
> > --- a/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
> > +++ b/arch/arm/dts/omap3-beagle-xm-ab-u-boot.dtsi
> > @@ -11,10 +11,6 @@
> >       };
> >   };
> >
> > -&mmc1 {
> > -     cd-inverted;
> > -};
> > -
> >   &uart1 {
> >       reg-shift = <2>;
> >   };
> > diff --git a/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi b/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
> > index 094f9557b7..41beaf0900 100644
> > --- a/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
> > +++ b/arch/arm/dts/omap3-beagle-xm-u-boot.dtsi
> > @@ -11,10 +11,6 @@
> >       };
> >   };
> >
> > -&mmc1 {
> > -     cd-inverted;
> > -};
> > -
> >   &uart1 {
> >       reg-shift = <2>;
> >   };
> > diff --git a/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi b/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
> > index b09ce0efb5..de411316d8 100644
> > --- a/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
> > +++ b/arch/arm/dts/omap3-evm-37xx-u-boot.dtsi
> > @@ -11,10 +11,6 @@
> >       };
> >   };
> >
> > -&mmc1 {
> > -     cd-inverted;
> > -};
> > -
> >   &uart1 {
> >       reg-shift = <2>;
> >   };
> > diff --git a/arch/arm/dts/omap3-evm-u-boot.dtsi b/arch/arm/dts/omap3-evm-u-boot.dtsi
> > index b09ce0efb5..de411316d8 100644
> > --- a/arch/arm/dts/omap3-evm-u-boot.dtsi
> > +++ b/arch/arm/dts/omap3-evm-u-boot.dtsi
> > @@ -11,10 +11,6 @@
> >       };
> >   };
> >
> > -&mmc1 {
> > -     cd-inverted;
> > -};
> > -
> >   &uart1 {
> >       reg-shift = <2>;
> >   };
>
> Shouldn't  logicpd-torpedo-37xx-devkit-u-boot.dtsi be updated ?

I did that already at:  https://patchwork.ozlabs.org/patch/965450/

adam
>
> JJ
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/3] gpio: omap_gpio: Add xlate function to support GPIO_ACTIVE_LOW

[Jean-Jacques Hiblot]

  Adam,


On 05/09/2018 11:35, Adam Ford wrote:
> The gpio driver doesn't current support knowing whether or not
> GPIO is active low or high.  It simply returns the value.  The
> side effect of this, is that the MMC routines which check
> the status card detect or write protect must use a u-boot specific
> dts modification to enable inverting the pin values when
> GPIO_ACTIVE_LOW is used.  This allows the invert option to be removed
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c
> index 555eba2662..0ecd2f374a 100644
> --- a/drivers/gpio/omap_gpio.c
> +++ b/drivers/gpio/omap_gpio.c
> @@ -24,6 +24,7 @@
>   #include <asm/io.h>
>   #include <linux/errno.h>
>   #include <malloc.h>
> +#include <dt-bindings/gpio/gpio.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> @@ -275,12 +276,25 @@ static int omap_gpio_get_function(struct udevice *dev, unsigned offset)
>   		return GPIOF_INPUT;
>   }
>   
> +static int omap_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
> +			   struct ofnode_phandle_args *args)
> +{
> +	desc->offset = args->args[0];
> +
> +	if (args->args[1] & GPIO_ACTIVE_LOW)
> +		desc->flags = GPIOD_ACTIVE_LOW;
> +	else
> +		desc->flags = 0;
> +	return 0;
> +}
Do we need this ? It looks a lot like the default behaviour (see 
gpio_xlate_offs_flags in gpio-uclass.c).

JJ
> Do we need this ? It looks like the default behaviour
>   static const struct dm_gpio_ops gpio_omap_ops = {
>   	.direction_input	= omap_gpio_direction_input,
>   	.direction_output	= omap_gpio_direction_output,
>   	.get_value		= omap_gpio_get_value,
>   	.set_value		= omap_gpio_set_value,
>   	.get_function		= omap_gpio_get_function,
> +	.xlate                  = omap_gpio_xlate,
>   };
>   
>   static int omap_gpio_probe(struct udevice *dev)
>

[U-Boot] [PATCH 1/3] gpio: omap_gpio: Add xlate function to support GPIO_ACTIVE_LOW

[Adam Ford]

On Wed, Sep 5, 2018 at 7:29 AM Jean-Jacques Hiblot <jjhiblot@ti.com> wrote:
>
>   Adam,
>
>
> On 05/09/2018 11:35, Adam Ford wrote:
> > The gpio driver doesn't current support knowing whether or not
> > GPIO is active low or high.  It simply returns the value.  The
> > side effect of this, is that the MMC routines which check
> > the status card detect or write protect must use a u-boot specific
> > dts modification to enable inverting the pin values when
> > GPIO_ACTIVE_LOW is used.  This allows the invert option to be removed
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c
> > index 555eba2662..0ecd2f374a 100644
> > --- a/drivers/gpio/omap_gpio.c
> > +++ b/drivers/gpio/omap_gpio.c
> > @@ -24,6 +24,7 @@
> >   #include <asm/io.h>
> >   #include <linux/errno.h>
> >   #include <malloc.h>
> > +#include <dt-bindings/gpio/gpio.h>
> >
> >   DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -275,12 +276,25 @@ static int omap_gpio_get_function(struct udevice *dev, unsigned offset)
> >               return GPIOF_INPUT;
> >   }
> >
> > +static int omap_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
> > +                        struct ofnode_phandle_args *args)
> > +{
> > +     desc->offset = args->args[0];
> > +
> > +     if (args->args[1] & GPIO_ACTIVE_LOW)
> > +             desc->flags = GPIOD_ACTIVE_LOW;
> > +     else
> > +             desc->flags = 0;
> > +     return 0;
> > +}
> Do we need this ? It looks a lot like the default behaviour (see
> gpio_xlate_offs_flags in gpio-uclass.c).

I don't know.  I saw that other gpio drivers had the xlate, so it
seemed like it made sense to have it here. I'll revert it, and keep
the other stuff and see what happens along with removing the entry
from the struct.
I don't have the other (non-Logic PD) boards, so I cannot test them.

adam
>
> JJ
> > Do we need this ? It looks like the default behaviour
> >   static const struct dm_gpio_ops gpio_omap_ops = {
> >       .direction_input        = omap_gpio_direction_input,
> >       .direction_output       = omap_gpio_direction_output,
> >       .get_value              = omap_gpio_get_value,
> >       .set_value              = omap_gpio_set_value,
> >       .get_function           = omap_gpio_get_function,
> > +     .xlate                  = omap_gpio_xlate,
> >   };
> >
> >   static int omap_gpio_probe(struct udevice *dev)
> >
>