[RFC PATCH 1/1] gpio: Handle NULL pointers gracefully

[Pratyush Yadav]

Prepare the way for a managed GPIO API by handling NULL pointers without
crashing or failing. validate_desc() comes from Linux with the prints
removed to reduce code size.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
Signed-off-by: Pratyush Yadav <p.yadav at ti.com>
---
 drivers/gpio/Kconfig       |  9 ++++
 drivers/gpio/gpio-uclass.c | 86 ++++++++++++++++++++++++++++++++++----
 include/asm-generic/gpio.h |  2 +-
 3 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d87f6cc105..f8b6bcdf44 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -36,6 +36,15 @@ config TPL_DM_GPIO
 	  particular GPIOs that they provide. The uclass interface
 	  is defined in include/asm-generic/gpio.h.

+config GPIO_VALIDATE_DESC
+	bool "Check if GPIO descriptor is NULL and bail out if it is"
+	depends on DM_GPIO
+	default y
+	help
+	  If a GPIO is optional, the GPIO descriptor is NULL. In that
+	  case, calls should bail out instead of causing NULL pointer
+	  access.
+
 config GPIO_HOG
 	bool "Enable GPIO hog support"
 	depends on DM_GPIO
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 9eeab22eef..6b97d3aaff 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -20,6 +20,25 @@

 DECLARE_GLOBAL_DATA_PTR;

+#ifdef CONFIG_GPIO_VALIDATE_DESC
+/*
+ * This descriptor validation needs to be inserted verbatim into each
+ * function taking a descriptor, so we need to use a preprocessor
+ * macro to avoid endless duplication. If the desc is NULL it is an
+ * optional GPIO and calls should just bail out.
+ */
+static inline int validate_desc(const struct gpio_desc *desc)
+{
+	if (!desc)
+		return 0;
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+	if (!desc->dev)
+		return -EINVAL;
+	return 1;
+}
+#endif
+
 /**
  * gpio_desc_init() - Initialize the GPIO descriptor
  *
@@ -303,11 +322,19 @@ int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc)

 int dm_gpio_request(struct gpio_desc *desc, const char *label)
 {
-	struct udevice *dev = desc->dev;
+	struct udevice *dev;
 	struct gpio_dev_priv *uc_priv;
 	char *str;
 	int ret;

+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
+	dev = desc->dev;
+
 	uc_priv = dev_get_uclass_priv(dev);
 	if (uc_priv->name[desc->offset])
 		return -EBUSY;
@@ -434,6 +461,14 @@ static int check_reserved(const struct gpio_desc *desc, const char *func)
 {
 	struct gpio_dev_priv *uc_priv;

+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	int ret;
+
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
 	if (!dm_gpio_is_valid(desc))
 		return -ENOENT;

@@ -510,6 +545,12 @@ int dm_gpio_get_value(const struct gpio_desc *desc)
 {
 	int ret;

+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
 	ret = check_reserved(desc, "get_value");
 	if (ret)
 		return ret;
@@ -521,6 +562,12 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value)
 {
 	int ret;

+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
 	ret = check_reserved(desc, "set_value");
 	if (ret)
 		return ret;
@@ -572,11 +619,21 @@ static int check_dir_flags(ulong flags)

 static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
 {
-	struct udevice *dev = desc->dev;
-	struct dm_gpio_ops *ops = gpio_get_ops(dev);
-	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct udevice *dev;
+	struct dm_gpio_ops *ops;
+	struct gpio_dev_priv *uc_priv;
 	int ret = 0;

+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
+	dev = desc->dev;
+	ops = gpio_get_ops(dev);
+	uc_priv = dev_get_uclass_priv(dev);
+
 	ret = check_dir_flags(flags);
 	if (ret) {
 		dev_dbg(dev,
@@ -1043,6 +1100,14 @@ int gpio_get_list_count(struct udevice *dev, const char *list_name)

 int dm_gpio_free(struct udevice *dev, struct gpio_desc *desc)
 {
+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	int ret;
+
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
 	/* For now, we don't do any checking of dev */
 	return _dm_gpio_free(desc->dev, desc->offset);
 }
@@ -1091,12 +1156,17 @@ static int gpio_renumber(struct udevice *removed_dev)

 int gpio_get_number(const struct gpio_desc *desc)
 {
-	struct udevice *dev = desc->dev;
 	struct gpio_dev_priv *uc_priv;

-	if (!dev)
-		return -1;
-	uc_priv = dev->uclass_priv;
+#ifdef CONFIG_GPIO_VALIDATE_DESC
+	int ret;
+
+	ret = validate_desc(desc);
+	if (ret <= 0)
+		return ret;
+#endif
+
+	uc_priv = dev_get_uclass_priv(desc->dev);

 	return uc_priv->gpio_base + desc->offset;
 }
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index e16c2f31d9..46007b1283 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -149,7 +149,7 @@ struct gpio_desc {
  */
 static inline bool dm_gpio_is_valid(const struct gpio_desc *desc)
 {
-	return desc->dev != NULL;
+	return desc && desc->dev;
 }

 /**
--
2.26.2