* [PATCH v2 3/6] can: sja1000: platform: use devm_* APIs
From: Florian Vaussard @ 2014-01-31 10:35 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: linux-can, netdev, linux-kernel, florian.vaussard
In-Reply-To: <1391164513-11529-1-git-send-email-florian.vaussard@epfl.ch>
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_platform.c | 46 ++++++++----------------------
1 file changed, 12 insertions(+), 34 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 29f9b63..779679a 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -79,34 +79,26 @@ static int sp_probe(struct platform_device *pdev)
pdata = dev_get_platdata(&pdev->dev);
if (!pdata) {
dev_err(&pdev->dev, "No platform data provided!\n");
- err = -ENODEV;
- goto exit;
+ return -ENODEV;
}
res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!res_mem || !res_irq) {
- err = -ENODEV;
- goto exit;
- }
+ if (!res_mem || !res_irq)
+ return -ENODEV;
- if (!request_mem_region(res_mem->start, resource_size(res_mem),
- DRV_NAME)) {
- err = -EBUSY;
- goto exit;
- }
+ if (!devm_request_mem_region(&pdev->dev, res_mem->start,
+ resource_size(res_mem), DRV_NAME))
+ return -EBUSY;
- addr = ioremap_nocache(res_mem->start, resource_size(res_mem));
- if (!addr) {
- err = -ENOMEM;
- goto exit_release;
- }
+ addr = devm_ioremap_nocache(&pdev->dev, res_mem->start,
+ resource_size(res_mem));
+ if (!addr)
+ return -ENOMEM;
dev = alloc_sja1000dev(0);
- if (!dev) {
- err = -ENOMEM;
- goto exit_iounmap;
- }
+ if (!dev)
+ return -ENOMEM;
priv = netdev_priv(dev);
dev->irq = res_irq->start;
@@ -151,28 +143,14 @@ static int sp_probe(struct platform_device *pdev)
exit_free:
free_sja1000dev(dev);
- exit_iounmap:
- iounmap(addr);
- exit_release:
- release_mem_region(res_mem->start, resource_size(res_mem));
- exit:
return err;
}
static int sp_remove(struct platform_device *pdev)
{
struct net_device *dev = platform_get_drvdata(pdev);
- struct sja1000_priv *priv = netdev_priv(dev);
- struct resource *res;
unregister_sja1000dev(dev);
-
- if (priv->reg_base)
- iounmap(priv->reg_base);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
free_sja1000dev(dev);
return 0;
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding
From: Florian Vaussard @ 2014-01-31 10:35 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: linux-can, netdev, linux-kernel, florian.vaussard, Grant Likely,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree
In-Reply-To: <1391164513-11529-1-git-send-email-florian.vaussard@epfl.ch>
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
Acked-by: Rob Herring <robh@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
^ permalink raw reply related
* Re: IGMP joins come from the wrong SA/interface
From: Hannes Frederic Sowa @ 2014-01-31 11:32 UTC (permalink / raw)
To: Julian Anastasov; +Cc: Steinar H. Gunderson, netdev
In-Reply-To: <alpine.LFD.2.11.1401310948270.1576@ja.home.ssi.bg>
On Fri, Jan 31, 2014 at 10:51:53AM +0200, Julian Anastasov wrote:
> To summarize: IGMP will use the same interface as
> selected from route to the joined multicast group but
> source can be from another device if for some reason there
> is no address on this interface.
Exactly that was my conclusion. As eth0.11 (as said in the second mail)
is a leg of a bridge I concluded that there is no ip address on that
interface.
Greetings,
Hannes
^ permalink raw reply
* Re: Suspected Copy Paste error in rtnl_bridge_notify
From: Tejun Heo @ 2014-01-31 11:47 UTC (permalink / raw)
To: Yogesh Gaur; +Cc: akpm, torvalds, linux-kernel, David S. Miller, netdev
In-Reply-To: <CAG6enPyV9bvYSwDrMp6X1LY6PXrmrP5MtjNk0N+7qs1LDY5pRg@mail.gmail.com>
Hey,
Seems suspicious but I don't know the code at all. cc'ing David and
netdev and quoting the whole body for them.
Thanks.
On Fri, Jan 31, 2014 at 11:32:55AM +0530, Yogesh Gaur wrote:
> Hello All,
>
> I want to clarify about following code-snippet from
> file: "net/core/rtnetlink.c"
> func: rtnl_bridge_notify
>
> ----------------------------------------------------------------------------
> | if ((!flags || (flags & BRIDGE_FLAGS_MASTER)) && |
> | br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
> |
> | err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
> |
> | if (err < 0)
> |
> | goto errout;
> |
> | }
> |
> |
> |
> | if ((flags & BRIDGE_FLAGS_SELF) &&
> |
> | dev->netdev_ops->ndo_bridge_getlink) {
> |
> | err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
> |
> | if (err < 0)
> |
> | goto errout;
> |
> ----------------------------------------------------------------------------
>
> In above code isn't code line:
> err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
> should be
> err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, br_dev, 0);
>
> If this needs to be changed then please review attached patch adding this
> change.
--
tejun
^ permalink raw reply
* Re: [PATCH v2 6/6] can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit register access
From: Marc Kleine-Budde @ 2014-01-31 12:28 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1391164513-11529-7-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> Add the 'reg-io-width' property for 8, 16 and 32-bit access, like
> what is currently done with IORESOURCE_MEM_{8,16,32}BIT for non-OF
> boot.
>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
> drivers/net/can/sja1000/sja1000_platform.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
> index 96a92a1..62ebc3d 100644
> --- a/drivers/net/can/sja1000/sja1000_platform.c
> +++ b/drivers/net/can/sja1000/sja1000_platform.c
> @@ -103,8 +103,17 @@ static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
> int err;
> u32 prop;
>
> - priv->read_reg = sp_read_reg8;
> - priv->write_reg = sp_write_reg8;
> + of_property_read_u32(of, "reg-io-width", &prop);
The new property might not be available in the DT, please check the
return value of of_property_read_u32(). Use 8 bit wide as default.
> + if (prop == 4) {
> + priv->read_reg = sp_read_reg32;
> + priv->write_reg = sp_write_reg32;
> + } else if (prop == 2) {
> + priv->read_reg = sp_read_reg16;
> + priv->write_reg = sp_write_reg16;
> + } else {
> + priv->read_reg = sp_read_reg8;
> + priv->write_reg = sp_write_reg8;
> + }
>
> err = of_property_read_u32(of, "nxp,external-clock-frequency", &prop);
> if (!err)
>
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 --]
^ permalink raw reply
* Re: [PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding
From: Marc Kleine-Budde @ 2014-01-31 12:29 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger
Cc: linux-can, netdev, linux-kernel, Grant Likely, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, devicetree
In-Reply-To: <1391164513-11529-6-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard 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
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
I think it makes sense to squash into patch 6.
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 --]
^ permalink raw reply
* Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform
From: Marc Kleine-Budde @ 2014-01-31 12:33 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1391164513-11529-5-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 7758 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> The OpenFirmware probe can be merged into the standard platform
> probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.
Please don't delete of of_platform driver, yet.
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
> drivers/net/can/sja1000/Kconfig | 13 +-
> drivers/net/can/sja1000/Makefile | 1 -
> drivers/net/can/sja1000/sja1000_of_platform.c | 218 --------------------------
> drivers/net/can/sja1000/sja1000_platform.c | 141 +++++++++++++----
> 4 files changed, 116 insertions(+), 257 deletions(-)
> delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c
[...]
> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
> index 779679a..96a92a1 100644
> --- a/drivers/net/can/sja1000/sja1000_platform.c
> +++ b/drivers/net/can/sja1000/sja1000_platform.c
> @@ -27,12 +27,16 @@
> #include <linux/can/dev.h>
> #include <linux/can/platform/sja1000.h>
> #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>
> #include "sja1000.h"
>
> #define DRV_NAME "sja1000_platform"
> +#define SJA1000_OFP_CAN_CLOCK (16000000 / 2)
This driver uses "SP" short for sja100_platform as namespace, please
convert.
>
> MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
> +MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
> MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
> MODULE_ALIAS("platform:" DRV_NAME);
> MODULE_LICENSE("GPL v2");
> @@ -67,24 +71,98 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val)
> iowrite8(val, priv->reg_base + reg * 4);
> }
>
> -static int sp_probe(struct platform_device *pdev)
> +static void sp_populate(struct sja1000_priv *priv,
> + struct sja1000_platform_data *pdata,
> + unsigned long resource_mem_flags)
> +{
> + /* The CAN clock frequency is half the oscillator clock frequency */
> + priv->can.clock.freq = pdata->osc_freq / 2;
> + priv->ocr = pdata->ocr;
> + priv->cdr = pdata->cdr;
> +
> + switch (resource_mem_flags & IORESOURCE_MEM_TYPE_MASK) {
> + case IORESOURCE_MEM_32BIT:
> + priv->read_reg = sp_read_reg32;
> + priv->write_reg = sp_write_reg32;
> + break;
> + case IORESOURCE_MEM_16BIT:
> + priv->read_reg = sp_read_reg16;
> + priv->write_reg = sp_write_reg16;
> + break;
> + case IORESOURCE_MEM_8BIT:
> + default:
> + priv->read_reg = sp_read_reg8;
> + priv->write_reg = sp_write_reg8;
> + break;
> + }
> +}
> +
> +#if defined(CONFIG_OF)
The driver also compiles on systems without CONFIG_OF without these
defines. Please remove.
> +static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
> {
> int err;
> + u32 prop;
> +
> + priv->read_reg = sp_read_reg8;
> + priv->write_reg = sp_write_reg8;
> +
> + err = of_property_read_u32(of, "nxp,external-clock-frequency", &prop);
> + if (!err)
> + priv->can.clock.freq = prop / 2;
> + else
> + priv->can.clock.freq = SJA1000_OFP_CAN_CLOCK; /* default */
> +
> + err = of_property_read_u32(of, "nxp,tx-output-mode", &prop);
> + if (!err)
> + priv->ocr |= prop & OCR_MODE_MASK;
> + else
> + priv->ocr |= OCR_MODE_NORMAL; /* default */
> +
> + err = of_property_read_u32(of, "nxp,tx-output-config", &prop);
> + if (!err)
> + priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
> + else
> + priv->ocr |= OCR_TX0_PULLDOWN; /* default */
> +
> + err = of_property_read_u32(of, "nxp,clock-out-frequency", &prop);
> + if (!err && prop) {
> + u32 divider = priv->can.clock.freq * 2 / prop;
> +
> + if (divider > 1)
> + priv->cdr |= divider / 2 - 1;
> + else
> + priv->cdr |= CDR_CLKOUT_MASK;
> + } else {
> + priv->cdr |= CDR_CLK_OFF; /* default */
> + }
> +
> + if (!of_property_read_bool(of, "nxp,no-comparator-bypass"))
> + priv->cdr |= CDR_CBP; /* default */
> +}
> +#else
> +static void sp_populate_of(struct sja1000_priv *priv, device_node *of)
> +{
> +}
> +#endif
> +
> +static int sp_probe(struct platform_device *pdev)
> +{
> + int err, irq = 0;
> void __iomem *addr;
> struct net_device *dev;
> struct sja1000_priv *priv;
> - struct resource *res_mem, *res_irq;
> + struct resource *res_mem, *res_irq = 0;
Please use NULL to initialize pointers.
> struct sja1000_platform_data *pdata;
> + struct device_node *of = pdev->dev.of_node;
>
> pdata = dev_get_platdata(&pdev->dev);
> - if (!pdata) {
> + if (!pdata && !of) {
> dev_err(&pdev->dev, "No platform data provided!\n");
> return -ENODEV;
> }
>
> res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> - if (!res_mem || !res_irq)
> + if (!res_mem)
> return -ENODEV;
>
> if (!devm_request_mem_region(&pdev->dev, res_mem->start,
> @@ -96,36 +174,34 @@ static int sp_probe(struct platform_device *pdev)
> if (!addr)
> return -ENOMEM;
>
> + if (of)
> + irq = irq_of_parse_and_map(of, 0);
> + else
> + res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +
> + if (!irq && !res_irq)
> + return -ENODEV;
> +
> dev = alloc_sja1000dev(0);
> if (!dev)
> return -ENOMEM;
> priv = netdev_priv(dev);
>
> - dev->irq = res_irq->start;
> - priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
> - if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
> - priv->irq_flags |= IRQF_SHARED;
> + if (res_irq) {
> + irq = res_irq->start;
> + priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
> + if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
> + priv->irq_flags |= IRQF_SHARED;
> + } else
> + priv->irq_flags = IRQF_SHARED;
Please add { } for else, too.
> +
> + dev->irq = irq;
> priv->reg_base = addr;
> - /* The CAN clock frequency is half the oscillator clock frequency */
> - priv->can.clock.freq = pdata->osc_freq / 2;
> - priv->ocr = pdata->ocr;
> - priv->cdr = pdata->cdr;
>
> - switch (res_mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> - case IORESOURCE_MEM_32BIT:
> - priv->read_reg = sp_read_reg32;
> - priv->write_reg = sp_write_reg32;
> - break;
> - case IORESOURCE_MEM_16BIT:
> - priv->read_reg = sp_read_reg16;
> - priv->write_reg = sp_write_reg16;
> - break;
> - case IORESOURCE_MEM_8BIT:
> - default:
> - priv->read_reg = sp_read_reg8;
> - priv->write_reg = sp_write_reg8;
> - break;
> - }
> + if (of)
> + sp_populate_of(priv, of);
> + else
> + sp_populate(priv, pdata, res_mem->flags);
>
> platform_set_drvdata(pdev, dev);
> SET_NETDEV_DEV(dev, &pdev->dev);
> @@ -156,12 +232,21 @@ static int sp_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#if defined(CONFIG_OF)
Please remove the ifdef.
> +static struct of_device_id sja1000_ofp_table[] = {
Please rename into sp_of_table or something similar.
> + {.compatible = "nxp,sja1000"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
> +#endif
> +
> static struct platform_driver sp_driver = {
> .probe = sp_probe,
> .remove = sp_remove,
> .driver = {
> .name = DRV_NAME,
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(sja1000_ofp_table),
The of_match_ptr is not needed anymore, please remove.
> },
> };
>
>
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 --]
^ permalink raw reply
* Re: [PATCH v2 3/6] can: sja1000: platform: use devm_* APIs
From: Marc Kleine-Budde @ 2014-01-31 12:34 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1391164513-11529-4-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 513 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> Simplify probe and remove functions by converting most of the resources
> to use devm_* APIs.
>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Looks good.
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 --]
^ permalink raw reply
* Re: [PATCH v2 2/6] can: sja1000: convert printk to use netdev API
From: Marc Kleine-Budde @ 2014-01-31 12:34 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1391164513-11529-3-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 454 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> Use netdev_* where applicable.
>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Loks good.
Tnx,
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 --]
^ permalink raw reply
* Re: [PATCH v2 1/6] can: sja1000: remove unused defines
From: Marc Kleine-Budde @ 2014-01-31 12:35 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1391164513-11529-2-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 612 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> Remove unused defines for the OF platform.
>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
I've overlooked in my first review that this patch is touching the
of_platform driver. Please remove the patch, as we want to remove the
driver sooner or later.
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 --]
^ permalink raw reply
* Re: [PATCH v2 0/6] can: sja1000: cleanups and new OF property
From: Marc Kleine-Budde @ 2014-01-31 12:37 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <1391164513-11529-1-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> Hello,
>
> Changes sinces v1:
> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>
> The first part of this series performs serveral small cleanups
> (patches 1 to 3).
>
> Patch 4 merges sja1000_of_platform.c into sja1000_platform.c.
> Changes are pretty conservatives (mostly copy/paste/move). IRQ
> is treated differently in the OF and non-OF versions, thus this
> is where the fused version differs the most.
>
> The final 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. This is needed on my system to correctly
> take into account the aliasing of the address bus.
Good work, please keep compatibility for DTs without the 'reg-io-width'
binding. The rest of my comments are only nitpicks. :)
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 --]
^ permalink raw reply
* Re: [PATCH v2 0/6] can: sja1000: cleanups and new OF property
From: Florian Vaussard @ 2014-01-31 12:41 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <52EB9908.2090903@pengutronix.de>
Hello Marc,
On 01/31/2014 01:37 PM, Marc Kleine-Budde wrote:
> On 01/31/2014 11:35 AM, Florian Vaussard wrote:
>> Hello,
>>
>> Changes sinces v1:
>> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>>
>> The first part of this series performs serveral small cleanups
>> (patches 1 to 3).
>>
>> Patch 4 merges sja1000_of_platform.c into sja1000_platform.c.
>> Changes are pretty conservatives (mostly copy/paste/move). IRQ
>> is treated differently in the OF and non-OF versions, thus this
>> is where the fused version differs the most.
>>
>> The final 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. This is needed on my system to correctly
>> take into account the aliasing of the address bus.
>
> Good work, please keep compatibility for DTs without the 'reg-io-width'
> binding. The rest of my comments are only nitpicks. :)
>
Good points. I will send a v3.
Regards,
Florian
^ permalink raw reply
* Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform
From: Florian Vaussard @ 2014-01-31 12:45 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <52EB9824.2010703@pengutronix.de>
Hello Marc,
On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
> On 01/31/2014 11:35 AM, Florian Vaussard wrote:
>> The OpenFirmware probe can be merged into the standard platform
>> probe to leverage common code.
>
> Good work, as we want to replace the existing driver, I'm quite picky on
> this patch, see more comments inline.
>
> Please don't delete of of_platform driver, yet.
>
Do you have any reason for not deleting of_platform? After this patch,
we will have duplicated functionalities, this may be misleading for
other people.
Regards,
Florian
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>> drivers/net/can/sja1000/Kconfig | 13 +-
>> drivers/net/can/sja1000/Makefile | 1 -
>> drivers/net/can/sja1000/sja1000_of_platform.c | 218 --------------------------
>> drivers/net/can/sja1000/sja1000_platform.c | 141 +++++++++++++----
>> 4 files changed, 116 insertions(+), 257 deletions(-)
>> delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c
> [...]
>
>> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
>> index 779679a..96a92a1 100644
>> --- a/drivers/net/can/sja1000/sja1000_platform.c
>> +++ b/drivers/net/can/sja1000/sja1000_platform.c
>> @@ -27,12 +27,16 @@
>> #include <linux/can/dev.h>
>> #include <linux/can/platform/sja1000.h>
>> #include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>>
>> #include "sja1000.h"
>>
>> #define DRV_NAME "sja1000_platform"
>> +#define SJA1000_OFP_CAN_CLOCK (16000000 / 2)
>
> This driver uses "SP" short for sja100_platform as namespace, please
> convert.
>
>>
>> MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
>> +MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
>> MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
>> MODULE_ALIAS("platform:" DRV_NAME);
>> MODULE_LICENSE("GPL v2");
>> @@ -67,24 +71,98 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val)
>> iowrite8(val, priv->reg_base + reg * 4);
>> }
>>
>> -static int sp_probe(struct platform_device *pdev)
>> +static void sp_populate(struct sja1000_priv *priv,
>> + struct sja1000_platform_data *pdata,
>> + unsigned long resource_mem_flags)
>> +{
>> + /* The CAN clock frequency is half the oscillator clock frequency */
>> + priv->can.clock.freq = pdata->osc_freq / 2;
>> + priv->ocr = pdata->ocr;
>> + priv->cdr = pdata->cdr;
>> +
>> + switch (resource_mem_flags & IORESOURCE_MEM_TYPE_MASK) {
>> + case IORESOURCE_MEM_32BIT:
>> + priv->read_reg = sp_read_reg32;
>> + priv->write_reg = sp_write_reg32;
>> + break;
>> + case IORESOURCE_MEM_16BIT:
>> + priv->read_reg = sp_read_reg16;
>> + priv->write_reg = sp_write_reg16;
>> + break;
>> + case IORESOURCE_MEM_8BIT:
>> + default:
>> + priv->read_reg = sp_read_reg8;
>> + priv->write_reg = sp_write_reg8;
>> + break;
>> + }
>> +}
>> +
>> +#if defined(CONFIG_OF)
>
> The driver also compiles on systems without CONFIG_OF without these
> defines. Please remove.
>
>> +static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
>> {
>> int err;
>> + u32 prop;
>> +
>> + priv->read_reg = sp_read_reg8;
>> + priv->write_reg = sp_write_reg8;
>> +
>> + err = of_property_read_u32(of, "nxp,external-clock-frequency", &prop);
>> + if (!err)
>> + priv->can.clock.freq = prop / 2;
>> + else
>> + priv->can.clock.freq = SJA1000_OFP_CAN_CLOCK; /* default */
>> +
>> + err = of_property_read_u32(of, "nxp,tx-output-mode", &prop);
>> + if (!err)
>> + priv->ocr |= prop & OCR_MODE_MASK;
>> + else
>> + priv->ocr |= OCR_MODE_NORMAL; /* default */
>> +
>> + err = of_property_read_u32(of, "nxp,tx-output-config", &prop);
>> + if (!err)
>> + priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
>> + else
>> + priv->ocr |= OCR_TX0_PULLDOWN; /* default */
>> +
>> + err = of_property_read_u32(of, "nxp,clock-out-frequency", &prop);
>> + if (!err && prop) {
>> + u32 divider = priv->can.clock.freq * 2 / prop;
>> +
>> + if (divider > 1)
>> + priv->cdr |= divider / 2 - 1;
>> + else
>> + priv->cdr |= CDR_CLKOUT_MASK;
>> + } else {
>> + priv->cdr |= CDR_CLK_OFF; /* default */
>> + }
>> +
>> + if (!of_property_read_bool(of, "nxp,no-comparator-bypass"))
>> + priv->cdr |= CDR_CBP; /* default */
>> +}
>> +#else
>> +static void sp_populate_of(struct sja1000_priv *priv, device_node *of)
>> +{
>> +}
>> +#endif
>> +
>> +static int sp_probe(struct platform_device *pdev)
>> +{
>> + int err, irq = 0;
>> void __iomem *addr;
>> struct net_device *dev;
>> struct sja1000_priv *priv;
>> - struct resource *res_mem, *res_irq;
>> + struct resource *res_mem, *res_irq = 0;
>
> Please use NULL to initialize pointers.
>
>> struct sja1000_platform_data *pdata;
>> + struct device_node *of = pdev->dev.of_node;
>>
>> pdata = dev_get_platdata(&pdev->dev);
>> - if (!pdata) {
>> + if (!pdata && !of) {
>> dev_err(&pdev->dev, "No platform data provided!\n");
>> return -ENODEV;
>> }
>>
>> res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> - if (!res_mem || !res_irq)
>> + if (!res_mem)
>> return -ENODEV;
>>
>> if (!devm_request_mem_region(&pdev->dev, res_mem->start,
>> @@ -96,36 +174,34 @@ static int sp_probe(struct platform_device *pdev)
>> if (!addr)
>> return -ENOMEM;
>>
>> + if (of)
>> + irq = irq_of_parse_and_map(of, 0);
>> + else
>> + res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> +
>> + if (!irq && !res_irq)
>> + return -ENODEV;
>> +
>> dev = alloc_sja1000dev(0);
>> if (!dev)
>> return -ENOMEM;
>> priv = netdev_priv(dev);
>>
>> - dev->irq = res_irq->start;
>> - priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
>> - if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
>> - priv->irq_flags |= IRQF_SHARED;
>> + if (res_irq) {
>> + irq = res_irq->start;
>> + priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
>> + if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
>> + priv->irq_flags |= IRQF_SHARED;
>> + } else
>> + priv->irq_flags = IRQF_SHARED;
>
> Please add { } for else, too.
>
>> +
>> + dev->irq = irq;
>> priv->reg_base = addr;
>> - /* The CAN clock frequency is half the oscillator clock frequency */
>> - priv->can.clock.freq = pdata->osc_freq / 2;
>> - priv->ocr = pdata->ocr;
>> - priv->cdr = pdata->cdr;
>>
>> - switch (res_mem->flags & IORESOURCE_MEM_TYPE_MASK) {
>> - case IORESOURCE_MEM_32BIT:
>> - priv->read_reg = sp_read_reg32;
>> - priv->write_reg = sp_write_reg32;
>> - break;
>> - case IORESOURCE_MEM_16BIT:
>> - priv->read_reg = sp_read_reg16;
>> - priv->write_reg = sp_write_reg16;
>> - break;
>> - case IORESOURCE_MEM_8BIT:
>> - default:
>> - priv->read_reg = sp_read_reg8;
>> - priv->write_reg = sp_write_reg8;
>> - break;
>> - }
>> + if (of)
>> + sp_populate_of(priv, of);
>> + else
>> + sp_populate(priv, pdata, res_mem->flags);
>>
>> platform_set_drvdata(pdev, dev);
>> SET_NETDEV_DEV(dev, &pdev->dev);
>> @@ -156,12 +232,21 @@ static int sp_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +#if defined(CONFIG_OF)
>
> Please remove the ifdef.
>
>> +static struct of_device_id sja1000_ofp_table[] = {
>
> Please rename into sp_of_table or something similar.
>
>> + {.compatible = "nxp,sja1000"},
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
>> +#endif
>> +
>> static struct platform_driver sp_driver = {
>> .probe = sp_probe,
>> .remove = sp_remove,
>> .driver = {
>> .name = DRV_NAME,
>> .owner = THIS_MODULE,
>> + .of_match_table = of_match_ptr(sja1000_ofp_table),
>
> The of_match_ptr is not needed anymore, please remove.
>
>> },
>> };
>>
>>
>
> Marc
>
^ permalink raw reply
* Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform
From: Marc Kleine-Budde @ 2014-01-31 12:49 UTC (permalink / raw)
To: florian.vaussard, Wolfgang Grandegger
Cc: linux-can, netdev, linux-kernel, Andreas Larsson
In-Reply-To: <52EB9AEA.4010503@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]
On 01/31/2014 01:45 PM, Florian Vaussard wrote:
> Hello Marc,
>
> On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
>> On 01/31/2014 11:35 AM, Florian Vaussard wrote:
>>> The OpenFirmware probe can be merged into the standard platform
>>> probe to leverage common code.
>>
>> Good work, as we want to replace the existing driver, I'm quite picky on
>> this patch, see more comments inline.
>>
>> Please don't delete of of_platform driver, yet.
>>
>
> Do you have any reason for not deleting of_platform? After this patch,
> we will have duplicated functionalities, this may be misleading for
> other people.
Okay.
IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
get a Tested-by from Andreas (Cc'ed).
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 --]
^ permalink raw reply
* Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform
From: Marc Kleine-Budde @ 2014-01-31 12:51 UTC (permalink / raw)
To: florian.vaussard, Wolfgang Grandegger
Cc: linux-can, netdev, linux-kernel, Andreas Larsson
In-Reply-To: <52EB9BE1.6060407@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]
On 01/31/2014 01:49 PM, Marc Kleine-Budde wrote:
> On 01/31/2014 01:45 PM, Florian Vaussard wrote:
>> Hello Marc,
>>
>> On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
>>> On 01/31/2014 11:35 AM, Florian Vaussard wrote:
>>>> The OpenFirmware probe can be merged into the standard platform
>>>> probe to leverage common code.
>>>
>>> Good work, as we want to replace the existing driver, I'm quite picky on
>>> this patch, see more comments inline.
>>>
>>> Please don't delete of of_platform driver, yet.
>>>
>>
>> Do you have any reason for not deleting of_platform? After this patch,
>> we will have duplicated functionalities, this may be misleading for
>> other people.
>
> Okay.
> IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
> get a Tested-by from Andreas (Cc'ed).
You have to use irq_of_parse_and_map() on sparc, but your patches does this.
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 --]
^ permalink raw reply
* Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform
From: Florian Vaussard @ 2014-01-31 13:00 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger
Cc: linux-can, netdev, linux-kernel, Andreas Larsson
In-Reply-To: <52EB9C48.3010205@pengutronix.de>
On 01/31/2014 01:51 PM, Marc Kleine-Budde wrote:
> On 01/31/2014 01:49 PM, Marc Kleine-Budde wrote:
>> On 01/31/2014 01:45 PM, Florian Vaussard wrote:
>>> Hello Marc,
>>>
>>> On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
>>>> On 01/31/2014 11:35 AM, Florian Vaussard wrote:
>>>>> The OpenFirmware probe can be merged into the standard platform
>>>>> probe to leverage common code.
>>>>
>>>> Good work, as we want to replace the existing driver, I'm quite picky on
>>>> this patch, see more comments inline.
>>>>
>>>> Please don't delete of of_platform driver, yet.
>>>>
>>>
>>> Do you have any reason for not deleting of_platform? After this patch,
>>> we will have duplicated functionalities, this may be misleading for
>>> other people.
>>
>> Okay.
>> IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
>> get a Tested-by from Andreas (Cc'ed).
>
So it is ok for you if I remove of_platform, right?
> You have to use irq_of_parse_and_map() on sparc, but your patches does this.
>
My first implementation was not using irq_of_parse_and_map(), as I do
not need it on ARM, but I suspected that other people might need it.
This is why the final code is slightly more complicated to handle both
cases.
Regards,
Florian
^ permalink raw reply
* Re: [RFC PATCH] net: ipv4: move inetpeer garbage collector work to power efficient workqueue
From: Sergei Shtylyov @ 2014-01-31 13:10 UTC (permalink / raw)
To: Zoran Markovic, linux-kernel
Cc: netdev, Shaibal Dutta, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1391125213-8119-1-git-send-email-zoran.markovic@linaro.org>
Hello.
On 31-01-2014 3:40, Zoran Markovic wrote:
> From: Shaibal Dutta <shaibal.dutta@broadcom.com>
> Garbage collector work does not have to be bound to the CPU that scheduled
> it. By moving work to the power-efficient workqueue, the selection of
> CPU executing the work is left to the scheduler. This extends idle
> residency times and conserves power.
> This functionality is enabled when CONFIG_WQ_POWER_EFFICIENT is selected.
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Shaibal Dutta <shaibal.dutta@broadcom.com>
> [zoran.markovic@linaro.org: Rebased to latest kernel version. Added
> commit message.]
> Signed-off-by: Zoran Markovic <zoran.markovic@linaro.org>
> ---
> net/ipv4/inetpeer.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
> index 48f4244..87155aa 100644
> --- a/net/ipv4/inetpeer.c
> +++ b/net/ipv4/inetpeer.c
> @@ -161,7 +161,8 @@ static void inetpeer_gc_worker(struct work_struct *work)
> list_splice(&list, &gc_list);
> spin_unlock_bh(&gc_lock);
>
> - schedule_delayed_work(&gc_work, gc_delay);
> + queue_delayed_work(system_power_efficient_wq,
> + &gc_work, gc_delay);
Please align the continuation line under the next character after ( on the
broken up line.
> @@ -576,7 +577,8 @@ static void inetpeer_inval_rcu(struct rcu_head *head)
> list_add_tail(&p->gc_list, &gc_list);
> spin_unlock_bh(&gc_lock);
>
> - schedule_delayed_work(&gc_work, gc_delay);
> + queue_delayed_work(system_power_efficient_wq,
> + &gc_work, gc_delay);
Same here. This is according to networking coding style.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform
From: Marc Kleine-Budde @ 2014-01-31 13:14 UTC (permalink / raw)
To: florian.vaussard, Wolfgang Grandegger
Cc: linux-can, netdev, linux-kernel, Andreas Larsson
In-Reply-To: <52EB9E54.2020008@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]
On 01/31/2014 02:00 PM, Florian Vaussard wrote:
>
>
> On 01/31/2014 01:51 PM, Marc Kleine-Budde wrote:
>> On 01/31/2014 01:49 PM, Marc Kleine-Budde wrote:
>>> On 01/31/2014 01:45 PM, Florian Vaussard wrote:
>>>> Hello Marc,
>>>>
>>>> On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
>>>>> On 01/31/2014 11:35 AM, Florian Vaussard wrote:
>>>>>> The OpenFirmware probe can be merged into the standard platform
>>>>>> probe to leverage common code.
>>>>>
>>>>> Good work, as we want to replace the existing driver, I'm quite picky on
>>>>> this patch, see more comments inline.
>>>>>
>>>>> Please don't delete of of_platform driver, yet.
>>>>>
>>>>
>>>> Do you have any reason for not deleting of_platform? After this patch,
>>>> we will have duplicated functionalities, this may be misleading for
>>>> other people.
>>>
>>> Okay.
>>> IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
>>> get a Tested-by from Andreas (Cc'ed).
>>
>
> So it is ok for you if I remove of_platform, right?
If you get a Tested-by for this patch by a sparc user.
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 --]
^ permalink raw reply
* Re: OOPS in nf_ct_unlink_expect_report using Polycom RealPresence Mobile
From: Mike Galbraith @ 2014-01-31 13:17 UTC (permalink / raw)
To: astx; +Cc: linux-kernel, netdev
In-Reply-To: <20140131120527.Horde.-3YzQu5S8WWBx7vuVVNdBA2@aws-it.at>
(CC netdev)
On Fri, 2014-01-31 at 12:05 +0100, astx wrote:
> Using Polycom video conferencing software my homebrew linux NAT router
> crashes with attached kernel oops message.
> This error can be reproduced also using kernel 3.2.54. Kernel 2.6.35
> seems to be stable.
>
> Disabling nf_nat_h323 and nf_conntrack_h323 avoids crash - but video
> conferencing software is no more usable.
>
>
> ===================================================================================
> BUG: unable to handle kernel paging request at 00100104
> IP: [<f8214f07>] nf_ct_unlink_expect_report+0x57/0xf0 [nf_conntrack]
> *pdpt = 00000000359aa001 *pde = 0000000000000000
> Oops: 0002 [#1] SMP
> Modules linked in: nf_conntrack_netlink nfnetlink xt_mac xt_TCPMSS
> ipt_MASQUERADE
> xt_pkttype xt_multiport xt_REDIRECT xt_nat iptable_mangle xt_LOG
> xt_limit af_packet
> act_mirred cls_u32 sch_ingress sch_hfsc ifb xt_tcpudp ip6t_REJECT ipt_REJECT
> ip6table_raw iptable_raw xt_CT iptable_filter nf_nat_pptp nf_nat_proto_gre
> nf_conntrack_proto_udplite nf_conntrack_proto_dccp ip6table_mangle
> iptable_nat
> nf_nat_ipv4 nf_nat_sip nf_nat_irc nf_nat_snmp_basic nf_conntrack_snmp
> nf_conntrack_broadcast nf_nat_h323 nf_nat_tftp nf_nat_ftp nf_nat
> nf_conntrack_h323
> nf_conntrack_tftp nf_conntrack_proto_sctp nf_conntrack_sip nf_conntrack_irc
> nf_conntrack_pptp nf_conntrack_proto_gre nf_conntrack_ftp nf_conntrack_ipv4
> nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack ip6table_filter ip6_tables
> x_tables padlock_sha padlock_aes e_powersaver freq_table mperf via_cputemp
> hwmon_vid serio_raw pcspkr i2c_viapro ehci_pci fan thermal processor 8139too
> sg thermal_sys button shpchp 8139cp pci_hotplug mii via_agp ext4 crc16 jbd2
> pata_via sata_via libata sd_mod scsi_mod ohci_hcd uhci_hcd ehci_hcd
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.28-9500-smp_m #1
> Hardware name: /CN700-8237, BIOS 6.00 PG 08/30/2007
> task: c07ce180 ti: f6408000 task.ti: c07c2000
> EIP: 0060:[<f8214f07>] EFLAGS: 00210206 CPU: 0
> EIP is at nf_ct_unlink_expect_report+0x57/0xf0 [nf_conntrack]
> EAX: 00100100 EBX: eb636bc0 ECX: 00000000 EDX: eb461540
> ESI: c0804e00 EDI: eb461544 EBP: f6409f08 ESP: f6409eec
> DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> CR0: 8005003b CR2: 00100104 CR3: 359d4000 CR4: 000006b0
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Stack:
> 00000000 00200286 f6409f08 c0244bd8 eb636bc0 00100100 00000000 f6409f18
> f8215687 f598ede8 c0804e00 f6409f28 f8211c99 f598ede8 f598ee50 f6409f5c
> f8212e5e 00000003 00000000 00000000 00000004 eb461514 f598ede8 00000000
> Call Trace:
> [<c0244bd8>] ? del_timer+0x48/0x70
> [<f8215687>] nf_ct_remove_expectations+0x47/0x60 [nf_conntrack]
> [<f8211c99>] nf_ct_delete_from_lists+0x59/0x90 [nf_conntrack]
> [<f8212e5e>] death_by_timeout+0x14e/0x1c0 [nf_conntrack]
> [<f8212d10>] ? nf_conntrack_set_hashsize+0x190/0x190 [nf_conntrack]
> [<c024442d>] call_timer_fn+0x1d/0x80
> [<c024461e>] run_timer_softirq+0x18e/0x1a0
> [<f8212d10>] ? nf_conntrack_set_hashsize+0x190/0x190 [nf_conntrack]
> [<c023e6f3>] __do_softirq+0xa3/0x170
> [<c023e650>] ? __local_bh_enable+0x70/0x70
> <IRQ>
> [<c023e587>] ? irq_exit+0x67/0xa0
> [<c0202af6>] ? do_IRQ+0x46/0xb0
> [<c027ad05>] ? clockevents_notify+0x35/0x110
> [<c066ac6c>] ? common_interrupt+0x2c/0x40
> [<c056e3c1>] ? cpuidle_enter_state+0x41/0xf0
> [<c056e6fb>] ? cpuidle_idle_call+0x8b/0x100
> [<c02085f8>] ? arch_cpu_idle+0x8/0x30
> [<c027314b>] ? cpu_idle_loop+0x4b/0x140
> [<c0273258>] ? cpu_startup_entry+0x18/0x20
> [<c066056d>] ? rest_init+0x5d/0x70
> [<c0813ac8>] ? start_kernel+0x2ec/0x2f2
> [<c081364f>] ? repair_env_string+0x5b/0x5b
> [<c0813269>] ? i386_start_kernel+0x33/0x35
> Code: 8b 7b 0c 8b b6 98 00 00 00 85 c0 89 07 74 03 89 78 04 c7 43 0c 00
> 02 20 00 83 ae ec 05 00 00 01 8b 03 8b 7b 04 85 c0 89 07 74 03 <89> 78
> 04 8b 43 7c c7 03 00 01 10 00 c7 43 04 00 02 20 00 80 6c
> EIP: [<f8214f07>] nf_ct_unlink_expect_report+0x57/0xf0 [nf_conntrack]
> SS:ESP 0068:f6409eec
> CR2: 0000000000100104
> ---[ end trace 79fe2e6b81f54dee ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> Rebooting in 300 seconds..
> ===================================================================================
>
>
> Polycom Version: 3.1-44477
> running on device: Apple iPad Mini
> using operating system: iOS Version: 7.0.4
>
>
> Attached also my kernel config. Hopefully someone could help...
>
> BR, Toni
^ permalink raw reply
* Re: [PATCH RFC 1/1] usb: Tell xhci when usb data might be misaligned
From: Peter Stuge @ 2014-01-31 13:21 UTC (permalink / raw)
To: David Laight
Cc: 'Sarah Sharp', linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Greg Kroah-Hartman, David Miller,
Dan Williams, Nyman, Mathias, Mark Lord, Alan Stern, Freddy Xin
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6B5B78@AcuExch.aculab.com>
David Laight wrote:
> > We shouldn't need to make userspace start to worry about alignment at
> > all. libusb worked in the past, before the link TRB fix went in. We
> > *cannot* break userspace USB drivers. The breakage needs to be fixed in
> > the USB core or the xHCI driver.
>
> Userspace doesn't care since everything gets copied into aligned
> kernel fragments - otherwise the other usb controllers wouldn't work.
OK, but not so great if someone wants to squeeze the most performance
possible out of USB also from userspace.
I'm going off on a tangent now but would it make sense to allow
userspace to do alignment if it wants to, and have a way to tell
the kernel when urb buffers are pre-aligned?
//Peter
^ permalink raw reply
* Re: igb and bnx2: "NETDEV WATCHDOG: transmit queue timed out" when skb has huge linear buffer
From: Zoltan Kiss @ 2014-01-31 13:29 UTC (permalink / raw)
To: Michael Chan, Zoltan Kiss
Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
John Ronciak, Tushar Dave, Akeem G Abodunrin, David S. Miller,
e1000-devel, netdev@vger.kernel.org, linux-kernel,
xen-devel@lists.xenproject.org
In-Reply-To: <1391114048.4804.2.camel@LTIRV-MCHAN1.corp.ad.broadcom.com>
On 30/01/14 21:34, Michael Chan wrote:
> On Thu, 2014-01-30 at 19:08 +0000, Zoltan Kiss wrote:
>> I've experienced some queue timeout problems mentioned in the subject
>> with igb and bnx2 cards.
> Please provide the full tx timeout dmesg. bnx2 dumps some diagnostic
> information during tx timeout that may be useful. Thanks.
Hi,
Here is some:
[ 5417.275463] ------------[ cut here ]------------
[ 5417.275472] WARNING: at net/sched/sch_generic.c:255
dev_watchdog+0x156/0x1f0()
[ 5417.275474] NETDEV WATCHDOG: eth1 (bnx2): transmit queue 2 timed out
[ 5417.275476] Modules linked in: tun nfsv3 nfs_acl rpcsec_gss_krb5
auth_rpcgss oid_registry nfsv4 nfs fscache lockd sunrpc ipv6
openvswitch(O) ipt_REJECT nf_conntrack_ipv
rack xt_tcpudp iptable_filter ip_tables x_tables nls_utf8 isofs
dm_multipath scsi_dh dm_mod dcdbas coretemp microcode psmouse serio_raw
lpc_ich mfd_core hid_generic ehci_p
sg hed bnx2 usbhid hid sr_mod cdrom sd_mod pata_acpi ata_generic
ata_piix libata uhci_hcd mptsas mptscsih mptbase scsi_transport_sas scsi_mod
[ 5417.275517] CPU: 0 PID: 3 Comm: ksoftirqd/0 Tainted: G O
3.10.11-0.xs1.8.50.170.377582 #1
[ 5417.275518] Hardware name: Dell Inc. PowerEdge R710/00W9X3, BIOS
1.2.6 07/17/2009
[ 5417.275520] 000000ff f008be08 c1488c53 f008be30 c1046664 c1658a88
f008be5c 000000ff
[ 5417.275525] c13fc146 c13fc146 ee96a000 00000002 00137d44 f008be48
c1046723 00000009
[ 5417.275530] f008be40 c1658a88 f008be5c f008be80 c13fc146 c16556e1
000000ff c1658a88
[ 5417.275535] Call Trace:
[ 5417.275539] [<c1488c53>] dump_stack+0x16/0x1b
[ 5417.275544] [<c1046664>] warn_slowpath_common+0x64/0x80
[ 5417.275546] [<c13fc146>] ? dev_watchdog+0x156/0x1f0
[ 5417.275549] [<c13fc146>] ? dev_watchdog+0x156/0x1f0
[ 5417.275551] [<c1046723>] warn_slowpath_fmt+0x33/0x40
[ 5417.275554] [<c13fc146>] dev_watchdog+0x156/0x1f0
[ 5417.275559] [<c10549ce>] call_timer_fn+0x3e/0xf0
[ 5417.275563] [<c107293e>] ? finish_task_switch+0x4e/0xb0
[ 5417.275565] [<c13fbff0>] ? __netdev_watchdog_up+0x60/0x60
[ 5417.275568] [<c1055c1b>] run_timer_softirq+0x1ab/0x210
[ 5417.275571] [<c13fbff0>] ? __netdev_watchdog_up+0x60/0x60
[ 5417.275574] [<c104e3f4>] __do_softirq+0xc4/0x200
[ 5417.275577] [<c1493547>] ? xen_do_upcall+0x7/0xc
[ 5417.275579] [<c104e550>] run_ksoftirqd+0x20/0x50
[ 5417.275582] [<c106f182>] smpboot_thread_fn+0x142/0x150
[ 5417.275586] [<c1067a2b>] kthread+0x9b/0xa0
[ 5417.275589] [<c106f040>] ? smpboot_create_threads+0x60/0x60
[ 5417.275591] [<c1070000>] ? cpu_rt_runtime_read+0x40/0x80
[ 5417.275594] [<c1492f77>] ret_from_kernel_thread+0x1b/0x28
[ 5417.275596] [<c1067990>] ? kthread_freezable_should_stop+0x60/0x60
[ 5417.275599] ---[ end trace 691f572d388226ca ]---
[ 5417.275602] bnx2 0000:01:00.1 eth1: <--- start FTQ dump --->
[ 5417.275622] bnx2 0000:01:00.1 eth1: RV2P_PFTQ_CTL 00010000
[ 5417.275629] bnx2 0000:01:00.1 eth1: RV2P_TFTQ_CTL 00020000
[ 5417.275636] bnx2 0000:01:00.1 eth1: RV2P_MFTQ_CTL 00004000
[ 5417.275643] bnx2 0000:01:00.1 eth1: TBDR_FTQ_CTL 00004002
[ 5417.275650] bnx2 0000:01:00.1 eth1: TDMA_FTQ_CTL 00010002
[ 5417.275657] bnx2 0000:01:00.1 eth1: TXP_FTQ_CTL 00010000
[ 5417.275663] bnx2 0000:01:00.1 eth1: TXP_FTQ_CTL 00010000
[ 5417.275670] bnx2 0000:01:00.1 eth1: TPAT_FTQ_CTL 00010000
[ 5417.275677] bnx2 0000:01:00.1 eth1: RXP_CFTQ_CTL 00008000
[ 5417.275684] bnx2 0000:01:00.1 eth1: RXP_FTQ_CTL 00100000
[ 5417.275690] bnx2 0000:01:00.1 eth1: COM_COMXQ_FTQ_CTL 00010000
[ 5417.275698] bnx2 0000:01:00.1 eth1: COM_COMTQ_FTQ_CTL 00020000
[ 5417.275705] bnx2 0000:01:00.1 eth1: COM_COMQ_FTQ_CTL 00010000
[ 5417.275712] bnx2 0000:01:00.1 eth1: CP_CPQ_FTQ_CTL 00004000
[ 5417.275718] bnx2 0000:01:00.1 eth1: CPU states:
[ 5417.275730] bnx2 0000:01:00.1 eth1: 045000 mode b84c state 80001000
evt_mask 500 pc 8001284 pc 8001284 instr 1440fffc
[ 5417.275746] bnx2 0000:01:00.1 eth1: 085000 mode b84c state 80005000
evt_mask 500 pc 8000a54 pc 8000a5c instr 10400016
[ 5417.275785] bnx2 0000:01:00.1 eth1: 0c5000 mode b84c state 80001000
evt_mask 500 pc 8004c20 pc 8004c20 instr 32050003
[ 5417.275801] bnx2 0000:01:00.1 eth1: 105000 mode b8cc state 80000000
evt_mask 500 pc 8000a8c pc 8000a94 instr 8c420020
[ 5417.275817] bnx2 0000:01:00.1 eth1: 145000 mode b880 state 80000000
evt_mask 500 pc 8000ab0 pc 800d1e8 instr 27bd0020
[ 5417.275834] bnx2 0000:01:00.1 eth1: 185000 mode b8cc state 80000000
evt_mask 500 pc 8000cb0 pc 8000930 instr 8ce800e8
[ 5417.275845] bnx2 0000:01:00.1 eth1: <--- end FTQ dump --->
[ 5417.275851] bnx2 0000:01:00.1 eth1: <--- start TBDC dump --->
[ 5417.275858] bnx2 0000:01:00.1 eth1: TBDC free cnt: 32
[ 5417.275864] bnx2 0000:01:00.1 eth1: LINE CID BIDX CMD VALIDS
[ 5417.275875] bnx2 0000:01:00.1 eth1: 00 001080 17c8 00 [0]
[ 5417.275886] bnx2 0000:01:00.1 eth1: 01 001080 17e0 00 [0]
[ 5417.275897] bnx2 0000:01:00.1 eth1: 02 001080 17e8 00 [0]
[ 5417.275907] bnx2 0000:01:00.1 eth1: 03 001080 17f8 00 [0]
[ 5417.275918] bnx2 0000:01:00.1 eth1: 04 001080 1800 00 [0]
[ 5417.275929] bnx2 0000:01:00.1 eth1: 05 001080 17d0 00 [0]
[ 5417.275940] bnx2 0000:01:00.1 eth1: 06 001080 17d8 00 [0]
[ 5417.275951] bnx2 0000:01:00.1 eth1: 07 001080 17f0 00 [0]
[ 5417.275961] bnx2 0000:01:00.1 eth1: 08 001080 1620 00 [0]
[ 5417.275972] bnx2 0000:01:00.1 eth1: 09 17de00 fbf8 78 [0]
[ 5417.275983] bnx2 0000:01:00.1 eth1: 0a 1bbf80 fef8 9f [0]
[ 5417.275994] bnx2 0000:01:00.1 eth1: 0b 1d2d80 f7f8 7f [0]
[ 5417.276005] bnx2 0000:01:00.1 eth1: 0c 148f00 f7b8 88 [0]
[ 5417.276016] bnx2 0000:01:00.1 eth1: 0d 16af80 f7d0 75 [0]
[ 5417.276026] bnx2 0000:01:00.1 eth1: 0e 1adf80 bfb0 26 [0]
[ 5417.276037] bnx2 0000:01:00.1 eth1: 0f 1ebf80 dd68 3c [0]
[ 5417.276048] bnx2 0000:01:00.1 eth1: 10 1cf700 d1f0 fc [0]
[ 5417.276059] bnx2 0000:01:00.1 eth1: 11 1cdc00 fbf0 7d [0]
[ 5417.276069] bnx2 0000:01:00.1 eth1: 12 15c900 f7f8 ef [0]
[ 5417.276081] bnx2 0000:01:00.1 eth1: 13 17cf00 d7d8 3f [0]
[ 5417.276093] bnx2 0000:01:00.1 eth1: 14 1ecf80 ffb0 b7 [0]
[ 5417.276107] bnx2 0000:01:00.1 eth1: 15 1cbd80 f3e8 bf [0]
[ 5417.276119] bnx2 0000:01:00.1 eth1: 16 179b80 d7f8 d7 [0]
[ 5417.276130] bnx2 0000:01:00.1 eth1: 17 1fdf00 f3e8 7e [0]
[ 5417.276141] bnx2 0000:01:00.1 eth1: 18 1f9780 b578 af [0]
[ 5417.276152] bnx2 0000:01:00.1 eth1: 19 1d7d80 fef0 ff [0]
[ 5417.276163] bnx2 0000:01:00.1 eth1: 1a 1d9e80 5fe8 d7 [0]
[ 5417.276174] bnx2 0000:01:00.1 eth1: 1b 1fff80 ebf8 f8 [0]
[ 5417.276186] bnx2 0000:01:00.1 eth1: 1c 1fbd80 f7d8 7f [0]
[ 5417.276200] bnx2 0000:01:00.1 eth1: 1d 16da80 2ef8 ff [0]
[ 5417.276211] bnx2 0000:01:00.1 eth1: 1e 1f9b80 bf50 8e [0]
[ 5417.276224] bnx2 0000:01:00.1 eth1: 1f 1bdf00 faf8 75 [0]
[ 5417.276231] bnx2 0000:01:00.1 eth1: <--- end TBDC dump --->
[ 5417.276246] bnx2 0000:01:00.1 eth1: DEBUG: intr_sem[0] PCI_CMD[00100406]
[ 5417.276258] bnx2 0000:01:00.1 eth1: DEBUG: PCI_PM[19002008]
PCI_MISC_CFG[92000088]
[ 5417.276269] bnx2 0000:01:00.1 eth1: DEBUG: EMAC_TX_STATUS[00000008]
EMAC_RX_STATUS[00000000]
[ 5417.276280] bnx2 0000:01:00.1 eth1: DEBUG: RPM_MGMT_PKT_CTRL[40000088]
[ 5417.276288] bnx2 0000:01:00.1 eth1: DEBUG:
HC_STATS_INTERRUPT_STATUS[01fb0004]
[ 5417.276298] bnx2 0000:01:00.1 eth1: DEBUG: PBA[00000000]
[ 5417.276304] bnx2 0000:01:00.1 eth1: <--- start MCP states dump --->
[ 5417.276314] bnx2 0000:01:00.1 eth1: DEBUG: MCP_STATE_P0[0003610e]
MCP_STATE_P1[0003610e]
[ 5417.276326] bnx2 0000:01:00.1 eth1: DEBUG: MCP mode[0000b880]
state[80000000] evt_mask[00000500]
[ 5417.276339] bnx2 0000:01:00.1 eth1: DEBUG: pc[0800d7b8] pc[08000cdc]
instr[00041880]
[ 5417.276349] bnx2 0000:01:00.1 eth1: DEBUG: shmem states:
[ 5417.276358] bnx2 0000:01:00.1 eth1: DEBUG: drv_mb[0d000004]
fw_mb[00000004] link_status[0000006f]
[ 5417.276369] drv_pulse_mb[00001485]
[ 5417.276373] bnx2 0000:01:00.1 eth1: DEBUG:
dev_info_signature[44564903] reset_type[01005254]
[ 5417.276383] condition[0003610e]
[ 5417.276389] bnx2 0000:01:00.1 eth1: DEBUG: 000001c0: 01005254
42530000 0003610e 00000000
[ 5417.276402] bnx2 0000:01:00.1 eth1: DEBUG: 000003cc: 44444444
44444444 44444444 00000a28
[ 5417.276416] bnx2 0000:01:00.1 eth1: DEBUG: 000003dc: 0004ffff
00000000 00000000 00000000
[ 5417.276430] bnx2 0000:01:00.1 eth1: DEBUG: 000003ec: 00000000
00000000 00000000 00000000
[ 5417.276440] bnx2 0000:01:00.1 eth1: DEBUG: 0x3fc[0000ffff]
^ permalink raw reply
* [PATCH v3 0/5] can: sja1000: cleanups and new OF property
From: Florian Vaussard @ 2014-01-31 13:34 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andreas Larsson, linux-can, netdev, sparclinux, linux-kernel,
florian.vaussard
Hello,
(could someone with a SJA1000 on SPARC perform a functional test
to see if interrupts are working? it would be great :-)
Changes since v2:
- Dropped patch 1 "can: sja1000: remove unused defines"
- Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
Changes since v1:
- Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
The first part of this series performs serveral small cleanups
(patches 1 and 2).
Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
Changes are pretty conservatives (mostly copy/paste/move). IRQ
is treated differently in the OF and non-OF versions, thus this
is where the fused version differs the most.
The final 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. This is needed on my system to correctly
take into account the aliasing of the address bus.
All patches were tested using OF boot on my OMAP3 system with a
memory-mapped SJA1000. Thus, the non-OF path is not tested, as
I do not have a platform data at hand.
Regards,
Florian
v1: http://thread.gmane.org/gmane.linux.kernel/1637835
v2: http://thread.gmane.org/gmane.linux.can/4831
Florian Vaussard (5):
can: sja1000: convert printk to use netdev API
can: sja1000: platform: use devm_* APIs
can: sja1000: fuse of_platform into platform
Documentation: devicetree: sja1000: add reg-io-width binding
can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit
register access
.../devicetree/bindings/net/can/sja1000.txt | 4 +
drivers/net/can/sja1000/Kconfig | 13 +-
drivers/net/can/sja1000/Makefile | 1 -
drivers/net/can/sja1000/sja1000.c | 3 +-
drivers/net/can/sja1000/sja1000_of_platform.c | 221 ---------------------
drivers/net/can/sja1000/sja1000_platform.c | 190 ++++++++++++------
6 files changed, 137 insertions(+), 295 deletions(-)
delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c
--
1.8.1.2
^ permalink raw reply
* [PATCH v3 1/5] can: sja1000: convert printk to use netdev API
From: Florian Vaussard @ 2014-01-31 13:34 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andreas Larsson, linux-can, netdev, sparclinux, linux-kernel,
florian.vaussard
In-Reply-To: <1391175277-19833-1-git-send-email-florian.vaussard@epfl.ch>
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
^ permalink raw reply related
* [PATCH v3 2/5] can: sja1000: platform: use devm_* APIs
From: Florian Vaussard @ 2014-01-31 13:34 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andreas Larsson, linux-can, netdev, sparclinux, linux-kernel,
florian.vaussard
In-Reply-To: <1391175277-19833-1-git-send-email-florian.vaussard@epfl.ch>
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_platform.c | 46 ++++++++----------------------
1 file changed, 12 insertions(+), 34 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 29f9b63..779679a 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -79,34 +79,26 @@ static int sp_probe(struct platform_device *pdev)
pdata = dev_get_platdata(&pdev->dev);
if (!pdata) {
dev_err(&pdev->dev, "No platform data provided!\n");
- err = -ENODEV;
- goto exit;
+ return -ENODEV;
}
res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!res_mem || !res_irq) {
- err = -ENODEV;
- goto exit;
- }
+ if (!res_mem || !res_irq)
+ return -ENODEV;
- if (!request_mem_region(res_mem->start, resource_size(res_mem),
- DRV_NAME)) {
- err = -EBUSY;
- goto exit;
- }
+ if (!devm_request_mem_region(&pdev->dev, res_mem->start,
+ resource_size(res_mem), DRV_NAME))
+ return -EBUSY;
- addr = ioremap_nocache(res_mem->start, resource_size(res_mem));
- if (!addr) {
- err = -ENOMEM;
- goto exit_release;
- }
+ addr = devm_ioremap_nocache(&pdev->dev, res_mem->start,
+ resource_size(res_mem));
+ if (!addr)
+ return -ENOMEM;
dev = alloc_sja1000dev(0);
- if (!dev) {
- err = -ENOMEM;
- goto exit_iounmap;
- }
+ if (!dev)
+ return -ENOMEM;
priv = netdev_priv(dev);
dev->irq = res_irq->start;
@@ -151,28 +143,14 @@ static int sp_probe(struct platform_device *pdev)
exit_free:
free_sja1000dev(dev);
- exit_iounmap:
- iounmap(addr);
- exit_release:
- release_mem_region(res_mem->start, resource_size(res_mem));
- exit:
return err;
}
static int sp_remove(struct platform_device *pdev)
{
struct net_device *dev = platform_get_drvdata(pdev);
- struct sja1000_priv *priv = netdev_priv(dev);
- struct resource *res;
unregister_sja1000dev(dev);
-
- if (priv->reg_base)
- iounmap(priv->reg_base);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
free_sja1000dev(dev);
return 0;
--
1.8.1.2
^ permalink raw reply related
* [PATCH v3 3/5] can: sja1000: fuse of_platform into platform
From: Florian Vaussard @ 2014-01-31 13:34 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andreas Larsson, linux-can, netdev, sparclinux, linux-kernel,
florian.vaussard
In-Reply-To: <1391175277-19833-1-git-send-email-florian.vaussard@epfl.ch>
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
drivers/net/can/sja1000/Kconfig | 13 +-
drivers/net/can/sja1000/Makefile | 1 -
drivers/net/can/sja1000/sja1000_of_platform.c | 221 --------------------------
drivers/net/can/sja1000/sja1000_platform.c | 134 ++++++++++++----
4 files changed, 109 insertions(+), 260 deletions(-)
delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c
diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index ff2ba86..4b18b87 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -17,16 +17,9 @@ config CAN_SJA1000_PLATFORM
the "platform bus" (Linux abstraction for directly to the
processor attached devices). Which can be found on various
boards from Phytec (http://www.phytec.de) like the PCM027,
- PCM038.
-
-config CAN_SJA1000_OF_PLATFORM
- tristate "Generic OF Platform Bus based SJA1000 driver"
- depends on OF
- ---help---
- This driver adds support for the SJA1000 chips connected to
- the OpenFirmware "platform bus" found on embedded systems with
- OpenFirmware bindings, e.g. if you have a PowerPC based system
- you may want to enable this option.
+ PCM038. It also provides the OpenFirmware "platform bus" found
+ on embedded systems with OpenFirmware bindings, e.g. if you
+ have a PowerPC based system you may want to enable this option.
config CAN_EMS_PCMCIA
tristate "EMS CPC-CARD Card"
diff --git a/drivers/net/can/sja1000/Makefile b/drivers/net/can/sja1000/Makefile
index b3d05cb..531d5fc 100644
--- a/drivers/net/can/sja1000/Makefile
+++ b/drivers/net/can/sja1000/Makefile
@@ -5,7 +5,6 @@
obj-$(CONFIG_CAN_SJA1000) += sja1000.o
obj-$(CONFIG_CAN_SJA1000_ISA) += sja1000_isa.o
obj-$(CONFIG_CAN_SJA1000_PLATFORM) += sja1000_platform.o
-obj-$(CONFIG_CAN_SJA1000_OF_PLATFORM) += sja1000_of_platform.o
obj-$(CONFIG_CAN_EMS_PCMCIA) += ems_pcmcia.o
obj-$(CONFIG_CAN_EMS_PCI) += ems_pci.o
obj-$(CONFIG_CAN_KVASER_PCI) += kvaser_pci.o
diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
deleted file mode 100644
index 047accd..0000000
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ /dev/null
@@ -1,221 +0,0 @@
-/*
- * Driver for SJA1000 CAN controllers on the OpenFirmware platform bus
- *
- * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the version 2 of the GNU General Public License
- * as published by the Free Software Foundation
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
- */
-
-/* This is a generic driver for SJA1000 chips on the OpenFirmware platform
- * bus found on embedded PowerPC systems. You need a SJA1000 CAN node
- * definition in your flattened device tree source (DTS) file similar to:
- *
- * can@3,100 {
- * compatible = "nxp,sja1000";
- * reg = <3 0x100 0x80>;
- * interrupts = <2 0>;
- * interrupt-parent = <&mpic>;
- * nxp,external-clock-frequency = <16000000>;
- * };
- *
- * See "Documentation/devicetree/bindings/net/can/sja1000.txt" for further
- * information.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/netdevice.h>
-#include <linux/delay.h>
-#include <linux/io.h>
-#include <linux/can/dev.h>
-
-#include <linux/of_platform.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-
-#include "sja1000.h"
-
-#define DRV_NAME "sja1000_of_platform"
-
-MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
-MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the OF platform bus");
-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);
-}
-
-static void sja1000_ofp_write_reg(const struct sja1000_priv *priv,
- int reg, u8 val)
-{
- iowrite8(val, priv->reg_base + reg);
-}
-
-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;
-}
-
-static int sja1000_ofp_probe(struct platform_device *ofdev)
-{
- struct device_node *np = ofdev->dev.of_node;
- struct net_device *dev;
- struct sja1000_priv *priv;
- struct resource res;
- u32 prop;
- int err, irq, res_size;
- void __iomem *base;
-
- err = of_address_to_resource(np, 0, &res);
- if (err) {
- dev_err(&ofdev->dev, "invalid address\n");
- return err;
- }
-
- res_size = resource_size(&res);
-
- if (!request_mem_region(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);
- if (!base) {
- dev_err(&ofdev->dev, "couldn't ioremap %pR\n", &res);
- err = -ENOMEM;
- goto exit_release_mem;
- }
-
- 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;
- }
-
- dev = alloc_sja1000dev(0);
- if (!dev) {
- err = -ENOMEM;
- goto exit_dispose_irq;
- }
-
- priv = netdev_priv(dev);
-
- priv->read_reg = sja1000_ofp_read_reg;
- priv->write_reg = sja1000_ofp_write_reg;
-
- err = of_property_read_u32(np, "nxp,external-clock-frequency", &prop);
- if (!err)
- priv->can.clock.freq = prop / 2;
- else
- priv->can.clock.freq = SJA1000_OFP_CAN_CLOCK; /* default */
-
- err = of_property_read_u32(np, "nxp,tx-output-mode", &prop);
- if (!err)
- priv->ocr |= prop & OCR_MODE_MASK;
- else
- priv->ocr |= OCR_MODE_NORMAL; /* default */
-
- err = of_property_read_u32(np, "nxp,tx-output-config", &prop);
- if (!err)
- priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
- else
- priv->ocr |= OCR_TX0_PULLDOWN; /* default */
-
- err = of_property_read_u32(np, "nxp,clock-out-frequency", &prop);
- if (!err && prop) {
- u32 divider = priv->can.clock.freq * 2 / prop;
-
- if (divider > 1)
- priv->cdr |= divider / 2 - 1;
- else
- priv->cdr |= CDR_CLKOUT_MASK;
- } else {
- priv->cdr |= CDR_CLK_OFF; /* default */
- }
-
- if (!of_property_read_bool(np, "nxp,no-comparator-bypass"))
- priv->cdr |= CDR_CBP; /* default */
-
- priv->irq_flags = IRQF_SHARED;
- priv->reg_base = base;
-
- dev->irq = irq;
-
- dev_info(&ofdev->dev,
- "reg_base=0x%p irq=%d clock=%d ocr=0x%02x cdr=0x%02x\n",
- priv->reg_base, dev->irq, priv->can.clock.freq,
- priv->ocr, priv->cdr);
-
- platform_set_drvdata(ofdev, dev);
- SET_NETDEV_DEV(dev, &ofdev->dev);
-
- err = register_sja1000dev(dev);
- if (err) {
- dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
- DRV_NAME, err);
- goto exit_free_sja1000;
- }
-
- return 0;
-
-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;
-}
-
-static struct of_device_id sja1000_ofp_table[] = {
- {.compatible = "nxp,sja1000"},
- {},
-};
-MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
-
-static struct platform_driver sja1000_ofp_driver = {
- .driver = {
- .owner = THIS_MODULE,
- .name = DRV_NAME,
- .of_match_table = sja1000_ofp_table,
- },
- .probe = sja1000_ofp_probe,
- .remove = sja1000_ofp_remove,
-};
-
-module_platform_driver(sja1000_ofp_driver);
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 779679a..50dece8 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -27,12 +27,16 @@
#include <linux/can/dev.h>
#include <linux/can/platform/sja1000.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
#include "sja1000.h"
#define DRV_NAME "sja1000_platform"
+#define SP_CAN_CLOCK (16000000 / 2)
MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
+MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
MODULE_ALIAS("platform:" DRV_NAME);
MODULE_LICENSE("GPL v2");
@@ -67,24 +71,92 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val)
iowrite8(val, priv->reg_base + reg * 4);
}
-static int sp_probe(struct platform_device *pdev)
+static void sp_populate(struct sja1000_priv *priv,
+ struct sja1000_platform_data *pdata,
+ unsigned long resource_mem_flags)
+{
+ /* The CAN clock frequency is half the oscillator clock frequency */
+ priv->can.clock.freq = pdata->osc_freq / 2;
+ priv->ocr = pdata->ocr;
+ priv->cdr = pdata->cdr;
+
+ switch (resource_mem_flags & IORESOURCE_MEM_TYPE_MASK) {
+ case IORESOURCE_MEM_32BIT:
+ priv->read_reg = sp_read_reg32;
+ priv->write_reg = sp_write_reg32;
+ break;
+ case IORESOURCE_MEM_16BIT:
+ priv->read_reg = sp_read_reg16;
+ priv->write_reg = sp_write_reg16;
+ break;
+ case IORESOURCE_MEM_8BIT:
+ default:
+ priv->read_reg = sp_read_reg8;
+ priv->write_reg = sp_write_reg8;
+ break;
+ }
+}
+
+static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
{
int err;
+ u32 prop;
+
+ priv->read_reg = sp_read_reg8;
+ priv->write_reg = sp_write_reg8;
+
+ err = of_property_read_u32(of, "nxp,external-clock-frequency", &prop);
+ if (!err)
+ priv->can.clock.freq = prop / 2;
+ else
+ priv->can.clock.freq = SP_CAN_CLOCK; /* default */
+
+ err = of_property_read_u32(of, "nxp,tx-output-mode", &prop);
+ if (!err)
+ priv->ocr |= prop & OCR_MODE_MASK;
+ else
+ priv->ocr |= OCR_MODE_NORMAL; /* default */
+
+ err = of_property_read_u32(of, "nxp,tx-output-config", &prop);
+ if (!err)
+ priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
+ else
+ priv->ocr |= OCR_TX0_PULLDOWN; /* default */
+
+ err = of_property_read_u32(of, "nxp,clock-out-frequency", &prop);
+ if (!err && prop) {
+ u32 divider = priv->can.clock.freq * 2 / prop;
+
+ if (divider > 1)
+ priv->cdr |= divider / 2 - 1;
+ else
+ priv->cdr |= CDR_CLKOUT_MASK;
+ } else {
+ priv->cdr |= CDR_CLK_OFF; /* default */
+ }
+
+ if (!of_property_read_bool(of, "nxp,no-comparator-bypass"))
+ priv->cdr |= CDR_CBP; /* default */
+}
+
+static int sp_probe(struct platform_device *pdev)
+{
+ int err, irq = 0;
void __iomem *addr;
struct net_device *dev;
struct sja1000_priv *priv;
- struct resource *res_mem, *res_irq;
+ struct resource *res_mem, *res_irq = NULL;
struct sja1000_platform_data *pdata;
+ struct device_node *of = pdev->dev.of_node;
pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
+ if (!pdata && !of) {
dev_err(&pdev->dev, "No platform data provided!\n");
return -ENODEV;
}
res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!res_mem || !res_irq)
+ if (!res_mem)
return -ENODEV;
if (!devm_request_mem_region(&pdev->dev, res_mem->start,
@@ -96,36 +168,35 @@ static int sp_probe(struct platform_device *pdev)
if (!addr)
return -ENOMEM;
+ if (of)
+ irq = irq_of_parse_and_map(of, 0);
+ else
+ res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+
+ if (!irq && !res_irq)
+ return -ENODEV;
+
dev = alloc_sja1000dev(0);
if (!dev)
return -ENOMEM;
priv = netdev_priv(dev);
- dev->irq = res_irq->start;
- priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
- if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
- priv->irq_flags |= IRQF_SHARED;
+ if (res_irq) {
+ irq = res_irq->start;
+ priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
+ if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
+ priv->irq_flags |= IRQF_SHARED;
+ } else {
+ priv->irq_flags = IRQF_SHARED;
+ }
+
+ dev->irq = irq;
priv->reg_base = addr;
- /* The CAN clock frequency is half the oscillator clock frequency */
- priv->can.clock.freq = pdata->osc_freq / 2;
- priv->ocr = pdata->ocr;
- priv->cdr = pdata->cdr;
- switch (res_mem->flags & IORESOURCE_MEM_TYPE_MASK) {
- case IORESOURCE_MEM_32BIT:
- priv->read_reg = sp_read_reg32;
- priv->write_reg = sp_write_reg32;
- break;
- case IORESOURCE_MEM_16BIT:
- priv->read_reg = sp_read_reg16;
- priv->write_reg = sp_write_reg16;
- break;
- case IORESOURCE_MEM_8BIT:
- default:
- priv->read_reg = sp_read_reg8;
- priv->write_reg = sp_write_reg8;
- break;
- }
+ if (of)
+ sp_populate_of(priv, of);
+ else
+ sp_populate(priv, pdata, res_mem->flags);
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -156,12 +227,19 @@ static int sp_remove(struct platform_device *pdev)
return 0;
}
+static struct of_device_id sp_of_table[] = {
+ {.compatible = "nxp,sja1000"},
+ {},
+};
+MODULE_DEVICE_TABLE(of, sp_of_table);
+
static struct platform_driver sp_driver = {
.probe = sp_probe,
.remove = sp_remove,
.driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
+ .of_match_table = sp_of_table,
},
};
--
1.8.1.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox