2014-07-25 06:20:19

by Mugunthan V N

[permalink] [raw]
Subject: [can-next PATCH 1/1] net: can: c_can_platform: Use devm_ioremap instead of devm_ioremap_resource.

From: George Cherian <[email protected]>

The raminit register is shared register for both can0 and can1.
Using devm_ioremap_resource fails the mapping for can1 interface
leading to a non functional can interface.

Signed-off-by: George Cherian <[email protected]>
Signed-off-by: Mugunthan V N <[email protected]>
---
drivers/net/can/c_can/c_can_platform.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index e29b6d0..5dede6e 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -278,7 +278,8 @@ static int c_can_plat_probe(struct platform_device *pdev)
break;
}

- priv->raminit_ctrlreg = devm_ioremap_resource(&pdev->dev, res);
+ priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
if (IS_ERR(priv->raminit_ctrlreg) || priv->instance < 0)
dev_info(&pdev->dev, "control memory is not used for raminit\n");
else
--
2.0.2.673.g9ab0882


2014-07-25 07:38:52

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [can-next PATCH 1/1] net: can: c_can_platform: Use devm_ioremap instead of devm_ioremap_resource.

On 07/25/2014 08:19 AM, Mugunthan V N wrote:
> From: George Cherian <[email protected]>
>
> The raminit register is shared register for both can0 and can1.
> Using devm_ioremap_resource fails the mapping for can1 interface
> leading to a non functional can interface.
>
> Signed-off-by: George Cherian <[email protected]>
> Signed-off-by: Mugunthan V N <[email protected]>

Applied to can, I've adjusted the commit message a bit, mentioning the
patch, that introduced the regression.

Thanks,
Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (242.00 B)
OpenPGP digital signature

2014-07-25 10:56:17

by Wolfram Sang

[permalink] [raw]
Subject: Re: [can-next PATCH 1/1] net: can: c_can_platform: Use devm_ioremap instead of devm_ioremap_resource.

Am 25-07-2014 08:19, schrieb Mugunthan V N:
> From: George Cherian <[email protected]>
>
> The raminit register is shared register for both can0 and can1.
> Using devm_ioremap_resource fails the mapping for can1 interface
> leading to a non functional can interface.
>
> Signed-off-by: George Cherian <[email protected]>
> Signed-off-by: Mugunthan V N <[email protected]>

The patch is a hack, or? It simply works because we drop all sanity
checking on the resource.

I am on holiday and can't really check, yet I believe the AM335x has
devicetree support for accessing single bits in SYSCONF. This is what
needs to be applied here, too, AFAICT.

> ---
> drivers/net/can/c_can/c_can_platform.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/c_can/c_can_platform.c
> b/drivers/net/can/c_can/c_can_platform.c
> index e29b6d0..5dede6e 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -278,7 +278,8 @@ static int c_can_plat_probe(struct platform_device
> *pdev)
> break;
> }
>
> - priv->raminit_ctrlreg = devm_ioremap_resource(&pdev->dev, res);
> + priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start,
> + resource_size(res));
> if (IS_ERR(priv->raminit_ctrlreg) || priv->instance < 0)
> dev_info(&pdev->dev, "control memory is not used for raminit\n");
> else

2014-07-25 13:06:22

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [can-next PATCH 1/1] net: can: c_can_platform: Use devm_ioremap instead of devm_ioremap_resource.

On 07/25/2014 12:49 PM, Wolfram Sang wrote:
> Am 25-07-2014 08:19, schrieb Mugunthan V N:
>> From: George Cherian <[email protected]>
>>
>> The raminit register is shared register for both can0 and can1.
>> Using devm_ioremap_resource fails the mapping for can1 interface
>> leading to a non functional can interface.
>>
>> Signed-off-by: George Cherian <[email protected]>
>> Signed-off-by: Mugunthan V N <[email protected]>
>
> The patch is a hack, or? It simply works because we drop all sanity
> checking on the resource.

Well, thinking about it, yes, a bit.

> I am on holiday and can't really check, yet I believe the AM335x has
> devicetree support for accessing single bits in SYSCONF. This is what
> needs to be applied here, too, AFAICT.

I cannot find any generic driver for the sysconf, USB and Ethernet have
an extra driver for their shared register space. I think the d_can can
access the raminit register via a syscon driver.

Enjoy your holiday :)
Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (242.00 B)
OpenPGP digital signature