| Message ID | 20180228155306.1006-1-rf@opensource.cirrus.com |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: devicetree: Fix dt_to_map_one_config handling of hogs | expand |
On Wed, Feb 28, 2018 at 4:53 PM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > When dt_to_map_one_config() is called with a pinctrl_dev passed > in, it should only be using this if the node being looked up > is a hog. The code was always using the passed pinctrl_dev > without checking whether the dt node referred to it. > > A pin controller can have pinctrl-n dependencies on other pin > controllers in these cases: > > - the pin controller hardware is external, for example I2C, so > needs other pin controller(s) to be setup to communicate with > the hardware device. > > - it is a child of a composite MFD so its of_node is shared with > the parent MFD and other children of that MFD. Any part of that > MFD could have dependencies on other pin controllers. > > Because of this, dt_to_map_one_config() can't assume that if it > has a pinctrl_dev passed in then the node it looks up must be > a hog. It could be a reference to some other pin controller. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Seems correct. Also the code is a little more clear after the patch. Patch applied. It'd be nice to see some testing of this, I hope linux-next is a good testing ground. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 02, 2018 at 09:42:54AM +0100, Linus Walleij wrote: > On Wed, Feb 28, 2018 at 4:53 PM, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: > > > When dt_to_map_one_config() is called with a pinctrl_dev passed > > in, it should only be using this if the node being looked up > > is a hog. The code was always using the passed pinctrl_dev > > without checking whether the dt node referred to it. > > > > A pin controller can have pinctrl-n dependencies on other pin > > controllers in these cases: > > > > - the pin controller hardware is external, for example I2C, so > > needs other pin controller(s) to be setup to communicate with > > the hardware device. > > > > - it is a child of a composite MFD so its of_node is shared with > > the parent MFD and other children of that MFD. Any part of that > > MFD could have dependencies on other pin controllers. > > > > Because of this, dt_to_map_one_config() can't assume that if it > > has a pinctrl_dev passed in then the node it looks up must be > > a hog. It could be a reference to some other pin controller. > > > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > > Seems correct. Also the code is a little more clear > after the patch. > > Patch applied. > > It'd be nice to see some testing of this, I hope linux-next is a good > testing ground. > Once we are happy on the testing front, how would you feel about getting this forwarded to 4.14 stable as well? The trouble is for our customers on 4.14 this bug would cause us to have to change the layout of the device tree bindings. We end up having to make a subnode for the pinctrl and move the hogs into there, keeping the main pin settings on the MFD device itself. And we know from the Madera series that such a DT arrangement is not acceptable upstream. Meaning without this patch on 4.14 our customers are forced to have DT bindings that won't be compatible with a mainline kernel. Thanks, Charles -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 7, 2018 at 5:12 PM, Charles Keepax <ckeepax@opensource.cirrus.com> wrote: > On Fri, Mar 02, 2018 at 09:42:54AM +0100, Linus Walleij wrote: >> On Wed, Feb 28, 2018 at 4:53 PM, Richard Fitzgerald >> <rf@opensource.cirrus.com> wrote: >> >> > When dt_to_map_one_config() is called with a pinctrl_dev passed >> > in, it should only be using this if the node being looked up >> > is a hog. The code was always using the passed pinctrl_dev >> > without checking whether the dt node referred to it. >> > >> > A pin controller can have pinctrl-n dependencies on other pin >> > controllers in these cases: >> > >> > - the pin controller hardware is external, for example I2C, so >> > needs other pin controller(s) to be setup to communicate with >> > the hardware device. >> > >> > - it is a child of a composite MFD so its of_node is shared with >> > the parent MFD and other children of that MFD. Any part of that >> > MFD could have dependencies on other pin controllers. >> > >> > Because of this, dt_to_map_one_config() can't assume that if it >> > has a pinctrl_dev passed in then the node it looks up must be >> > a hog. It could be a reference to some other pin controller. >> > >> > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> >> >> Seems correct. Also the code is a little more clear >> after the patch. >> >> Patch applied. >> >> It'd be nice to see some testing of this, I hope linux-next is a good >> testing ground. >> > > Once we are happy on the testing front, how would you feel about > getting this forwarded to 4.14 stable as well? Sadly too late for me to tag as stable, but you can suggest to Greg to pick it up for stable individually after v4.17-rc1. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index 1ff6c3573493..b601039d6c69 100644 --- a/drivers/pinctrl/devicetree.c +++ b/drivers/pinctrl/devicetree.c @@ -122,8 +122,10 @@ static int dt_to_map_one_config(struct pinctrl *p, /* OK let's just assume this will appear later then */ return -EPROBE_DEFER; } - if (!pctldev) - pctldev = get_pinctrl_dev_from_of_node(np_pctldev); + /* If we're creating a hog we can use the passed pctldev */ + if (pctldev && (np_pctldev == p->dev->of_node)) + break; + pctldev = get_pinctrl_dev_from_of_node(np_pctldev); if (pctldev) break; /* Do not defer probing of hogs (circular loop) */
When dt_to_map_one_config() is called with a pinctrl_dev passed in, it should only be using this if the node being looked up is a hog. The code was always using the passed pinctrl_dev without checking whether the dt node referred to it. A pin controller can have pinctrl-n dependencies on other pin controllers in these cases: - the pin controller hardware is external, for example I2C, so needs other pin controller(s) to be setup to communicate with the hardware device. - it is a child of a composite MFD so its of_node is shared with the parent MFD and other children of that MFD. Any part of that MFD could have dependencies on other pin controllers. Because of this, dt_to_map_one_config() can't assume that if it has a pinctrl_dev passed in then the node it looks up must be a hog. It could be a reference to some other pin controller. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- drivers/pinctrl/devicetree.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)