You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "michallenc (via GitHub)" <gi...@apache.org> on 2023/08/02 09:27:13 UTC
[GitHub] [nuttx] michallenc opened a new pull request, #10005: ioexpander: add option to register GPIO by name for IOexpander
michallenc opened a new pull request, #10005:
URL: https://github.com/apache/nuttx/pull/10005
## Summary
Function `gpio_lower_half()` could only use GPIO minor number for pin registration. This adds option to register GPIO pin by name as supported for standard GPIOs without expander.
## Impact
General driver change, could affect BSPs.
## Testing
Tested on SAMv7 with pcf8575 expander
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] acassis commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1281898674
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
@xiaoxiang781216 I think using a single function in this case is fine, it avoids adding a new function to do same thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] michallenc commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1282426712
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
@xiaoxiang781216 I think I agree with the consintency and that it should be under `gpio_lower_half_byname()`. But how about also introducing something like `static gpio_lower_half_internal()` that would basically wrap lines +- 392 to 431 in an internal function. This way we would avoid repeating the same code. Then we would have
```C
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, int minor)
{
some variables init
gpio_lower_half_internal(some args);
ret = gpio_pin_register(gpio, minor);
}
```
and
```C
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, char *name)
{
some variables init
gpio_lower_half_internal(some args);
ret = gpio_pin_register_byname(gpio, name);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1282558919
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
Yes, it looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] michallenc commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1282741624
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1281884368
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
should we add a new api:
```
int gpio_lower_halfby_name(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, int minor);
enum gpio_pintype_e pintype, int minor, char *name);
```
like gpio_pin_register_byname to keep the compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 merged pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #10005:
URL: https://github.com/apache/nuttx/pull/10005
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] davids5 commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "davids5 (via GitHub)" <gi...@apache.org>.
davids5 commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1281907460
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
consistency > simple
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] michallenc commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1282426712
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
@xiaoxiang781216 I think I agree with the consintency and that it should be under `gpio_lower_half_byname()` and not in the same function. But how about also introducing something like `static gpio_lower_half_internal()` that would basically wrap lines +- 392 to 431 in an internal function. This way we would avoid repeating the same code. Then we would have
```C
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, int minor)
{
some variables init
gpio_lower_half_internal(some args);
ret = gpio_pin_register(gpio, minor);
}
```
and
```C
int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, char *name)
{
some variables init
gpio_lower_half_internal(some args);
ret = gpio_pin_register_byname(gpio, name);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1282787991
##########
drivers/ioexpander/gpio_lower_half.c:
##########
@@ -427,9 +415,105 @@ int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
{
gpioerr("ERROR: gplh_setpintype() failed: %d\n", ret);
kmm_free(priv);
- return ret;
+ return NULL;
}
+ return priv;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_lower_half_byname
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * name - gpio device name
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe,
+ unsigned int pin, enum gpio_pintype_e pintype,
+ char *name)
+{
+ FAR struct gplh_dev_s *priv;
+ FAR struct gpio_dev_s *gpio;
+ int ret;
+
+ DEBUGASSERT(name != NULL);
+
+ /* Initialize gplh_dev_s structure and set pin type */
+
+ priv = gpio_lower_half_internal(ioe, pin, pintype);
+ if (priv == NULL)
+ {
+ gpioerr("ERROR: gpio_lower_half_internal() failed: %d\n", ret);
+ }
+
+ gpio = &priv->gpio;
+ gpio->gp_ops = &g_gplh_ops;
+
+ /* Register GPIO device by name */
+
+ ret = gpio_pin_register_byname(gpio, name);
+ if (ret < 0)
+ {
+ gpioerr("ERROR: gpio_pin_register_byname() failed: %d\n", ret);
+ kmm_free(priv);
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: gpio_lower_half
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * minor - The minor device number to use when registering the device,
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
+ enum gpio_pintype_e pintype, int minor)
+{
+ FAR struct gplh_dev_s *priv;
+ FAR struct gpio_dev_s *gpio;
+ int ret;
+
+ /* Initialize gplh_dev_s structure and set pin type */
+
+ priv = gpio_lower_half_internal(ioe, pin, pintype);
+ if (priv == NULL)
+ {
+ gpioerr("ERROR: gpio_lower_half_internal() failed: %d\n", ret);
+ }
+
+ gpio = &priv->gpio;
Review Comment:
```suggestion
gpio = &priv->gpio;
gpio->gp_ops = &g_gplh_ops;
```
##########
drivers/ioexpander/gpio_lower_half.c:
##########
@@ -427,9 +415,105 @@ int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
{
gpioerr("ERROR: gplh_setpintype() failed: %d\n", ret);
kmm_free(priv);
- return ret;
+ return NULL;
}
+ return priv;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_lower_half_byname
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * name - gpio device name
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe,
+ unsigned int pin, enum gpio_pintype_e pintype,
+ char *name)
Review Comment:
```suggestion
FAR char *name)
```
##########
drivers/ioexpander/gpio_lower_half.c:
##########
@@ -364,30 +364,18 @@ static int gplh_setpintype(FAR struct gpio_dev_s *gpio,
}
/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: gpio_lower_half
+ * Name: gpio_lower_half_internal
*
* Description:
- * Create a GPIO pin device driver instance for an I/O expander pin.
- * The I/O expander pin must have already been configured by the caller
- * for the particular pintype.
- *
- * Input Parameters:
- * ioe - An instance of the I/O expander interface
- * pin - The I/O expander pin number for the driver
- * pintype - See enum gpio_pintype_e
- * minor - The minor device number to use when registering the device
- *
- * Returned Value:
- * Zero (OK) on success; a negated errno value on failure.
+ * Internal handler for gpio_lower_half and gpio_lower_half_byname
+ * functions. Initializes gplh_dev_s structure and sets pin type.
*
****************************************************************************/
-int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor)
+static struct gplh_dev_s *gpio_lower_half_internal(
Review Comment:
```suggestion
static FAR struct gplh_dev_s *
gpio_lower_half_internal(FAR struct ioexpander_dev_s *ioe,
unsigned int pin,
enum gpio_pintype_e pintype)
```
##########
drivers/ioexpander/gpio_lower_half.c:
##########
@@ -427,9 +415,105 @@ int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
{
gpioerr("ERROR: gplh_setpintype() failed: %d\n", ret);
kmm_free(priv);
- return ret;
+ return NULL;
}
+ return priv;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_lower_half_byname
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * name - gpio device name
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe,
+ unsigned int pin, enum gpio_pintype_e pintype,
+ char *name)
+{
+ FAR struct gplh_dev_s *priv;
+ FAR struct gpio_dev_s *gpio;
+ int ret;
+
+ DEBUGASSERT(name != NULL);
+
+ /* Initialize gplh_dev_s structure and set pin type */
+
+ priv = gpio_lower_half_internal(ioe, pin, pintype);
+ if (priv == NULL)
+ {
+ gpioerr("ERROR: gpio_lower_half_internal() failed: %d\n", ret);
Review Comment:
```suggestion
return -ENOMEM;
```
##########
drivers/ioexpander/gpio_lower_half.c:
##########
@@ -427,9 +415,105 @@ int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
{
gpioerr("ERROR: gplh_setpintype() failed: %d\n", ret);
kmm_free(priv);
- return ret;
+ return NULL;
}
+ return priv;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_lower_half_byname
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * name - gpio device name
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe,
+ unsigned int pin, enum gpio_pintype_e pintype,
+ char *name)
+{
+ FAR struct gplh_dev_s *priv;
+ FAR struct gpio_dev_s *gpio;
+ int ret;
+
+ DEBUGASSERT(name != NULL);
+
+ /* Initialize gplh_dev_s structure and set pin type */
+
+ priv = gpio_lower_half_internal(ioe, pin, pintype);
+ if (priv == NULL)
+ {
+ gpioerr("ERROR: gpio_lower_half_internal() failed: %d\n", ret);
+ }
+
+ gpio = &priv->gpio;
Review Comment:
```suggestion
gpio = &priv->gpio;
gpio->gp_ops = &g_gplh_ops;
```
##########
drivers/ioexpander/gpio_lower_half.c:
##########
@@ -427,9 +415,105 @@ int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
{
gpioerr("ERROR: gplh_setpintype() failed: %d\n", ret);
kmm_free(priv);
- return ret;
+ return NULL;
}
+ return priv;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_lower_half_byname
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * name - gpio device name
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe,
+ unsigned int pin, enum gpio_pintype_e pintype,
+ char *name)
+{
+ FAR struct gplh_dev_s *priv;
+ FAR struct gpio_dev_s *gpio;
+ int ret;
+
+ DEBUGASSERT(name != NULL);
+
+ /* Initialize gplh_dev_s structure and set pin type */
+
+ priv = gpio_lower_half_internal(ioe, pin, pintype);
+ if (priv == NULL)
+ {
+ gpioerr("ERROR: gpio_lower_half_internal() failed: %d\n", ret);
+ }
+
+ gpio = &priv->gpio;
+ gpio->gp_ops = &g_gplh_ops;
+
+ /* Register GPIO device by name */
+
+ ret = gpio_pin_register_byname(gpio, name);
+ if (ret < 0)
+ {
+ gpioerr("ERROR: gpio_pin_register_byname() failed: %d\n", ret);
+ kmm_free(priv);
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: gpio_lower_half
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * minor - The minor device number to use when registering the device,
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
+ enum gpio_pintype_e pintype, int minor)
+{
+ FAR struct gplh_dev_s *priv;
+ FAR struct gpio_dev_s *gpio;
+ int ret;
+
+ /* Initialize gplh_dev_s structure and set pin type */
+
+ priv = gpio_lower_half_internal(ioe, pin, pintype);
+ if (priv == NULL)
+ {
+ gpioerr("ERROR: gpio_lower_half_internal() failed: %d\n", ret);
Review Comment:
```suggestion
return -ENOMEM;
```
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -271,6 +271,32 @@ int gpio_pin_unregister(FAR struct gpio_dev_s *dev, int minor);
int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
FAR const char *pinname);
+/****************************************************************************
+ * Name: gpio_lower_half_byname
+ *
+ * Description:
+ * Create a GPIO pin device driver instance for an I/O expander pin.
+ * The I/O expander pin must have already been configured by the caller
+ * for the particular pintype.
+ *
+ * Input Parameters:
+ * ioe - An instance of the I/O expander interface
+ * pin - The I/O expander pin number for the driver
+ * pintype - See enum gpio_pintype_e
+ * name - gpio device name
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_GPIO_LOWER_HALF
+struct ioexpander_dev_s;
+int gpio_lower_half_byname(FAR struct ioexpander_dev_s *ioe,
+ unsigned int pin, enum gpio_pintype_e pintype,
+ char *name);
Review Comment:
add FAR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] michallenc commented on a diff in pull request #10005: ioexpander: add option to register GPIO by name for IOexpander
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #10005:
URL: https://github.com/apache/nuttx/pull/10005#discussion_r1282426712
##########
include/nuttx/ioexpander/gpio.h:
##########
@@ -293,7 +295,7 @@ int gpio_pin_unregister_byname(FAR struct gpio_dev_s *dev,
#ifdef CONFIG_GPIO_LOWER_HALF
struct ioexpander_dev_s;
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
- enum gpio_pintype_e pintype, int minor);
+ enum gpio_pintype_e pintype, int minor, char *name);
Review Comment:
@xiaoxiang781216 I think I agree with the consintency and that it should be under `gpio_lower_half_byname()` and not in the same function. But how about also introducing something like `static gpio_lower_half_internal()` that would basically wrap lines +- 392 to 431 in an internal function. This way we would avoid repeating the same code. Then we would have
```C
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, int minor)
{
some variables init
gpio_lower_half_internal(some args);
ret = gpio_pin_register(gpio, minor);
}
```
and
```C
int gpio_lower_half(FAR struct ioexpander_dev_s *ioe, unsigned int pin,
enum gpio_pintype_e pintype, char *name)
{
some variables init
gpio_lower_half_internal(some args);
ret = gpio_pin_register_byname(gpio, name);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org