Skip to contentSkip to conversation

Some pinctrl fixes for pinmux modules

PATCH In review Tony Lindgren 3 participants 17 messages Jan 20, 2012 – Jan 26, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Some pinctrl fixes for pinmux modules

Hi,

Here are few fixes mostly to make it easier to develop loadable pinmux modules.

Regards,

Tony

---

Tony Lindgren (4):
pinctrl: Free debugfs entries when unloading a pinmux driver
pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set
pinctrl: Add checks for empty names in pinmux_search_function
pinctrl: Fix some pinmux typos

 Documentation/pinctrl.txt |    2 +-
 drivers/pinctrl/core.c    |   14 +++++++++++++-
 drivers/pinctrl/core.h    |    3 +++
 drivers/pinctrl/pinmux.c  |   23 ++++++++++++++---------
 4 files changed, 31 insertions(+), 11 deletions(-)
Signature · 1 line
Signature
#1 lore raw

PATCH 1/4 pinctrl: Free debugfs entries when unloading a pinmux driver

Free debugfs entries when unloading a pinmux driver

---
 drivers/pinctrl/core.c |   14 +++++++++++++-
 drivers/pinctrl/core.h |    3 +++
 2 files changed, 16 insertions(+), 1 deletions(-)
drivers/pinctrl/core.c +13 -1
index 569bdb3..d9d35fc 100644
@@ -510,10 +510,12 @@ static struct dentry *debugfs_root;
 
 static void pinctrl_init_device_debugfs(struct pinctrl_dev *pctldev)
 {
-	static struct dentry *device_root;
+	struct dentry *device_root;
 
 	device_root = debugfs_create_dir(dev_name(pctldev->dev),
 					 debugfs_root);
+	pctldev->device_root = device_root;
+
 	if (IS_ERR(device_root) || !device_root) {
 		pr_warn("failed to create debugfs directory for %s\n",
 			dev_name(pctldev->dev));
@@ -529,6 +531,11 @@ static void pinctrl_init_device_debugfs(struct pinctrl_dev *pctldev)
 	pinconf_init_device_debugfs(device_root, pctldev);
 }
 
+static void pinctrl_remove_device_debugfs(struct pinctrl_dev *pctldev)
+{
+	debugfs_remove_recursive(pctldev->device_root);
+}
+
 static void pinctrl_init_debugfs(void)
 {
 	debugfs_root = debugfs_create_dir("pinctrl", NULL);
@@ -553,6 +560,10 @@ static void pinctrl_init_debugfs(void)
 {
 }
 
+static void pinctrl_remove_device_debugfs(struct pinctrl_dev *pctldev)
+{
+}
+
 #endif
 
 /**
@@ -641,6 +652,7 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
 	if (pctldev == NULL)
 		return;
 
+	pinctrl_remove_device_debugfs(pctldev);
 	pinmux_unhog_maps(pctldev);
 	/* TODO: check that no pinmuxes are still active? */
 	mutex_lock(&pinctrldev_list_mutex);
drivers/pinctrl/core.h +3 -0
index 177a331..cfa86da 100644
@@ -41,6 +41,9 @@ struct pinctrl_dev {
 	struct device *dev;
 	struct module *owner;
 	void *driver_data;
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *device_root;
+#endif
 #ifdef CONFIG_PINMUX
 	struct mutex pinmux_hogs_lock;
 	struct list_head pinmux_hogs;
#2 lore raw

Re: [PATCH 1/4] pinctrl: Free debugfs entries when unloading a pinmux driver

Quoted 3 lines · Tony Lindgren

Free debugfs entries when unloading a pinmux driver

Thanks, patch applied.

Yours,
Linus Walleij

#3 lore raw

PATCH 2/4 pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

In reply to Tony Lindgren

The ctrl_dev_name is optional for struct pinmux_map assuming that ctrl_dev is set. Without this patch we can get:

Unable to handle kernel NULL pointer dereference at virtual address 00000000

...
(pinmux_hog_maps+0xa4/0x20c)
(pinctrl_register+0x2a4/0x378)
...

---
 drivers/pinctrl/pinmux.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
drivers/pinctrl/pinmux.c +5 -2
index a76a348..06b8943 100644
@@ -992,9 +992,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
 
 	for (i = 0; i < pinmux_maps_num; i++) {
 		struct pinmux_map const *map = &pinmux_maps[i];
+		int match_found = 0;
 
-		if (((map->ctrl_dev == dev) ||
-		     !strcmp(map->ctrl_dev_name, devname)) &&
+		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
+				match_found = 1;
+
+		if (((map->ctrl_dev == dev) || match_found) &&
 		    map->hog_on_boot) {
 			/* OK time to hog! */
 			ret = pinmux_hog_map(pctldev, map);
#4 lore raw

RE: [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

Tony Lindgren wrote at Friday, January 20, 2012 9:17 AM:

Quoted 13 lines

The ctrl_dev_name is optional for struct pinmux_map assuming that ctrl_dev is set. Without this patch we can get:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
drivers/pinctrl/pinmux.c +1 -0
@@ -992,9 +992,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
 
 	for (i = 0; i < pinmux_maps_num; i++) {
 		struct pinmux_map const *map = &pinmux_maps[i];
+		int match_found = 0;

drivers/pinctrl/pinmux.c int pinmux_hog_maps(struct pinctrl_dev *pctldev) · line 1003

Quoted 10 lines
drivers/pinctrl/pinmux.c +4 -2
 
-		if (((map->ctrl_dev == dev) ||
-		     !strcmp(map->ctrl_dev_name, devname)) &&
+		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
+				match_found = 1;
+
+		if (((map->ctrl_dev == dev) || match_found) &&
 		    map->hog_on_boot) {
 			/* OK time to hog! */
 			ret = pinmux_hog_map(pctldev, map);

Wouldn't it be better if match_found was true for all matching cases, in other words for the new code to be:

		int match_found = 0;

		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
				match_found = 1;
		if (map->ctrl_dev == dev)
				match_found = 1;
		if (match_found && map->hog_on_boot) {
Signature · 3 lines
nvpublic

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m����������?�I�
#5 lore raw

Re: [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

Quoted 28 lines · Stephen Warren

Tony Lindgren wrote at Friday, January 20, 2012 9:17 AM:

Quoted 13 lines

The ctrl_dev_name is optional for struct pinmux_map assuming that ctrl_dev is set. Without this patch we can get:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
drivers/pinctrl/pinmux.c +1 -0
@@ -992,9 +992,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
 
 	for (i = 0; i < pinmux_maps_num; i++) {
 		struct pinmux_map const *map = &pinmux_maps[i];
+		int match_found = 0;

drivers/pinctrl/pinmux.c int pinmux_hog_maps(struct pinctrl_dev *pctldev) · line 1003

Quoted 10 lines
drivers/pinctrl/pinmux.c +4 -2
 
-		if (((map->ctrl_dev == dev) ||
-		     !strcmp(map->ctrl_dev_name, devname)) &&
+		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
+				match_found = 1;
+
+		if (((map->ctrl_dev == dev) || match_found) &&
 		    map->hog_on_boot) {
 			/* OK time to hog! */
 			ret = pinmux_hog_map(pctldev, map);

Wouldn't it be better if match_found was true for all matching cases, in other words for the new code to be:

Quoted 7 lines
		int match_found = 0;

		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
				match_found = 1;
		if (map->ctrl_dev == dev)
				match_found = 1;
		if (match_found && map->hog_on_boot) {

OK will take a look, that would make the code easier to read.

Regards,

Tony

#6 lore raw

Re: [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

Quoted 31 lines · Tony Lindgren
Quoted 30 lines · Stephen Warren

Tony Lindgren wrote at Friday, January 20, 2012 9:17 AM:

Quoted 13 lines

The ctrl_dev_name is optional for struct pinmux_map assuming that ctrl_dev is set. Without this patch we can get:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
drivers/pinctrl/pinmux.c +1 -0
@@ -992,9 +992,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
 
 	for (i = 0; i < pinmux_maps_num; i++) {
 		struct pinmux_map const *map = &pinmux_maps[i];
+		int match_found = 0;

drivers/pinctrl/pinmux.c int pinmux_hog_maps(struct pinctrl_dev *pctldev) · line 1003

Quoted 10 lines
drivers/pinctrl/pinmux.c +4 -2
 
-		if (((map->ctrl_dev == dev) ||
-		     !strcmp(map->ctrl_dev_name, devname)) &&
+		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
+				match_found = 1;
+
+		if (((map->ctrl_dev == dev) || match_found) &&
 		    map->hog_on_boot) {
 			/* OK time to hog! */
 			ret = pinmux_hog_map(pctldev, map);

Wouldn't it be better if match_found was true for all matching cases, in other words for the new code to be:

		int match_found = 0;
Quoted 7 lines
Quoted 5 lines
		if (map->ctrl_dev_name && !strcmp(map->ctrl_dev_name, devname))
				match_found = 1;
		if (map->ctrl_dev == dev)
				match_found = 1;
		if (match_found && map->hog_on_boot) {

OK will take a look, that would make the code easier to read.

Here's this one updated, looks like we can keep it readable even without match_found if we continue the search early based on !map->hog_on_boot.

Regards,

Tony

From: Tony Lindgren <[email protected]>
Date: Fri, 20 Jan 2012 07:43:53 -0800
Subject: [PATCH] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

The ctrl_dev_name is optional for struct pinmux_map assuming that ctrl_dev is set. Without this patch we can get:

Unable to handle kernel NULL pointer dereference at virtual address 00000000

...
(pinmux_hog_maps+0xa4/0x20c)
(pinctrl_register+0x2a4/0x378)
...

Fix this by adding adding a test for map->ctrl_dev. Additionally move the test for map->ctrl_dev earlier to optimize out the loop a bit.

drivers/pinctrl/pinmux.c +6 -3
@@ -993,9 +993,12 @@ int pinmux_hog_maps(struct pinctrl_dev *pctldev)
 	for (i = 0; i < pinmux_maps_num; i++) {
 		struct pinmux_map const *map = &pinmux_maps[i];
 
-		if (((map->ctrl_dev == dev) ||
-		     !strcmp(map->ctrl_dev_name, devname)) &&
-		    map->hog_on_boot) {
+		if (!map->hog_on_boot)
+			continue;
+
+		if ((map->ctrl_dev == dev) ||
+			(map->ctrl_dev_name &&
+				!strcmp(map->ctrl_dev_name, devname))) {
 			/* OK time to hog! */
 			ret = pinmux_hog_map(pctldev, map);
 			if (ret)
#7 lore raw

RE: [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

Tony Lindgren wrote at Tuesday, January 24, 2012 5:24 PM:
...

Quoted 9 lines

The ctrl_dev_name is optional for struct pinmux_map assuming that ctrl_dev is set. Without this patch we can get:

Unable to handle kernel NULL pointer dereference at virtual address 00000000

...
(pinmux_hog_maps+0xa4/0x20c)
(pinctrl_register+0x2a4/0x378)
...

Quoted 5 lines

Fix this by adding adding a test for map->ctrl_dev. Additionally move the test for map->ctrl_dev earlier to optimize out the loop a bit.

Signature · 1 line
nvpublic
#8 lore raw

Re: [PATCH 2/4] pinctrl: Fix pinmux_hog_maps when ctrl_dev_name is not set

In reply to Tony Lindgren
Quoted 5 lines · Tony Lindgren

* Tony Lindgren <[email protected]> [120120 09:06]:

Here's this one updated, looks like we can keep it readable even without match_found if we continue the search early based on !map->hog_on_boot.

Thanks, patch applied with Stephens ACK.

Yours,
Linus Walleij

#9 lore raw

PATCH 3/4 pinctrl: Add checks for empty names in pinmux_search_function

In reply to Tony Lindgren

Otherwise we can get the following when dealing with buggy data in a pinmux driver:

Unable to handle kernel NULL pointer dereference at virtual address 00000000

...
PC is at strcmp+0xc/0x34
LR is at pinmux_get+0x350/0x8f4
...

---
 drivers/pinctrl/pinmux.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
drivers/pinctrl/pinmux.c +7 -0
index 06b8943..ffe633d 100644
@@ -584,6 +584,13 @@ static int pinmux_search_function(struct pinctrl_dev *pctldev,
 							   selector);
 		int ret;
 
+		if (!fname) {
+			pr_warning("no name for function%i\n",
+				   selector);
+			selector++;
+			continue;
+		}
+
 		if (!strcmp(map->function, fname)) {
 			/* Found the function, check pin group */
 			ret = pinmux_check_pin_group(pctldev, selector,
#10 lore raw

RE: [PATCH 3/4] pinctrl: Add checks for empty names in pinmux_search_function

Tony Lindgren wrote at Friday, January 20, 2012 9:18 AM:

Quoted 13 lines

Otherwise we can get the following when dealing with buggy data in a pinmux driver:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
drivers/pinctrl/pinmux.c +0 -0
index 06b8943..ffe633d 100644
@@ -584,6 +584,13 @@ static int pinmux_search_function(struct pinctrl_dev *pctldev,
 							   selector);
 		int ret;
 

drivers/pinctrl/pinmux.c static int pinmux_search_function(struct pinctrl_dev *pctldev, · line 596

Quoted 10 lines
drivers/pinctrl/pinmux.c +7 -0
+		if (!fname) {
+			pr_warning("no name for function%i\n",
+				   selector);
+			selector++;
+			continue;
+		}
+
 		if (!strcmp(map->function, fname)) {
 			/* Found the function, check pin group */
 			ret = pinmux_check_pin_group(pctldev, selector,

Shouldn't this be BUG_ON(!fname)?

There are lots of other places that pmxops->get_function_name() is called. Wouldn't it be better to enhance e.g. pinmux_check_ops() to validate that all functions have a name during pinctrl registration?

Signature · 3 lines
nvpublic

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m����������?�I�
#11 lore raw

Re: [PATCH 3/4] pinctrl: Add checks for empty names in pinmux_search_function

Quoted 27 lines · Stephen Warren

Tony Lindgren wrote at Friday, January 20, 2012 9:18 AM:

Quoted 13 lines

Otherwise we can get the following when dealing with buggy data in a pinmux driver:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
drivers/pinctrl/pinmux.c +0 -0
index 06b8943..ffe633d 100644
@@ -584,6 +584,13 @@ static int pinmux_search_function(struct pinctrl_dev *pctldev,
 							   selector);
 		int ret;
 

drivers/pinctrl/pinmux.c static int pinmux_search_function(struct pinctrl_dev *pctldev, · line 596

Quoted 10 lines
drivers/pinctrl/pinmux.c +7 -0
+		if (!fname) {
+			pr_warning("no name for function%i\n",
+				   selector);
+			selector++;
+			continue;
+		}
+
 		if (!strcmp(map->function, fname)) {
 			/* Found the function, check pin group */
 			ret = pinmux_check_pin_group(pctldev, selector,

Shouldn't this be BUG_ON(!fname)?

Quoted 3 lines

There are lots of other places that pmxops->get_function_name() is called. Wouldn't it be better to enhance e.g. pinmux_check_ops() to validate that all functions have a name during pinctrl registration?

Maybe yeah. I'll take a look next week when I'm back home.

Tony

#12 lore raw

Re: [PATCH 3/4] pinctrl: Add checks for empty names in pinmux_search_function

Quoted 28 lines · Tony Lindgren
Quoted 27 lines · Stephen Warren

Tony Lindgren wrote at Friday, January 20, 2012 9:18 AM:

Quoted 13 lines

Otherwise we can get the following when dealing with buggy data in a pinmux driver:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
drivers/pinctrl/pinmux.c +0 -0
index 06b8943..ffe633d 100644
@@ -584,6 +584,13 @@ static int pinmux_search_function(struct pinctrl_dev *pctldev,
 							   selector);
 		int ret;
 

drivers/pinctrl/pinmux.c static int pinmux_search_function(struct pinctrl_dev *pctldev, · line 596

Quoted 10 lines
drivers/pinctrl/pinmux.c +7 -0
+		if (!fname) {
+			pr_warning("no name for function%i\n",
+				   selector);
+			selector++;
+			continue;
+		}
+
 		if (!strcmp(map->function, fname)) {
 			/* Found the function, check pin group */
 			ret = pinmux_check_pin_group(pctldev, selector,

Shouldn't this be BUG_ON(!fname)?

Quoted 5 lines
Quoted 3 lines

There are lots of other places that pmxops->get_function_name() is called. Wouldn't it be better to enhance e.g. pinmux_check_ops() to validate that all functions have a name during pinctrl registration?

Maybe yeah. I'll take a look next week when I'm back home.

There's a little bit of a init order issue here to add checking for the function names during registration. That's because we don't allocate pctldev until after pinmux_check_ops() in pinctrl_register. This means we can't currently call ops->list_functions until unless we change the init order.

To fix this we need to allocate pctldev earlier, and then change the *_check_ops functions to use pctldev, see below.

Regards,

Tony

From: Tony Lindgren <[email protected]>
Date: Tue, 24 Jan 2012 16:28:08 -0800
Subject: [PATCH] pinctrl: Add checks for empty function names

This is needed as otherwise we can get the following when dealing with buggy data in a pinmux driver for pinmux_search_function:

Unable to handle kernel NULL pointer dereference at virtual address 00000000

...
PC is at strcmp+0xc/0x34
LR is at pinmux_get+0x350/0x8f4
...

As we need pctldev initialized to call ops->list_functions, let's initialize it before check_ops calls and pass the pctldev to the check_ops functions. Do this for both pinmux and pinconf check_ops functions.

drivers/pinctrl/core.c +18 -18
@@ -583,40 +583,40 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
 	if (pctldesc->name == NULL)
 		return NULL;
 
+	pctldev = kzalloc(sizeof(struct pinctrl_dev), GFP_KERNEL);
+	if (pctldev == NULL)
+		return NULL;
+
+	/* Initialize pin control device struct */
+	pctldev->owner = pctldesc->owner;
+	pctldev->desc = pctldesc;
+	pctldev->driver_data = driver_data;
+	INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
+	spin_lock_init(&pctldev->pin_desc_tree_lock);
+	INIT_LIST_HEAD(&pctldev->gpio_ranges);
+	mutex_init(&pctldev->gpio_ranges_lock);
+	pctldev->dev = dev;
+
 	/* If we're implementing pinmuxing, check the ops for sanity */
 	if (pctldesc->pmxops) {
-		ret = pinmux_check_ops(pctldesc->pmxops);
+		ret = pinmux_check_ops(pctldev);
 		if (ret) {
 			pr_err("%s pinmux ops lacks necessary functions\n",
 			       pctldesc->name);
-			return NULL;
+			goto out_err;
 		}
 	}
 
 	/* If we're implementing pinconfig, check the ops for sanity */
 	if (pctldesc->confops) {
-		ret = pinconf_check_ops(pctldesc->confops);
+		ret = pinconf_check_ops(pctldev);
 		if (ret) {
 			pr_err("%s pin config ops lacks necessary functions\n",
 			       pctldesc->name);
-			return NULL;
+			goto out_err;
 		}
 	}
 
-	pctldev = kzalloc(sizeof(struct pinctrl_dev), GFP_KERNEL);
-	if (pctldev == NULL)
-		return NULL;
-
-	/* Initialize pin control device struct */
-	pctldev->owner = pctldesc->owner;
-	pctldev->desc = pctldesc;
-	pctldev->driver_data = driver_data;
-	INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
-	spin_lock_init(&pctldev->pin_desc_tree_lock);
-	INIT_LIST_HEAD(&pctldev->gpio_ranges);
-	mutex_init(&pctldev->gpio_ranges_lock);
-	pctldev->dev = dev;
-
 	/* Register all the pins */
 	pr_debug("try to register %d pins on %s...\n",
 		 pctldesc->npins, pctldesc->name);
drivers/pinctrl/pinconf.c +3 -1
@@ -205,8 +205,10 @@ int pin_config_group_set(const char *dev_name, const char *pin_group,
 }
 EXPORT_SYMBOL(pin_config_group_set);
 
-int pinconf_check_ops(const struct pinconf_ops *ops)
+int pinconf_check_ops(struct pinctrl_dev *pctldev)
 {
+	const struct pinconf_ops *ops = pctldev->desc->confops;
+
 	/* We must be able to read out pin status */
 	if (!ops->pin_config_get && !ops->pin_config_group_get)
 		return -EINVAL;
drivers/pinctrl/pinconf.h +2 -2
@@ -13,7 +13,7 @@
 
 #ifdef CONFIG_PINCONF
 
-int pinconf_check_ops(const struct pinconf_ops *ops);
+int pinconf_check_ops(struct pinctrl_dev *pctldev);
 void pinconf_init_device_debugfs(struct dentry *devroot,
 				 struct pinctrl_dev *pctldev);
 int pin_config_get_for_pin(struct pinctrl_dev *pctldev, unsigned pin,
@@ -23,7 +23,7 @@ int pin_config_set_for_pin(struct pinctrl_dev *pctldev, unsigned pin,
 
 #else
 
-static inline int pinconf_check_ops(const struct pinconf_ops *ops)
+static inline int pinconf_check_ops(struct pinctrl_dev *pctldev)
 {
 	return 0;
 }
drivers/pinctrl/pinmux.c +16 -1
@@ -904,8 +904,11 @@ void pinmux_disable(struct pinmux *pmx)
 }
 EXPORT_SYMBOL_GPL(pinmux_disable);
 
-int pinmux_check_ops(const struct pinmux_ops *ops)
+int pinmux_check_ops(struct pinctrl_dev *pctldev)
 {
+	const struct pinmux_ops *ops = pctldev->desc->pmxops;
+	unsigned selector = 0;
+
 	/* Check that we implement required operations */
 	if (!ops->list_functions ||
 	    !ops->get_function_name ||
@@ -914,6 +917,18 @@ int pinmux_check_ops(const struct pinmux_ops *ops)
 	    !ops->disable)
 		return -EINVAL;
 
+	/* Check that all functions registered have names */
+	while (ops->list_functions(pctldev, selector) >= 0) {
+		const char *fname = ops->get_function_name(pctldev,
+							   selector);
+		if (!fname) {
+			pr_err("pinmux ops has no name for function%u\n",
+				selector);
+			return -EINVAL;
+		}
+		selector++;
+	}
+
 	return 0;
 }
 
drivers/pinctrl/pinmux.h +2 -2
@@ -12,7 +12,7 @@
  */
 #ifdef CONFIG_PINMUX
 
-int pinmux_check_ops(const struct pinmux_ops *ops);
+int pinmux_check_ops(struct pinctrl_dev *pctldev);
 void pinmux_init_device_debugfs(struct dentry *devroot,
 				struct pinctrl_dev *pctldev);
 void pinmux_init_debugfs(struct dentry *subsys_root);
@@ -21,7 +21,7 @@ void pinmux_unhog_maps(struct pinctrl_dev *pctldev);
 
 #else
 
-static inline int pinmux_check_ops(const struct pinmux_ops *ops)
+static inline int pinmux_check_ops(struct pinctrl_dev *pctldev)
 {
 	return 0;
 }
#13 lore raw

Re: [PATCH 3/4] pinctrl: Add checks for empty names in pinmux_search_function

Quoted 9 lines · Tony Lindgren

This is needed as otherwise we can get the following when dealing with buggy data in a pinmux driver for pinmux_search_function:

Unable to handle kernel NULL pointer dereference at virtual address 00000000

...
PC is at strcmp+0xc/0x34
LR is at pinmux_get+0x350/0x8f4
...

Quoted 6 lines

As we need pctldev initialized to call ops->list_functions, let's initialize it before check_ops calls and pass the pctldev to the check_ops functions. Do this for both pinmux and pinconf check_ops functions.

This looks correct to me, so applied it and pushed to -next for testing.

Yours,
Linus Walleij

#14 lore raw

PATCH 4/4 pinctrl: Fix some pinmux typos

In reply to Tony Lindgren

Fix some pinmux typos so implementing pinmux drivers is a bit easier.

---
 Documentation/pinctrl.txt |    2 +-
 drivers/pinctrl/pinmux.c  |    9 ++-------
 2 files changed, 3 insertions(+), 8 deletions(-)
Documentation/pinctrl.txt +1 -1
index 6727b92..4af17a0 100644
@@ -1025,7 +1025,7 @@ it, disables and releases it, and muxes it in on the pins defined by group B:
 
 foo_switch()
 {
-	struct pinmux pmx;
+	struct pinmux *pmx;
 
 	/* Enable on position A */
 	pmx = pinmux_get(&device, "spi0-pos-A");
drivers/pinctrl/pinmux.c +2 -7
index ffe633d..8acf54e 100644
@@ -53,11 +53,6 @@ struct pinmux_group {
  * @dev: the device using this pinmux
  * @usecount: the number of active users of this mux setting, used to keep
  *	track of nested use cases
- * @pins: an array of discrete physical pins used in this mapping, taken
- *	from the global pin enumeration space (copied from pinmux map)
- * @num_pins: the number of pins in this mapping array, i.e. the number of
- *	elements in .pins so we can iterate over that array (copied from
- *	pinmux map)
  * @pctldev: pin control device handling this pinmux
  * @func_selector: the function selector for the pinmux device handling
  *	this pinmux
@@ -411,7 +406,7 @@ int __init pinmux_register_mappings(struct pinmux_map const *maps,
 }
 
 /**
- * acquire_pins() - acquire all the pins for a certain funcion on a pinmux
+ * acquire_pins() - acquire all the pins for a certain function on a pinmux
  * @pctldev: the device to take the pins on
  * @func_selector: the function selector to acquire the pins for
  * @group_selector: the group selector containing the pins to acquire
@@ -458,7 +453,7 @@ static int acquire_pins(struct pinctrl_dev *pctldev,
 
 /**
  * release_pins() - release pins taken by earlier acquirement
- * @pctldev: the device to free the pinx on
+ * @pctldev: the device to free the pins on
  * @group_selector: the group selector containing the pins to free
  */
 static void release_pins(struct pinctrl_dev *pctldev,
#15 lore raw

Re: [PATCH 4/4] pinctrl: Fix some pinmux typos

Quoted 4 lines · Tony Lindgren

Fix some pinmux typos so implementing pinmux drivers is a bit easier.

Thanks for spotting these, patch applied.

Yours,
Linus Walleij

#16 lore raw
In reply to Tony Lindgren
Quoted 2 lines · Tony Lindgren

Here are few fixes mostly to make it easier to develop loadable pinmux modules.

So applied 1/4 and 4/4 waiting for v2 of 2/4 and 3/4 in accordance with Stephens comments.

Yours,
Linus Walleij