[PATCH] pinctrl: samsung: Fix memory mapping code

[PATCH] pinctrl: samsung: Fix memory mapping code

[Andrzej Hajda]

Some pinctrls share memory regions, and devm_ioremap_resource does not
allow to share resources, in opposition to devm_ioremap.
This patch restores back usage of devm_ioremap function, but with proper
error handling and logging.

Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/pinctrl/samsung/pinctrl-samsung.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index f9ddba7..ebecff8 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -988,9 +988,12 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
 
 	for (i = 0; i < ctrl->nr_ext_resources + 1; i++) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		virt_base[i] = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(virt_base[i]))
-			return ERR_CAST(virt_base[i]);
+		virt_base[i] = devm_ioremap(&pdev->dev, res->start,
+						resource_size(res));
+		if (!virt_base[i]) {
+			dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
+			return ERR_PTR(-EIO);
+		}
 	}
 
 	bank = d->pin_banks;
-- 
2.7.4

Re: [PATCH] pinctrl: samsung: Fix memory mapping code

[Marek Szyprowski]

Hi,

On 2017-02-22 08:44, Andrzej Hajda wrote:
> Some pinctrls share memory regions, and devm_ioremap_resource does not
> allow to share resources, in opposition to devm_ioremap.
> This patch restores back usage of devm_ioremap function, but with proper
> error handling and logging.
>
> Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/pinctrl/samsung/pinctrl-samsung.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> index f9ddba7..ebecff8 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -988,9 +988,12 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
>   
>   	for (i = 0; i < ctrl->nr_ext_resources + 1; i++) {
>   		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> -		virt_base[i] = devm_ioremap_resource(&pdev->dev, res);
> -		if (IS_ERR(virt_base[i]))
> -			return ERR_CAST(virt_base[i]);
> +		virt_base[i] = devm_ioremap(&pdev->dev, res->start,
> +						resource_size(res));
> +		if (!virt_base[i]) {
> +			dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
> +			return ERR_PTR(-EIO);
> +		}
>   	}
>   
>   	bank = d->pin_banks;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] pinctrl: samsung: Fix memory mapping code

[Krzysztof Kozlowski]

On Wed, Feb 22, 2017 at 08:44:02AM +0100, Andrzej Hajda wrote:
> Some pinctrls share memory regions, and devm_ioremap_resource does not
> allow to share resources, in opposition to devm_ioremap.
> This patch restores back usage of devm_ioremap function, but with proper
> error handling and logging.
> 
> Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/pinctrl/samsung/pinctrl-samsung.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> index f9ddba7..ebecff8 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -988,9 +988,12 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
>  
>  	for (i = 0; i < ctrl->nr_ext_resources + 1; i++) {
>  		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> -		virt_base[i] = devm_ioremap_resource(&pdev->dev, res);
> -		if (IS_ERR(virt_base[i]))
> -			return ERR_CAST(virt_base[i]);
> +		virt_base[i] = devm_ioremap(&pdev->dev, res->start,
> +						resource_size(res));

A little bit out of scope of this patch: don't we need here a check
for if (!res)? With devm_ioremap_resource it wasn't needed but
platform_get_resource might return NULL and here you are dereferencing
it.

This might be though a separate patch as you try to revert broken
behavior to previous code.


Best regards,
Krzysztof


> +		if (!virt_base[i]) {
> +			dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
> +			return ERR_PTR(-EIO);
> +		}
>  	}
>  
>  	bank = d->pin_banks;
> -- 
> 2.7.4
> 

[PATCH v2] pinctrl: samsung: Fix memory mapping code

[Andrzej Hajda]

Some pinctrls share memory regions, and devm_ioremap_resource does not
allow to share resources, in opposition to devm_ioremap.
This patch restores back usage of devm_ioremap function, but with proper
error handling and logging.

Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2:
- added platform_get_resource error check (thanks to Krzysztof)

 drivers/pinctrl/samsung/pinctrl-samsung.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index f9ddba7..d7aa22c 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -988,9 +988,16 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
 
 	for (i = 0; i < ctrl->nr_ext_resources + 1; i++) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		virt_base[i] = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(virt_base[i]))
-			return ERR_CAST(virt_base[i]);
+		if (!res) {
+			dev_err(&pdev->dev, "failed to get mem%d resource\n", i);
+			return ERR_PTR(-EINVAL);
+		}
+		virt_base[i] = devm_ioremap(&pdev->dev, res->start,
+						resource_size(res));
+		if (!virt_base[i]) {
+			dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
+			return ERR_PTR(-EIO);
+		}
 	}
 
 	bank = d->pin_banks;
-- 
2.7.4

Re: [PATCH v2] pinctrl: samsung: Fix memory mapping code

[Krzysztof Kozlowski]

On Thu, Feb 23, 2017 at 12:12 PM, Andrzej Hajda <a.hajda@samsung.com> wrote:
> Some pinctrls share memory regions, and devm_ioremap_resource does not
> allow to share resources, in opposition to devm_ioremap.
> This patch restores back usage of devm_ioremap function, but with proper
> error handling and logging.
>
> Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v2:
> - added platform_get_resource error check (thanks to Krzysztof)
>
>  drivers/pinctrl/samsung/pinctrl-samsung.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)

Thanks for update. Looks good:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2] pinctrl: samsung: Fix memory mapping code

[Linus Walleij]

On Thu, Feb 23, 2017 at 11:12 AM, Andrzej Hajda <a.hajda@samsung.com> wrote:

> Some pinctrls share memory regions, and devm_ioremap_resource does not
> allow to share resources, in opposition to devm_ioremap.
> This patch restores back usage of devm_ioremap function, but with proper
> error handling and logging.
>
> Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v2:
> - added platform_get_resource error check (thanks to Krzysztof)

This conflicts with the revert that Marek urged me to do for the
fixes branch.

Please check the resulting tree on my fixes branch and rebase on
that.

Yours,
Linus Walleij

Re: [PATCH v2] pinctrl: samsung: Fix memory mapping code

[Andrzej Hajda]

Hi Linus,

On 14.03.2017 14:35, Linus Walleij wrote:
> On Thu, Feb 23, 2017 at 11:12 AM, Andrzej Hajda <a.hajda@samsung.com> wrote:
>
>> Some pinctrls share memory regions, and devm_ioremap_resource does not
>> allow to share resources, in opposition to devm_ioremap.
>> This patch restores back usage of devm_ioremap function, but with proper
>> error handling and logging.
>>
>> Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> v2:
>> - added platform_get_resource error check (thanks to Krzysztof)
> This conflicts with the revert that Marek urged me to do for the
> fixes branch.
>
> Please check the resulting tree on my fixes branch and rebase on
> that.

I do not see your revert on fixes branch of [1].
The most recent patch is:
96e1ce8 pinctrl: uniphier: change pin names of aio/xirq for LD11

And on this branch my patch applies cleanly :)


Regards
Andrzej


[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git

>
> Yours,
> Linus Walleij
>
>
>

Re: [PATCH v2] pinctrl: samsung: Fix memory mapping code

[Linus Walleij]

On Tue, Mar 14, 2017 at 3:28 PM, Andrzej Hajda <a.hajda@samsung.com> wrote:

>> Please check the resulting tree on my fixes branch and rebase on
>> that.
>
> I do not see your revert on fixes branch of [1].
> The most recent patch is:
> 96e1ce8 pinctrl: uniphier: change pin names of aio/xirq for LD11

Yeah I had not had time to push it out until in the evening.

> And on this branch my patch applies cleanly :)

As per your advice in the other thread I took out the revert
and applied this instead, thanks!

Yours,
Linus Walleij

Re: [PATCH v2] pinctrl: samsung: Fix memory mapping code

[Linus Walleij]

On Thu, Feb 23, 2017 at 11:12 AM, Andrzej Hajda <a.hajda@samsung.com> wrote:

> Some pinctrls share memory regions, and devm_ioremap_resource does not
> allow to share resources, in opposition to devm_ioremap.
> This patch restores back usage of devm_ioremap function, but with proper
> error handling and logging.
>
> Fixes: baafaca ("pinctrl: samsung: Fix return value check in samsung_pinctrl_get_soc_data()")
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v2:
> - added platform_get_resource error check (thanks to Krzysztof)

I removed the revert and applied this instead, adding Marek's tested-by
and Krzysztof's reviewed-by.

Yours,
Linus Walleij