Devicetree
 help / color / mirror / Atom feed
* [PATCHv2 13/16] ARM: OMAP2+: extend iommu pdata-quirks to OMAP5
From: Suman Anna @ 2014-02-13 18:15 UTC (permalink / raw)
  To: Joerg Roedel, Tony Lindgren
  Cc: Florian Vaussard, Laurent Pinchart, iommu, devicetree, linux-omap,
	linux-arm-kernel, Suman Anna
In-Reply-To: <1392315347-32967-1-git-send-email-s-anna@ti.com>

OMAP5 has the same iommus as OMAP4, so extend the OMAP4
iommu pdata quirks for OMAP5 as well.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/pdata-quirks.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 74e094a..551877f 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -193,7 +193,9 @@ static void __init omap4_panda_legacy_init(void)
 	legacy_init_ehci_clk("auxclk3_ck");
 	legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
 }
+#endif
 
+#if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
 static struct iommu_platform_data omap4_iommu_pdata = {
 	.reset_name = "mmu_cache",
 	.assert_reset = omap_device_assert_hardreset,
@@ -264,6 +266,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
 #ifdef CONFIG_ARCH_OMAP4
 	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
 	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
+#endif
+#if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
 	OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
 		       &omap4_iommu_pdata),
 	OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
-- 
1.8.5.3


^ permalink raw reply related

* [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs
From: Suman Anna @ 2014-02-13 18:15 UTC (permalink / raw)
  To: Joerg Roedel, Tony Lindgren
  Cc: Florian Vaussard, Laurent Pinchart, iommu, devicetree, linux-omap,
	linux-arm-kernel, Suman Anna
In-Reply-To: <1392315347-32967-1-git-send-email-s-anna@ti.com>

From: Florian Vaussard <florian.vaussard@epfl.ch>

The irq numbers, ocp address space and device attribute data
have all been cleaned up for OMAP3 IOMMUs. All this data is
populated via the corresponding dt node.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 46 ------------------------------
 1 file changed, 46 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 9c7e23a..d68c131 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -24,7 +24,6 @@
 #include "l4_3xxx.h"
 #include <linux/platform_data/asoc-ti-mcbsp.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
-#include <linux/platform_data/iommu-omap.h>
 #include <linux/platform_data/mailbox-omap.h>
 #include <plat/dmtimer.h>
 
@@ -2991,83 +2990,39 @@ static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
 
 /* mmu isp */
 
-static struct omap_mmu_dev_attr mmu_isp_dev_attr = {
-	.da_start	= 0x0,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 8,
-};
-
 static struct omap_hwmod omap3xxx_mmu_isp_hwmod;
-static struct omap_hwmod_irq_info omap3xxx_mmu_isp_irqs[] = {
-	{ .irq = 24 + OMAP_INTC_START, },
-	{ .irq = -1 }
-};
-
-static struct omap_hwmod_addr_space omap3xxx_mmu_isp_addrs[] = {
-	{
-		.pa_start	= 0x480bd400,
-		.pa_end		= 0x480bd47f,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
 
 /* l4_core -> mmu isp */
 static struct omap_hwmod_ocp_if omap3xxx_l4_core__mmu_isp = {
 	.master		= &omap3xxx_l4_core_hwmod,
 	.slave		= &omap3xxx_mmu_isp_hwmod,
-	.addr		= omap3xxx_mmu_isp_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
 static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
 	.name		= "mmu_isp",
 	.class		= &omap3xxx_mmu_hwmod_class,
-	.mpu_irqs	= omap3xxx_mmu_isp_irqs,
 	.main_clk	= "cam_ick",
-	.dev_attr	= &mmu_isp_dev_attr,
 	.flags		= HWMOD_NO_IDLEST,
 };
 
 /* mmu iva */
 
-static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
-	.da_start	= 0x11000000,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 32,
-};
-
 static struct omap_hwmod omap3xxx_mmu_iva_hwmod;
-static struct omap_hwmod_irq_info omap3xxx_mmu_iva_irqs[] = {
-	{ .irq = 28 + OMAP_INTC_START, },
-	{ .irq = -1 }
-};
-
 static struct omap_hwmod_rst_info omap3xxx_mmu_iva_resets[] = {
 	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
 };
 
-static struct omap_hwmod_addr_space omap3xxx_mmu_iva_addrs[] = {
-	{
-		.pa_start	= 0x5d000000,
-		.pa_end		= 0x5d00007f,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
-
 /* l3_main -> iva mmu */
 static struct omap_hwmod_ocp_if omap3xxx_l3_main__mmu_iva = {
 	.master		= &omap3xxx_l3_main_hwmod,
 	.slave		= &omap3xxx_mmu_iva_hwmod,
-	.addr		= omap3xxx_mmu_iva_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
 static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
 	.name		= "mmu_iva",
 	.class		= &omap3xxx_mmu_hwmod_class,
-	.mpu_irqs	= omap3xxx_mmu_iva_irqs,
 	.clkdm_name	= "iva2_clkdm",
 	.rst_lines	= omap3xxx_mmu_iva_resets,
 	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_mmu_iva_resets),
@@ -3080,7 +3035,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
 			.idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
 		},
 	},
-	.dev_attr	= &mmu_iva_dev_attr,
 	.flags		= HWMOD_NO_IDLEST,
 };
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCHv2 15/16] ARM: OMAP4: hwmod data: cleanup data for IOMMUs
From: Suman Anna @ 2014-02-13 18:15 UTC (permalink / raw)
  To: Joerg Roedel, Tony Lindgren
  Cc: Florian Vaussard, Laurent Pinchart, iommu, devicetree, linux-omap,
	linux-arm-kernel, Suman Anna
In-Reply-To: <1392315347-32967-1-git-send-email-s-anna@ti.com>

From: Florian Vaussard <florian.vaussard@epfl.ch>

The device attribute data and ocp address space have all been
cleaned up for OMAP4 iommus. All this data is populated via
the corresponding dt node.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 35 ------------------------------
 1 file changed, 35 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 3318cae9..caca6c2 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -29,7 +29,6 @@
 
 #include <linux/platform_data/spi-omap2-mcspi.h>
 #include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <linux/platform_data/iommu-omap.h>
 #include <plat/dmtimer.h>
 
 #include "omap_hwmod.h"
@@ -2083,32 +2082,16 @@ static struct omap_hwmod_class omap44xx_mmu_hwmod_class = {
 
 /* mmu ipu */
 
-static struct omap_mmu_dev_attr mmu_ipu_dev_attr = {
-	.da_start	= 0x0,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 32,
-};
-
 static struct omap_hwmod omap44xx_mmu_ipu_hwmod;
 static struct omap_hwmod_rst_info omap44xx_mmu_ipu_resets[] = {
 	{ .name = "mmu_cache", .rst_shift = 2 },
 };
 
-static struct omap_hwmod_addr_space omap44xx_mmu_ipu_addrs[] = {
-	{
-		.pa_start	= 0x55082000,
-		.pa_end		= 0x550820ff,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
-
 /* l3_main_2 -> mmu_ipu */
 static struct omap_hwmod_ocp_if omap44xx_l3_main_2__mmu_ipu = {
 	.master		= &omap44xx_l3_main_2_hwmod,
 	.slave		= &omap44xx_mmu_ipu_hwmod,
 	.clk		= "l3_div_ck",
-	.addr		= omap44xx_mmu_ipu_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
@@ -2127,37 +2110,20 @@ static struct omap_hwmod omap44xx_mmu_ipu_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.dev_attr	= &mmu_ipu_dev_attr,
 };
 
 /* mmu dsp */
 
-static struct omap_mmu_dev_attr mmu_dsp_dev_attr = {
-	.da_start	= 0x0,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 32,
-};
-
 static struct omap_hwmod omap44xx_mmu_dsp_hwmod;
 static struct omap_hwmod_rst_info omap44xx_mmu_dsp_resets[] = {
 	{ .name = "mmu_cache", .rst_shift = 1 },
 };
 
-static struct omap_hwmod_addr_space omap44xx_mmu_dsp_addrs[] = {
-	{
-		.pa_start	= 0x4a066000,
-		.pa_end		= 0x4a0660ff,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
-
 /* l4_cfg -> dsp */
 static struct omap_hwmod_ocp_if omap44xx_l4_cfg__mmu_dsp = {
 	.master		= &omap44xx_l4_cfg_hwmod,
 	.slave		= &omap44xx_mmu_dsp_hwmod,
 	.clk		= "l4_div_ck",
-	.addr		= omap44xx_mmu_dsp_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
@@ -2176,7 +2142,6 @@ static struct omap_hwmod omap44xx_mmu_dsp_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.dev_attr	= &mmu_dsp_dev_attr,
 };
 
 /*
-- 
1.8.5.3


^ permalink raw reply related

* [PATCHv2 16/16] ARM: OMAP2+: Remove legacy omap-iommu.c
From: Suman Anna @ 2014-02-13 18:15 UTC (permalink / raw)
  To: Joerg Roedel, Tony Lindgren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Florian Vaussard,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1392315347-32967-1-git-send-email-s-anna-l0cyMroinI0@public.gmane.org>

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

With full DT boot, the legacy mode of platform device creation
for OMAP IOMMUs is not needed anymore.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
---
 arch/arm/mach-omap2/Makefile     |  3 --
 arch/arm/mach-omap2/omap-iommu.c | 79 ----------------------------------------
 2 files changed, 82 deletions(-)
 delete mode 100644 arch/arm/mach-omap2/omap-iommu.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index e6eec6f..242933f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -226,9 +226,6 @@ obj-$(CONFIG_SOC_DRA7XX)		+= omap_hwmod_7xx_data.o
 obj-$(CONFIG_OMAP3_EMU)			+= emu.o
 obj-$(CONFIG_HW_PERF_EVENTS)		+= pmu.o
 
-iommu-$(CONFIG_OMAP_IOMMU)		:= omap-iommu.o
-obj-y					+= $(iommu-m) $(iommu-y)
-
 ifneq ($(CONFIG_TIDSPBRIDGE),)
 obj-y					+= dsp.o
 endif
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
deleted file mode 100644
index f1fab56..0000000
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * omap iommu: omap device registration
- *
- * Copyright (C) 2008-2009 Nokia Corporation
- *
- * Written by Hiroshi DOYU <Hiroshi.DOYU-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/of.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/err.h>
-#include <linux/slab.h>
-
-#include <linux/platform_data/iommu-omap.h>
-#include "soc.h"
-#include "omap_hwmod.h"
-#include "omap_device.h"
-
-static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
-{
-	struct platform_device *pdev;
-	struct iommu_platform_data *pdata;
-	struct omap_mmu_dev_attr *a = (struct omap_mmu_dev_attr *)oh->dev_attr;
-	static int i;
-
-	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return -ENOMEM;
-
-	pdata->name = oh->name;
-	pdata->nr_tlb_entries = a->nr_tlb_entries;
-	pdata->da_start = a->da_start;
-	pdata->da_end = a->da_end;
-
-	if (oh->rst_lines_cnt == 1) {
-		pdata->reset_name = oh->rst_lines->name;
-		pdata->assert_reset = omap_device_assert_hardreset;
-		pdata->deassert_reset = omap_device_deassert_hardreset;
-	}
-
-	pdev = omap_device_build("omap-iommu", i, oh, pdata, sizeof(*pdata));
-
-	kfree(pdata);
-
-	if (IS_ERR(pdev)) {
-		pr_err("%s: device build err: %ld\n", __func__, PTR_ERR(pdev));
-		return PTR_ERR(pdev);
-	}
-
-	i++;
-
-	return 0;
-}
-
-static int __init omap_iommu_init(void)
-{
-	/* If dtb is there, the devices will be created dynamically */
-	if (of_have_populated_dt())
-		return -ENODEV;
-
-	return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
-}
-/* must be ready before omap3isp is probed */
-omap_subsys_initcall(omap_iommu_init);
-
-static void __exit omap_iommu_exit(void)
-{
-	/* Do nothing */
-}
-module_exit(omap_iommu_exit);
-
-MODULE_AUTHOR("Hiroshi DOYU");
-MODULE_DESCRIPTION("omap iommu: omap device registration");
-MODULE_LICENSE("GPL v2");
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH 0/4] OMAP IOMMU DTS nodes
From: Suman Anna @ 2014-02-13 18:22 UTC (permalink / raw)
  To: Benoit Cousson, Tony Lindgren
  Cc: Florian Vaussard, Laurent Pinchart, devicetree, iommu, linux-omap,
	linux-arm-kernel, Suman Anna

Hi,

This series includes patches that adds the iommu DT nodes on
OMAP3 (IVA), and OMAP4 and OMAP5 SoCs. It also includes an
updated OMAP3 ISP iommu DT node patch posted previously [1].

Posting the series separately from the driver DT adapation
changes [2]. The series adds the DTS patches in line with the
new OMAP IOMMU bindings [3] posted alongside the DT adaptation.

[1] http://marc.info/?l=linux-omap&m=138728486000628&w=2
[2] http://marc.info/?l=linux-omap&m=139231544416973&w=2
[3] http://marc.info/?l=linux-omap&m=139231545116981&w=2

Florian Vaussard (3):
  ARM: dts: OMAP3: Update ISP IOMMU node
  ARM: dts: OMAP3: Add IVA IOMMU node
  ARM: dts: OMAP4: Add IOMMU nodes

Suman Anna (1):
  ARM: dts: OMAP5: Add IOMMU nodes

 arch/arm/boot/dts/omap3.dtsi | 17 ++++++++++++++---
 arch/arm/boot/dts/omap4.dtsi | 17 +++++++++++++++++
 arch/arm/boot/dts/omap5.dtsi | 17 +++++++++++++++++
 3 files changed, 48 insertions(+), 3 deletions(-)

-- 
1.8.5.3


^ permalink raw reply

* [PATCH 1/4] ARM: dts: OMAP3: Update ISP IOMMU node
From: Suman Anna @ 2014-02-13 18:22 UTC (permalink / raw)
  To: Benoit Cousson, Tony Lindgren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Florian Vaussard,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1392315776-33197-1-git-send-email-s-anna-l0cyMroinI0@public.gmane.org>

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

Update the IOMMU node for the camera subsystem as per the
OMAP IOMMU bindings.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
[s-anna-l0cyMroinI0@public.gmane.org: corrected interrupt number]
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/omap3.dtsi | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index a5fc83b..ac91cc3 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -411,10 +411,12 @@
 		};
 
 		mmu_isp: mmu@480bd400 {
-			compatible = "ti,omap3-mmu-isp";
-			ti,hwmods = "mmu_isp";
+			compatible = "ti,omap2-iommu";
 			reg = <0x480bd400 0x80>;
-			interrupts = <8>;
+			interrupts = <24>;
+			ti,hwmods = "mmu_isp";
+			ti,#tlb-entries = <8>;
+			dma-window = <0 0xfffff000>;
 		};
 
 		wdt2: wdt@48314000 {
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH 2/4] ARM: dts: OMAP3: Add IVA IOMMU node
From: Suman Anna @ 2014-02-13 18:22 UTC (permalink / raw)
  To: Benoit Cousson, Tony Lindgren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Florian Vaussard,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1392315776-33197-1-git-send-email-s-anna-l0cyMroinI0@public.gmane.org>

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

Add the DT node for the IOMMU within the DSP subsystem. The entry
is disabled to keep in line with the current hwmod usage.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
[s-anna-l0cyMroinI0@public.gmane.org: split the entry and disable the node]
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/omap3.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index ac91cc3..9607187 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -419,6 +419,15 @@
 			dma-window = <0 0xfffff000>;
 		};
 
+		mmu_iva: mmu@5d000000 {
+			compatible = "ti,omap2-iommu";
+			reg = <0x5d000000 0x80>;
+			interrupts = <28>;
+			ti,hwmods = "mmu_iva";
+			dma-window = <0x11000000 0xeefff000>;
+			status = "disabled";
+		};
+
 		wdt2: wdt@48314000 {
 			compatible = "ti,omap3-wdt";
 			reg = <0x48314000 0x80>;
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH 3/4] ARM: dts: OMAP4: Add IOMMU nodes
From: Suman Anna @ 2014-02-13 18:22 UTC (permalink / raw)
  To: Benoit Cousson, Tony Lindgren
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Florian Vaussard,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1392315776-33197-1-git-send-email-s-anna-l0cyMroinI0@public.gmane.org>

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

Add the IOMMU nodes for the DSP and IPU subsystems. The external
address space for DSP starts at 0x20000000 in OMAP4 compared to
0x11000000 in OMAP3, and the addresses beyond 0xE0000000 are
private address space for the Cortex-M3 cores in the IPU subsystem.
The MMU within the IPU sub-system also supports a bus error back
capability, not available on the DSP MMU.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
[s-anna-l0cyMroinI0@public.gmane.org: dma-window updates and bus error back addition]
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/omap4.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index d3f8a6e..1885f90 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -461,6 +461,23 @@
 			dma-names = "tx", "rx";
 		};
 
+		mmu_dsp: mmu@4a066000 {
+			compatible = "ti,omap4-iommu";
+			reg = <0x4a066000 0xff>;
+			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "mmu_dsp";
+			dma-window = <0x20000000 0xdffff000>;
+		};
+
+		mmu_ipu: mmu@55082000 {
+			compatible = "ti,omap4-iommu";
+			reg = <0x55082000 0xff>;
+			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "mmu_ipu";
+			dma-window = <0 0xdffff000>;
+			ti,iommu-bus-err-back;
+		};
+
 		wdt2: wdt@4a314000 {
 			compatible = "ti,omap4-wdt", "ti,omap3-wdt";
 			reg = <0x4a314000 0x80>;
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH 4/4] ARM: dts: OMAP5: Add IOMMU nodes
From: Suman Anna @ 2014-02-13 18:22 UTC (permalink / raw)
  To: Benoit Cousson, Tony Lindgren
  Cc: Florian Vaussard, Laurent Pinchart, devicetree, iommu, linux-omap,
	linux-arm-kernel, Suman Anna
In-Reply-To: <1392315776-33197-1-git-send-email-s-anna@ti.com>

The IOMMU DT nodes have been added for the DSP and IPU
subsystems. The MMUs in OMAP5 are identical to those in
OMAP4, including the bus error back capability on IPU.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/boot/dts/omap5.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index a72813a..a78fdaa 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -513,6 +513,23 @@
 			dma-names = "tx", "rx";
 		};
 
+		mmu_dsp: mmu@4a066000 {
+			compatible = "ti,omap4-iommu";
+			reg = <0x4a066000 0xff>;
+			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "mmu_dsp";
+			dma-window = <0x20000000 0xdffff000>;
+		};
+
+		mmu_ipu: mmu@55082000 {
+			compatible = "ti,omap4-iommu";
+			reg = <0x55082000 0xff>;
+			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "mmu_ipu";
+			dma-window = <0 0xdffff000>;
+			ti,iommu-bus-err-back;
+		};
+
 		keypad: keypad@4ae1c000 {
 			compatible = "ti,omap4-keypad";
 			reg = <0x4ae1c000 0x400>;
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH net-next v2 04/10] net: phy: add Broadcom BCM7xxx internal PHY driver
From: Florian Fainelli @ 2014-02-13 18:41 UTC (permalink / raw)
  To: Francois Romieu
  Cc: netdev, David Miller, Kevin Cernekee, devicetree@vger.kernel.org
In-Reply-To: <20140213103454.GA14941@electric-eye.fr.zoreil.com>

Hi Francois,

2014-02-13 2:34 GMT-08:00 Francois Romieu <romieu@fr.zoreil.com>:
> Florian Fainelli <f.fainelli@gmail.com> :
> [...]
>> diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
>> new file mode 100644
>> index 0000000..6aea6e2
>> --- /dev/null
>> +++ b/drivers/net/phy/bcm7xxx.c
> [...]
>> +static int bcm7445_config_init(struct phy_device *phydev)
>> +{
>> +     int ret;
>
> It could be declared after 'i' below.
>
>> +     const struct bcm7445_regs {
>
> static const
>
>> +             int reg;
>> +             u16 value;
>> +     } bcm7445_regs_cfg[] = {
>> +             /* increases ADC latency by 24ns */
>> +             { 0x17, 0x0038 },
>> +             { 0x15, 0xAB95 },
>> +             /* increases internal 1V LDO voltage by 5% */
>> +             { 0x17, 0x2038 },
>> +             { 0x15, 0xBB22 },
>> +             /* reduce RX low pass filter corner frequency */
>> +             { 0x17, 0x6038 },
>> +             { 0x15, 0xFFC5 },
>> +             /* reduce RX high pass filter corner frequency */
>> +             { 0x17, 0x003a },
>> +             { 0x15, 0x2002 },
>> +     };
>> +     unsigned int i;
>> +
>> +     for (i = 0; i < ARRAY_SIZE(bcm7445_regs_cfg); i++) {
>> +             ret = phy_write(phydev,
>> +                             bcm7445_regs_cfg[i].reg,
>> +                             bcm7445_regs_cfg[i].value);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static void phy_write_exp(struct phy_device *phydev,
>> +                                     u16 reg, u16 value)
>
> static void phy_write_exp(struct phy_device *phydev, u16 reg, u16 value)
>
>> +{
>> +     phy_write(phydev, 0x17, 0xf00 | reg);
>> +     phy_write(phydev, 0x15, value);
>> +}
>> +
>> +static void phy_write_misc(struct phy_device *phydev,
>> +                                     u16 reg, u16 chl, u16 value)
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ all tabs that don't line up
>
> static void phy_write_misc(struct phy_device *phydev,
>                            u16 reg, u16 chl, u16 value)
>
> static void phy_write_misc(struct phy_device *phydev, u16 reg, u16 chl,
>                            u16 value)
>
> static void phy_write_misc(struct phy_device *phydev, u16 reg, u16 chl, u16 val)
>
>
>> +{
>> +     int tmp;
>> +
>> +     phy_write(phydev, 0x18, 0x7);
>> +
>> +     tmp = phy_read(phydev, 0x18);
>> +     tmp |= 0x800;
>> +     phy_write(phydev, 0x18, tmp);
>> +
>> +     tmp = (chl * 0x2000) | reg;
>> +     phy_write(phydev, 0x17, tmp);
>> +
>> +     phy_write(phydev, 0x15, value);
>
> You may use some #define for the 0x15, 0x17 and 0x18 values.

Right, those are actually inherited from the BCM54xx PHY driver, I
will move those register to a common location e.g: brcmphy.h

>
>> +}
>> +
>> +static int bcm7xxx_28nm_afe_config_init(struct phy_device *phydev)
>> +{
>> +     /* write AFE_RXCONFIG_0 */
>> +     phy_write_misc(phydev, 0x38, 0x0000, 0xeb19);
>> +
>> +     /* write AFE_RXCONFIG_1 */
>> +     phy_write_misc(phydev, 0x38, 0x0001, 0x9a3f);
>> +
>> +     /* write AFE_RX_LP_COUNTER */
>> +     phy_write_misc(phydev, 0x38, 0x0003, 0x7fc7);
>> +
>> +     /* write AFE_HPF_TRIM_OTHERS */
>> +     phy_write_misc(phydev, 0x3A, 0x0000, 0x000b);
>> +
>> +     /* write AFTE_TX_CONFIG */
>> +     phy_write_misc(phydev, 0x39, 0x0000, 0x0800);
>
> Some #define may be welcome to replace the comments.

I would rather keep those as comments as they might change over time
if I get to incorporate a new workaround sequence.

>
> [...]
>> +static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
>> +{
>> +     int ret;
>> +
>> +     ret = bcm7445_config_init(phydev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return bcm7xxx_28nm_afe_config_init(phydev);
>> +}
>> +
>> +static int phy_set_clr_bits(struct phy_device *dev, int location,
>> +                                     int set_mask, int clr_mask)
>> +{
>> +     int v, ret;
>> +
>> +     v = phy_read(dev, location);
>> +     if (v < 0)
>> +             return v;
>> +
>> +     v &= ~clr_mask;
>> +     v |= set_mask;
>> +
>> +     ret = phy_write(dev, location, v);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return v;
>> +}
>> +
>> +static int bcm7xxx_config_init(struct phy_device *phydev)
>> +{
>> +     /* Enable 64 clock MDIO */
>> +     phy_write(phydev, 0x1d, 0x1000);
>> +     phy_read(phydev, 0x1d);
>> +
>> +     /* Workaround only required for 100Mbits/sec */
>> +     if (!(phydev->dev_flags & PHY_BRCM_100MBPS_WAR))
>> +             return 0;
>> +
>> +     /* set shadow mode 2 */
>> +     phy_set_clr_bits(phydev, 0x1f, 0x0004, 0x0004);
>
> phy_set_clr_bits returned status code is not checked.
>
>> +
>> +     /* set iddq_clkbias */
>> +     phy_write(phydev, 0x14, 0x0F00);
>> +     udelay(10);
>> +
>> +     /* reset iddq_clkbias */
>> +     phy_write(phydev, 0x14, 0x0C00);
>> +
>> +     phy_write(phydev, 0x13, 0x7555);
>> +
>> +     /* reset shadow mode 2 */
>> +     phy_set_clr_bits(phydev, 0x1f, 0x0004, 0);
>
> phy_set_clr_bits returned status code is not checked.
>
>> +
>> +     return 0;
>> +}
>> +
>> +/* Workaround for putting the PHY in IDDQ mode, required
>> + * for all BCM7XXX PHYs
>> + */
>> +static int bcm7xxx_suspend(struct phy_device *phydev)
>
> Factor out with bcm7445_config_init and some helper ?

I would rather keep this function simple like it is today since this
really is only required for entering suspend mode properly while
bcm7445_config_init() as the name suggests is specific to 7445 only.

Thanks for the review!

>
>> +{
>> +     int ret;
>> +     const struct bcm7xxx_regs {
>> +             int reg;
>> +             u16 value;
>> +     } bcm7xxx_suspend_cfg[] = {
>> +             { 0x1f, 0x008b },
>> +             { 0x10, 0x01c0 },
>> +             { 0x14, 0x7000 },
>> +             { 0x1f, 0x000f },
>> +             { 0x10, 0x20d0 },
>> +             { 0x1f, 0x000b },
>> +     };
>> +     unsigned int i;
>
> --
> Ueimor



-- 
Florian

^ permalink raw reply

* Re: [PATCH] of: give priority to the compatible match in __of_match_node()
From: Rob Herring @ 2014-02-13 19:01 UTC (permalink / raw)
  To: Kevin Hao
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxppc-dev,
	Sebastian Hesselbarth, Stephen N Chivers, Chris Proctor,
	Arnd Bergmann, Scott Wood, Grant Likely, Rob Herring
In-Reply-To: <1392205084-2351-1-git-send-email-haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> When the device node do have a compatible property, we definitely
> prefer the compatible match besides the type and name. Only if
> there is no such a match, we then consider the candidate which
> doesn't have compatible entry but do match the type or name with
> the device node.
>
> This is based on a patch from Sebastian Hesselbarth.
>   http://patchwork.ozlabs.org/patch/319434/
>
> I did some code refactoring and also fixed a bug in the original patch.

I'm inclined to just revert this once again and avoid possibly
breaking yet another platform.

However, I think I would like to see this structured differently. We
basically have 2 ways of matching: the existing pre-3.14 way and the
desired match on best compatible only. All new bindings should match
with the new way and the old way needs to be kept for compatibility.
So lets structure the code that way. Search the match table first for
best compatible with name and type NULL, then search the table the old
way. I realize it appears you are doing this, but it is not clear this
is the intent of the code. I would like to see this written as a patch
with commit 105353145eafb3ea919 reverted first and you add a new match
function to call first and then fallback to the existing function.

Rob

>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/of/base.c | 55 +++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ff85450d5683..9d655df458bd 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -730,32 +730,45 @@ out:
>  }
>  EXPORT_SYMBOL(of_find_node_with_property);
>
> +static int of_match_type_or_name(const struct device_node *node,
> +                               const struct of_device_id *m)
> +{
> +       int match = 1;
> +
> +       if (m->name[0])
> +               match &= node->name && !strcmp(m->name, node->name);
> +
> +       if (m->type[0])
> +               match &= node->type && !strcmp(m->type, node->type);
> +
> +       return match;
> +}
> +
>  static
>  const struct of_device_id *__of_match_node(const struct of_device_id *matches,
>                                            const struct device_node *node)
>  {
>         const char *cp;
>         int cplen, l;
> +       const struct of_device_id *m;
> +       int match;
>
>         if (!matches)
>                 return NULL;
>
>         cp = __of_get_property(node, "compatible", &cplen);
> -       do {
> -               const struct of_device_id *m = matches;
> +       while (cp && (cplen > 0)) {
> +               m = matches;
>
>                 /* Check against matches with current compatible string */
>                 while (m->name[0] || m->type[0] || m->compatible[0]) {
> -                       int match = 1;
> -                       if (m->name[0])
> -                               match &= node->name
> -                                       && !strcmp(m->name, node->name);
> -                       if (m->type[0])
> -                               match &= node->type
> -                                       && !strcmp(m->type, node->type);
> -                       if (m->compatible[0])
> -                               match &= cp
> -                                       && !of_compat_cmp(m->compatible, cp,
> +                       if (!m->compatible[0]) {
> +                               m++;
> +                               continue;
> +                       }
> +
> +                       match = of_match_type_or_name(node, m);
> +                       match &= cp && !of_compat_cmp(m->compatible, cp,
>                                                         strlen(m->compatible));
>                         if (match)
>                                 return m;
> @@ -763,12 +776,18 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
>                 }
>
>                 /* Get node's next compatible string */
> -               if (cp) {
> -                       l = strlen(cp) + 1;
> -                       cp += l;
> -                       cplen -= l;
> -               }
> -       } while (cp && (cplen > 0));
> +               l = strlen(cp) + 1;
> +               cp += l;
> +               cplen -= l;
> +       }
> +
> +       m = matches;
> +       /* Check against matches without compatible string */
> +       while (m->name[0] || m->type[0] || m->compatible[0]) {
> +               if (!m->compatible[0] && of_match_type_or_name(node, m))
> +                       return m;
> +               m++;
> +       }
>
>         return NULL;
>  }
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/3] driver core & of: Mark of_nodes of added device as populated
From: Rob Herring @ 2014-02-13 19:15 UTC (permalink / raw)
  To: Pawel Moll
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
	arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Rob Herring,
	Mark Rutland, Ian Campbell, Kumar Gala, Greg Kroah-Hartman,
	Mike Turquette, Jean Delvare, Guenter Roeck, Samuel Ortiz,
	Lee Jones, Arnd Bergmann, Grant Likely, Dmitry Eremin-Solenikov,
	David Woodhouse, Liam Girdwood, Mark Brown
In-Reply-To: <1392137610-27842-2-git-send-email-pawel.moll-5wv7dgnIgG8@public.gmane.org>

On Tue, Feb 11, 2014 at 10:53 AM, Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org> wrote:
> In "Device Tree powered" systems, platform devices are usually
> massively populated with of_platform_populate() call, executed
> at some level of initcalls, either by generic architecture
> or by platform-specific code.
>
> There are situations though where certain devices must be
> created (and bound with drivers) before all the others.
> This presents small challenge in DT-driven systems, as
> devices explicitly created in early code would be created
> again by of_platform_populate().
>
> This patch tries to solve that issue in a generic way,
> adding a "populated" flag which is set in the device_node
> structure when a device is being created in the core.
> Later, of_platform_populate() skips such nodes (and
> its children) in a similar way to the non-available ones.
>
> Signed-off-by: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/base/core.c       |  4 ++++
>  drivers/of/device.c       | 16 ++++++++++++++++
>  drivers/of/platform.c     |  6 ++++--
>  include/linux/of.h        |  6 ++++++
>  include/linux/of_device.h | 11 +++++++++++
>  5 files changed, 41 insertions(+), 2 deletions(-)

Go read Grant's and my comments from your last posting.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] can: xilinx CAN controller support.
From: Marc Kleine-Budde @ 2014-02-13 19:20 UTC (permalink / raw)
  To: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
	linux-can
  Cc: netdev, linux-arm-kernel, linux-kernel, devicetree,
	Kedareswara rao Appana
In-Reply-To: <0dc128ac-e5cd-47bd-9ab1-3a545c3e2043@CO9EHSMHS012.ehs.local>

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

On 02/12/2014 08:10 AM, Kedareswara rao Appana wrote:
> This patch adds xilinx CAN controller support.
> This driver supports both ZYNQ CANPS IP and
> Soft IP AXI CAN controller.
> 
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> This patch is rebased on the 3.14 rc2 kernel.
> Changes for v2:
> - Updated with the review comments.
> - Removed unnecessary debug prints.
> - included tx,rx fifo depths in ZYNQ CANPS case also.
> ---
>  .../devicetree/bindings/net/can/xilinx_can.txt     |   45 +
>  drivers/net/can/Kconfig                            |    7 +
>  drivers/net/can/Makefile                           |    1 +
>  drivers/net/can/xilinx_can.c                       | 1153 ++++++++++++++++++++
>  4 files changed, 1206 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt
>  create mode 100644 drivers/net/can/xilinx_can.c
> 
> diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> new file mode 100644
> index 0000000..0e57103
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> @@ -0,0 +1,45 @@
> +Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings
> +---------------------------------------------------------
> +
> +Required properties:
> +- compatible		: Should be "xlnx,zynq-can-1.00.a" for Zynq CAN
> +			  controllers and "xlnx,axi-can-1.00.a" for Axi CAN
> +			  controllers.
> +- reg			: Physical base address and size of the Axi CAN/Zynq
> +			  CANPS registers map.
> +- interrupts		: Property with a value describing the interrupt
> +			  number.
> +- interrupt-parent	: Must be core interrupt controller
> +- clock-names		: List of input clock names - "ref_clk", "aper_clk"
> +			  (See clock bindings for details. Two clocks are
> +			   required for Zynq CAN. For Axi CAN
> +			   case it is one(ref_clk)).
> +- clocks		: Clock phandles (see clock bindings for details).
> +- tx-fifo-depth		: Can Tx fifo depth.
> +- rx-fifo-depth		: Can Rx fifo depth.
> +
> +
> +Example:
> +
> +For Zynq CANPS Dts file:
> +	zynq_can_0: zynq-can@e0008000 {
> +			compatible = "xlnx,zynq-can-1.00.a";
> +			clocks = <&clkc 19>, <&clkc 36>;
> +			clock-names = "ref_clk", "aper_clk";
> +			reg = <0xe0008000 0x1000>;
> +			interrupts = <0 28 4>;
> +			interrupt-parent = <&intc>;
> +			tx-fifo-depth = <0x40>;
> +			rx-fifo-depth = <0x40>;
> +		};
> +For Axi CAN Dts file:
> +	axi_can_0: axi-can@40000000 {
> +			compatible = "xlnx,axi-can-1.00.a";
> +			clocks = <&clkc 0>;
> +			clock-names = "ref_clk" ;
> +			reg = <0x40000000 0x10000>;
> +			interrupt-parent = <&intc>;
> +			interrupts = <0 59 1>;
> +			tx-fifo-depth = <0x40>;
> +			rx-fifo-depth = <0x40>;
> +		};
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 9e7d95d..b180239 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -125,6 +125,13 @@ config CAN_GRCAN
>  	  endian syntheses of the cores would need some modifications on
>  	  the hardware level to work.
>  
> +config CAN_XILINXCAN
> +	tristate "Xilinx CAN"
> +	depends on ARCH_ZYNQ || MICROBLAZE
> +	---help---
> +	  Xilinx CAN driver. This driver supports both soft AXI CAN IP and
> +	  Zynq CANPS IP.
> +
>  source "drivers/net/can/mscan/Kconfig"
>  
>  source "drivers/net/can/sja1000/Kconfig"
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index c744039..0b8e11e 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
>  obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  obj-$(CONFIG_PCH_CAN)		+= pch_can.o
>  obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
> +obj-$(CONFIG_CAN_XILINXCAN)	+= xilinx_can.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> new file mode 100644
> index 0000000..642e6b4
> --- /dev/null
> +++ b/drivers/net/can/xilinx_can.c
> @@ -0,0 +1,1153 @@
> +/* Xilinx CAN device driver
> + *
> + * Copyright (C) 2012 - 2014 Xilinx, Inc.
> + * Copyright (C) 2009 PetaLogix. All rights reserved.
> + *
> + * Description:
> + * This driver is developed for Axi CAN IP and for Zynq CANPS Controller.
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/skbuff.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/can/led.h>
> +
> +#define DRIVER_NAME	"XILINX_CAN"
> +
> +/* CAN registers set */
> +#define XCAN_SRR_OFFSET			0x00 /* Software reset */
> +#define XCAN_MSR_OFFSET			0x04 /* Mode select */
> +#define XCAN_BRPR_OFFSET		0x08 /* Baud rate prescaler */
> +#define XCAN_BTR_OFFSET			0x0C /* Bit timing */
> +#define XCAN_ECR_OFFSET			0x10 /* Error counter */
> +#define XCAN_ESR_OFFSET			0x14 /* Error status */
> +#define XCAN_SR_OFFSET			0x18 /* Status */
> +#define XCAN_ISR_OFFSET			0x1C /* Interrupt status */
> +#define XCAN_IER_OFFSET			0x20 /* Interrupt enable */
> +#define XCAN_ICR_OFFSET			0x24 /* Interrupt clear */
> +#define XCAN_TXFIFO_ID_OFFSET		0x30 /* TX FIFO ID */
> +#define XCAN_TXFIFO_DLC_OFFSET		0x34 /* TX FIFO DLC */
> +#define XCAN_TXFIFO_DW1_OFFSET		0x38 /* TX FIFO Data Word 1 */
> +#define XCAN_TXFIFO_DW2_OFFSET		0x3C /* TX FIFO Data Word 2 */
> +#define XCAN_RXFIFO_ID_OFFSET		0x50 /* RX FIFO ID */
> +#define XCAN_RXFIFO_DLC_OFFSET		0x54 /* RX FIFO DLC */
> +#define XCAN_RXFIFO_DW1_OFFSET		0x58 /* RX FIFO Data Word 1 */
> +#define XCAN_RXFIFO_DW2_OFFSET		0x5C /* RX FIFO Data Word 2 */

Can you define all register offsets via an enum please.

> +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */
> +#define XCAN_SRR_CEN_MASK		0x00000002 /* CAN enable */
> +#define XCAN_SRR_RESET_MASK		0x00000001 /* Soft Reset the CAN core */
> +#define XCAN_MSR_LBACK_MASK		0x00000002 /* Loop back mode select */
> +#define XCAN_MSR_SLEEP_MASK		0x00000001 /* Sleep mode select */
> +#define XCAN_BRPR_BRP_MASK		0x000000FF /* Baud rate prescaler */
> +#define XCAN_BTR_SJW_MASK		0x00000180 /* Synchronous jump width */
> +#define XCAN_BTR_TS2_MASK		0x00000070 /* Time segment 2 */
> +#define XCAN_BTR_TS1_MASK		0x0000000F /* Time segment 1 */
> +#define XCAN_ECR_REC_MASK		0x0000FF00 /* Receive error counter */
> +#define XCAN_ECR_TEC_MASK		0x000000FF /* Transmit error counter */
> +#define XCAN_ESR_ACKER_MASK		0x00000010 /* ACK error */
> +#define XCAN_ESR_BERR_MASK		0x00000008 /* Bit error */
> +#define XCAN_ESR_STER_MASK		0x00000004 /* Stuff error */
> +#define XCAN_ESR_FMER_MASK		0x00000002 /* Form error */
> +#define XCAN_ESR_CRCER_MASK		0x00000001 /* CRC error */
> +#define XCAN_SR_TXFLL_MASK		0x00000400 /* TX FIFO is full */
> +#define XCAN_SR_ESTAT_MASK		0x00000180 /* Error status */
> +#define XCAN_SR_ERRWRN_MASK		0x00000040 /* Error warning */
> +#define XCAN_SR_NORMAL_MASK		0x00000008 /* Normal mode */
> +#define XCAN_SR_LBACK_MASK		0x00000002 /* Loop back mode */
> +#define XCAN_SR_CONFIG_MASK		0x00000001 /* Configuration mode */
> +#define XCAN_IXR_TXFEMP_MASK		0x00004000 /* TX FIFO Empty */
> +#define XCAN_IXR_WKUP_MASK		0x00000800 /* Wake up interrupt */
> +#define XCAN_IXR_SLP_MASK		0x00000400 /* Sleep interrupt */
> +#define XCAN_IXR_BSOFF_MASK		0x00000200 /* Bus off interrupt */
> +#define XCAN_IXR_ERROR_MASK		0x00000100 /* Error interrupt */
> +#define XCAN_IXR_RXNEMP_MASK		0x00000080 /* RX FIFO NotEmpty intr */
> +#define XCAN_IXR_RXOFLW_MASK		0x00000040 /* RX FIFO Overflow intr */
> +#define XCAN_IXR_RXOK_MASK		0x00000010 /* Message received intr */
> +#define XCAN_IXR_TXOK_MASK		0x00000002 /* TX successful intr */
> +#define XCAN_IXR_ARBLST_MASK		0x00000001 /* Arbitration lost intr */
> +#define XCAN_IDR_ID1_MASK		0xFFE00000 /* Standard msg identifier */
> +#define XCAN_IDR_SRR_MASK		0x00100000 /* Substitute remote TXreq */
> +#define XCAN_IDR_IDE_MASK		0x00080000 /* Identifier extension */
> +#define XCAN_IDR_ID2_MASK		0x0007FFFE /* Extended message ident */
> +#define XCAN_IDR_RTR_MASK		0x00000001 /* Remote TX request */
> +#define XCAN_DLCR_DLC_MASK		0xF0000000 /* Data length code */
> +
> +#define XCAN_INTR_ALL		(XCAN_IXR_TXOK_MASK | XCAN_IXR_BSOFF_MASK |\
> +				 XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK | \
> +				 XCAN_IXR_RXNEMP_MASK | XCAN_IXR_ERROR_MASK | \
> +				 XCAN_IXR_ARBLST_MASK | XCAN_IXR_RXOK_MASK)
> +
> +/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
> +#define XCAN_BTR_SJW_SHIFT		7  /* Synchronous jump width */
> +#define XCAN_BTR_TS2_SHIFT		4  /* Time segment 2 */
> +#define XCAN_IDR_ID1_SHIFT		21 /* Standard Messg Identifier */
> +#define XCAN_IDR_ID2_SHIFT		1  /* Extended Message Identifier */
> +#define XCAN_DLCR_DLC_SHIFT		28 /* Data length code */
> +#define XCAN_ESR_REC_SHIFT		8  /* Rx Error Count */
> +
> +/* CAN frame length constants */
> +#define XCAN_ECHO_SKB_MAX		64
> +#define XCAN_FRAME_MAX_DATA_LEN		8
> +#define XCAN_TIMEOUT			(50 * HZ)

This is 50 seconds, is this intentional?

> +
> +/**
> + * struct xcan_priv - This definition define CAN driver instance
> + * @can:			CAN private data structure.
> + * @open_time:			For holding timeout values

Please remove open_time completely from the driver.

> + * @waiting_ech_skb_index:	Pointer for skb
> + * @ech_skb_next:		This tell the next packet in the queue
> + * @waiting_ech_skb_num:	Gives the number of packets waiting
> + * @xcan_echo_skb_max_tx:	Maximum number packets the driver can send
> + * @xcan_echo_skb_max_rx:	Maximum number packets the driver can receive
> + * @napi:			NAPI structure
> + * @ech_skb_lock:		For spinlock purpose
> + * @read_reg:			For reading data from CAN registers
> + * @write_reg:			For writing data to CAN registers
> + * @dev:			Network device data structure
> + * @reg_base:			Ioremapped address to registers
> + * @irq_flags:			For request_irq()
> + * @aperclk:			Pointer to struct clk
> + * @devclk:			Pointer to struct clk
> + */
> +struct xcan_priv {
> +	struct can_priv can;
> +	int open_time;
> +	int waiting_ech_skb_index;
> +	int ech_skb_next;

please make them:

unsigned int tx_head;
unsigned int tx_tail;

I'll explain how to use them later. Have a look at the ti_hecc driver.

> +	int waiting_ech_skb_num;
> +	int xcan_echo_skb_max_tx;
> +	int xcan_echo_skb_max_rx;
> +	struct napi_struct napi;
> +	spinlock_t ech_skb_lock;
> +	u32 (*read_reg)(const struct xcan_priv *priv, int reg);
> +	void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);

Please remove read_reg, write_reg, as long as there isn't any BE support
in the driver, call them directly.

> +	struct net_device *dev;
> +	void __iomem *reg_base;
> +	unsigned long irq_flags;
> +	struct clk *aperclk;
> +	struct clk *devclk;
> +};
> +
> +/* CAN Bittiming constants as per Xilinx CAN specs */
> +static const struct can_bittiming_const xcan_bittiming_const = {
> +	.name = DRIVER_NAME,
> +	.tseg1_min = 1,
> +	.tseg1_max = 16,
> +	.tseg2_min = 1,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 256,
> +	.brp_inc = 1,
> +};
> +
> +/**
> + * xcan_write_reg - Write a value to the device register
> + * @priv:	Driver private data structure
> + * @reg:	Register offset
> + * @val:	Value to write at the Register offset
> + *
> + * Write data to the paricular CAN register
> + */
> +static void xcan_write_reg(const struct xcan_priv *priv, int reg, u32 val)

Please use the enum for instead of an int for the reg.

> +{
> +	writel(val, priv->reg_base + reg);
> +}
> +
> +/**
> + * xcan_read_reg - Read a value from the device register
> + * @priv:	Driver private data structure
> + * @reg:	Register offset
> + *
> + * Read data from the particular CAN register
> + * Return: value read from the CAN register
> + */
> +static u32 xcan_read_reg(const struct xcan_priv *priv, int reg)
> +{

same here

> +	return readl(priv->reg_base + reg);
> +}
> +
> +/**
> + * set_reset_mode - Resets the CAN device mode
> + * @ndev:	Pointer to net_device structure
> + *
> + * This is the driver reset mode routine.The driver
> + * enters into configuration mode.
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int set_reset_mode(struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	unsigned long timeout;
> +
> +	priv->can.state = CAN_STATE_STOPPED;
> +
> +	timeout = jiffies + XCAN_TIMEOUT;
> +	while (!(priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_CONFIG_MASK)) {
> +		if (time_after(jiffies, timeout)) {
> +			netdev_warn(ndev, "timedout waiting for config mode\n");
> +			return -ETIMEDOUT;
> +		}
> +		usleep_range(500, 10000);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * xcan_set_bittiming - CAN set bit timing routine
> + * @ndev:	Pointer to net_device structure
> + *
> + * This is the driver set bittiming  routine.
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_set_bittiming(struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	u32 btr0, btr1;
> +	u32 is_config_mode;
> +
> +	/* Check whether Xilinx CAN is in configuration mode.
> +	 * It cannot set bit timing if Xilinx CAN is not in configuration mode.
> +	 */
> +	is_config_mode = priv->read_reg(priv, XCAN_SR_OFFSET) &
> +				XCAN_SR_CONFIG_MASK;
> +	if (!is_config_mode) {
> +		netdev_alert(ndev,
> +			"Cannot set bittiming can is not in config mode\n");
> +		return -EPERM;
> +	}
> +
> +	/* Setting Baud Rate prescalar value in BRPR Register */
> +	btr0 = (bt->brp - 1) & XCAN_BRPR_BRP_MASK;
> +
> +	/* Setting Time Segment 1 in BTR Register */
> +	btr1 = (bt->prop_seg + bt->phase_seg1 - 1) & XCAN_BTR_TS1_MASK;
> +
> +	/* Setting Time Segment 2 in BTR Register */
> +	btr1 |= ((bt->phase_seg2 - 1) << XCAN_BTR_TS2_SHIFT) &
> +		XCAN_BTR_TS2_MASK;
> +
> +	/* Setting Synchronous jump width in BTR Register */
> +	btr1 |= ((bt->sjw - 1) << XCAN_BTR_SJW_SHIFT) & XCAN_BTR_SJW_MASK;

All the masking should not be needed, as the bit timing is calculated
within the bounds you specified.

> +	priv->write_reg(priv, XCAN_BRPR_OFFSET, btr0);
> +	priv->write_reg(priv, XCAN_BTR_OFFSET, btr1);
> +
> +	netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n",
> +			priv->read_reg(priv, XCAN_BRPR_OFFSET),
> +			priv->read_reg(priv, XCAN_BTR_OFFSET));
> +
> +	return 0;
> +}
> +
> +/**
> + * xcan_start - This the drivers start routine
> + * @ndev:	Pointer to net_device structure
> + *
> + * This is the drivers start routine.
> + * Based on the State of the CAN device it puts
> + * the CAN device into a proper mode.
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_start(struct net_device *ndev)

Please name the function xcan_chip_start(), to for a common naming like
the flexcan and at91 driver.

> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	u32 err;
> +	unsigned long timeout;
> +
> +	/* Check if it is in reset mode */
> +	if (priv->can.state != CAN_STATE_STOPPED)

Don't depend on any state here, I suggest to do a softreset (or
equivalent) of you CAN core and configure everything.

> +		err = set_reset_mode(ndev);
> +		if (err < 0)
> +			return err;
> +
> +	/* Enable interrupts */
> +	priv->write_reg(priv, XCAN_IER_OFFSET, XCAN_INTR_ALL);
> +
> +	/* Check whether it is loopback mode or normal mode  */
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
> +		/* Put device into loopback mode */
> +		priv->write_reg(priv, XCAN_MSR_OFFSET, XCAN_MSR_LBACK_MASK);
> +	else
> +		/* The device is in normal mode */
> +		priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> +
> +	if (priv->can.state == CAN_STATE_STOPPED) {
> +		/* Enable Xilinx CAN */
> +		priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +		timeout = jiffies + XCAN_TIMEOUT;
> +		if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> +			while ((priv->read_reg(priv, XCAN_SR_OFFSET)
> +					& XCAN_SR_LBACK_MASK) == 0) {
> +				if (time_after(jiffies, timeout)) {
> +					netdev_warn(ndev,
> +						"timedout for loopback mode\n");
> +					return -ETIMEDOUT;
> +				}
> +				usleep_range(500, 10000);
> +			}
> +		} else {
> +			while ((priv->read_reg(priv, XCAN_SR_OFFSET)
> +					& XCAN_SR_NORMAL_MASK) == 0) {
> +				if (time_after(jiffies, timeout)) {
> +					netdev_warn(ndev,
> +						"timedout for normal mode\n");
> +					return -ETIMEDOUT;
> +				}
> +				usleep_range(500, 10000);
> +			}
> +		}
> +		netdev_dbg(ndev, "status:#x%08x\n",
> +				priv->read_reg(priv, XCAN_SR_OFFSET));
> +	}
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +	return 0;
> +}
> +
> +/**
> + * xcan_do_set_mode - This sets the mode of the driver
> + * @ndev:	Pointer to net_device structure
> + * @mode:	Tells the mode of the driver
> + *
> + * This check the drivers state and calls the
> + * the corresponding modes to set.
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	int ret;
> +
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		ret = xcan_start(ndev);
> +		if (ret < 0)
> +			netdev_err(ndev, "xcan_start failed!\n");
> +		netif_wake_queue(ndev);
> +		break;
> +	default:
> +		ret = -EOPNOTSUPP;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * xcan_start_xmit - Starts the transmission
> + * @skb:	sk_buff pointer that contains data to be Txed
> + * @ndev:	Pointer to net_device structure
> + *
> + * This function is invoked from upper layers to initiate transmission. This
> + * function uses the next available free txbuff and populates their fields to
> + * start the transmission.
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	u32 id, dlc, data[2] = {0, 0}, rtr = 0;

I think you can drop the rtr varibale and use cf->can_id & CAN_RTR_FLAG
instead.

> +	unsigned long flags;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;
> +
> +	/* Watch carefully on the bit sequence */
> +	if (cf->can_id & CAN_EFF_FLAG) {
> +		/* Extended CAN ID format */
> +		id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT) &
> +			XCAN_IDR_ID2_MASK;
> +		id |= (((cf->can_id & CAN_EFF_MASK) >>
> +			(CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
> +			XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK;
> +
> +		/* The substibute remote TX request bit should be "1"
> +		 * for extended frames as in the Xilinx CAN datasheet
> +		 */
> +		id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK;
> +
> +		if (cf->can_id & CAN_RTR_FLAG) {
> +			/* Extended frames remote TX request */
> +			id |= XCAN_IDR_RTR_MASK;
> +			rtr = 1;
> +		}
> +	} else {
> +		/* Standard CAN ID format */
> +		id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT) &
> +			XCAN_IDR_ID1_MASK;
> +
> +		if (cf->can_id & CAN_RTR_FLAG) {
> +			/* Extended frames remote TX request */
> +			id |= XCAN_IDR_SRR_MASK;
> +			rtr = 1;
> +		}
> +	}
> +
> +	dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT;

No need to mask dlc, it's valid.

> +
> +	if (dlc > 0)

You've copied my speudo code :)
But you have to use (cf->can_dlc > 0) here, as dlc is the shifted value.

> +		data[0] = be32_to_cpup((__be32 *)(cf->data + 0));
> +	if (dlc > 4)
> +		data[1] = be32_to_cpup((__be32 *)(cf->data + 4));
> +
> +	can_put_echo_skb(skb, ndev, priv->ech_skb_next);

	can_put_echo_skb(skb, ndev,
		priv->tx_head % priv->xcan_echo_skb_max_tx);

	priv->tx_head++;

> +
> +	/* Write the Frame to Xilinx CAN TX FIFO */
> +	priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id);
> +	priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
> +	if (!rtr) {
> +		priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data[0]);
> +		priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data[1]);
> +		stats->tx_bytes += cf->can_dlc;

Please add a comment which write triggers the tx. What in case of the
rtr? Which write triggers the tx then?

> +	}
> +
> +	priv->ech_skb_next = (priv->ech_skb_next + 1) %
> +					priv->xcan_echo_skb_max_tx;

Please remove, it's not needed.

> +
> +	spin_lock_irqsave(&priv->ech_skb_lock, flags);
> +	priv->waiting_ech_skb_num++;
> +	spin_unlock_irqrestore(&priv->ech_skb_lock, flags);

All 3 not needed.

> +
> +	/* Check if the TX buffer is full */
> +	if (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TXFLL_MASK) {
> +		netif_stop_queue(ndev);
> +		netdev_err(ndev, "TX register is still full!\n");
> +		return NETDEV_TX_BUSY;

If this is true, there is a Bug in the flow control. It should be moved
to the beginning of the function, see at91_can's xmit function.

	/* Check if the TX buffer is full */
	if (unlikely(priv->read_reg(priv, XCAN_SR_OFFSET) &
		XCAN_SR_TXFLL_MASK)) {
		netif_stop_queue(ndev);
		netdev_err(ndev,
			"BUG!, TX FIFO full when queue awake!\n");
		return NETDEV_TX_BUSY;
	}

> +	} else if (priv->waiting_ech_skb_num == priv->xcan_echo_skb_max_tx) {
> +		netif_stop_queue(ndev);
> +		netdev_err(ndev, "waiting:0x%08x, max:0x%08x\n",
> +			priv->waiting_ech_skb_num, priv->xcan_echo_skb_max_tx);
> +		return NETDEV_TX_BUSY;
> +	}

This is a the regular flow control function and must be called before a
TX complete interrupt can trigger. Your tx-complete interrupt is
probably always enabled?

So here you check the fill level of the FIFO:

	if ((priv->tx_head - priv->tx_tail) ==
			priv->xcan_echo_skb_max_tx)
		netif_stop_queue(ndev);

If it's full, stop the queue. The you trigger the tx, the tx complete
interrupt gets called and the queue will be restarted.

> +
> +	return NETDEV_TX_OK;
> +}
> +
> +/**
> + * xcan_rx -  Is called from CAN isr to complete the received
> + *		frame  processing
> + * @ndev:	Pointer to net_device structure
> + *
> + * This function is invoked from the CAN isr(poll) to process the Rx frames. It
> + * does minimal processing and invokes "netif_receive_skb" to complete further
> + * processing.
> + * Return: 0 on success and negative error value on error
> + */
> +static int xcan_rx(struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 id_xcan, dlc, data[2] = {0, 0}, rtr = 0;
> +
> +	skb = alloc_can_skb(ndev, &cf);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	/* Read a frame from Xilinx zynq CANPS */
> +	id_xcan = priv->read_reg(priv, XCAN_RXFIFO_ID_OFFSET);
> +	dlc = priv->read_reg(priv, XCAN_RXFIFO_DLC_OFFSET) & XCAN_DLCR_DLC_MASK;

Better do the shift to dlc.

> +
> +	/* Change Xilinx CAN data length format to socketCAN data format */
> +	cf->can_dlc = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
> +				XCAN_DLCR_DLC_SHIFT);

Then it's just: get_can_dlc(dlc);

> +
> +	/* Change Xilinx CAN ID format to socketCAN ID format */
> +	if (id_xcan & XCAN_IDR_IDE_MASK) {
> +		/* The received frame is an Extended format frame */
> +		cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
> +		cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
> +				XCAN_IDR_ID2_SHIFT;
> +		cf->can_id |= CAN_EFF_FLAG;
> +		if (id_xcan & XCAN_IDR_RTR_MASK) {
> +			cf->can_id |= CAN_RTR_FLAG;
> +			rtr = 1;
> +		}
> +	} else {
> +		/* The received frame is a standard format frame */
> +		cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
> +				XCAN_IDR_ID1_SHIFT;
> +		if (id_xcan & XCAN_IDR_RTR_MASK) {
> +			cf->can_id |= CAN_RTR_FLAG;
> +			rtr = 1;
> +		}
> +	}
> +
> +	if (!rtr) {
> +		data[0] = priv->read_reg(priv, XCAN_RXFIFO_DW1_OFFSET);
> +		data[1] = priv->read_reg(priv, XCAN_RXFIFO_DW2_OFFSET);
> +
> +		/* Change Xilinx CAN data format to socketCAN data format */
> +		*(__be32 *)(cf->data) = cpu_to_be32(data[0]);
> +		if (cf->can_dlc > 4)
> +			*(__be32 *)(cf->data + 4) = cpu_to_be32(data[1]);
> +	}
> +	can_led_event(ndev, CAN_LED_EVENT_RX);
> +
> +	netif_receive_skb(skb);
> +
> +	stats->rx_bytes += cf->can_dlc;
> +	stats->rx_packets++;
> +	return 0;
> +}
> +
> +/**
> + * xcan_err_interrupt - error frame Isr
> + * @ndev:	net_device pointer
> + * @isr:	interrupt status register value
> + *
> + * This is the CAN error interrupt and it will
> + * check the the type of error and forward the error
> + * frame to upper layers.
> + */
> +static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	u32 err_status, status;
> +
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb) {
> +		netdev_err(ndev, "alloc_can_err_skb() failed!\n");
> +		return;
> +	}
> +
> +	err_status = priv->read_reg(priv, XCAN_ESR_OFFSET);
> +	priv->write_reg(priv, XCAN_ESR_OFFSET, err_status);
> +	status = priv->read_reg(priv, XCAN_SR_OFFSET);
> +
> +	if (isr & XCAN_IXR_BSOFF_MASK) {
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		priv->can.can_stats.bus_off++;
> +		/* Leave device in Config Mode in bus-off state */
> +		priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
> +		can_bus_off(ndev);
> +	} else if ((status & XCAN_SR_ESTAT_MASK) == XCAN_SR_ESTAT_MASK) {
> +		cf->can_id |= CAN_ERR_CRTL;
> +		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> +		priv->can.can_stats.error_passive++;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE |
> +					CAN_ERR_CRTL_TX_PASSIVE;
> +	} else if (status & XCAN_SR_ERRWRN_MASK) {
> +		cf->can_id |= CAN_ERR_CRTL;
> +		priv->can.state = CAN_STATE_ERROR_WARNING;
> +		priv->can.can_stats.error_warning++;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING |
> +					CAN_ERR_CRTL_TX_WARNING;
> +	}
> +
> +	/* Check for Arbitration lost interrupt */
> +	if (isr & XCAN_IXR_ARBLST_MASK) {
> +		cf->can_id |= CAN_ERR_LOSTARB;
> +		cf->data[0] = CAN_ERR_LOSTARB_UNSPEC;
> +		priv->can.can_stats.arbitration_lost++;
> +	}
> +
> +	/* Check for RX FIFO Overflow interrupt */
> +	if (isr & XCAN_IXR_RXOFLW_MASK) {
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> +		stats->rx_over_errors++;
> +		stats->rx_errors++;
> +		priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
> +	}
> +
> +	/* Check for error interrupt */
> +	if (isr & XCAN_IXR_ERROR_MASK) {
> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +		cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> +
> +		/* Check for Ack error interrupt */
> +		if (err_status & XCAN_ESR_ACKER_MASK) {
> +			cf->can_id |= CAN_ERR_ACK;
> +			cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> +			stats->tx_errors++;
> +		}
> +
> +		/* Check for Bit error interrupt */
> +		if (err_status & XCAN_ESR_BERR_MASK) {
> +			cf->can_id |= CAN_ERR_PROT;
> +			cf->data[2] = CAN_ERR_PROT_BIT;
> +			stats->tx_errors++;
> +		}
> +
> +		/* Check for Stuff error interrupt */
> +		if (err_status & XCAN_ESR_STER_MASK) {
> +			cf->can_id |= CAN_ERR_PROT;
> +			cf->data[2] = CAN_ERR_PROT_STUFF;
> +			stats->rx_errors++;
> +		}
> +
> +		/* Check for Form error interrupt */
> +		if (err_status & XCAN_ESR_FMER_MASK) {
> +			cf->can_id |= CAN_ERR_PROT;
> +			cf->data[2] = CAN_ERR_PROT_FORM;
> +			stats->rx_errors++;
> +		}
> +
> +		/* Check for CRC error interrupt */
> +		if (err_status & XCAN_ESR_CRCER_MASK) {
> +			cf->can_id |= CAN_ERR_PROT;
> +			cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ |
> +					CAN_ERR_PROT_LOC_CRC_DEL;
> +			stats->rx_errors++;
> +		}
> +			priv->can.can_stats.bus_error++;
> +	}
> +
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +
> +	netdev_dbg(ndev, "%s: error status register:0x%x\n",
> +			__func__, priv->read_reg(priv, XCAN_ESR_OFFSET));
> +}
> +
> +/**
> + * xcan_state_interrupt - It will check the state of the CAN device
> + * @ndev:	net_device pointer
> + * @isr:	interrupt status register value
> + *
> + * This will checks the state of the CAN device
> + * and puts the device into appropriate state.
> + */
> +static void xcan_state_interrupt(struct net_device *ndev, u32 isr)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +
> +	/* Check for Sleep interrupt if set put CAN device in sleep state */
> +	if (isr & XCAN_IXR_SLP_MASK)
> +		priv->can.state = CAN_STATE_SLEEPING;
> +
> +	/* Check for Wake up interrupt if set put CAN device in Active state */
> +	if (isr & XCAN_IXR_WKUP_MASK)
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +}
> +
> +/**
> + * xcan_rx_poll - Poll routine for rx packets (NAPI)
> + * @napi:	napi structure pointer
> + * @quota:	Max number of rx packets to be processed.
> + *
> + * This is the poll routine for rx part.
> + * It will process the packets maximux quota value.
> + *
> + * Return: number of packets received
> + */
> +static int xcan_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct net_device *ndev = napi->dev;
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	u32 isr, ier;
> +	int work_done = 0;
> +
> +	isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> +	while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
> +		if (isr & XCAN_IXR_RXOK_MASK) {
> +			priv->write_reg(priv, XCAN_ICR_OFFSET,
> +				XCAN_IXR_RXOK_MASK);
> +			if (xcan_rx(ndev) < 0)
> +				return work_done;
> +			work_done++;
> +		} else {
> +			priv->write_reg(priv, XCAN_ICR_OFFSET,
> +				XCAN_IXR_RXNEMP_MASK);
> +			break;
> +		}

What does the XCAN_IXR_RXOK_MASK mean if it's send and undset?

> +		priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_RXNEMP_MASK);
> +		isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> +	}
> +
> +	if (work_done < quota) {
> +		napi_complete(napi);
> +		ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> +		ier |= (XCAN_IXR_RXOK_MASK | XCAN_IXR_RXNEMP_MASK);
> +		priv->write_reg(priv, XCAN_IER_OFFSET, ier);

Is this a read-modify-write register? I mean will an interrupt get
disabled, if you write a 0-bit in the IER register? What does the ICR
register?

> +	}
> +	return work_done;
> +}
> +
> +/**
> + * xcan_tx_interrupt - Tx Done Isr
> + * @ndev:	net_device pointer
> + */
> +static void xcan_tx_interrupt(struct net_device *ndev)
> +{
> +	unsigned long flags;
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	u32 processed = 0, txpackets;
> +
> +	stats->tx_packets++;
> +	netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
> +			priv->waiting_ech_skb_num, priv->waiting_ech_skb_index);
> +
> +	txpackets = priv->waiting_ech_skb_num;
> +
> +	if (txpackets) {
> +		can_get_echo_skb(ndev, priv->waiting_ech_skb_index);
> +		priv->waiting_ech_skb_index =
> +			(priv->waiting_ech_skb_index + 1) %
> +			priv->xcan_echo_skb_max_tx;
> +		processed++;
> +		txpackets--;
> +	}
> +
> +	spin_lock_irqsave(&priv->ech_skb_lock, flags);
> +	priv->waiting_ech_skb_num -= processed;
> +	spin_unlock_irqrestore(&priv->ech_skb_lock, flags);

This all simplyfies to a:
	can_get_echo_skb(ndev, priv->tx_tail %
		priv->xcan_echo_skb_max_tx);
	priv->tx_tail++;

I think you should add some kind of loop here, it there is more than one
tx-complete per IRQ.

> +
> +	netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
> +			priv->waiting_ech_skb_num, priv->waiting_ech_skb_index);
> +
> +	netif_wake_queue(ndev);
> +
> +	can_led_event(ndev, CAN_LED_EVENT_TX);
> +}
> +
> +/**
> + * xcan_interrupt - CAN Isr
> + * @irq:	irq number
> + * @dev_id:	device id poniter
> + *
> + * This is the xilinx CAN Isr. It checks for the type of interrupt
> + * and invokes the corresponding ISR.
> + *
> + * Return:
> + * IRQ_NONE - If CAN device is in sleep mode, IRQ_HANDLED otherwise
> + */
> +static irqreturn_t xcan_interrupt(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = (struct net_device *)dev_id;
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	u32 isr, ier;
> +
> +	/* Get the interrupt status from Xilinx CAN */
> +	isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> +	if (!isr)
> +		return IRQ_NONE;
> +
> +	netdev_dbg(ndev, "%s: isr:#x%08x, err:#x%08x\n", __func__,
> +			isr, priv->read_reg(priv, XCAN_ESR_OFFSET));
> +
> +	/* Check for the type of interrupt and Processing it */
> +	if (isr & (XCAN_IXR_SLP_MASK | XCAN_IXR_WKUP_MASK)) {
> +		priv->write_reg(priv, XCAN_ICR_OFFSET, (XCAN_IXR_SLP_MASK |
> +				XCAN_IXR_WKUP_MASK));
> +		xcan_state_interrupt(ndev, isr);
> +	}
> +
> +	/* Check for Tx interrupt and Processing it */
> +	if (isr & XCAN_IXR_TXOK_MASK) {
> +		priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXOK_MASK);
> +		xcan_tx_interrupt(ndev);
> +	}
> +
> +	/* Check for the type of error interrupt and Processing it */
> +	if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
> +			XCAN_IXR_BSOFF_MASK | XCAN_IXR_ARBLST_MASK)) {
> +		priv->write_reg(priv, XCAN_ICR_OFFSET, (XCAN_IXR_ERROR_MASK |
> +				XCAN_IXR_RXOFLW_MASK | XCAN_IXR_BSOFF_MASK |
> +				XCAN_IXR_ARBLST_MASK));
> +		xcan_err_interrupt(ndev, isr);
> +	}
> +
> +	/* Check for the type of receive interrupt and Processing it */
> +	if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
> +		ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> +		ier &= ~(XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK);
> +		priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> +		napi_schedule(&priv->napi);
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * xcan_stop - Driver stop routine
> + * @ndev:	Pointer to net_device structure
> + *
> + * This is the drivers stop routine. It will disable the
> + * interrupts and put the device into configuration mode.
> + */
> +static void xcan_stop(struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	u32 ier;
> +
> +	/* Disable interrupts and leave the can in configuration mode */
> +	ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> +	ier &= ~XCAN_INTR_ALL;
> +	priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> +	priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
> +	priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +/**
> + * xcan_open - Driver open routine
> + * @ndev:	Pointer to net_device structure
> + *
> + * This is the driver open routine.
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_open(struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	int ret;
> +
> +	ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
> +			ndev->name, (void *)ndev);
> +	if (ret < 0) {
> +		netdev_err(ndev, "Irq allocation for CAN failed\n");
> +		return ret;
> +	}
> +
> +	/* Set chip into reset mode */
> +	ret = set_reset_mode(ndev);
> +	if (ret < 0)
> +		netdev_err(ndev, "mode resetting failed failed!\n");

Is this critical?

> +
> +	/* Common open */
> +	ret = open_candev(ndev);
> +	if (ret)
> +		return ret;

You should free the interrupt handler if this fails.

> +
> +	ret = xcan_start(ndev);
> +	if (ret < 0)
> +		netdev_err(ndev, "xcan_start failed!\n");
> +
> +
> +	can_led_event(ndev, CAN_LED_EVENT_OPEN);
> +	napi_enable(&priv->napi);
> +	netif_start_queue(ndev);
> +
> +	return 0;
> +}
> +
> +/**
> + * xcan_close - Driver close routine
> + * @ndev:	Pointer to net_device structure
> + *
> + * Return: 0 always
> + */
> +static int xcan_close(struct net_device *ndev)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	napi_disable(&priv->napi);
> +	xcan_stop(ndev);
> +	free_irq(ndev->irq, ndev);
> +	close_candev(ndev);
> +
> +	can_led_event(ndev, CAN_LED_EVENT_STOP);
> +
> +	return 0;
> +}
> +
> +/**
> + * xcan_get_berr_counter - error counter routine
> + * @ndev:	Pointer to net_device structure
> + * @bec:	Pointer to can_berr_counter structure
> + *
> + * This is the driver error counter routine.
> + * Return: 0 always
> + */
> +static int xcan_get_berr_counter(const struct net_device *ndev,
> +					struct can_berr_counter *bec)
> +{
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +
> +	bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) & XCAN_ECR_TEC_MASK;
> +	bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
> +			XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
> +	return 0;
> +}
> +
> +static const struct net_device_ops xcan_netdev_ops = {
> +	.ndo_open	= xcan_open,
> +	.ndo_stop	= xcan_close,
> +	.ndo_start_xmit	= xcan_start_xmit,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +/**
> + * xcan_suspend - Suspend method for the driver
> + * @_dev:	Address of the platform_device structure
> + *
> + * Put the driver into low power mode.
> + * Return: 0 always
> + */
> +static int xcan_suspend(struct device *_dev)
> +{
> +	struct platform_device *pdev = container_of(_dev,
> +			struct platform_device, dev);
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +
> +	priv->write_reg(priv, XCAN_MSR_OFFSET, XCAN_MSR_SLEEP_MASK);
> +	priv->can.state = CAN_STATE_SLEEPING;
> +
> +	clk_disable(priv->aperclk);
> +	clk_disable(priv->devclk);
> +
> +	return 0;
> +}
> +
> +/**
> + * xcan_resume - Resume from suspend
> + * @dev:	Address of the platformdevice structure
> + *
> + * Resume operation after suspend.
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = container_of(dev,
> +			struct platform_device, dev);
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +	int ret;
> +
> +	ret = clk_enable(priv->aperclk);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable clock.\n");
> +		return ret;
> +	}
> +	ret = clk_enable(priv->devclk);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable clock.\n");
> +		return ret;
> +	}
> +
> +	priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> +	priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend, xcan_resume);
> +
> +/**
> + * xcan_probe - Platform registration call
> + * @pdev:	Handle to the platform device structure
> + *
> + * This function does all the memory allocation and registration for the CAN
> + * device.
> + *
> + * Return: 0 on success and failure value on error
> + */
> +static int xcan_probe(struct platform_device *pdev)
> +{
> +	struct resource *res; /* IO mem resources */
> +	struct net_device *ndev;
> +	struct xcan_priv *priv;
> +	int ret, fifodep;
> +
> +	/* Create a CAN device instance */
> +	ndev = alloc_candev(sizeof(struct xcan_priv), XCAN_ECHO_SKB_MAX);
> +	if (!ndev)
> +		return -ENOMEM;
> +
> +	priv = netdev_priv(ndev);
> +	priv->dev = ndev;
> +	priv->can.bittiming_const = &xcan_bittiming_const;
> +	priv->can.do_set_bittiming = xcan_set_bittiming;
> +	priv->can.do_set_mode = xcan_do_set_mode;
> +	priv->can.do_get_berr_counter = xcan_get_berr_counter;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> +					CAN_CTRLMODE_BERR_REPORTING;
> +
> +	/* Get IRQ for the device */
> +	ndev->irq = platform_get_irq(pdev, 0);
> +
> +	spin_lock_init(&priv->ech_skb_lock);
> +	ndev->flags |= IFF_ECHO;	/* We support local echo */
> +
> +	platform_set_drvdata(pdev, ndev);
> +	SET_NETDEV_DEV(ndev, &pdev->dev);
> +	ndev->netdev_ops = &xcan_netdev_ops;
> +
> +	/* Get the virtual base address for the device */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->reg_base)) {
> +		ret = PTR_ERR(priv->reg_base);
> +		goto err_free;
> +	}
> +	ndev->mem_start = res->start;
> +	ndev->mem_end = res->end;
> +
> +	priv->write_reg = xcan_write_reg;
> +	priv->read_reg = xcan_read_reg;
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "tx-fifo-depth",
> +				&fifodep);
> +	if (ret < 0)
> +		goto err_free;
> +	priv->xcan_echo_skb_max_tx = fifodep;
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "rx-fifo-depth",
> +				&fifodep);
> +	if (ret < 0)
> +		goto err_free;
> +	priv->xcan_echo_skb_max_rx = fifodep;
> +
> +	/* Getting the CAN devclk info */
> +	priv->devclk = devm_clk_get(&pdev->dev, "ref_clk");
> +	if (IS_ERR(priv->devclk)) {
> +		dev_err(&pdev->dev, "Device clock not found.\n");
> +		ret = PTR_ERR(priv->devclk);
> +		goto err_free;
> +	}
> +
> +	/* Check for type of CAN device */
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "xlnx,zynq-can-1.00.a")) {
> +		priv->aperclk = devm_clk_get(&pdev->dev, "aper_clk");
> +		if (IS_ERR(priv->aperclk)) {
> +			dev_err(&pdev->dev, "aper clock not found\n");
> +			ret = PTR_ERR(priv->aperclk);
> +			goto err_free;
> +		}
> +	} else {
> +		priv->aperclk = priv->devclk;
> +	}
> +
> +	ret = clk_prepare_enable(priv->devclk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "unable to enable device clock\n");
> +		goto err_free;
> +	}
> +
> +	ret = clk_prepare_enable(priv->aperclk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "unable to enable aper clock\n");
> +		goto err_unprepar_disabledev;
> +	}

Can you keep your clocks disaled if the interface is not up?

> +
> +	priv->can.clock.freq = clk_get_rate(priv->devclk);
> +
> +	netif_napi_add(ndev, &priv->napi, xcan_rx_poll,
> +				priv->xcan_echo_skb_max_rx);
> +	ret = register_candev(ndev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "fail to register failed (err=%d)\n", ret);
> +		goto err_unprepar_disableaper;
> +	}
> +
> +	devm_can_led_init(ndev);
> +	dev_info(&pdev->dev,
> +			"reg_base=0x%p irq=%d clock=%d, tx fifo depth:%d\n",
> +			priv->reg_base, ndev->irq, priv->can.clock.freq,
> +			priv->xcan_echo_skb_max_tx);
> +
> +	return 0;
> +
> +err_unprepar_disableaper:
> +	clk_disable_unprepare(priv->aperclk);
> +err_unprepar_disabledev:
> +	clk_disable_unprepare(priv->devclk);
> +err_free:
> +	free_candev(ndev);
> +
> +	return ret;
> +}
> +
> +/**
> + * xcan_remove - Unregister the device after releasing the resources
> + * @pdev:	Handle to the platform device structure
> + *
> + * This function frees all the resources allocated to the device.
> + * Return: 0 always
> + */
> +static int xcan_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct xcan_priv *priv = netdev_priv(ndev);
> +
> +	if (set_reset_mode(ndev) < 0)
> +		netdev_err(ndev, "mode resetting failed!\n");
> +
> +	unregister_candev(ndev);
> +	netif_napi_del(&priv->napi);
> +	clk_disable_unprepare(priv->aperclk);
> +	clk_disable_unprepare(priv->devclk);
> +
> +	free_candev(ndev);
> +
> +	return 0;
> +}
> +
> +/* Match table for OF platform binding */
> +static struct of_device_id xcan_of_match[] = {
> +	{ .compatible = "xlnx,zynq-can-1.00.a", },
> +	{ .compatible = "xlnx,axi-can-1.00.a", },
> +	{ /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, xcan_of_match);
> +
> +static struct platform_driver xcan_driver = {
> +	.probe = xcan_probe,
> +	.remove	= xcan_remove,
> +	.driver	= {
> +		.owner = THIS_MODULE,
> +		.name = DRIVER_NAME,
> +		.pm = &xcan_dev_pm_ops,
> +		.of_match_table	= xcan_of_match,
> +	},
> +};
> +
> +module_platform_driver(xcan_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Xilinx Inc");
> +MODULE_DESCRIPTION("Xilinx CAN interface");
> 

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 0/6] net: cpsw: Support for am335x chip MACIDs
From: Uwe Kleine-König @ 2014-02-13 19:33 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Mugunthan V N, Markus Pargmann, David S. Miller,
	Benoît Cousson, linux-omap, devicetree, linux-arm-kernel,
	kernel
In-Reply-To: <20131218171301.GT4970@saruman.home>

Hello,

On Wed, Dec 18, 2013 at 11:13:01AM -0600, Felipe Balbi wrote:
> On Wed, Dec 18, 2013 at 10:40:58PM +0530, Mugunthan V N wrote:
> > On Wednesday 18 December 2013 10:38 PM, Felipe Balbi wrote:
> > > On Wed, Dec 18, 2013 at 10:30:55PM +0530, Mugunthan V N wrote:
> > >> On Wednesday 18 December 2013 10:17 PM, Markus Pargmann wrote:
> > >> Mac ID is to be filled by U-Boot and this kind of approach is already
> > >> rejected in linux-omap list.
> > >>
> > >> If proper ethaddr/eth*addr is populated in U-boot environment variable
> > >> then mac-address dt property in ethernet* device nodes will be populated
> > >> before boot kernel in U-boot. So I don't think this patch series is
> > >> required.
> > > but will u-boot read MACID from control module ?
> > >
> > Yes, U-Boot will read the MACID from control module and if a customer
> > wants to have his own MACID, U-boot ENV variable ethaddr/eth1addr must
> > be updated.
> 
> cool, then I agree this series shouldn't be applied ;-)
But even then I'd suggest to take at least patches 1 and 2.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 4/6] net: cpsw: Use cpsw-ctrl-macid driver
From: Uwe Kleine-König @ 2014-02-13 19:37 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: David S. Miller, Benoît Cousson, linux-omap, devicetree,
	linux-arm-kernel, kernel
In-Reply-To: <1387385242-1161-5-git-send-email-mpa@pengutronix.de>

Hello Markus,

On Wed, Dec 18, 2013 at 05:47:20PM +0100, Markus Pargmann wrote:
> Use ctrl-macid driver to obtain the macids stored in the processor. This
> is only done when defined in DT.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/net/cpsw.txt |  5 +++++
>  drivers/net/ethernet/ti/cpsw.c                 | 18 ++++++++++++++----
>  drivers/net/ethernet/ti/cpsw.h                 |  2 ++
>  3 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index c39f077..b95c38b 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -34,6 +34,11 @@ Required properties:
>  Optional properties:
>  - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
>  - mac-address		: Specifies slave MAC address
> +- ti,mac-address-ctrl	: When cpsw-ctrl-macid support is compiledin, this can
> +			  be set to a phandle with one argument, see
> +			  cpsw-ctrl-macid.txt. If this method fails, cpsw falls
> +			  back to mac-address or random mac-address.
> +
>  
>  Note: "ti,hwmods" field is used to fetch the base address and irq
>  resources from TI, omap hwmod data base during device registration.
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 5120d9c..382d793 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -1804,9 +1804,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>  		snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
>  			 PHY_ID_FMT, mdio->name, phyid);
>  
> -		mac_addr = of_get_mac_address(slave_node);
> -		if (mac_addr)
> -			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
> +		ret = cpsw_ctrl_macid_read(slave_node, slave_data->mac_addr);
> +		if (ret) {
> +			if (ret == -EPROBE_DEFER)
> +				return ret;
> +
> +			mac_addr = of_get_mac_address(slave_node);
> +			if (mac_addr)
> +				memcpy(slave_data->mac_addr, mac_addr,
> +						ETH_ALEN);
> +		}
I'd do it the other way round: Use the contents from an explicit
"mac-address" or "local-mac-address" property (i.e. of_get_mac_address)
and if that doesn't return anything use the mac-address-ctrl as
fallback.

>  
>  		slave_data->phy_if = of_get_phy_mode(slave_node);
>  
> @@ -1946,10 +1953,13 @@ static int cpsw_probe(struct platform_device *pdev)
>  	/* Select default pin state */
>  	pinctrl_pm_select_default_state(&pdev->dev);
>  
> -	if (cpsw_probe_dt(&priv->data, pdev)) {
> +	ret = cpsw_probe_dt(&priv->data, pdev);
> +	if (ret == -EINVAL) {
>  		pr_err("cpsw: platform data missing\n");
>  		ret = -ENODEV;
>  		goto clean_runtime_disable_ret;
> +	} else if (ret) {
> +		goto clean_runtime_disable_ret;
>  	}
>  	data = &priv->data;
>  
> diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
> index 1b71067..222eebe 100644
> --- a/drivers/net/ethernet/ti/cpsw.h
> +++ b/drivers/net/ethernet/ti/cpsw.h
> @@ -42,4 +42,6 @@ struct cpsw_platform_data {
>  
>  void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave);
>  
> +int cpsw_ctrl_macid_read(struct device_node *np, u8 *mac_addr);
> +
>  #endif /* __CPSW_H__ */
> -- 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 3/6] net: cpsw: Add control-module macid driver
From: Uwe Kleine-König @ 2014-02-13 19:44 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: David S. Miller, Benoît Cousson,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1387385242-1161-4-git-send-email-mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On Wed, Dec 18, 2013 at 05:47:19PM +0100, Markus Pargmann wrote:
> This driver extracts the hardware macid from the control module of
> am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw
> to get the macid from within the processor.
> 
> This driver is not used, unless it is defined in DT and referenced by a
> cpsw slave with a phandle.
> 
> Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  .../devicetree/bindings/net/cpsw-ctrl-macid.txt    |  31 +++++
>  drivers/net/ethernet/ti/Kconfig                    |   8 ++
>  drivers/net/ethernet/ti/Makefile                   |   1 +
>  drivers/net/ethernet/ti/cpsw-ctrl-macid.c          | 138 +++++++++++++++++++++
>  4 files changed, 178 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
>  create mode 100644 drivers/net/ethernet/ti/cpsw-ctrl-macid.c
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> new file mode 100644
> index 0000000..abff2af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> @@ -0,0 +1,31 @@
> +TI CPSW ctrl macid Devicetree bindings
> +--------------------------------------
> +
> +Required properties:
> + - compatible		: Should be "ti,am3352-cpsw-ctrl-macid"
> + - reg			: physical base address and size of the cpsw
> +			  registers map
> + - reg-names		: names of the register map given in "reg" node
> + - #ti,cpsw-ctrl-macid	: Should be <1>
#ti,mac-address-ctrl-cells?

> +
> +When used from cpsw, "ti,mac-address-ctrl" should be a phandle to this device
> +node with one argument, 0 or 1 to select the macid 0 or 1.
> +
> +Examples:
> +
> +	cpsw_ctrl_macid: cpsw-ctrl-macid@44e10630 {
> +		compatible = "ti,am3352-cpsw-ctrl-macid";
> +		#ti,mac-address-ctrl-cells = <1>;
> +		reg = <0x44e10630 0x16>;
s/0x16/0x10/

> +		reg-names = "ctrl-macid";
> +	};
> +
> +Used in cpsw slave nodes like this:
> +
> +	cpsw_emac0: slave@4a100200 {
> +		ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
> +	};
> +
> +	cpsw_emac1: slave@4a100300 {
> +		ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
> +	};
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index 53150c2..24819ef 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -56,12 +56,20 @@ config TI_CPSW_PHY_SEL
>  	  This driver supports configuring of the phy mode connected to
>  	  the CPSW.
>  
> +config TI_CPSW_CTRL_MACID
> +	boolean "TI CPSW internal MACID support"
> +	depends on TI_CPSW
> +	---help---
> +	  This driver supports reading the hardcoded MACID from am33xx
> +	  processors control module.
> +
Would it be nicer to put this after the TI_CPSW definition. (Think
$(make config).)

>  config TI_CPSW
>  	tristate "TI CPSW Switch Support"
>  	depends on ARM && (ARCH_DAVINCI || SOC_AM33XX)
>  	select TI_DAVINCI_CPDMA
>  	select TI_DAVINCI_MDIO
>  	select TI_CPSW_PHY_SEL
> +	select TI_CPSW_CTRL_MACID
If TI_CPSW selects TI_CPSW_CTRL_MACID the latter doesn't need to depend
on the former. So this optin is user visible but never
user-(de)selectable. I'd say drop the Kconfig symbol and just add
cpsw-ctrl-macid.o to ti_cpsw-y in the Makefile (or really make it
optional).

>  	---help---
>  	  This driver supports TI's CPSW Ethernet Switch.
>  
> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
> index 9cfaab8..5a31c2b 100644
> --- a/drivers/net/ethernet/ti/Makefile
> +++ b/drivers/net/ethernet/ti/Makefile
> @@ -8,5 +8,6 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += davinci_emac.o
>  obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o
>  obj-$(CONFIG_TI_DAVINCI_CPDMA) += davinci_cpdma.o
>  obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o
> +obj-$(CONFIG_TI_CPSW_CTRL_MACID) += cpsw-ctrl-macid.o
>  obj-$(CONFIG_TI_CPSW) += ti_cpsw.o
>  ti_cpsw-y := cpsw_ale.o cpsw.o cpts.o
> diff --git a/drivers/net/ethernet/ti/cpsw-ctrl-macid.c b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
> new file mode 100644
> index 0000000..e18c957
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/cpsw-ctrl-macid.c
> @@ -0,0 +1,138 @@
> +/* CPSW Control Module MACID driver
> + *
> + * Copyright (C) 2013 Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include "cpsw.h"
> +
> +#define AM33XX_CTRL_MAC_LO_REG(id) (0x8 * id)
> +#define AM33XX_CTRL_MAC_HI_REG(id) (0x8 * id + 0x4)
> +
> +struct cpsw_ctrl_macid {
> +	struct device *dev;
> +	u8 __iomem *ctrl_macid;
> +	void (*cpsw_macid_get)(struct cpsw_ctrl_macid *priv, int slave,
> +			u8 *mac_addr);
> +};
> +
> +
> +static void cpsw_ctrl_get_macid(struct cpsw_ctrl_macid *priv, int slave,
> +		u8 *mac_addr)
> +{
> +	u32 macid_lo;
> +	u32 macid_hi;
> +
> +	macid_lo = readl(priv->ctrl_macid + AM33XX_CTRL_MAC_LO_REG(slave));
> +	macid_hi = readl(priv->ctrl_macid + AM33XX_CTRL_MAC_HI_REG(slave));
> +
> +	mac_addr[5] = (macid_lo >> 8) & 0xff;
> +	mac_addr[4] = macid_lo & 0xff;
> +	mac_addr[3] = (macid_hi >> 24) & 0xff;
> +	mac_addr[2] = (macid_hi >> 16) & 0xff;
> +	mac_addr[1] = (macid_hi >> 8) & 0xff;
> +	mac_addr[0] = macid_hi & 0xff;
> +}
> +
> +static struct platform_driver cpsw_ctrl_macid_driver;
> +
> +static int match(struct device *dev, void *data)
> +{
> +	struct device_node *node = (struct device_node *)data;
> +
> +	return dev->of_node == node &&
> +		dev->driver == &cpsw_ctrl_macid_driver.driver;
> +}
> +
> +int cpsw_ctrl_macid_read(struct device_node *np, u8 *mac_addr)
> +{
> +	struct device *ctrl_dev;
> +	struct cpsw_ctrl_macid *priv;
> +	struct of_phandle_args args;
> +	int ret;
> +
> +	ret = of_parse_phandle_with_args(np, "ti,mac-address-ctrl",
> +			"#ti,mac-address-ctrl-cells", 0, &args);
> +	if (ret)
> +		return ret;
> +
> +	if (args.args_count != 1 || args.args[0] < 0 || args.args[0] > 1) {
> +		pr_err("Failed to parse ti,mac-address-module phandle because of invalid arguments\n");
> +		return -EINVAL;
> +	}
> +
> +	ctrl_dev = bus_find_device(&platform_bus_type, NULL, args.np, match);
> +	priv = dev_get_drvdata(ctrl_dev);
> +	of_node_put(args.np);
> +	if (priv == NULL)
> +		return -EPROBE_DEFER;
> +
> +	priv->cpsw_macid_get(priv, args.args[0], mac_addr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cpsw_ctrl_macid_read);
> +
> +static const struct of_device_id cpsw_ctrl_macid_of_ids[] = {
> +	{
> +		.compatible	= "ti,am3352-cpsw-ctrl-macid",
> +		.data		= &cpsw_ctrl_get_macid,
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, cpsw_ctrl_macid_of_ids);
> +
> +static int cpsw_ctrl_macid_probe(struct platform_device *pdev)
> +{
> +	struct resource	*res;
> +	const struct of_device_id *of_id;
> +	struct cpsw_ctrl_macid *priv;
> +
> +	of_id = of_match_node(cpsw_ctrl_macid_of_ids, pdev->dev.of_node);
> +	if (!of_id)
> +		return -EINVAL;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(&pdev->dev, "unable to alloc memory for cpsw-ctrl-macid\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv->cpsw_macid_get = of_id->data;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl-macid");
> +	priv->ctrl_macid = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->ctrl_macid))
> +		return PTR_ERR(priv->ctrl_macid);
> +
> +	dev_set_drvdata(&pdev->dev, priv);
> +
> +	dev_info(&pdev->dev, "TI CPSW ctrl macid loaded\n");
> +	return 0;
> +}
> +
> +static struct platform_driver cpsw_ctrl_macid_driver = {
> +	.probe		= cpsw_ctrl_macid_probe,
> +	.driver		= {
> +		.name	= "cpsw-ctrl-macid",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(cpsw_ctrl_macid_of_ids),
> +	},
> +};
> +
> +module_platform_driver(cpsw_ctrl_macid_driver);
> +MODULE_AUTHOR("Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.8.5.1
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 6/6] arm: dts: am335x beagle bone use processor macids
From: Uwe Kleine-König @ 2014-02-13 19:47 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: David S. Miller, Benoît Cousson, linux-omap, devicetree,
	linux-arm-kernel, kernel
In-Reply-To: <1387385242-1161-7-git-send-email-mpa@pengutronix.de>

Hello,

On Wed, Dec 18, 2013 at 05:47:22PM +0100, Markus Pargmann wrote:
> Use macids stored in the am335x chip.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  arch/arm/boot/dts/am335x-bone.dts      | 8 ++++++++
>  arch/arm/boot/dts/am335x-boneblack.dts | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index 94ee427..9b65a62 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -10,6 +10,14 @@
>  #include "am33xx.dtsi"
>  #include "am335x-bone-common.dtsi"
>  
> +&cpsw_emac0 {
> +	ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
> +};
> +
> +&cpsw_emac1 {
> +	ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
> +};
with the mac-address property overwriting the addresses found in the
mac-address-ctrl block as I suggested in reply to the respective patch,
I'd add this property not per machine but directly in am33xx.dtsi.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/5] drivers: of: add initialization code for reserved memory
From: Josh Cartwright @ 2014-02-13 19:48 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Benjamin Herrenschmidt, Grant Likely, Tomasz Figa,
	Marek Szyprowski, linux-kernel, linux-arm-kernel, linaro-mm-sig,
	devicetree, linux-doc, Kyungmin Park, Arnd Bergmann,
	Michal Nazarewicz, Sascha Hauer, Laura Abbott, Rob Herring,
	Olof Johansson, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Kumar Gala, Nishanth Peethambaran, Marc
In-Reply-To: <52FA87B8.8040203@gmail.com>

On Tue, Feb 11, 2014 at 09:27:36PM +0100, Tomasz Figa wrote:
> On 11.02.2014 21:19, Josh Cartwright wrote:
> >On Tue, Feb 11, 2014 at 09:04:21PM +0100, Tomasz Figa wrote:
> > >On 11.02.2014 21:02, Benjamin Herrenschmidt wrote:
> > > >On Tue, 2014-02-11 at 19:01 +0000, Grant Likely wrote:
> > > > > > except that the former IMHO better suits the definition of memory
> > > > > > region, which I see as a single contiguous range of memory and can be
> > > > > > simplified to have a single reg entry per region.
> > > > >
> > > > > My point is rather if multiple reg tuples are found in a reserved memory
> > > > > node, the kernel must respect them and reserve the memory. I'm not
> > > > > arguing about whether or not that makes for a good binding.
> > > >
> > > > agreed.
> > >
> > > My point is why, if the binding defines that just a single tuple should be
> > > provided.
> >
> > FWIW, the usecase I had mentioned in reply to Grant in the patch 5/5
> > thread [1] could make use of this.  The shared memory region is split
> > into a main chunk and several "auxiliary" chunk, but collectively these
> > regions all share the same heap state.
> >
> >   Josh
> >
> > 1: http://lkml.kernel.org/r/20140205192502.GO20228@joshc.qualcomm.com
>
> The use case seems fine, but I believe it could be properly represented in
> device tree using multiple single-reg regions as well, unless the consumer
> can request a block of memory that crosses boundary of two sub-regions
> specified by reg entries of single region.

I could probably make a only-one-reg-entry policy work for me, but it
makes things a bit more awkward.  I'd lose the ability to describe
"this set of regions need to be logically handled together" directly in
the reserved memory node, and would need to push it up a layer.

	reserved-memory {
		smem: smem {
			reg = <...>;
		};
		aux1: auxiliary1 {
			reg = <...>;
		};
		aux2: auxiliary2 {
			reg = <...>;
		};
		...
	};

	heap : heap {
		compatible = "qcom,shared-memory";
		memory-region = <&smem &aux1 &aux2>;
		#smem-cells = <2>;
	};

	actual_consumer1 {
		compatible = "...";
		smem = <&heap IDENTIFIER1 0x1000>;
	};

	actual_consumer2 {
		compatible = "...";
		smem = <&heap IDENTIFIER2 0x1000>;
	};

Maybe that's better off, I don't know.  This would also eliminate my
need for a #memory-region-cells property.

Thanks,
  Josh

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply

* Re: [PATCH net-next v2 01/10] net: phy: add "internal" PHY mode
From: David Miller @ 2014-02-13 20:34 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, cernekee, devicetree
In-Reply-To: <1392269395-23513-2-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 12 Feb 2014 21:29:46 -0800

> On some systems, the PHY can be internal, in the same package as the
> Ethernet MAC, and still be responding to a specific address on the MDIO
> bus, in that case, the Ethernet MAC might need to know about it to
> properly configure a port multiplexer to switch to an internal or
> external PHY. Add a new PHY interface mode for this and update the
> Device Tree of_get_phy_mode() function to look for it.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Changes since v1:
> - rebased against lastest net-next master branch

This is over-engineering.

The only thing that even uses this value is phy_is_internal(), and
the only user of phy_is_internal() is the generic PHY layer ethtool
operation for get-settings.

The PHY layer already has a place to indicate whether a PHY is
internal or not, overriding that using the PHY mode is trouble
waiting to happen.

Please, just provide some way to propagate this device tree property
into phy->is_internal.

^ permalink raw reply

* Re: [PATCH net-next v2 01/10] net: phy: add "internal" PHY mode
From: Florian Fainelli @ 2014-02-13 20:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Kevin Cernekee, devicetree@vger.kernel.org
In-Reply-To: <20140213.153409.1193335923050383308.davem@davemloft.net>

2014-02-13 12:34 GMT-08:00 David Miller <davem@davemloft.net>:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Wed, 12 Feb 2014 21:29:46 -0800
>
>> On some systems, the PHY can be internal, in the same package as the
>> Ethernet MAC, and still be responding to a specific address on the MDIO
>> bus, in that case, the Ethernet MAC might need to know about it to
>> properly configure a port multiplexer to switch to an internal or
>> external PHY. Add a new PHY interface mode for this and update the
>> Device Tree of_get_phy_mode() function to look for it.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> Changes since v1:
>> - rebased against lastest net-next master branch
>
> This is over-engineering.
>
> The only thing that even uses this value is phy_is_internal(), and
> the only user of phy_is_internal() is the generic PHY layer ethtool
> operation for get-settings.
>
> The PHY layer already has a place to indicate whether a PHY is
> internal or not, overriding that using the PHY mode is trouble
> waiting to happen.
>
> Please, just provide some way to propagate this device tree property
> into phy->is_internal.

I just realized that I am able to drop this change completely since I
add a PHY driver for the Ethernet MAC which already flags particular
devices of interest as internal PHYs. This change originally came up
as I needed to know that before probing for the PHY, which can be
resolved by doing some re-ordering.

Thanks for the feedback.
-- 
Florian

^ permalink raw reply

* Re: [PATCH v3 0/3] net: stmmac: Add STi GMAC ethernet
From: David Miller @ 2014-02-13 21:25 UTC (permalink / raw)
  To: srinivas.kandagatla
  Cc: netdev, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	rob, linux, stuart.menefy, peppe.cavallaro, devicetree, linux-doc,
	linux-kernel, linux-arm-kernel, kernel
In-Reply-To: <1392112716-30803-1-git-send-email-srinivas.kandagatla@st.com>

From: <srinivas.kandagatla@st.com>
Date: Tue, 11 Feb 2014 09:58:36 +0000

> Dave, Can I request you to take the first patch via net tree for v3.15, I can
> request Arnd or Olof to take the DT patches via the arm-soc tree for v3.15.

Since it's a new driver, I decided to add it to the plain 'net' tree.

Thanks.


^ permalink raw reply

* Re: [PATCH v2] bus: imx-weim: support CS GPR config for imx6q-weim
From: Philippe De Muyter @ 2014-02-13 22:38 UTC (permalink / raw)
  To: Shawn Guo
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Huang Shijie,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1392296342-21115-1-git-send-email-shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Hi Shawn,

On Thu, Feb 13, 2014 at 08:59:02PM +0800, Shawn Guo wrote:
> For imx6q-weim type of device, there might a WEIM CS space configuration
> register in General Purpose Register controller, e.g. IOMUXC_GPR1 on
> i.MX6Q.
> 
> Depending on which configuration of the following 4 is chosen for given
> system, IOMUXC_GPR1[11:0] should be set up as 05, 033, 0113 or 01111
> correspondingly.
> 
> 	CS0(128M) CS1(0M)  CS2(0M)  CS3(0M)
> 	CS0(64M)  CS1(64M) CS2(0M)  CS3(0M)
> 	CS0(64M)  CS1(32M) CS2(32M) CS3(0M)
> 	CS0(32M)  CS1(32M) CS2(32M) CS3(32M)
> 
> The patch creates a function for imx6q-weim type of device, which scans
> 'ranges' property of WEIM node and build the GPR value incrementally.
> Thus the WEIM CS GPR can be set up automatically at boot time.

Please add that text to Documentation/devicetree/bindings/bus/imx-weim.txt.

> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Changes since v1:
>  - Drop device tree property fsl,weim-cs-gpr
>  - Use octal value for IOMUXC_GPR1[11:0] bit field
>  - Use of_property_for_each_u32() to build gprval incrementally
> 
>  drivers/bus/imx-weim.c |   59 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index 3ef58c8..a120c93 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -11,6 +11,9 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/regmap.h>
>  
>  struct imx_weim_devtype {
>  	unsigned int	cs_count;
> @@ -56,6 +59,59 @@ static const struct of_device_id weim_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, weim_id_table);
>  
> +static int __init imx6q_weim_gpr_setup(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct property *prop;
> +	const __be32 *p;
> +	struct regmap *gpr;
> +	u32 gprvals[4] = {
> +		05,	/* CS0(128M) CS1(0M)  CS2(0M)  CS3(0M) */
> +		033,	/* CS0(64M)  CS1(64M) CS2(0M)  CS3(0M) */
> +		0113,	/* CS0(64M)  CS1(32M) CS2(32M) CS3(0M) */
> +		0111,	/* CS0(64M)  CS1(32M) CS2(32M) CS3(0M) */

		01111,	/* CS0(32M)  CS1(32M) CS2(32M) CS3(32M) */

> +	};
> +	u32 gprval = 0;
> +	u32 val;
> +	int len;
> +	int cs = 0;
> +	int i = 0;
> +
> +	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> +	if (IS_ERR(gpr)) {
> +		dev_err(&pdev->dev, "failed to find fsl,imx6q-iomuxc-gpr\n");
> +		return -ENOENT;
> +	}
> +
> +	prop = of_find_property(np, "ranges", &len);
> +	if (prop == NULL)
> +		return -ENOENT;
> +	if (len % (sizeof(u32) * 4))
> +		return -EINVAL;
> +
> +	of_property_for_each_u32(np, "ranges", prop, p, val) {
> +		if (i % 4 == 0) {
> +			cs = val;
> +		} else if (i % 4 == 3 && val) {
> +			val /= SZ_64M;
> +			val = (val << 1) | 1;

or
			val = (val / SZ_32M) | 1;

> +			gprval |= val << cs * 3;
> +		}
> +		i++;
> +	}

You may replace the tests above (from "prop = of_find_property" to "EINVAL;")
by :
	if (i == 0 || (i % 4))
		goto err;
> +
> +	for (i = 0; i < ARRAY_SIZE(gprvals); i++) {
> +		if (gprval == gprvals[i]) {
> +			/* Found it. Set up IOMUXC_GPR1[11:0] with it. */
> +			regmap_update_bits(gpr, IOMUXC_GPR1, 0xfff, gprval);
> +			return 0;
> +		}
> +	}
> +

   err:

> +	dev_err(&pdev->dev, "Invalid 'ranges' configuration\n");
> +	return -EINVAL;
> +}
> +
>  /* Parse and set the timing for this device. */
>  static int __init weim_timing_setup(struct device_node *np, void __iomem *base,
>  				    const struct imx_weim_devtype *devtype)
> @@ -92,6 +148,9 @@ static int __init weim_parse_dt(struct platform_device *pdev,
>  	struct device_node *child;
>  	int ret;
>  
> +	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx6q-weim"))
> +		imx6q_weim_gpr_setup(pdev);
> +
>  	for_each_child_of_node(pdev->dev.of_node, child) {
>  		if (!child->name)
>  			continue;
> -- 
> 1.7.9.5
> 

I have tested this v2 patch, and, except for the typo (0111 instead of 01111),
it works perfectly.

Thanks

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT
From: David Miller @ 2014-02-13 22:39 UTC (permalink / raw)
  To: stefan.sorensen-usnHOLptxrsHrNJx0XZkJA
  Cc: richardcochran-Re5JQEeQqe8AvxtiuMwx3w,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1392302129-24947-1-git-send-email-stefan.sorensen-usnHOLptxrsHrNJx0XZkJA@public.gmane.org>

From: Stefan Sørensen <stefan.sorensen-usnHOLptxrsHrNJx0XZkJA@public.gmane.org>
Date: Thu, 13 Feb 2014 15:35:22 +0100

> This patch series add DT configuration to the DP83640 PHY driver and makes
> the configuration of periodic output pins dynamic.
> 
> Changes since v2:
>  - Add patch to properly configure perout triggers 0+1
>  - Keep extts and perout numbers separate
>  - Shorten pr_err lines
> 
> Changes since v1:
>  - Add bindings documentation
>  - Keep module parameters
>  - Rename gpio->pin
>  - Split patch into DT part and dynamic periodic output support
> 
> Stefan Sørensen (3):
>   net:phy:dp83640: Program pulsewidth2 values of perout triggers 0 and 1
>   net:phy:dp83640: Support a configurable number of periodic outputs
>   net:phy:dp83640: Get pin and master/slave configuration from DT

I'm not applying this.

This is a great example of why I hate module parameters.

Whoever added these module parameters in the first place took the easy
road and didn't have to think about whether this was a good way or not
to specify these parameters.  Do you really think we can, like magic,
just remove this module parameter without having to think about the
consequences of that action at all?

The entities currently specifying this parameter, do they just lose?

Sorry, that's unacceptable.

You'll have to code this in a way such that people who currently specify
this parameter keep working.

If you don't like it, be angry at whoever added this module parameter
in the first place, not me.  Module parameters are 99 times out of 100
not the right way to configure something, really.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: dts: omap3-gta04: Add EOC irq gpio line handling.
From: Tony Lindgren @ 2014-02-13 22:44 UTC (permalink / raw)
  To: Belisko Marek
  Cc: Benoit Cousson, robh+dt, Pawel Moll, Mark Rutland,
	ijc+devicetree@hellion.org.uk, Kumar Gala,
	Russell King - ARM Linux, linux-omap@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel, LKML,
	Dr. H. Nikolaus Schaller, Marek Belisko
In-Reply-To: <CAAfyv36Y+N9DpFWPf5SLmJv=fcnvktVPd30QBOEsNGFdr-A0xA@mail.gmail.com>

* Belisko Marek <marek.belisko@gmail.com> [140120 12:27]:
> Ping? Benoit can you please merge this trivial update. Thanks.

Applying into omap-for-v3.14/fixes thanks.

Tony

^ permalink raw reply

* Re: [PATCH 0/2] ARM: DTS: am335x-evmsk: Audio and MMC1 fix (for 3.13?)
From: Tony Lindgren @ 2014-02-13 22:46 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Benoit Cousson, linux-omap, linux-arm-kernel, devicetree, rogerq,
	balbi
In-Reply-To: <52CBB788.9070404@ti.com>

* Peter Ujfalusi <peter.ujfalusi@ti.com> [140107 00:16]:
> Hi Benoit,
> 
> On 12/23/2013 11:28 AM, Peter Ujfalusi wrote:
> > Hi,
> > 
> > The audio frequency has been incorrectly set in the DTS file which results
> > incorrect playback frequency on EVM-SK.
> > 
> > The SD card can not be used without the second patch on 3.13-rc5.
> 
> Can you take a look at these and if it is possible send it for 3.13?

Applying both into omap-for-v3.14/fixes thanks.

Tony

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox