RE: [PATCH] pinctrl: devicetree: Fix pctldev pointer overwrite

["A.s. Dong" <aisheng.dong@xxxxxxx>]

> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@xxxxxxxxx]
> Sent: Friday, June 8, 2018 12:52 AM
> To: linus.walleij@xxxxxxxxxx
> Cc: rf@xxxxxxxxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx;
> slongerbeam@xxxxxxxxx; shawnguo@xxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> dl-linux-imx <linux-imx@xxxxxxx>; mika.penttila@xxxxxxxxxxxx; Fabio
> Estevam <fabio.estevam@xxxxxxx>; stable@xxxxxxxxxxxxxxx
> Subject: [PATCH] pinctrl: devicetree: Fix pctldev pointer overwrite
> 
> From: Fabio Estevam <fabio.estevam@xxxxxxx>
> 
> Commit b89405b6102f ("pinctrl: devicetree: Fix dt_to_map_one_config
> handling of hogs") causes the pinctrl hog pins to not get initialized on i.MX
> platforms leaving them with the IOMUX settings untouched.
> 
> This causes several regressions on i.MX such as:
> 
> - OV5640 camera driver can not be probed anymore on imx6qdl-sabresd
> because the camera clock pin is in a pinctrl_hog group and since its pinctrl
> initialization is skipped, the camera clock is kept in GPIO functionality instead
> of CLK_CKO function.
> 
> - Audio stopped working on imx6qdl-wandboard and imx53-qsb for the same
> reason.
> 
> Richard Fitzgerald explains the problem:
> 
> "I see the bug. If the hog node isn't a 1st level child of the pinctrl parent node
> it will go around the for(;;) loop again but on the first pass I overwrite pctldev
> with the result of
> get_pinctrl_dev_from_of_node() so it doesn't point to the pinctrl driver any
> more."
> 
> Fix the issue by stashing the original pctldev so it doesn't get overwritten.
> 
> Fixes:  b89405b6102f ("pinctrl: devicetree: Fix dt_to_map_one_config
> handling of hogs")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Reported-by: Mika Penttilä <mika.penttila@xxxxxxxxxxxx>
> Reported-by: Steve Longerbeam <slongerbeam@xxxxxxxxx>
> Suggested-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxx>

Looks good to me.

Reviewed-by: Dong Aisheng <aisheng.dong@xxxxxxx>

Regards
Dong Aisheng

> ---
>  drivers/pinctrl/devicetree.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index
> b601039..c4aa411 100644
> --- a/drivers/pinctrl/devicetree.c
> +++ b/drivers/pinctrl/devicetree.c
> @@ -101,10 +101,11 @@ struct pinctrl_dev *of_pinctrl_get(struct
> device_node *np)  }
> 
>  static int dt_to_map_one_config(struct pinctrl *p,
> -				struct pinctrl_dev *pctldev,
> +				struct pinctrl_dev *hog_pctldev,
>  				const char *statename,
>  				struct device_node *np_config)
>  {
> +	struct pinctrl_dev *pctldev = NULL;
>  	struct device_node *np_pctldev;
>  	const struct pinctrl_ops *ops;
>  	int ret;
> @@ -123,8 +124,10 @@ static int dt_to_map_one_config(struct pinctrl *p,
>  			return -EPROBE_DEFER;
>  		}
>  		/* If we're creating a hog we can use the passed pctldev */
> -		if (pctldev && (np_pctldev == p->dev->of_node))
> +		if (hog_pctldev && (np_pctldev == p->dev->of_node)) {
> +			pctldev = hog_pctldev;
>  			break;
> +		}
>  		pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
>  		if (pctldev)
>  			break;
> --
> 2.7.4

��.n��������+%������w��{.n�����{��
b���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f