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
Remove unused defines for the OF platform.
Signed-off-by: Florian Vaussard <[email protected]>
---
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
Add the reg-io-width property to describe the width of the memory
accesses.
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: [email protected]
Signed-off-by: Florian Vaussard <[email protected]>
---
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
Add routines for 8, 16 and 32-bit access like in sja1000_platform.c
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: [email protected]
Signed-off-by: Florian Vaussard <[email protected]>
---
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
Simplify probe and remove functions by converting most of the resources
to use devm_* APIs.
Signed-off-by: Florian Vaussard <[email protected]>
---
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
Use netdev_* where applicable.
Signed-off-by: Florian Vaussard <[email protected]>
---
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
On Thu, Jan 30, 2014 at 8:29 AM, Florian Vaussard
<[email protected]> wrote:
> Add the reg-io-width property to describe the width of the memory
> accesses.
>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Pawel Moll <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Ian Campbell <[email protected]>
> Cc: Kumar Gala <[email protected]>
> Cc: [email protected]
> Signed-off-by: Florian Vaussard <[email protected]>
Acked-by: Rob Herring <[email protected]>
> ---
> 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
>
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 |
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
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 |
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
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 |