[PATCH 0/5] can: sja1000: cleanups and new OF property

[PATCH 0/5] can: sja1000: cleanups and new OF property

[Florian Vaussard]

Hello,

The first part of this series performs serveral small cleanups
(patches 1 to 3).

The second part introduces the 'reg-io-width' binding (already used
by some other drivers) to perform a similar job as what was done
with IORESOURCE_MEM_XXBIT on the sja1000_platform. This is needed
on my system to correctly take into account the aliasing of the
address bus.

All patches were tested on my OMAP3 system with a memory-mapped
SJA1000.

Regards,
Florian

Florian Vaussard (5):
  can: sja1000: remove unused defines
  can: sja1000: convert printk to use netdev API
  can: sja1000: of: use devm_* APIs
  Documentation: devicetree: sja1000: add reg-io-width binding
  can: sja1000: of: add read/write routines for 8, 16 and 32-bit
    register access

 .../devicetree/bindings/net/can/sja1000.txt        |  4 ++
 drivers/net/can/sja1000/sja1000.c                  |  3 +-
 drivers/net/can/sja1000/sja1000_of_platform.c      | 66 ++++++++++++++--------
 3 files changed, 46 insertions(+), 27 deletions(-)

-- 
1.8.1.2

[PATCH 1/5] can: sja1000: remove unused defines

[Florian Vaussard]

Remove unused defines for the OF platform.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000_of_platform.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index 047accd..2f29eb9 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -55,9 +55,6 @@ MODULE_LICENSE("GPL v2");
 
 #define SJA1000_OFP_CAN_CLOCK  (16000000 / 2)
 
-#define SJA1000_OFP_OCR        OCR_TX0_PULLDOWN
-#define SJA1000_OFP_CDR        (CDR_CBP | CDR_CLK_OFF)
-
 static u8 sja1000_ofp_read_reg(const struct sja1000_priv *priv, int reg)
 {
 	return ioread8(priv->reg_base + reg);
-- 
1.8.1.2

[PATCH 2/5] can: sja1000: convert printk to use netdev API

[Florian Vaussard]

Use netdev_* where applicable.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index f17c301..55cce47 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -106,8 +106,7 @@ static int sja1000_probe_chip(struct net_device *dev)
 	struct sja1000_priv *priv = netdev_priv(dev);
 
 	if (priv->reg_base && sja1000_is_absent(priv)) {
-		printk(KERN_INFO "%s: probing @0x%lX failed\n",
-		       DRV_NAME, dev->base_addr);
+		netdev_err(dev, "probing failed\n");
 		return 0;
 	}
 	return -1;
-- 
1.8.1.2

[PATCH 3/5] can: sja1000: of: use devm_* APIs

[Florian Vaussard]

Simplify probe and remove functions by converting most of the resources
to use devm_* APIs.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000_of_platform.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index 2f29eb9..8ebb4af 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -69,18 +69,11 @@ static void sja1000_ofp_write_reg(const struct sja1000_priv *priv,
 static int sja1000_ofp_remove(struct platform_device *ofdev)
 {
 	struct net_device *dev = platform_get_drvdata(ofdev);
-	struct sja1000_priv *priv = netdev_priv(dev);
-	struct device_node *np = ofdev->dev.of_node;
-	struct resource res;
 
 	unregister_sja1000dev(dev);
 	free_sja1000dev(dev);
-	iounmap(priv->reg_base);
 	irq_dispose_mapping(dev->irq);
 
-	of_address_to_resource(np, 0, &res);
-	release_mem_region(res.start, resource_size(&res));
-
 	return 0;
 }
 
@@ -102,23 +95,22 @@ static int sja1000_ofp_probe(struct platform_device *ofdev)
 
 	res_size = resource_size(&res);
 
-	if (!request_mem_region(res.start, res_size, DRV_NAME)) {
+	if (!devm_request_mem_region(&ofdev->dev,
+				     res.start, res_size, DRV_NAME)) {
 		dev_err(&ofdev->dev, "couldn't request %pR\n", &res);
 		return -EBUSY;
 	}
 
-	base = ioremap_nocache(res.start, res_size);
+	base = devm_ioremap_nocache(&ofdev->dev, res.start, res_size);
 	if (!base) {
 		dev_err(&ofdev->dev, "couldn't ioremap %pR\n", &res);
-		err = -ENOMEM;
-		goto exit_release_mem;
+		return -ENOMEM;
 	}
 
 	irq = irq_of_parse_and_map(np, 0);
 	if (irq == 0) {
 		dev_err(&ofdev->dev, "no irq found\n");
-		err = -ENODEV;
-		goto exit_unmap_mem;
+		return -ENODEV;
 	}
 
 	dev = alloc_sja1000dev(0);
@@ -191,10 +183,6 @@ exit_free_sja1000:
 	free_sja1000dev(dev);
 exit_dispose_irq:
 	irq_dispose_mapping(irq);
-exit_unmap_mem:
-	iounmap(base);
-exit_release_mem:
-	release_mem_region(res.start, res_size);
 
 	return err;
 }
-- 
1.8.1.2

[PATCH 4/5] Documentation: devicetree: sja1000: add reg-io-width binding

[Florian Vaussard]

Add the reg-io-width property to describe the width of the memory
accesses.

Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 Documentation/devicetree/bindings/net/can/sja1000.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt
index f2105a4..b4a6d53 100644
--- a/Documentation/devicetree/bindings/net/can/sja1000.txt
+++ b/Documentation/devicetree/bindings/net/can/sja1000.txt
@@ -12,6 +12,10 @@ Required properties:
 
 Optional properties:
 
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+	should be performed on the device.  Valid value is 1, 2 or 4.
+	Default to 1 (8 bits).
+
 - nxp,external-clock-frequency : Frequency of the external oscillator
 	clock in Hz. Note that the internal clock frequency used by the
 	SJA1000 is half of that value. If not specified, a default value
-- 
1.8.1.2

[PATCH 5/5] can: sja1000: of: add read/write routines for 8, 16 and 32-bit register access

[Florian Vaussard]

Add routines for 8, 16 and 32-bit access like in sja1000_platform.c

Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000_of_platform.c | 41 +++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index 8ebb4af..a9a0696 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -55,17 +55,39 @@ MODULE_LICENSE("GPL v2");
 
 #define SJA1000_OFP_CAN_CLOCK  (16000000 / 2)
 
-static u8 sja1000_ofp_read_reg(const struct sja1000_priv *priv, int reg)
+static u8 sja1000_ofp_read_reg8(const struct sja1000_priv *priv, int reg)
 {
 	return ioread8(priv->reg_base + reg);
 }
 
-static void sja1000_ofp_write_reg(const struct sja1000_priv *priv,
-				  int reg, u8 val)
+static void sja1000_ofp_write_reg8(const struct sja1000_priv *priv,
+				   int reg, u8 val)
 {
 	iowrite8(val, priv->reg_base + reg);
 }
 
+static u8 sja1000_ofp_read_reg16(const struct sja1000_priv *priv, int reg)
+{
+	return ioread8(priv->reg_base + reg * 2);
+}
+
+static void sja1000_ofp_write_reg16(const struct sja1000_priv *priv,
+				    int reg, u8 val)
+{
+	iowrite8(val, priv->reg_base + reg * 2);
+}
+
+static u8 sja1000_ofp_read_reg32(const struct sja1000_priv *priv, int reg)
+{
+	return ioread8(priv->reg_base + reg * 4);
+}
+
+static void sja1000_ofp_write_reg32(const struct sja1000_priv *priv,
+				    int reg, u8 val)
+{
+	iowrite8(val, priv->reg_base + reg * 4);
+}
+
 static int sja1000_ofp_remove(struct platform_device *ofdev)
 {
 	struct net_device *dev = platform_get_drvdata(ofdev);
@@ -121,8 +143,17 @@ static int sja1000_ofp_probe(struct platform_device *ofdev)
 
 	priv = netdev_priv(dev);
 
-	priv->read_reg = sja1000_ofp_read_reg;
-	priv->write_reg = sja1000_ofp_write_reg;
+	of_property_read_u32(np, "reg-io-width", &prop);
+	if (prop == 4) {
+		priv->read_reg = sja1000_ofp_read_reg32;
+		priv->write_reg = sja1000_ofp_write_reg32;
+	} else if (prop == 2) {
+		priv->read_reg = sja1000_ofp_read_reg16;
+		priv->write_reg = sja1000_ofp_write_reg16;
+	} else {
+		priv->read_reg = sja1000_ofp_read_reg8;
+		priv->write_reg = sja1000_ofp_write_reg8;
+	}
 
 	err = of_property_read_u32(np, "nxp,external-clock-frequency", &prop);
 	if (!err)
-- 
1.8.1.2

Re: [PATCH 4/5] Documentation: devicetree: sja1000: add reg-io-width binding

[Rob Herring]

On Thu, Jan 30, 2014 at 8:29 AM, Florian Vaussard
<florian.vaussard@epfl.ch> wrote:
> Add the reg-io-width property to describe the width of the memory
> accesses.
>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  Documentation/devicetree/bindings/net/can/sja1000.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt
> index f2105a4..b4a6d53 100644
> --- a/Documentation/devicetree/bindings/net/can/sja1000.txt
> +++ b/Documentation/devicetree/bindings/net/can/sja1000.txt
> @@ -12,6 +12,10 @@ Required properties:
>
>  Optional properties:
>
> +- reg-io-width : Specify the size (in bytes) of the IO accesses that
> +       should be performed on the device.  Valid value is 1, 2 or 4.
> +       Default to 1 (8 bits).
> +
>  - nxp,external-clock-frequency : Frequency of the external oscillator
>         clock in Hz. Note that the internal clock frequency used by the
>         SJA1000 is half of that value. If not specified, a default value
> --
> 1.8.1.2
>

Re: [PATCH 0/5] can: sja1000: cleanups and new OF property

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

Hello Florian,

On 01/30/2014 03:29 PM, Florian Vaussard wrote:
> The first part of this series performs serveral small cleanups
> (patches 1 to 3).

Thanks for your contribution. I like patches 1 and 2.

> The second part introduces the 'reg-io-width' binding (already used
> by some other drivers) to perform a similar job as what was done
> with IORESOURCE_MEM_XXBIT on the sja1000_platform. This is needed
> on my system to correctly take into account the aliasing of the
> address bus.

And I appreciate the improvements for the of_platform driver. However
that driver was written back when it was not possible to have platform
and of bindings in the same driver. So I'd like to see that the
of_platform driver gets merged into the platform driver.

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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH 0/5] can: sja1000: cleanups and new OF property

[Florian Vaussard]

Hello,

On 01/30/2014 04:22 PM, Marc Kleine-Budde wrote:
> Hello Florian,
> 
> On 01/30/2014 03:29 PM, Florian Vaussard wrote:
>> The first part of this series performs serveral small cleanups
>> (patches 1 to 3).
> 
> Thanks for your contribution. I like patches 1 and 2.
> 
>> The second part introduces the 'reg-io-width' binding (already used
>> by some other drivers) to perform a similar job as what was done
>> with IORESOURCE_MEM_XXBIT on the sja1000_platform. This is needed
>> on my system to correctly take into account the aliasing of the
>> address bus.
> 
> And I appreciate the improvements for the of_platform driver. However
> that driver was written back when it was not possible to have platform
> and of bindings in the same driver. So I'd like to see that the
> of_platform driver gets merged into the platform driver.
> 

Fine. Is an incremental patch on top of this series ok for you ?

Regards,

Florian

Re: [PATCH 0/5] can: sja1000: cleanups and new OF property

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

On 01/30/2014 04:27 PM, Florian Vaussard wrote:
> Hello,
> 
> On 01/30/2014 04:22 PM, Marc Kleine-Budde wrote:
>> Hello Florian,
>>
>> On 01/30/2014 03:29 PM, Florian Vaussard wrote:
>>> The first part of this series performs serveral small cleanups
>>> (patches 1 to 3).
>>
>> Thanks for your contribution. I like patches 1 and 2.
>>
>>> The second part introduces the 'reg-io-width' binding (already used
>>> by some other drivers) to perform a similar job as what was done
>>> with IORESOURCE_MEM_XXBIT on the sja1000_platform. This is needed
>>> on my system to correctly take into account the aliasing of the
>>> address bus.
>>
>> And I appreciate the improvements for the of_platform driver. However
>> that driver was written back when it was not possible to have platform
>> and of bindings in the same driver. So I'd like to see that the
>> of_platform driver gets merged into the platform driver.
>>
> 
> Fine. Is an incremental patch on top of this series ok for you ?

I'd rather see patches 1 and 2 you have already posted, then probably a
modernization patch which converts the platform driver to use devm_ and
friends. Then a patch adding the existing of bindings [1]. This patch is
probably quite small if you prepare the driver in the modernization
patch properly. The last patch will add the new reg-io-width property.

Marc

[1] Maybe also deleting the existing of_platform driver, but I'm not sure.
-- 
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH 0/5] can: sja1000: cleanups and new OF property

[Florian Vaussard]

On 01/30/2014 04:36 PM, Marc Kleine-Budde wrote:
> On 01/30/2014 04:27 PM, Florian Vaussard wrote:
>> Hello,
>>
>> On 01/30/2014 04:22 PM, Marc Kleine-Budde wrote:
>>> Hello Florian,
>>>
>>> On 01/30/2014 03:29 PM, Florian Vaussard wrote:
>>>> The first part of this series performs serveral small cleanups
>>>> (patches 1 to 3).
>>>
>>> Thanks for your contribution. I like patches 1 and 2.
>>>
>>>> The second part introduces the 'reg-io-width' binding (already used
>>>> by some other drivers) to perform a similar job as what was done
>>>> with IORESOURCE_MEM_XXBIT on the sja1000_platform. This is needed
>>>> on my system to correctly take into account the aliasing of the
>>>> address bus.
>>>
>>> And I appreciate the improvements for the of_platform driver. However
>>> that driver was written back when it was not possible to have platform
>>> and of bindings in the same driver. So I'd like to see that the
>>> of_platform driver gets merged into the platform driver.
>>>
>>
>> Fine. Is an incremental patch on top of this series ok for you ?
> 
> I'd rather see patches 1 and 2 you have already posted, then probably a
> modernization patch which converts the platform driver to use devm_ and
> friends. Then a patch adding the existing of bindings [1]. This patch is
> probably quite small if you prepare the driver in the modernization
> patch properly. The last patch will add the new reg-io-width property.
> 

Ok, I will do it.

Regards,
Florian

Re: [PATCH 0/5] can: sja1000: cleanups and new OF property

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

On 01/30/2014 05:51 PM, Florian Vaussard wrote:
>>> Fine. Is an incremental patch on top of this series ok for you ?
>>
>> I'd rather see patches 1 and 2 you have already posted, then probably a
>> modernization patch which converts the platform driver to use devm_ and
>> friends. Then a patch adding the existing of bindings [1]. This patch is
>> probably quite small if you prepare the driver in the modernization
>> patch properly. The last patch will add the new reg-io-width property.
>>
> 
> Ok, I will do it.

If you have questions or work in progress patches feel free to ask.

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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]