Re: [PATCH v3] gpio: Use separate bitfield array to indicate GPIO is claimed

2023-08-15 Thread Tom Rini
On Wed, Aug 02, 2023 at 01:26:02AM +0200, Marek Vasut wrote:

> The current gpio-uclass design uses name field in struct gpio_dev_priv as
> an indicator that GPIO is claimed by consumer. This overloads the function
> of name field and does not work well for named pins not configured as GPIO
> pins.
> 
> Introduce separate bitfield array as the claim indicator.
> 
> This unbreaks dual-purpose AF and GPIO operation on STM32MP since commit
> 2c38f7c31806 ("pinctrl: pinctrl_stm32: Populate uc_priv->name[] with pinmux 
> node's name")
> where any pin which has already been configured as AF could no longer be
> claimed as dual-purpose GPIO. This is important for pins like STM32 MMCI
> st,cmd-gpios .
> 
> Signed-off-by: Marek Vasut 
> Reviewed-by: Simon Glass 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3] gpio: Use separate bitfield array to indicate GPIO is claimed

2023-08-02 Thread Simon Glass
On Tue, 1 Aug 2023 at 17:26, Marek Vasut  wrote:
>
> The current gpio-uclass design uses name field in struct gpio_dev_priv as
> an indicator that GPIO is claimed by consumer. This overloads the function
> of name field and does not work well for named pins not configured as GPIO
> pins.
>
> Introduce separate bitfield array as the claim indicator.
>
> This unbreaks dual-purpose AF and GPIO operation on STM32MP since commit
> 2c38f7c31806 ("pinctrl: pinctrl_stm32: Populate uc_priv->name[] with pinmux 
> node's name")
> where any pin which has already been configured as AF could no longer be
> claimed as dual-purpose GPIO. This is important for pins like STM32 MMCI
> st,cmd-gpios .
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Michal Suchanek 
> Cc: Patrice Chotard 
> Cc: Patrick Delaunay 
> Cc: Rasmus Villemoes 
> Cc: Samuel Holland 
> Cc: Simon Glass 
> ---
> V2: Add set/clear helpers
> V3: Define GPIO_ALLOC_BITS, update calloc() invocation accordingly
> ---
>  drivers/gpio/gpio-uclass.c | 64 +++---
>  include/asm-generic/gpio.h |  2 ++
>  2 files changed, 61 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass 

The GPIO tests should already provide coverage for this so I don't
believe this needs any more tests.


[PATCH v3] gpio: Use separate bitfield array to indicate GPIO is claimed

2023-08-01 Thread Marek Vasut
The current gpio-uclass design uses name field in struct gpio_dev_priv as
an indicator that GPIO is claimed by consumer. This overloads the function
of name field and does not work well for named pins not configured as GPIO
pins.

Introduce separate bitfield array as the claim indicator.

This unbreaks dual-purpose AF and GPIO operation on STM32MP since commit
2c38f7c31806 ("pinctrl: pinctrl_stm32: Populate uc_priv->name[] with pinmux 
node's name")
where any pin which has already been configured as AF could no longer be
claimed as dual-purpose GPIO. This is important for pins like STM32 MMCI
st,cmd-gpios .

Signed-off-by: Marek Vasut 
---
Cc: Michal Suchanek 
Cc: Patrice Chotard 
Cc: Patrick Delaunay 
Cc: Rasmus Villemoes 
Cc: Samuel Holland 
Cc: Simon Glass 
---
V2: Add set/clear helpers
V3: Define GPIO_ALLOC_BITS, update calloc() invocation accordingly
---
 drivers/gpio/gpio-uclass.c | 64 +++---
 include/asm-generic/gpio.h |  2 ++
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 712119c3415..3e33a12456b 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -28,6 +28,8 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#define GPIO_ALLOC_BITS32
+
 /**
  * gpio_desc_init() - Initialize the GPIO descriptor
  *
@@ -75,6 +77,46 @@ static int gpio_to_device(unsigned int gpio, struct 
gpio_desc *desc)
return -ENOENT;
 }
 
+/**
+ * gpio_is_claimed() - Test whether GPIO is claimed by consumer
+ *
+ * Test whether GPIO is claimed by consumer already.
+ *
+ * @uc_priv:   gpio_dev_priv pointer.
+ * @offset:gpio offset within the device
+ * @return:true if claimed, false if not claimed
+ */
+static bool gpio_is_claimed(struct gpio_dev_priv *uc_priv, unsigned int offset)
+{
+   return !!(uc_priv->claimed[offset / GPIO_ALLOC_BITS] & BIT(offset % 
GPIO_ALLOC_BITS));
+}
+
+/**
+ * gpio_set_claim() - Set GPIO claimed by consumer
+ *
+ * Set a bit which indicate the GPIO is claimed by consumer
+ *
+ * @uc_priv:   gpio_dev_priv pointer.
+ * @offset:gpio offset within the device
+ */
+static void gpio_set_claim(struct gpio_dev_priv *uc_priv, unsigned int offset)
+{
+   uc_priv->claimed[offset / GPIO_ALLOC_BITS] |= BIT(offset % 
GPIO_ALLOC_BITS);
+}
+
+/**
+ * gpio_clear_claim() - Clear GPIO claimed by consumer
+ *
+ * Clear a bit which indicate the GPIO is claimed by consumer
+ *
+ * @uc_priv:   gpio_dev_priv pointer.
+ * @offset:gpio offset within the device
+ */
+static void gpio_clear_claim(struct gpio_dev_priv *uc_priv, unsigned int 
offset)
+{
+   uc_priv->claimed[offset / GPIO_ALLOC_BITS] &= ~BIT(offset % 
GPIO_ALLOC_BITS);
+}
+
 #if CONFIG_IS_ENABLED(DM_GPIO_LOOKUP_LABEL)
 /**
  * dm_gpio_lookup_label() - look for name in gpio device
@@ -94,7 +136,7 @@ static int dm_gpio_lookup_label(const char *name,
 
*offset = -1;
for (i = 0; i < uc_priv->gpio_count; i++) {
-   if (!uc_priv->name[i])
+   if (!gpio_is_claimed(uc_priv, i))
continue;
if (!strcmp(name, uc_priv->name[i])) {
*offset = i;
@@ -350,7 +392,7 @@ int dm_gpio_request(struct gpio_desc *desc, const char 
*label)
int ret;
 
uc_priv = dev_get_uclass_priv(dev);
-   if (uc_priv->name[desc->offset])
+   if (gpio_is_claimed(uc_priv, desc->offset))
return -EBUSY;
str = strdup(label);
if (!str)
@@ -362,6 +404,8 @@ int dm_gpio_request(struct gpio_desc *desc, const char 
*label)
return ret;
}
}
+
+   gpio_set_claim(uc_priv, desc->offset);
uc_priv->name[desc->offset] = str;
 
return 0;
@@ -438,7 +482,7 @@ int _dm_gpio_free(struct udevice *dev, uint offset)
int ret;
 
uc_priv = dev_get_uclass_priv(dev);
-   if (!uc_priv->name[offset])
+   if (!gpio_is_claimed(uc_priv, offset))
return -ENXIO;
if (ops->rfree) {
ret = ops->rfree(dev, offset);
@@ -446,6 +490,7 @@ int _dm_gpio_free(struct udevice *dev, uint offset)
return ret;
}
 
+   gpio_clear_claim(uc_priv, offset);
free(uc_priv->name[offset]);
uc_priv->name[offset] = NULL;
 
@@ -480,7 +525,7 @@ static int check_reserved(const struct gpio_desc *desc, 
const char *func)
return -ENOENT;
 
uc_priv = dev_get_uclass_priv(desc->dev);
-   if (!uc_priv->name[desc->offset]) {
+   if (!gpio_is_claimed(uc_priv, desc->offset)) {
printf("%s: %s: error: gpio %s%d not reserved\n",
   desc->dev->name, func,
   uc_priv->bank_name ? uc_priv->bank_name : "",
@@ -826,7 +871,7 @@ static int get_function(struct udevice *dev, int offset, 
bool skip_unused,
return -EINVAL;
if (namep)
*namep = uc_priv->name[offset];
-   if (ski