gpiolib: add gpiod_get_array and gpiod_put_array functions
Subsystems (2)
PATCH v3 2/4 gpiolib: add gpiod_get_array and gpiod_put_array functions
Introduce new functions for conveniently obtaining and disposing of an entire array of GPIOs with one function call.
---
Change log:
v3: - rebase on current linux-gpio devel branch
- fix ACPI GPIO counting
- allow for zero-sized arrays
- make the flags argument mandatory for the new functions
- clarify documentation
v2: change interface
Documentation/gpio/consumer.txt | 33 +++++- drivers/gpio/gpiolib.c | 231 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 46 ++++++++ 3 files changed, 307 insertions(+), 3 deletions(-)
Documentation/gpio/consumer.txt +30 -3
@@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function: - struct gpio_desc *gpiod_get_optional(struct device *dev, const char *con_id, enum gpiod_flags flags)
@@ -68,6 +67,27 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: unsigned int index, enum gpiod_flags flags) +For a function using multiple GPIOs all of those can be obtained with one call: + + struct gpio_descs *gpiod_get_array(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + +This function returns a struct gpio_descs which contains an array of +descriptors: + + struct gpio_descs { + unsigned int array_size; + struct gpio_desc *desc_array[]; + } + +The following function returns NULL instead of -ENOENT if no GPIOs have been +assigned to the requested function: + + struct gpio_descs *gpiod_get_array_optional(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + Device-managed variants of these functions are also defined: struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id,
@@ -91,8 +111,15 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: void gpiod_put(struct gpio_desc *desc) -It is strictly forbidden to use a descriptor after calling this function. The -device-managed variant is, unsurprisingly: +For an array of GPIOs this function can be used: + + void gpiod_put_array(struct gpio_descs *descs) + +It is strictly forbidden to use a descriptor after calling these functions. +It is also not allowed to individually release descriptors (using gpiod_put()) +from an array acquired with gpiod_get_array(). + +The device-managed variant is, unsurprisingly: void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
drivers/gpio/gpiolib.c +231 -0
249 lines not on this page · raw message
include/linux/gpio/consumer.h +46 -0
83 lines not on this page · raw message
Signature · 1 line
2.0.5
Quoted 37 lines · Rojhalat Ibrahim
Introduce new functions for conveniently obtaining and disposing of an entire array of GPIOs with one function call.
Suggested-by Alexandre Courbot <[email protected]>Signed-off-by Rojhalat Ibrahim <[email protected]>---
Change log:
v3: - rebase on current linux-gpio devel branch
- fix ACPI GPIO counting
- allow for zero-sized arrays
- make the flags argument mandatory for the new functions
- clarify documentation
v2: change interfaceDocumentation/gpio/consumer.txt | 33 +++++- drivers/gpio/gpiolib.c | 231 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 46 ++++++++ 3 files changed, 307 insertions(+), 3 deletions(-)Documentation/gpio/consumer.txt +2
-1@@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function: - struct gpio_desc *gpiod_get_optional(struct device *dev, const char *con_id, enum gpiod_flags flags)@@ -68,6 +67,27 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: unsigned int index, enum gpiod_flags flags) +For a function using multiple GPIOs all of those can be obtained with one call: +
Documentation/gpio/consumer.txt instead of -ENOENT if no GPIO has been assigned to the requested function: · line 81
Quoted 10 lines
Documentation/gpio/consumer.txt +10
-0+ struct gpio_descs *gpiod_get_array(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + +This function returns a struct gpio_descs which contains an array of +descriptors: + + struct gpio_descs { + unsigned int array_size; + struct gpio_desc *desc_array[];
I missed this in the previous revision, but how about having shorter names for these members? Like
array_size -> ndescsdesc_array -> desc
These would be more comfortable to work with I believe.
Quoted 127 lines
Documentation/gpio/consumer.txt +18
-2+ } + +The following function returns NULL instead of -ENOENT if no GPIOs have been +assigned to the requested function: + + struct gpio_descs *gpiod_get_array_optional(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + Device-managed variants of these functions are also defined: struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id,@@ -91,8 +111,15 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: void gpiod_put(struct gpio_desc *desc) -It is strictly forbidden to use a descriptor after calling this function. The -device-managed variant is, unsurprisingly: +For an array of GPIOs this function can be used: + + void gpiod_put_array(struct gpio_descs *descs) + +It is strictly forbidden to use a descriptor after calling these functions. +It is also not allowed to individually release descriptors (using gpiod_put()) +from an array acquired with gpiod_get_array(). + +The device-managed variant is, unsurprisingly: void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)drivers/gpio/gpiolib.c +89
-0@@ -1804,6 +1804,158 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,return desc; } +static int dt_gpio_count (struct device *dev,const char *con_id) +{ +int ret; +char propname[32 ]; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]); +else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =of_gpio_named_count (dev->of_node, propname); +if (ret >=0 ) +break ; + } +return ret; +} + +#ifdef CONFIG_ACPI + +static unsigned int acpi_gpio_package_count (const union acpi_object *obj) +{ +const union acpi_object *element = obj->package.elements; +const union acpi_object *end = element + obj->package.count; +unsigned int count =0 ; + +while (element < end) { +if (element->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count++; + + element++; + } +return count; +} + +static int acpi_find_gpio_count (struct acpi_resource *ares,void *data) +{ +unsigned int *count = data; + +if (ares->type ==ACPI_RESOURCE_TYPE_GPIO ) + *count += ares->data.gpio.pin_table_length; + +return 1 ; +} + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +struct acpi_device *adev =ACPI_COMPANION (dev); +const union acpi_object *obj; +const struct acpi_gpio_mapping *gm; +int count = -ENOENT ; +int ret; +char propname[32 ]; +unsigned int i; + +/* Try first from _DSD */ +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id &&strcmp (con_id,"gpios" )) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]); +else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =acpi_dev_get_property (adev, propname,ACPI_TYPE_ANY , + &obj); +if (ret ==0 ) { +if (obj->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count =1 ; +else if (obj->type ==ACPI_TYPE_PACKAGE ) + count =acpi_gpio_package_count (obj); + }else if (adev->driver_gpios) { +for (gm = adev->driver_gpios; gm->name; gm++) +if (strcmp (propname, gm->name) ==0 ) { + count = gm->size; +break ; + } + } +if (count >=0 ) +break ; + } + +/* Then from plain _CRS GPIOs */ +if (count <0 ) { +struct list_head resource_list; +unsigned int crs_count =0 ;
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1905
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+ +INIT_LIST_HEAD (&resource_list); +acpi_dev_get_resources (adev, &resource_list, + acpi_find_gpio_count, &crs_count); +acpi_dev_free_resource_list (&resource_list); +if (crs_count >0 ) + count = crs_count; + } +return count; +}
Mika, does the ACPI part look good to you?
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1916
Quoted 11 lines
drivers/gpio/gpiolib.c +11
-0+ +#else + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +return -ENODEV ; +} + +#endif + +static int lut_gpio_count (struct device *dev,const char *con_id)
I did not reply to your previous comment, but I'd prefer if this was named platform_gpio_count, for clarity.
These are my last nits - after they are fixed, please feel free to add my
to the next revision.
[+Rafael]
Quoted 5 lines · Rojhalat Ibrahim
Introduce new functions for conveniently obtaining and disposing of an entire array of GPIOs with one function call.
Suggested-by Alexandre Courbot <[email protected]>Signed-off-by Rojhalat Ibrahim <[email protected]>
Can you please next time CC me the whole series? Now it fails with:
drivers/gpio/gpiolib.c: In function ‘dt_gpio_count’:drivers/gpio/gpiolib.c:1812:29: error: ‘gpio_suffixes’ undeclared (first use in this function)
for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
^include/linux/kernel.h:54:33: note: in definition of macro ‘ARRAY_SIZE’
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +__must_be_array(arr))
and so on.
Quoted 121 lines
---
Change log:
v3: - rebase on current linux-gpio devel branch
- fix ACPI GPIO counting
- allow for zero-sized arrays
- make the flags argument mandatory for the new functions
- clarify documentation
v2: change interfaceDocumentation/gpio/consumer.txt | 33 +++++- drivers/gpio/gpiolib.c | 231 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 46 ++++++++ 3 files changed, 307 insertions(+), 3 deletions(-)Documentation/gpio/consumer.txt +30
-3@@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function: - struct gpio_desc *gpiod_get_optional(struct device *dev, const char *con_id, enum gpiod_flags flags)@@ -68,6 +67,27 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: unsigned int index, enum gpiod_flags flags) +For a function using multiple GPIOs all of those can be obtained with one call: + + struct gpio_descs *gpiod_get_array(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + +This function returns a struct gpio_descs which contains an array of +descriptors: + + struct gpio_descs { + unsigned int array_size; + struct gpio_desc *desc_array[]; + } + +The following function returns NULL instead of -ENOENT if no GPIOs have been +assigned to the requested function: + + struct gpio_descs *gpiod_get_array_optional(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + Device-managed variants of these functions are also defined: struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id,@@ -91,8 +111,15 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: void gpiod_put(struct gpio_desc *desc) -It is strictly forbidden to use a descriptor after calling this function. The -device-managed variant is, unsurprisingly: +For an array of GPIOs this function can be used: + + void gpiod_put_array(struct gpio_descs *descs) + +It is strictly forbidden to use a descriptor after calling these functions. +It is also not allowed to individually release descriptors (using gpiod_put()) +from an array acquired with gpiod_get_array(). + +The device-managed variant is, unsurprisingly: void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)drivers/gpio/gpiolib.c +41
-0@@ -1804,6 +1804,158 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,return desc; } +static int dt_gpio_count (struct device *dev,const char *con_id) +{ +int ret; +char propname[32 ]; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]); +else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =of_gpio_named_count (dev->of_node, propname); +if (ret >=0 ) +break ; + } +return ret; +} + +#ifdef CONFIG_ACPI + +static unsigned int acpi_gpio_package_count (const union acpi_object *obj) +{ +const union acpi_object *element = obj->package.elements; +const union acpi_object *end = element + obj->package.count; +unsigned int count =0 ; + +while (element < end) { +if (element->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count++; + + element++; + } +return count; +} + +static int acpi_find_gpio_count (struct acpi_resource *ares,void *data) +{ +unsigned int *count = data;
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1857
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+ +if (ares->type ==ACPI_RESOURCE_TYPE_GPIO ) + *count += ares->data.gpio.pin_table_length; + +return 1 ; +} + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +struct acpi_device *adev =ACPI_COMPANION (dev);
Should you check that the ACPI companion device actually exists here or is it checked somewhere before already?
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1869
Quoted 12 lines
drivers/gpio/gpiolib.c +12
-0+const union acpi_object *obj; +const struct acpi_gpio_mapping *gm; +int count = -ENOENT ; +int ret; +char propname[32 ]; +unsigned int i; + +/* Try first from _DSD */ +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id &&strcmp (con_id,"gpios" )) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]);
I'm assuming some previous patch added gpio_suffixes[].
Quoted 26 lines
drivers/gpio/gpiolib.c +26
-0+else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =acpi_dev_get_property (adev, propname,ACPI_TYPE_ANY , + &obj); +if (ret ==0 ) { +if (obj->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count =1 ; +else if (obj->type ==ACPI_TYPE_PACKAGE ) + count =acpi_gpio_package_count (obj); + }else if (adev->driver_gpios) { +for (gm = adev->driver_gpios; gm->name; gm++) +if (strcmp (propname, gm->name) ==0 ) { + count = gm->size; +break ; + } + } +if (count >=0 ) +break ; + } + +/* Then from plain _CRS GPIOs */ +if (count <0 ) { +struct list_head resource_list; +unsigned int crs_count =0 ;
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1905
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+ +INIT_LIST_HEAD (&resource_list); +acpi_dev_get_resources (adev, &resource_list, + acpi_find_gpio_count, &crs_count); +acpi_dev_free_resource_list (&resource_list); +if (crs_count >0 ) + count = crs_count; + } +return count; +}
Otherwise the ACPI parts look good to me. I still want to test this, though.
Quoted 35 lines
drivers/gpio/gpiolib.c +35
-0+ +#else + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +return -ENODEV ; +} + +#endif + +static int lut_gpio_count (struct device *dev,const char *con_id) +{ +struct gpiod_lookup_table *table; +struct gpiod_lookup *p; +unsigned int count =0 ; + + table =gpiod_find_lookup_table (dev); +if (!table) +return -ENOENT ; + +for (p = &table->table[0 ]; p->chip_label; p++) { +if ((con_id && p->con_id && !strcmp (con_id, p->con_id)) || + (!con_id && !p->con_id)) + count++; + } +if (!count) +return -ENOENT ; + +return count; +} + +/** +* gpiod_count - return the number of GPIOs associated with a device / function +* or -ENOENT if no GPIO has been assigned to the requested function +* @dev: GPIO consumer, can be NULL for system-global GPIOs
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1950
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+* @con_id: function within the GPIO consumer +*/ +int gpiod_count (struct device *dev,const char *con_id) +{ +int count = -ENOENT ; + +if (IS_ENABLED (CONFIG_OF ) && dev && dev->of_node) + count =dt_gpio_count (dev, con_id); +else if (IS_ENABLED (CONFIG_ACPI ) && dev &&ACPI_HANDLE (dev)) + count =acpi_gpio_count (dev, con_id);
Aha, so you here you checked for the ACPI compation, all right then.
Quoted 30 lines
drivers/gpio/gpiolib.c +23
-0+ +if (count <0 ) + count =lut_gpio_count (dev, con_id); + +return count; +} +EXPORT_SYMBOL_GPL (gpiod_count); +/** * gpiod_get - obtain a GPIO for a given GPIO function * @dev: GPIO consumer, can be NULL for system-global GPIOs @@ -2005,6 +2157,70 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev,EXPORT_SYMBOL_GPL (__gpiod_get_index_optional);/** +* gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function +* @dev: GPIO consumer, can be NULL for system-global GPIOs +* @con_id: function within the GPIO consumer +* @flags: optional GPIO initialization flags +* +* This function acquires all the GPIOs defined under a given function. +* +* Return a struct gpio_descs containing an array of descriptors, -ENOENT if +* no GPIO has been assigned to the requested function, or another IS_ERR() +* code if an error occurred while trying to acquire the GPIOs. +*/ +struct gpio_descs *__must_check gpiod_get_array (struct device *dev, +const char *con_id, +enum gpiod_flags flags) +{
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2184
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+struct gpio_desc *desc; +struct gpio_descs *descs; +int count; + + count =gpiod_count (dev, con_id); +if (count <0 ) +return ERR_PTR (count); + + descs =kzalloc (sizeof (*descs) +sizeof (descs->desc_array[0 ]) * count, +GFP_KERNEL );
Should you return ERR_PTR(-ENOMEM) here if the allocation fails?
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2186
Quoted 2 lines
drivers/gpio/gpiolib.c +2
-0+ +for (descs->array_size =0 ; descs->array_size < count; ) {
^
|
+-- extra whitespacedrivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2190
Quoted 4 lines
drivers/gpio/gpiolib.c +4
-0+ desc =gpiod_get_index (dev, con_id, descs->array_size, flags); +if (IS_ERR (desc)) { +gpiod_put_array (descs); +return ERR_PTR (PTR_ERR (desc));
Uh, why not
return desc
?
Quoted 17 lines
drivers/gpio/gpiolib.c +17
-0+ } + descs->desc_array[descs->array_size] = desc; + descs->array_size++; + } +return descs; +} +EXPORT_SYMBOL_GPL (gpiod_get_array); + +/** +* gpiod_get_array_optional - obtain multiple GPIOs from a multi-index GPIO +* function +* @dev: GPIO consumer, can be NULL for system-global GPIOs +* @con_id: function within the GPIO consumer +* @flags: optional GPIO initialization flags +* +* This is equivalent to gpiod_get_array(), except that when no GPIO was +* assigned to the requested function it will return NULL.
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2217
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+*/ +struct gpio_descs *__must_check gpiod_get_array_optional (struct device *dev, +const char *con_id, +enum gpiod_flags flags) +{ +struct gpio_descs *descs; + + descs =gpiod_get_array (dev, con_id, flags); +if (IS_ERR (descs) && (PTR_ERR (descs) == -ENOENT )) +return NULL ;
The caller still needs to handle other error codes (than -ENOENT), so would it be better to just have only gpiod_get_array()?
Quoted 89 lines
drivers/gpio/gpiolib.c +21
-0+ +return descs; +} +EXPORT_SYMBOL_GPL (gpiod_get_array_optional); + +/** * gpiod_put - dispose of a GPIO descriptor * @desc: GPIO descriptor to dispose of * @@ -2016,6 +2232,21 @@ void gpiod_put(struct gpio_desc *desc) }EXPORT_SYMBOL_GPL (gpiod_put); +/** +* gpiod_put_array - dispose of multiple GPIO descriptors +* @descs: struct gpio_descs containing an array of descriptors +*/ +void gpiod_put_array (struct gpio_descs *descs) +{ +unsigned int i; + +for (i =0 ; i < descs->array_size; i++) +gpiod_put (descs->desc_array[i]); + +kfree (descs); +} +EXPORT_SYMBOL_GPL (gpiod_put_array); +#ifdef CONFIG_DEBUG_FS static void gpiolib_dbg_show (struct seq_file *s,struct gpio_chip *chip)include/linux/gpio/consumer.h +24
-0@@ -16,6 +16,15 @@ struct device;*/ struct gpio_desc ; +/** +* Struct containing an array of descriptors that can be obtained using +* gpiod_get_array(). +*/ +struct gpio_descs { +unsigned int array_size; +struct gpio_desc *desc_array[]; +}; +#define GPIOD_FLAGS_BIT_DIR_SET BIT(0)#define GPIOD_FLAGS_BIT_DIR_OUT BIT(1)#define GPIOD_FLAGS_BIT_DIR_VAL BIT(2)@@ -34,6 +43,9 @@ enum gpiod_flags {#ifdef CONFIG_GPIOLIB +/* Return the number of GPIOs associated with a device / function */ +int gpiod_count (struct device *dev,const char *con_id); +/* Acquire and dispose GPIOs */ struct gpio_desc *__must_check __gpiod_get(struct device *dev, const char *con_id,@@ -49,7 +61,14 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev,const char *con_id, unsigned int index, enum gpiod_flags flags); +struct gpio_descs *__must_check gpiod_get_array (struct device *dev, +const char *con_id, +enum gpiod_flags flags); +struct gpio_descs *__must_check gpiod_get_array_optional (struct device *dev, +const char *con_id, +enum gpiod_flags flags);void gpiod_put (struct gpio_desc *desc); +void gpiod_put_array (struct gpio_descs *descs); struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id,@@ -113,6 +132,11 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, struct fwnode_handle *child);#else /* CONFIG_GPIOLIB */ +static inline int gpiod_count (struct device *dev,const char *con_id) +{ +return 0 ; +} +static inline struct gpio_desc *__must_check __gpiod_get (struct device *dev,const char *con_id,enum gpiod_flags flags)
include/linux/gpio/consumer.h __gpiod_get_index_optional(struct device *dev, const char *con_id, · line 173
Quoted 8 lines
include/linux/gpio/consumer.h +5
-0@@ -142,6 +166,20 @@ __gpiod_get_index_optional(struct device *dev, const char *con_id,return ERR_PTR (-ENOSYS ); } +static inline struct gpio_descs *__must_check +gpiod_get_array (struct device *dev,const char *con_id, +enum gpiod_flags flags) +{ +return ERR_PTR (-ENOSYS );
That should be -ENXIO.
include/linux/gpio/consumer.h __gpiod_get_index_optional(struct device *dev, const char *con_id, · line 180
Quoted 7 lines
include/linux/gpio/consumer.h +7
-0+} + +static inline struct gpio_descs *__must_check +gpiod_get_array_optional (struct device *dev,const char *con_id, +enum gpiod_flags flags) +{ +return ERR_PTR (-ENOSYS );
Ditto.
Quoted 23 lines
include/linux/gpio/consumer.h +10
-0+} +static inline void gpiod_put (struct gpio_desc *desc) {might_sleep ();@@ -150,6 +188,14 @@ static inline void gpiod_put(struct gpio_desc *desc)WARN_ON (1 ); } +static inline void gpiod_put_array (struct gpio_descs *descs) +{ +might_sleep (); + +/* GPIO can never have been requested */ +WARN_ON (1 ); +} +static inline struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id,Signature · 1 line
2.0.5
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 11 lines · On Tuesday 10 February 2015 12:40:34 Mika Westerberg wrote:
[+Rafael]
Quoted 5 lines · Rojhalat Ibrahim
Introduce new functions for conveniently obtaining and disposing of an entire array of GPIOs with one function call.
Suggested-by Alexandre Courbot <[email protected]>Signed-off-by Rojhalat Ibrahim <[email protected]>Can you please next time CC me the whole series? Now it fails with:
Sure. Sorry about that.
Quoted 192 lines
drivers/gpio/gpiolib.c: In function ‘dt_gpio_count’:drivers/gpio/gpiolib.c:1812:29: error: ‘gpio_suffixes’ undeclared (first use in this function)for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { ^
include/linux/kernel.h:54:33: note: in definition of macro ‘ARRAY_SIZE’
#defineARRAY_SIZE(arr)(sizeof(arr) / sizeof((arr)[0]) +__must_be_array(arr))and so on.
Quoted 121 lines
---
Change log:
v3: - rebase on current linux-gpio devel branch
- fix ACPI GPIO counting
- allow for zero-sized arrays
- make the flags argument mandatory for the new functions
- clarify documentation
v2: change interfaceDocumentation/gpio/consumer.txt | 33 +++++- drivers/gpio/gpiolib.c | 231 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 46 ++++++++ 3 files changed, 307 insertions(+), 3 deletions(-)Documentation/gpio/consumer.txt +30
-3@@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function: - struct gpio_desc *gpiod_get_optional(struct device *dev, const char *con_id, enum gpiod_flags flags)@@ -68,6 +67,27 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: unsigned int index, enum gpiod_flags flags) +For a function using multiple GPIOs all of those can be obtained with one call: + + struct gpio_descs *gpiod_get_array(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + +This function returns a struct gpio_descs which contains an array of +descriptors: + + struct gpio_descs { + unsigned int array_size; + struct gpio_desc *desc_array[]; + } + +The following function returns NULL instead of -ENOENT if no GPIOs have been +assigned to the requested function: + + struct gpio_descs *gpiod_get_array_optional(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + Device-managed variants of these functions are also defined: struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id,@@ -91,8 +111,15 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: void gpiod_put(struct gpio_desc *desc) -It is strictly forbidden to use a descriptor after calling this function. The -device-managed variant is, unsurprisingly: +For an array of GPIOs this function can be used: + + void gpiod_put_array(struct gpio_descs *descs) + +It is strictly forbidden to use a descriptor after calling these functions. +It is also not allowed to individually release descriptors (using gpiod_put()) +from an array acquired with gpiod_get_array(). + +The device-managed variant is, unsurprisingly: void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)drivers/gpio/gpiolib.c +41
-0@@ -1804,6 +1804,158 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,return desc; } +static int dt_gpio_count (struct device *dev,const char *con_id) +{ +int ret; +char propname[32 ]; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]); +else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =of_gpio_named_count (dev->of_node, propname); +if (ret >=0 ) +break ; + } +return ret; +} + +#ifdef CONFIG_ACPI + +static unsigned int acpi_gpio_package_count (const union acpi_object *obj) +{ +const union acpi_object *element = obj->package.elements; +const union acpi_object *end = element + obj->package.count; +unsigned int count =0 ; + +while (element < end) { +if (element->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count++; + + element++; + } +return count; +} + +static int acpi_find_gpio_count (struct acpi_resource *ares,void *data) +{ +unsigned int *count = data;drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1857
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+ +if (ares->type ==ACPI_RESOURCE_TYPE_GPIO ) + *count += ares->data.gpio.pin_table_length; + +return 1 ; +} + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +struct acpi_device *adev =ACPI_COMPANION (dev);Should you check that the ACPI companion device actually exists here or is it checked somewhere before already?
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1869
Quoted 12 lines
drivers/gpio/gpiolib.c +12
-0+const union acpi_object *obj; +const struct acpi_gpio_mapping *gm; +int count = -ENOENT ; +int ret; +char propname[32 ]; +unsigned int i; + +/* Try first from _DSD */ +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id &&strcmp (con_id,"gpios" )) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]);I'm assuming some previous patch added gpio_suffixes[].
Quoted 31 lines
drivers/gpio/gpiolib.c +31
-0+else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =acpi_dev_get_property (adev, propname,ACPI_TYPE_ANY , + &obj); +if (ret ==0 ) { +if (obj->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count =1 ; +else if (obj->type ==ACPI_TYPE_PACKAGE ) + count =acpi_gpio_package_count (obj); + }else if (adev->driver_gpios) { +for (gm = adev->driver_gpios; gm->name; gm++) +if (strcmp (propname, gm->name) ==0 ) { + count = gm->size; +break ; + } + } +if (count >=0 ) +break ; + } + +/* Then from plain _CRS GPIOs */ +if (count <0 ) { +struct list_head resource_list; +unsigned int crs_count =0 ; + +INIT_LIST_HEAD (&resource_list); +acpi_dev_get_resources (adev, &resource_list, + acpi_find_gpio_count, &crs_count); +acpi_dev_free_resource_list (&resource_list);
Quoted 9 lines
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1905
Quoted 5 lines
drivers/gpio/gpiolib.c +5
-0+if (crs_count >0 ) + count = crs_count; + } +return count; +}Otherwise the ACPI parts look good to me. I still want to test this, though.
Great.
Quoted 82 lines
Quoted 35 lines
drivers/gpio/gpiolib.c +35
-0+ +#else + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +return -ENODEV ; +} + +#endif + +static int lut_gpio_count (struct device *dev,const char *con_id) +{ +struct gpiod_lookup_table *table; +struct gpiod_lookup *p; +unsigned int count =0 ; + + table =gpiod_find_lookup_table (dev); +if (!table) +return -ENOENT ; + +for (p = &table->table[0 ]; p->chip_label; p++) { +if ((con_id && p->con_id && !strcmp (con_id, p->con_id)) || + (!con_id && !p->con_id)) + count++; + } +if (!count) +return -ENOENT ; + +return count; +} + +/** +* gpiod_count - return the number of GPIOs associated with a device / function +* or -ENOENT if no GPIO has been assigned to the requested function +* @dev: GPIO consumer, can be NULL for system-global GPIOs drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1950
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+* @con_id: function within the GPIO consumer +*/ +int gpiod_count (struct device *dev,const char *con_id) +{ +int count = -ENOENT ; + +if (IS_ENABLED (CONFIG_OF ) && dev && dev->of_node) + count =dt_gpio_count (dev, con_id); +else if (IS_ENABLED (CONFIG_ACPI ) && dev &&ACPI_HANDLE (dev)) + count =acpi_gpio_count (dev, con_id);Aha, so you here you checked for the ACPI compation, all right then.
Quoted 34 lines
drivers/gpio/gpiolib.c +27
-0+ +if (count <0 ) + count =lut_gpio_count (dev, con_id); + +return count; +} +EXPORT_SYMBOL_GPL (gpiod_count); +/** * gpiod_get - obtain a GPIO for a given GPIO function * @dev: GPIO consumer, can be NULL for system-global GPIOs @@ -2005,6 +2157,70 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev,EXPORT_SYMBOL_GPL (__gpiod_get_index_optional);/** +* gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function +* @dev: GPIO consumer, can be NULL for system-global GPIOs +* @con_id: function within the GPIO consumer +* @flags: optional GPIO initialization flags +* +* This function acquires all the GPIOs defined under a given function. +* +* Return a struct gpio_descs containing an array of descriptors, -ENOENT if +* no GPIO has been assigned to the requested function, or another IS_ERR() +* code if an error occurred while trying to acquire the GPIOs. +*/ +struct gpio_descs *__must_check gpiod_get_array (struct device *dev, +const char *con_id, +enum gpiod_flags flags) +{ +struct gpio_desc *desc; +struct gpio_descs *descs; +int count; +
Quoted 9 lines
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2184
Quoted 6 lines
drivers/gpio/gpiolib.c +6
-0+ count =gpiod_count (dev, con_id); +if (count <0 ) +return ERR_PTR (count); + + descs =kzalloc (sizeof (*descs) +sizeof (descs->desc_array[0 ]) * count, +GFP_KERNEL );Should you return
ERR_PTR(-ENOMEM)here if the allocation fails?
Of course. I'll put it in the next revision.
Quoted 13 lines
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2186
Quoted 2 lines
drivers/gpio/gpiolib.c +2
-0+ +for (descs->array_size =0 ; descs->array_size < count; ) {^ | +-- extra whitespacedrivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2190
Quoted 4 lines
drivers/gpio/gpiolib.c +4
-0+ desc =gpiod_get_index (dev, con_id, descs->array_size, flags); +if (IS_ERR (desc)) { +gpiod_put_array (descs); +return ERR_PTR (PTR_ERR (desc));Uh, why not
Quoted 3 lines
return desc
?
Because of the different pointer type. desc is a pointer to a GPIO descriptor whereas the return type of the function is a pointer to struct gpio_descs.
Quoted 23 lines
Quoted 23 lines
drivers/gpio/gpiolib.c +22
-0+ } + descs->desc_array[descs->array_size] = desc; + descs->array_size++; + } +return descs; +} +EXPORT_SYMBOL_GPL (gpiod_get_array); + +/** +* gpiod_get_array_optional - obtain multiple GPIOs from a multi-index GPIO +* function +* @dev: GPIO consumer, can be NULL for system-global GPIOs +* @con_id: function within the GPIO consumer +* @flags: optional GPIO initialization flags +* +* This is equivalent to gpiod_get_array(), except that when no GPIO was +* assigned to the requested function it will return NULL. +*/ +struct gpio_descs *__must_check gpiod_get_array_optional (struct device *dev, +const char *con_id, +enum gpiod_flags flags) +{
Quoted 9 lines
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2217
Quoted 5 lines
drivers/gpio/gpiolib.c +5
-0+struct gpio_descs *descs; + + descs =gpiod_get_array (dev, con_id, flags); +if (IS_ERR (descs) && (PTR_ERR (descs) == -ENOENT )) +return NULL ;The caller still needs to handle other error codes (than
-ENOENT), so would it be better to just have onlygpiod_get_array()?
This follows the same pattern as gpiod_get_optional() and gpiod_get_index_optional()
From Documentation/gpio/consumer.txt:
This is useful to discriminate between mere
errors and an absence of GPIO for optional GPIO parameters. For the common pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function
Quoted 92 lines
Quoted 92 lines
drivers/gpio/gpiolib.c +21
-0+ +return descs; +} +EXPORT_SYMBOL_GPL (gpiod_get_array_optional); + +/** * gpiod_put - dispose of a GPIO descriptor * @desc: GPIO descriptor to dispose of * @@ -2016,6 +2232,21 @@ void gpiod_put(struct gpio_desc *desc) }EXPORT_SYMBOL_GPL (gpiod_put); +/** +* gpiod_put_array - dispose of multiple GPIO descriptors +* @descs: struct gpio_descs containing an array of descriptors +*/ +void gpiod_put_array (struct gpio_descs *descs) +{ +unsigned int i; + +for (i =0 ; i < descs->array_size; i++) +gpiod_put (descs->desc_array[i]); + +kfree (descs); +} +EXPORT_SYMBOL_GPL (gpiod_put_array); +#ifdef CONFIG_DEBUG_FS static void gpiolib_dbg_show (struct seq_file *s,struct gpio_chip *chip)include/linux/gpio/consumer.h +24
-0@@ -16,6 +16,15 @@ struct device;*/ struct gpio_desc ; +/** +* Struct containing an array of descriptors that can be obtained using +* gpiod_get_array(). +*/ +struct gpio_descs { +unsigned int array_size; +struct gpio_desc *desc_array[]; +}; +#define GPIOD_FLAGS_BIT_DIR_SET BIT(0)#define GPIOD_FLAGS_BIT_DIR_OUT BIT(1)#define GPIOD_FLAGS_BIT_DIR_VAL BIT(2)@@ -34,6 +43,9 @@ enum gpiod_flags {#ifdef CONFIG_GPIOLIB +/* Return the number of GPIOs associated with a device / function */ +int gpiod_count (struct device *dev,const char *con_id); +/* Acquire and dispose GPIOs */ struct gpio_desc *__must_check __gpiod_get(struct device *dev, const char *con_id,@@ -49,7 +61,14 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev,const char *con_id, unsigned int index, enum gpiod_flags flags); +struct gpio_descs *__must_check gpiod_get_array (struct device *dev, +const char *con_id, +enum gpiod_flags flags); +struct gpio_descs *__must_check gpiod_get_array_optional (struct device *dev, +const char *con_id, +enum gpiod_flags flags);void gpiod_put (struct gpio_desc *desc); +void gpiod_put_array (struct gpio_descs *descs); struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id,@@ -113,6 +132,11 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, struct fwnode_handle *child);#else /* CONFIG_GPIOLIB */ +static inline int gpiod_count (struct device *dev,const char *con_id) +{ +return 0 ; +} +static inline struct gpio_desc *__must_check __gpiod_get (struct device *dev,const char *con_id,enum gpiod_flags flags)@@ -142,6 +166,20 @@ __gpiod_get_index_optional(struct device *dev, const char *con_id,return ERR_PTR (-ENOSYS ); }
Quoted 8 lines
include/linux/gpio/consumer.h __gpiod_get_index_optional(struct device *dev, const char *con_id, · line 173
Quoted 5 lines
include/linux/gpio/consumer.h +5
-0+static inline struct gpio_descs *__must_check +gpiod_get_array (struct device *dev,const char *con_id, +enum gpiod_flags flags) +{ +return ERR_PTR (-ENOSYS );That should be
-ENXIO.
Why? All the other functions in this section return -ENOSYS.
According to include/uapi/asm-generic/errno.h ENOSYS means "Function not implemented"
and in include/uapi/asm-generic/errno-base.h it says ENXIO stands for "No such device or address".
If ENXIO is more correct here (and I don't see why), then it should at least be consistent for all functions, right?
Quoted 19 lines
include/linux/gpio/consumer.h __gpiod_get_index_optional(struct device *dev, const char *con_id, · line 180
Quoted 7 lines
include/linux/gpio/consumer.h +7
-0+} + +static inline struct gpio_descs *__must_check +gpiod_get_array_optional (struct device *dev,const char *con_id, +enum gpiod_flags flags) +{ +return ERR_PTR (-ENOSYS );Ditto.
Quoted 9 lines
include/linux/gpio/consumer.h +2
-0+} +static inline void gpiod_put (struct gpio_desc *desc) {might_sleep ();@@ -150,6 +188,14 @@ static inline void gpiod_put(struct gpio_desc *desc)WARN_ON (1 ); }
Quoted 16 lines
include/linux/gpio/consumer.h static inline void gpiod_put(struct gpio_desc *desc) · line 201
Quoted 11 lines
include/linux/gpio/consumer.h +8
-0+static inline void gpiod_put_array (struct gpio_descs *descs) +{ +might_sleep (); + +/* GPIO can never have been requested */ +WARN_ON (1 ); +} +static inline struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id,--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 326 lines · Rojhalat Ibrahim
Quoted 11 lines · On Tuesday 10 February 2015 12:40:34 Mika Westerberg wrote:
[+Rafael]
Quoted 5 lines · Rojhalat Ibrahim
Introduce new functions for conveniently obtaining and disposing of an entire array of GPIOs with one function call.
Suggested-by Alexandre Courbot <[email protected]>Signed-off-by Rojhalat Ibrahim <[email protected]>Can you please next time CC me the whole series? Now it fails with:
Sure. Sorry about that.
Quoted 192 lines
drivers/gpio/gpiolib.c: In function ‘dt_gpio_count’:drivers/gpio/gpiolib.c:1812:29: error: ‘gpio_suffixes’ undeclared (first use in this function)for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) { ^
include/linux/kernel.h:54:33: note: in definition of macro ‘ARRAY_SIZE’
#defineARRAY_SIZE(arr)(sizeof(arr) / sizeof((arr)[0]) +__must_be_array(arr))and so on.
Quoted 121 lines
---
Change log:
v3: - rebase on current linux-gpio devel branch
- fix ACPI GPIO counting
- allow for zero-sized arrays
- make the flags argument mandatory for the new functions
- clarify documentation
v2: change interfaceDocumentation/gpio/consumer.txt | 33 +++++- drivers/gpio/gpiolib.c | 231 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 46 ++++++++ 3 files changed, 307 insertions(+), 3 deletions(-)Documentation/gpio/consumer.txt +30
-3@@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function: - struct gpio_desc *gpiod_get_optional(struct device *dev, const char *con_id, enum gpiod_flags flags)@@ -68,6 +67,27 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: unsigned int index, enum gpiod_flags flags) +For a function using multiple GPIOs all of those can be obtained with one call: + + struct gpio_descs *gpiod_get_array(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + +This function returns a struct gpio_descs which contains an array of +descriptors: + + struct gpio_descs { + unsigned int array_size; + struct gpio_desc *desc_array[]; + } + +The following function returns NULL instead of -ENOENT if no GPIOs have been +assigned to the requested function: + + struct gpio_descs *gpiod_get_array_optional(struct device *dev, + const char *con_id, + enum gpiod_flags flags) + Device-managed variants of these functions are also defined: struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id,@@ -91,8 +111,15 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: void gpiod_put(struct gpio_desc *desc) -It is strictly forbidden to use a descriptor after calling this function. The -device-managed variant is, unsurprisingly: +For an array of GPIOs this function can be used: + + void gpiod_put_array(struct gpio_descs *descs) + +It is strictly forbidden to use a descriptor after calling these functions. +It is also not allowed to individually release descriptors (using gpiod_put()) +from an array acquired with gpiod_get_array(). + +The device-managed variant is, unsurprisingly: void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)drivers/gpio/gpiolib.c +41
-0@@ -1804,6 +1804,158 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,return desc; } +static int dt_gpio_count (struct device *dev,const char *con_id) +{ +int ret; +char propname[32 ]; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]); +else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =of_gpio_named_count (dev->of_node, propname); +if (ret >=0 ) +break ; + } +return ret; +} + +#ifdef CONFIG_ACPI + +static unsigned int acpi_gpio_package_count (const union acpi_object *obj) +{ +const union acpi_object *element = obj->package.elements; +const union acpi_object *end = element + obj->package.count; +unsigned int count =0 ; + +while (element < end) { +if (element->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count++; + + element++; + } +return count; +} + +static int acpi_find_gpio_count (struct acpi_resource *ares,void *data) +{ +unsigned int *count = data;drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1857
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+ +if (ares->type ==ACPI_RESOURCE_TYPE_GPIO ) + *count += ares->data.gpio.pin_table_length; + +return 1 ; +} + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +struct acpi_device *adev =ACPI_COMPANION (dev);Should you check that the ACPI companion device actually exists here or is it checked somewhere before already?
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1869
Quoted 12 lines
drivers/gpio/gpiolib.c +12
-0+const union acpi_object *obj; +const struct acpi_gpio_mapping *gm; +int count = -ENOENT ; +int ret; +char propname[32 ]; +unsigned int i; + +/* Try first from _DSD */ +for (i =0 ; i <ARRAY_SIZE (gpio_suffixes); i++) { +if (con_id &&strcmp (con_id,"gpios" )) +snprintf (propname,sizeof (propname),"%s-%s" , + con_id, gpio_suffixes[i]);I'm assuming some previous patch added gpio_suffixes[].
Quoted 31 lines
drivers/gpio/gpiolib.c +31
-0+else +snprintf (propname,sizeof (propname),"%s" , + gpio_suffixes[i]); + + ret =acpi_dev_get_property (adev, propname,ACPI_TYPE_ANY , + &obj); +if (ret ==0 ) { +if (obj->type ==ACPI_TYPE_LOCAL_REFERENCE ) + count =1 ; +else if (obj->type ==ACPI_TYPE_PACKAGE ) + count =acpi_gpio_package_count (obj); + }else if (adev->driver_gpios) { +for (gm = adev->driver_gpios; gm->name; gm++) +if (strcmp (propname, gm->name) ==0 ) { + count = gm->size; +break ; + } + } +if (count >=0 ) +break ; + } + +/* Then from plain _CRS GPIOs */ +if (count <0 ) { +struct list_head resource_list; +unsigned int crs_count =0 ; + +INIT_LIST_HEAD (&resource_list); +acpi_dev_get_resources (adev, &resource_list, + acpi_find_gpio_count, &crs_count); +acpi_dev_free_resource_list (&resource_list);Quoted 9 lines
drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1905
Quoted 5 lines
drivers/gpio/gpiolib.c +5
-0+if (crs_count >0 ) + count = crs_count; + } +return count; +}Otherwise the ACPI parts look good to me. I still want to test this, though.
Great.
Quoted 82 lines
Quoted 35 lines
drivers/gpio/gpiolib.c +35
-0+ +#else + +static int acpi_gpio_count (struct device *dev,const char *con_id) +{ +return -ENODEV ; +} + +#endif + +static int lut_gpio_count (struct device *dev,const char *con_id) +{ +struct gpiod_lookup_table *table; +struct gpiod_lookup *p; +unsigned int count =0 ; + + table =gpiod_find_lookup_table (dev); +if (!table) +return -ENOENT ; + +for (p = &table->table[0 ]; p->chip_label; p++) { +if ((con_id && p->con_id && !strcmp (con_id, p->con_id)) || + (!con_id && !p->con_id)) + count++; + } +if (!count) +return -ENOENT ; + +return count; +} + +/** +* gpiod_count - return the number of GPIOs associated with a device / function +* or -ENOENT if no GPIO has been assigned to the requested function +* @dev: GPIO consumer, can be NULL for system-global GPIOs drivers/gpio/gpiolib.c static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id, · line 1950
Quoted 10 lines
drivers/gpio/gpiolib.c +10
-0+* @con_id: function within the GPIO consumer +*/ +int gpiod_count (struct device *dev,const char *con_id) +{ +int count = -ENOENT ; + +if (IS_ENABLED (CONFIG_OF ) && dev && dev->of_node) + count =dt_gpio_count (dev, con_id); +else if (IS_ENABLED (CONFIG_ACPI ) && dev &&ACPI_HANDLE (dev)) + count =acpi_gpio_count (dev, con_id);Aha, so you here you checked for the ACPI compation, all right then.
Quoted 34 lines
drivers/gpio/gpiolib.c +27
-0+ +if (count <0 ) + count =lut_gpio_count (dev, con_id); + +return count; +} +EXPORT_SYMBOL_GPL (gpiod_count); +/** * gpiod_get - obtain a GPIO for a given GPIO function * @dev: GPIO consumer, can be NULL for system-global GPIOs @@ -2005,6 +2157,70 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev,EXPORT_SYMBOL_GPL (__gpiod_get_index_optional);/** +* gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function +* @dev: GPIO consumer, can be NULL for system-global GPIOs +* @con_id: function within the GPIO consumer +* @flags: optional GPIO initialization flags +* +* This function acquires all the GPIOs defined under a given function. +* +* Return a struct gpio_descs containing an array of descriptors, -ENOENT if +* no GPIO has been assigned to the requested function, or another IS_ERR() +* code if an error occurred while trying to acquire the GPIOs. +*/ +struct gpio_descs *__must_check gpiod_get_array (struct device *dev, +const char *con_id, +enum gpiod_flags flags) +{ +struct gpio_desc *desc; +struct gpio_descs *descs; +int count; +Quoted 9 lines
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2184
Quoted 6 lines
drivers/gpio/gpiolib.c +6
-0+ count =gpiod_count (dev, con_id); +if (count <0 ) +return ERR_PTR (count); + + descs =kzalloc (sizeof (*descs) +sizeof (descs->desc_array[0 ]) * count, +GFP_KERNEL );Should you return
ERR_PTR(-ENOMEM)here if the allocation fails?Of course. I'll put it in the next revision.
Quoted 13 lines
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2186
Quoted 2 lines
drivers/gpio/gpiolib.c +2
-0+ +for (descs->array_size =0 ; descs->array_size < count; ) {^ | +-- extra whitespacedrivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2190
Quoted 4 lines
drivers/gpio/gpiolib.c +4
-0+ desc =gpiod_get_index (dev, con_id, descs->array_size, flags); +if (IS_ERR (desc)) { +gpiod_put_array (descs); +return ERR_PTR (PTR_ERR (desc));Uh, why not
Quoted 6 lines
Quoted 3 lines
return desc
?
Because of the different pointer type. desc is a pointer to a GPIO descriptor whereas the return type of the function is a pointer to struct
gpio_descs.
Right. I wonder if ERR_CAST() may be used here.
Quoted 37 lines
Quoted 24 lines
Quoted 24 lines
drivers/gpio/gpiolib.c +22
-0+ } + descs->desc_array[descs->array_size] = desc; + descs->array_size++; + } +return descs; +} +EXPORT_SYMBOL_GPL (gpiod_get_array); + +/** +* gpiod_get_array_optional - obtain multiple GPIOs from a multi-index GPIO +* function +* @dev: GPIO consumer, can be NULL for system-global GPIOs +* @con_id: function within the GPIO consumer +* @flags: optional GPIO initialization flags +* +* This is equivalent to gpiod_get_array(), except that when no GPIO was +* assigned to the requested function it will return NULL. +*/ +struct gpio_descs *__must_check gpiod_get_array_optional (struct device *dev, +const char *con_id, +enum gpiod_flags flags) +{Quoted 9 lines
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2217
Quoted 5 lines
drivers/gpio/gpiolib.c +5
-0+struct gpio_descs *descs; + + descs =gpiod_get_array (dev, con_id, flags); +if (IS_ERR (descs) && (PTR_ERR (descs) == -ENOENT )) +return NULL ;The caller still needs to handle other error codes (than
-ENOENT), so would it be better to just have onlygpiod_get_array()?This follows the same pattern as
gpiod_get_optional()andgpiod_get_index_optional()
Quoted 7 lines
Quoted 1 line
From
Documentation/gpio/consumer.txt:This is useful to discriminate between mere
errors and an absence of GPIO for optional GPIO parameters. For the common pattern where a GPIO is optional, thegpiod_get_optional()andgpiod_get_index_optional()functions can be used. These functions return NULL instead of-ENOENTif no GPIO has been assigned to the requested function
Fair enough.
Quoted 101 lines
Quoted 92 lines
Quoted 92 lines
drivers/gpio/gpiolib.c +21
-0+ +return descs; +} +EXPORT_SYMBOL_GPL (gpiod_get_array_optional); + +/** * gpiod_put - dispose of a GPIO descriptor * @desc: GPIO descriptor to dispose of * @@ -2016,6 +2232,21 @@ void gpiod_put(struct gpio_desc *desc) }EXPORT_SYMBOL_GPL (gpiod_put); +/** +* gpiod_put_array - dispose of multiple GPIO descriptors +* @descs: struct gpio_descs containing an array of descriptors +*/ +void gpiod_put_array (struct gpio_descs *descs) +{ +unsigned int i; + +for (i =0 ; i < descs->array_size; i++) +gpiod_put (descs->desc_array[i]); + +kfree (descs); +} +EXPORT_SYMBOL_GPL (gpiod_put_array); +#ifdef CONFIG_DEBUG_FS static void gpiolib_dbg_show (struct seq_file *s,struct gpio_chip *chip)include/linux/gpio/consumer.h +24
-0@@ -16,6 +16,15 @@ struct device;*/ struct gpio_desc ; +/** +* Struct containing an array of descriptors that can be obtained using +* gpiod_get_array(). +*/ +struct gpio_descs { +unsigned int array_size; +struct gpio_desc *desc_array[]; +}; +#define GPIOD_FLAGS_BIT_DIR_SET BIT(0)#define GPIOD_FLAGS_BIT_DIR_OUT BIT(1)#define GPIOD_FLAGS_BIT_DIR_VAL BIT(2)@@ -34,6 +43,9 @@ enum gpiod_flags {#ifdef CONFIG_GPIOLIB +/* Return the number of GPIOs associated with a device / function */ +int gpiod_count (struct device *dev,const char *con_id); +/* Acquire and dispose GPIOs */ struct gpio_desc *__must_check __gpiod_get(struct device *dev, const char *con_id,@@ -49,7 +61,14 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev,const char *con_id, unsigned int index, enum gpiod_flags flags); +struct gpio_descs *__must_check gpiod_get_array (struct device *dev, +const char *con_id, +enum gpiod_flags flags); +struct gpio_descs *__must_check gpiod_get_array_optional (struct device *dev, +const char *con_id, +enum gpiod_flags flags);void gpiod_put (struct gpio_desc *desc); +void gpiod_put_array (struct gpio_descs *descs); struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id,@@ -113,6 +132,11 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, struct fwnode_handle *child);#else /* CONFIG_GPIOLIB */ +static inline int gpiod_count (struct device *dev,const char *con_id) +{ +return 0 ; +} +static inline struct gpio_desc *__must_check __gpiod_get (struct device *dev,const char *con_id,enum gpiod_flags flags)@@ -142,6 +166,20 @@ __gpiod_get_index_optional(struct device *dev, const char *con_id,return ERR_PTR (-ENOSYS ); }Quoted 8 lines
include/linux/gpio/consumer.h __gpiod_get_index_optional(struct device *dev, const char *con_id, · line 173
Quoted 5 lines
include/linux/gpio/consumer.h +5
-0+static inline struct gpio_descs *__must_check +gpiod_get_array (struct device *dev,const char *con_id, +enum gpiod_flags flags) +{ +return ERR_PTR (-ENOSYS );That should be
-ENXIO.
Quoted 1 line
Why? All the other functions in this section return
-ENOSYS.
It means system call not implemented and should only be returned from those.
Quoted 6 lines
According to
include/uapi/asm-generic/errno.hENOSYS means "Function not implemented"
and ininclude/uapi/asm-generic/errno-base.hit says ENXIO stands for "No such device or address".If ENXIO is more correct here (and I don't see why),
IIRC it was decided (perhaps in last Kernel Summit) that in these cases we return -ENXIO, not -ENOSYS. But...
Quoted 1 line
then it should at least be consistent for all functions, right?
...you are right here. It should be consistent accross the file.
Quoted 19 lines
Quoted 19 lines
include/linux/gpio/consumer.h __gpiod_get_index_optional(struct device *dev, const char *con_id, · line 180
Quoted 7 lines
include/linux/gpio/consumer.h +7
-0+} + +static inline struct gpio_descs *__must_check +gpiod_get_array_optional (struct device *dev,const char *con_id, +enum gpiod_flags flags) +{ +return ERR_PTR (-ENOSYS );Ditto.
Quoted 9 lines
include/linux/gpio/consumer.h +2
-0+} +static inline void gpiod_put (struct gpio_desc *desc) {might_sleep ();@@ -150,6 +188,14 @@ static inline void gpiod_put(struct gpio_desc *desc)WARN_ON (1 ); }
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
include/linux/gpio/consumer.h +7
-0+static inline void gpiod_put_array (struct gpio_descs *descs) +{ +might_sleep (); + +/* GPIO can never have been requested */ +WARN_ON (1 ); +}Quoted 9 lines
include/linux/gpio/consumer.h static inline void gpiod_put(struct gpio_desc *desc) · line 201
Quoted 4 lines
include/linux/gpio/consumer.h +1
-0+static inline struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id,--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 16 lines · On Tuesday 10 February 2015 14:41:18 Mika Westerberg wrote:
Quoted 15 lines · Rojhalat Ibrahim
Quoted 13 lines · On Tuesday 10 February 2015 12:40:34 Mika Westerberg wrote:
drivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2186
Quoted 2 lines
drivers/gpio/gpiolib.c +2
-0+ +for (descs->array_size =0 ; descs->array_size < count; ) {^ | +-- extra whitespacedrivers/gpio/gpiolib.c struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, · line 2190
Quoted 4 lines
drivers/gpio/gpiolib.c +4
-0+ desc =gpiod_get_index (dev, con_id, descs->array_size, flags); +if (IS_ERR (desc)) { +gpiod_put_array (descs); +return ERR_PTR (PTR_ERR (desc));Uh, why not
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
return desc
?
Because of the different pointer type. desc is a pointer to a GPIO descriptor whereas the return type of the function is a pointer to struct
gpio_descs.Right. I wonder if
ERR_CAST()may be used here.
Didn't know about that one. I'll use it in the next revision.
Thanks for looking at this.