All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Adding DT support for TI HECC module
@ 2017-01-11 14:05 yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  2017-01-11 14:05 ` [PATCH v2 1/3] ARM: dts: AM35x: Add hecc node yegorslists
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-01-11 14:05 UTC (permalink / raw)
  To: linux-can-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ,
	andrej.skvortzov-Re5JQEeQqe8AvxtiuMwx3w, hs-ynQEQJNshbs,
	Yegor Yefremov

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This is an attempt to revive DT support for TI HECC that was started in 2015.

I haven't changed much because not all questions could be fully answered:

* Should HECC use "am3505" as compatible?
* What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?

Anton Glukhov (3):
  ARM: dts: AM35x: Add hecc node
  can: ti_hecc: Add TI HECC DT binding documentation
  can: ti_hecc: Add DT support for TI HECC module

 .../devicetree/bindings/net/can/ti_hecc.txt        | 31 +++++++++++++
 arch/arm/boot/dts/am3517.dtsi                      | 13 ++++++
 drivers/net/can/ti_hecc.c                          | 54 ++++++++++++++++++++--
 3 files changed, 94 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc.txt

-- 
2.1.4

--
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	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] ARM: dts: AM35x: Add hecc node
  2017-01-11 14:05 [PATCH v2 0/3] Adding DT support for TI HECC module yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
@ 2017-01-11 14:05 ` yegorslists
  2017-01-11 14:05 ` [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation yegorslists
       [not found] ` <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
  2 siblings, 0 replies; 16+ messages in thread
From: yegorslists @ 2017-01-11 14:05 UTC (permalink / raw)
  To: linux-can
  Cc: linux-omap, devicetree, robh+dt, jean-michel.hautbois,
	andrej.skvortzov, hs, Anton Glukhov, Yegor Yefremov

From: Anton Glukhov <anton.a.glukhov@gmail.com>

HECC node description for am35x SOCs

Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v1 -> v2:
	change compatible to "ti,am3505"

 arch/arm/boot/dts/am3517.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/am3517.dtsi b/arch/arm/boot/dts/am3517.dtsi
index 9fe545d..3bb8687 100644
--- a/arch/arm/boot/dts/am3517.dtsi
+++ b/arch/arm/boot/dts/am3517.dtsi
@@ -13,6 +13,7 @@
 / {
 	aliases {
 		serial3 = &uart4;
+		can = &hecc;
 	};
 
 	ocp@68000000 {
@@ -72,6 +73,18 @@
 			pinctrl-single,register-width = <16>;
 			pinctrl-single,function-mask = <0xff1f>;
 		};
+
+		hecc: can@5c050000 {
+			compatible = "ti,am3505";
+			status = "disabled";
+			reg = <0x5c050000 0x4000>;
+			interrupts = <24>;
+			clocks = <&hecc_ck>;
+			ti,scc-ram-offset = <0x3000>;
+			ti,hecc-ram-offset = <0x3000>;
+			ti,mbx-offset = <0x2000>;
+			ti,int-line = <0>;
+		};
 	};
 };
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation
  2017-01-11 14:05 [PATCH v2 0/3] Adding DT support for TI HECC module yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  2017-01-11 14:05 ` [PATCH v2 1/3] ARM: dts: AM35x: Add hecc node yegorslists
@ 2017-01-11 14:05 ` yegorslists
       [not found]   ` <1484143521-4898-3-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
       [not found] ` <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
  2 siblings, 1 reply; 16+ messages in thread
From: yegorslists @ 2017-01-11 14:05 UTC (permalink / raw)
  To: linux-can
  Cc: linux-omap, devicetree, robh+dt, jean-michel.hautbois,
	andrej.skvortzov, hs, Anton Glukhov, Yegor Yefremov

From: Anton Glukhov <anton.a.glukhov@gmail.com>

DT binding documentation for TI High End CAN Controller

Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v1 -> v2:
	change compatible to "ti,am3505"

 .../devicetree/bindings/net/can/ti_hecc.txt        | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc.txt

diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc.txt b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
new file mode 100644
index 0000000..ce015cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
@@ -0,0 +1,31 @@
+* Texas Instruments High End CAN Controller (HECC)
+
+This file provides information, what the device node
+for the hecc interface contains.
+
+Required properties:
+- compatible: "ti,am3505"
+- reg: offset and length of the register set for the device
+- interrupts: interrupt mapping for the hecc interrupts sources
+- clocks: clock phandles (see clock bindings for details)
+- ti,scc-ram-offset: offset to scc module ram
+- ti,hecc-ram-offset: offset to hecc module ram
+- ti,mbx-offset: offset to mailbox ram
+
+Optional properties:
+- ti,int-line: interrupt line
+
+Example:
+
+For am3517evm board:
+	hecc: can@0x5c050000 {
+		compatible = "ti,am3505";
+		status = "disabled";
+		reg = <0x5c050000 0x4000>;
+		interrupts = <24>;
+		clocks = <&hecc_ck>;
+		ti,scc-ram-offset = <0x3000>;
+		ti,hecc-ram-offset = <0x3000>;
+		ti,mbx-offset = <0x2000>;
+		ti,int-line = <0>;
+	};
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module
       [not found] ` <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-11 14:05   ` yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
       [not found]     ` <1484143521-4898-4-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
  2017-01-11 21:50   ` [PATCH v2 0/3] Adding " Yegor Yefremov
  1 sibling, 1 reply; 16+ messages in thread
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-01-11 14:05 UTC (permalink / raw)
  To: linux-can-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ,
	andrej.skvortzov-Re5JQEeQqe8AvxtiuMwx3w, hs-ynQEQJNshbs,
	Anton Glukhov, Yegor Yefremov

From: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

These patch set adds device tree support for TI HECC module.

Signed-off-by: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Changes v1 -> v2:
	- change compatible to "ti,am3505"
	- remove CONFIG_OF
	- don't set int_line to 0 explicitly

 drivers/net/can/ti_hecc.c | 54 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 680d1ff..fd3d9fc 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -46,6 +46,8 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
@@ -872,19 +874,62 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
 	.ndo_change_mtu		= can_change_mtu,
 };
 
+static const struct of_device_id ti_hecc_dt_ids[] = {
+	{
+		.compatible = "ti,am3505",
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
+
+static struct ti_hecc_platform_data *hecc_parse_dt(struct device *dev)
+{
+	struct ti_hecc_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+
+	pdata = devm_kzalloc(dev, sizeof(struct ti_hecc_platform_data), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	if (of_property_read_u32(np, "ti,scc-ram-offset", &pdata->scc_ram_offset)) {
+		dev_err(dev, "Missing scc-ram-offset property in the DT.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "ti,hecc-ram-offset", &pdata->hecc_ram_offset)) {
+		dev_err(dev, "Missing hecc-ram-offset property in the DT.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "ti,mbx-offset", &pdata->mbx_offset)) {
+		dev_err(dev, "Missing mbx-offset property in the DT.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line);
+
+	return pdata;
+}
+
 static int ti_hecc_probe(struct platform_device *pdev)
 {
 	struct net_device *ndev = (struct net_device *)0;
 	struct ti_hecc_priv *priv;
-	struct ti_hecc_platform_data *pdata;
+	struct ti_hecc_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *mem, *irq;
 	void __iomem *addr;
 	int err = -ENODEV;
 
-	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata && np) {
+		pdata = hecc_parse_dt(&pdev->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
 	if (!pdata) {
-		dev_err(&pdev->dev, "No platform data\n");
-		goto probe_exit;
+		dev_err(&pdev->dev, "Platform data missing\n");
+		return -EINVAL;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1037,6 +1082,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
 static struct platform_driver ti_hecc_driver = {
 	.driver = {
 		.name    = DRV_NAME,
+		.of_match_table = ti_hecc_dt_ids,
 	},
 	.probe = ti_hecc_probe,
 	.remove = ti_hecc_remove,
-- 
2.1.4

--
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 related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module
       [not found]     ` <1484143521-4898-4-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-11 14:24       ` Yegor Yefremov
  2017-01-17 15:31         ` Yegor Yefremov
  0 siblings, 1 reply; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-11 14:24 UTC (permalink / raw)
  To: linux-can-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs,
	Anton Glukhov, Yegor Yefremov

On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> From: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> These patch set adds device tree support for TI HECC module.
>
> Signed-off-by: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> Changes v1 -> v2:
>         - change compatible to "ti,am3505"
>         - remove CONFIG_OF
>         - don't set int_line to 0 explicitly
>
>  drivers/net/can/ti_hecc.c | 54 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
> index 680d1ff..fd3d9fc 100644
> --- a/drivers/net/can/ti_hecc.c
> +++ b/drivers/net/can/ti_hecc.c
> @@ -46,6 +46,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
>  #include <linux/can/dev.h>
>  #include <linux/can/error.h>
> @@ -872,19 +874,62 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
>         .ndo_change_mtu         = can_change_mtu,
>  };
>
> +static const struct of_device_id ti_hecc_dt_ids[] = {
> +       {
> +               .compatible = "ti,am3505",
> +       },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
> +
> +static struct ti_hecc_platform_data *hecc_parse_dt(struct device *dev)
> +{
> +       struct ti_hecc_platform_data *pdata;
> +       struct device_node *np = dev->of_node;
> +
> +       pdata = devm_kzalloc(dev, sizeof(struct ti_hecc_platform_data), GFP_KERNEL);
> +       if (!pdata)
> +               return ERR_PTR(-ENOMEM);
> +
> +       if (of_property_read_u32(np, "ti,scc-ram-offset", &pdata->scc_ram_offset)) {
> +               dev_err(dev, "Missing scc-ram-offset property in the DT.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (of_property_read_u32(np, "ti,hecc-ram-offset", &pdata->hecc_ram_offset)) {
> +               dev_err(dev, "Missing hecc-ram-offset property in the DT.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (of_property_read_u32(np, "ti,mbx-offset", &pdata->mbx_offset)) {
> +               dev_err(dev, "Missing mbx-offset property in the DT.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line);
> +
> +       return pdata;
> +}
> +
>  static int ti_hecc_probe(struct platform_device *pdev)
>  {
>         struct net_device *ndev = (struct net_device *)0;
>         struct ti_hecc_priv *priv;
> -       struct ti_hecc_platform_data *pdata;
> +       struct ti_hecc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> +       struct device_node *np = pdev->dev.of_node;
>         struct resource *mem, *irq;
>         void __iomem *addr;
>         int err = -ENODEV;
>
> -       pdata = dev_get_platdata(&pdev->dev);
> +       if (!pdata && np) {
> +               pdata = hecc_parse_dt(&pdev->dev);
> +               if (IS_ERR(pdata))
> +                       return PTR_ERR(pdata);
> +       }
> +
>         if (!pdata) {
> -               dev_err(&pdev->dev, "No platform data\n");
> -               goto probe_exit;
> +               dev_err(&pdev->dev, "Platform data missing\n");
> +               return -EINVAL;
>         }
>
>         mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1037,6 +1082,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
>  static struct platform_driver ti_hecc_driver = {
>         .driver = {
>                 .name    = DRV_NAME,
> +               .of_match_table = ti_hecc_dt_ids,
>         },
>         .probe = ti_hecc_probe,
>         .remove = ti_hecc_remove,
> --
> 2.1.4
>

So far the CAN interface will be registered, but I get following issue
with clock for HECC:

[    6.574174] CAN device driver interface
[    6.663318] ------------[ cut here ]------------
[    6.668220] WARNING: CPU: 0 PID: 115 at drivers/clk/clk.c:652
clk_core_enable+0xdc/0x268
[    6.676676] Modules linked in: ti_hecc(+) can_dev
snd_soc_omap_mcbsp omap_wdt snd_soc_omap snd_soc_tlv320aic23_i2c
snd_soc_tlv320aic23 snd_soc_core ehci_omap snd_pcm_dmaengine snd_pcm
ehci_hcd snd_timer snd at24 soundcore nvmem_core gpio_pca953x usbcore
[    6.700227] CPU: 0 PID: 115 Comm: udevd Not tainted 4.10.0-rc3 #2
[    6.706595] Hardware name: Generic AM3517 (Flattened Device Tree)
[    6.713000] [<c010fff8>] (unwind_backtrace) from [<c010c178>]
(show_stack+0x10/0x14)
[    6.721108] [<c010c178>] (show_stack) from [<c0518fcc>]
(dump_stack+0xac/0xe0)
[    6.728682] [<c0518fcc>] (dump_stack) from [<c0136d60>] (__warn+0xd8/0x104)
[    6.735973] [<c0136d60>] (__warn) from [<c0136e38>]
(warn_slowpath_null+0x20/0x28)
[    6.743898] [<c0136e38>] (warn_slowpath_null) from [<c0595530>]
(clk_core_enable+0xdc/0x268)
[    6.752730] [<c0595530>] (clk_core_enable) from [<c05966d0>]
(clk_core_enable_lock+0x18/0x2c)
[    6.761670] [<c05966d0>] (clk_core_enable_lock) from [<bf17202c>]
(ti_hecc_probe+0x188/0x3d0 [ti_hecc])
[    6.771616] [<bf17202c>] (ti_hecc_probe [ti_hecc]) from
[<c05f5930>] (platform_drv_probe+0x50/0xb0)
[    6.781095] [<c05f5930>] (platform_drv_probe) from [<c05f3998>]
(driver_probe_device+0x1f8/0x2cc)
[    6.790379] [<c05f3998>] (driver_probe_device) from [<c05f3b2c>]
(__driver_attach+0xc0/0xc4)
[    6.799208] [<c05f3b2c>] (__driver_attach) from [<c05f1e3c>]
(bus_for_each_dev+0x6c/0xa0)
[    6.807763] [<c05f1e3c>] (bus_for_each_dev) from [<c05f2ef8>]
(bus_add_driver+0x100/0x210)
[    6.816410] [<c05f2ef8>] (bus_add_driver) from [<c05f4968>]
(driver_register+0x78/0xf4)
[    6.824785] [<c05f4968>] (driver_register) from [<c0101874>]
(do_one_initcall+0x3c/0x170)
[    6.833342] [<c0101874>] (do_one_initcall) from [<c023b4c8>]
(do_init_module+0x5c/0x1b8)
[    6.841818] [<c023b4c8>] (do_init_module) from [<c01d9a08>]
(load_module+0x1d2c/0x23f8)
[    6.850194] [<c01d9a08>] (load_module) from [<c01da2f0>]
(SyS_finit_module+0xa4/0xb8)
[    6.858392] [<c01da2f0>] (SyS_finit_module) from [<c0107760>]
(ret_fast_syscall+0x0/0x1c)
[    6.866939] ---[ end trace b502c707901327dc ]---
[    6.875799] ti_hecc 5c050000.can: device registered
(reg_base=d0b1c000, irq=40)

Going to take a closer look at this.

Yegor
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
       [not found] ` <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
  2017-01-11 14:05   ` [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
@ 2017-01-11 21:50   ` Yegor Yefremov
       [not found]     ` <CAGm1_kvWXfZ_f3PPL1VJj8AhBf59Pax_GFHDVdejuMBRDu9y6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-11 21:50 UTC (permalink / raw)
  To: linux-can-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs,
	Yegor Yefremov

On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>
> This is an attempt to revive DT support for TI HECC that was started in 2015.
>
> I haven't changed much because not all questions could be fully answered:
>
> * Should HECC use "am3505" as compatible?

I mean "ti,am3505-hecc"

> * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?

First submission and related comments:
http://comments.gmane.org/gmane.linux.can/8616

Yegor
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
       [not found]     ` <CAGm1_kvWXfZ_f3PPL1VJj8AhBf59Pax_GFHDVdejuMBRDu9y6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-12  0:47       ` Tony Lindgren
       [not found]         ` <20170112004757.GZ2630-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Tony Lindgren @ 2017-01-12  0:47 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs

* Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170111 13:52]:
> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >
> > This is an attempt to revive DT support for TI HECC that was started in 2015.
> >
> > I haven't changed much because not all questions could be fully answered:
> >
> > * Should HECC use "am3505" as compatible?
> 
> I mean "ti,am3505-hecc"

Yeah it should use the device name for the driver.

> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?

The devicetree maintainers need to ack the binding doc. Maybe
send that as a first patch?

Regards,

Tony

> First submission and related comments:
> http://comments.gmane.org/gmane.linux.can/8616
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
       [not found]         ` <20170112004757.GZ2630-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
@ 2017-01-12  7:59           ` Yegor Yefremov
       [not found]             ` <CAGm1_kvNcmpayN-=mMmkCn1=wXaykhENUrHK2-MVmZLC+Cca0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-12  7:59 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs,
	Marc Kleine-Budde

On Thu, Jan 12, 2017 at 1:47 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> * Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170111 13:52]:
>> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> >
>> > This is an attempt to revive DT support for TI HECC that was started in 2015.
>> >
>> > I haven't changed much because not all questions could be fully answered:
>> >
>> > * Should HECC use "am3505" as compatible?
>>
>> I mean "ti,am3505-hecc"
>
> Yeah it should use the device name for the driver.
>
>> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
>
> The devicetree maintainers need to ack the binding doc. Maybe
> send that as a first patch?

The question is whether to place these settings into dtsi (as it was
done in the original patch) or in the driver itself.

Yegor
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
       [not found]             ` <CAGm1_kvNcmpayN-=mMmkCn1=wXaykhENUrHK2-MVmZLC+Cca0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-12 15:32               ` Yegor Yefremov
       [not found]                 ` <CAGm1_ksOZ591TQHVo5u0MHP_H5fzPX3ip5Jf3eunfT2OOW7fZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2017-01-12 15:41               ` Tony Lindgren
  1 sibling, 1 reply; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-12 15:32 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs,
	Marc Kleine-Budde

On Thu, Jan 12, 2017 at 8:59 AM, Yegor Yefremov
<yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> On Thu, Jan 12, 2017 at 1:47 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
>> * Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170111 13:52]:
>>> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>>> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>> >
>>> > This is an attempt to revive DT support for TI HECC that was started in 2015.
>>> >
>>> > I haven't changed much because not all questions could be fully answered:
>>> >
>>> > * Should HECC use "am3505" as compatible?
>>>
>>> I mean "ti,am3505-hecc"
>>
>> Yeah it should use the device name for the driver.

I've looked at drivers/net/ethernet/ti/davinci_emac.c and there
"ti,am3517-emac" will be used. Should we stick to am3517? am3505 won't
be used in any dts[i] file.

>>> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
>>
>> The devicetree maintainers need to ack the binding doc. Maybe
>> send that as a first patch?
>
> The question is whether to place these settings into dtsi (as it was
> done in the original patch) or in the driver itself.

Yegor
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
       [not found]             ` <CAGm1_kvNcmpayN-=mMmkCn1=wXaykhENUrHK2-MVmZLC+Cca0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2017-01-12 15:32               ` Yegor Yefremov
@ 2017-01-12 15:41               ` Tony Lindgren
  2017-01-16  9:34                 ` Yegor Yefremov
  1 sibling, 1 reply; 16+ messages in thread
From: Tony Lindgren @ 2017-01-12 15:41 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs,
	Marc Kleine-Budde

* Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170112 00:00]:
> On Thu, Jan 12, 2017 at 1:47 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170111 13:52]:
> >> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> >> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >> >
> >> > This is an attempt to revive DT support for TI HECC that was started in 2015.
> >> >
> >> > I haven't changed much because not all questions could be fully answered:
> >> >
> >> > * Should HECC use "am3505" as compatible?
> >>
> >> I mean "ti,am3505-hecc"
> >
> > Yeah it should use the device name for the driver.
> >
> >> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
> >
> > The devicetree maintainers need to ack the binding doc. Maybe
> > send that as a first patch?
> 
> The question is whether to place these settings into dtsi (as it was
> done in the original patch) or in the driver itself.

Well where are they on the SoC? Each driver should only access registers
that belong to the driver module.

If the ti,scc-ram-offset and ti,hecc-ram-offset are not within the ECC
driver module, probably you should use a separate driver for them
such as drivers/misc/sram.c.

Also, sounds like the ti,mbx-offset should just be using the mailbox
framework like remoteproc is doing with include/linux/omap-mailbox.h?

Regards,

Tony
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
       [not found]                 ` <CAGm1_ksOZ591TQHVo5u0MHP_H5fzPX3ip5Jf3eunfT2OOW7fZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-12 15:44                   ` Tony Lindgren
  0 siblings, 0 replies; 16+ messages in thread
From: Tony Lindgren @ 2017-01-12 15:44 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Andrey Skvortsov, hs-ynQEQJNshbs,
	Marc Kleine-Budde

* Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170112 07:33]:
> On Thu, Jan 12, 2017 at 8:59 AM, Yegor Yefremov
> <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > On Thu, Jan 12, 2017 at 1:47 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> >> * Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170111 13:52]:
> >>> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> >>> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >>> >
> >>> > This is an attempt to revive DT support for TI HECC that was started in 2015.
> >>> >
> >>> > I haven't changed much because not all questions could be fully answered:
> >>> >
> >>> > * Should HECC use "am3505" as compatible?
> >>>
> >>> I mean "ti,am3505-hecc"
> >>
> >> Yeah it should use the device name for the driver.
> 
> I've looked at drivers/net/ethernet/ti/davinci_emac.c and there
> "ti,am3517-emac" will be used. Should we stick to am3517? am3505 won't
> be used in any dts[i] file.

If they are the same, then usually the earliest hardware naming can be
used for later versions too. I don't know which was first am3517 or
3505. If you think "ti,am3517-hecc" makes sense I'm fine with that.

Regards,

Tony
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation
       [not found]   ` <1484143521-4898-3-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-13 19:56     ` Rob Herring
  2017-01-16 10:59       ` Yegor Yefremov
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2017-01-13 19:56 UTC (permalink / raw)
  To: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ,
	andrej.skvortzov-Re5JQEeQqe8AvxtiuMwx3w, hs-ynQEQJNshbs,
	Anton Glukhov

On Wed, Jan 11, 2017 at 03:05:20PM +0100, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
> From: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> DT binding documentation for TI High End CAN Controller
> 
> Signed-off-by: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> Changes v1 -> v2:
> 	change compatible to "ti,am3505"
> 
>  .../devicetree/bindings/net/can/ti_hecc.txt        | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc.txt b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
> new file mode 100644
> index 0000000..ce015cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
> @@ -0,0 +1,31 @@
> +* Texas Instruments High End CAN Controller (HECC)
> +
> +This file provides information, what the device node
> +for the hecc interface contains.
> +
> +Required properties:
> +- compatible: "ti,am3505"
> +- reg: offset and length of the register set for the device
> +- interrupts: interrupt mapping for the hecc interrupts sources
> +- clocks: clock phandles (see clock bindings for details)

> +- ti,scc-ram-offset: offset to scc module ram
> +- ti,hecc-ram-offset: offset to hecc module ram
> +- ti,mbx-offset: offset to mailbox ram

Is there not a common case that would be the default?

> +
> +Optional properties:
> +- ti,int-line: interrupt line

Needs a better description. What are valid values? This is some internal 
setting about which pin to route the interrupt output to I'm guessing.

> +
> +Example:
> +
> +For am3517evm board:
> +	hecc: can@0x5c050000 {
> +		compatible = "ti,am3505";
> +		status = "disabled";
> +		reg = <0x5c050000 0x4000>;
> +		interrupts = <24>;
> +		clocks = <&hecc_ck>;
> +		ti,scc-ram-offset = <0x3000>;
> +		ti,hecc-ram-offset = <0x3000>;
> +		ti,mbx-offset = <0x2000>;
> +		ti,int-line = <0>;
> +	};
> -- 
> 2.1.4
> 
--
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	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
  2017-01-12 15:41               ` Tony Lindgren
@ 2017-01-16  9:34                 ` Yegor Yefremov
  2017-01-16 17:38                   ` Tony Lindgren
  0 siblings, 1 reply; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-16  9:34 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-can, linux-omap, devicetree, robh+dt, Andrey Skvortsov, hs,
	Marc Kleine-Budde

On Thu, Jan 12, 2017 at 4:41 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Yegor Yefremov <yegorslists@googlemail.com> [170112 00:00]:
>> On Thu, Jan 12, 2017 at 1:47 AM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Yegor Yefremov <yegorslists@googlemail.com> [170111 13:52]:
>> >> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists@googlemail.com> wrote:
>> >> > From: Yegor Yefremov <yegorslists@googlemail.com>
>> >> >
>> >> > This is an attempt to revive DT support for TI HECC that was started in 2015.
>> >> >
>> >> > I haven't changed much because not all questions could be fully answered:
>> >> >
>> >> > * Should HECC use "am3505" as compatible?
>> >>
>> >> I mean "ti,am3505-hecc"
>> >
>> > Yeah it should use the device name for the driver.
>> >
>> >> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
>> >
>> > The devicetree maintainers need to ack the binding doc. Maybe
>> > send that as a first patch?
>>
>> The question is whether to place these settings into dtsi (as it was
>> done in the original patch) or in the driver itself.
>
> Well where are they on the SoC? Each driver should only access registers
> that belong to the driver module.
>
> If the ti,scc-ram-offset and ti,hecc-ram-offset are not within the ECC
> driver module, probably you should use a separate driver for them
> such as drivers/misc/sram.c.
>
> Also, sounds like the ti,mbx-offset should just be using the mailbox
> framework like remoteproc is doing with include/linux/omap-mailbox.h?

AFAIK all offsets are in RAM and belong to ioremapped space:

        mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        if (!mem) {
                dev_err(&pdev->dev, "No mem resources\n");
                goto probe_exit;
        }
        irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
        if (!irq) {
                dev_err(&pdev->dev, "No irq resource\n");
                goto probe_exit;
        }
        if (!request_mem_region(mem->start, resource_size(mem), pdev->name)) {
                dev_err(&pdev->dev, "HECC region already claimed\n");
                err = -EBUSY;
                goto probe_exit;
        }
        addr = ioremap(mem->start, resource_size(mem));
        if (!addr) {
                dev_err(&pdev->dev, "ioremap failed\n");
                err = -ENOMEM;
                goto probe_exit_free_region;
        }

hecc_ram_offset usage:

static inline void hecc_write_lam(struct ti_hecc_priv *priv, u32 mbxno, u32 val)
{
        __raw_writel(val, priv->base + priv->hecc_ram_offset + mbxno * 4);
}

mbx_offset usage:

static inline void hecc_write_mbx(struct ti_hecc_priv *priv, u32 mbxno,
        u32 reg, u32 val)
{
        __raw_writel(val, priv->base + priv->mbx_offset + mbxno * 0x10 +
                        reg);
}

static inline u32 hecc_read_mbx(struct ti_hecc_priv *priv, u32 mbxno, u32 reg)
{
        return __raw_readl(priv->base + priv->mbx_offset + mbxno * 0x10 +
                        reg);
}

scc_ram_offset won't be used at all. CAN controller will be used in
HECC mode only:

        /* SCC compat mode NOT supported (and not needed too) */
        hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_SCM);

@Marc is mailbox infra applicable here?

Yegor

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation
  2017-01-13 19:56     ` Rob Herring
@ 2017-01-16 10:59       ` Yegor Yefremov
  0 siblings, 0 replies; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-16 10:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-can, linux-omap, devicetree, Andrey Skvortsov, hs,
	Anton Glukhov, Marc Kleine-Budde, Grim, Dennis

On Fri, Jan 13, 2017 at 8:56 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 11, 2017 at 03:05:20PM +0100, yegorslists@googlemail.com wrote:
>> From: Anton Glukhov <anton.a.glukhov@gmail.com>
>>
>> DT binding documentation for TI High End CAN Controller
>>
>> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> ---
>> Changes v1 -> v2:
>>       change compatible to "ti,am3505"
>>
>>  .../devicetree/bindings/net/can/ti_hecc.txt        | 31 ++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc.txt b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
>> new file mode 100644
>> index 0000000..ce015cf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
>> @@ -0,0 +1,31 @@
>> +* Texas Instruments High End CAN Controller (HECC)
>> +
>> +This file provides information, what the device node
>> +for the hecc interface contains.
>> +
>> +Required properties:
>> +- compatible: "ti,am3505"
>> +- reg: offset and length of the register set for the device
>> +- interrupts: interrupt mapping for the hecc interrupts sources
>> +- clocks: clock phandles (see clock bindings for details)
>
>> +- ti,scc-ram-offset: offset to scc module ram
>> +- ti,hecc-ram-offset: offset to hecc module ram
>> +- ti,mbx-offset: offset to mailbox ram
>
> Is there not a common case that would be the default?

So far HECC is only implemented in two SoCs am3505/am3517. Both are
identical in this regard and differ in having/not having a 3D graphics
engine. So perhaps it makes sense just to move these offsets to the
driver itself?

>> +
>> +Optional properties:
>> +- ti,int-line: interrupt line

Marc suggested to convert this option to bool. Though it should be
also renamed then. HECC has basically two interrupts HECC0 and HECC1.
The one is for mailbox interrupts the other for system interrupts. But
one can map all interrupts to one "pin". This is also made in the
driver. The user can decide which one to use for all interrupts.

I'd suggest following name:

ti,use-hecc1int: if provided configures HECC to produce all interrupts
on HECC1INT interrupt line. By default HECC0INT interrupt line will be
used.

Yegor

> Needs a better description. What are valid values? This is some internal
> setting about which pin to route the interrupt output to I'm guessing.
>
>> +
>> +Example:
>> +
>> +For am3517evm board:
>> +     hecc: can@0x5c050000 {
>> +             compatible = "ti,am3505";
>> +             status = "disabled";
>> +             reg = <0x5c050000 0x4000>;
>> +             interrupts = <24>;
>> +             clocks = <&hecc_ck>;
>> +             ti,scc-ram-offset = <0x3000>;
>> +             ti,hecc-ram-offset = <0x3000>;
>> +             ti,mbx-offset = <0x2000>;
>> +             ti,int-line = <0>;
>> +     };
>> --
>> 2.1.4
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
  2017-01-16  9:34                 ` Yegor Yefremov
@ 2017-01-16 17:38                   ` Tony Lindgren
  0 siblings, 0 replies; 16+ messages in thread
From: Tony Lindgren @ 2017-01-16 17:38 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: linux-can, linux-omap, devicetree, robh+dt, Andrey Skvortsov, hs,
	Marc Kleine-Budde

* Yegor Yefremov <yegorslists@googlemail.com> [170116 01:36]:
> On Thu, Jan 12, 2017 at 4:41 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Yegor Yefremov <yegorslists@googlemail.com> [170112 00:00]:
> >> On Thu, Jan 12, 2017 at 1:47 AM, Tony Lindgren <tony@atomide.com> wrote:
> >> > * Yegor Yefremov <yegorslists@googlemail.com> [170111 13:52]:
> >> >> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists@googlemail.com> wrote:
> >> >> > From: Yegor Yefremov <yegorslists@googlemail.com>
> >> >> >
> >> >> > This is an attempt to revive DT support for TI HECC that was started in 2015.
> >> >> >
> >> >> > I haven't changed much because not all questions could be fully answered:
> >> >> >
> >> >> > * Should HECC use "am3505" as compatible?
> >> >>
> >> >> I mean "ti,am3505-hecc"
> >> >
> >> > Yeah it should use the device name for the driver.
> >> >
> >> >> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
> >> >
> >> > The devicetree maintainers need to ack the binding doc. Maybe
> >> > send that as a first patch?
> >>
> >> The question is whether to place these settings into dtsi (as it was
> >> done in the original patch) or in the driver itself.
> >
> > Well where are they on the SoC? Each driver should only access registers
> > that belong to the driver module.
> >
> > If the ti,scc-ram-offset and ti,hecc-ram-offset are not within the ECC
> > driver module, probably you should use a separate driver for them
> > such as drivers/misc/sram.c.
> >
> > Also, sounds like the ti,mbx-offset should just be using the mailbox
> > framework like remoteproc is doing with include/linux/omap-mailbox.h?
> 
> AFAIK all offsets are in RAM and belong to ioremapped space:
> 
>         mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         if (!mem) {
>                 dev_err(&pdev->dev, "No mem resources\n");
>                 goto probe_exit;
>         }
>         irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>         if (!irq) {
>                 dev_err(&pdev->dev, "No irq resource\n");
>                 goto probe_exit;
>         }
>         if (!request_mem_region(mem->start, resource_size(mem), pdev->name)) {
>                 dev_err(&pdev->dev, "HECC region already claimed\n");
>                 err = -EBUSY;
>                 goto probe_exit;
>         }
>         addr = ioremap(mem->start, resource_size(mem));
>         if (!addr) {
>                 dev_err(&pdev->dev, "ioremap failed\n");
>                 err = -ENOMEM;
>                 goto probe_exit_free_region;

If these are all within the HECC address space, then all you need
is just multiple reg entries and "reg-names" property.
Then the driver can get the reg entry by name and no custom properties
are needed for those.

Regards,

Tony

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module
  2017-01-11 14:24       ` Yegor Yefremov
@ 2017-01-17 15:31         ` Yegor Yefremov
  0 siblings, 0 replies; 16+ messages in thread
From: Yegor Yefremov @ 2017-01-17 15:31 UTC (permalink / raw)
  To: linux-can
  Cc: linux-omap, devicetree, robh+dt, Andrey Skvortsov, hs,
	Anton Glukhov, Yegor Yefremov, Marc Kleine-Budde

On Wed, Jan 11, 2017 at 3:24 PM, Yegor Yefremov
<yegorslists@googlemail.com> wrote:
> On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists@googlemail.com> wrote:
>> From: Anton Glukhov <anton.a.glukhov@gmail.com>
>>
>> These patch set adds device tree support for TI HECC module.
>>
>> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> ---
>> Changes v1 -> v2:
>>         - change compatible to "ti,am3505"
>>         - remove CONFIG_OF
>>         - don't set int_line to 0 explicitly
>>
>>  drivers/net/can/ti_hecc.c | 54 +++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 50 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
>> index 680d1ff..fd3d9fc 100644
>> --- a/drivers/net/can/ti_hecc.c
>> +++ b/drivers/net/can/ti_hecc.c
>> @@ -46,6 +46,8 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/clk.h>
>>  #include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>>
>>  #include <linux/can/dev.h>
>>  #include <linux/can/error.h>
>> @@ -872,19 +874,62 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
>>         .ndo_change_mtu         = can_change_mtu,
>>  };
>>
>> +static const struct of_device_id ti_hecc_dt_ids[] = {
>> +       {
>> +               .compatible = "ti,am3505",
>> +       },
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
>> +
>> +static struct ti_hecc_platform_data *hecc_parse_dt(struct device *dev)
>> +{
>> +       struct ti_hecc_platform_data *pdata;
>> +       struct device_node *np = dev->of_node;
>> +
>> +       pdata = devm_kzalloc(dev, sizeof(struct ti_hecc_platform_data), GFP_KERNEL);
>> +       if (!pdata)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       if (of_property_read_u32(np, "ti,scc-ram-offset", &pdata->scc_ram_offset)) {
>> +               dev_err(dev, "Missing scc-ram-offset property in the DT.\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +
>> +       if (of_property_read_u32(np, "ti,hecc-ram-offset", &pdata->hecc_ram_offset)) {
>> +               dev_err(dev, "Missing hecc-ram-offset property in the DT.\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +
>> +       if (of_property_read_u32(np, "ti,mbx-offset", &pdata->mbx_offset)) {
>> +               dev_err(dev, "Missing mbx-offset property in the DT.\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +
>> +       of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line);
>> +
>> +       return pdata;
>> +}
>> +
>>  static int ti_hecc_probe(struct platform_device *pdev)
>>  {
>>         struct net_device *ndev = (struct net_device *)0;
>>         struct ti_hecc_priv *priv;
>> -       struct ti_hecc_platform_data *pdata;
>> +       struct ti_hecc_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> +       struct device_node *np = pdev->dev.of_node;
>>         struct resource *mem, *irq;
>>         void __iomem *addr;
>>         int err = -ENODEV;
>>
>> -       pdata = dev_get_platdata(&pdev->dev);
>> +       if (!pdata && np) {
>> +               pdata = hecc_parse_dt(&pdev->dev);
>> +               if (IS_ERR(pdata))
>> +                       return PTR_ERR(pdata);
>> +       }
>> +
>>         if (!pdata) {
>> -               dev_err(&pdev->dev, "No platform data\n");
>> -               goto probe_exit;
>> +               dev_err(&pdev->dev, "Platform data missing\n");
>> +               return -EINVAL;
>>         }
>>
>>         mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> @@ -1037,6 +1082,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
>>  static struct platform_driver ti_hecc_driver = {
>>         .driver = {
>>                 .name    = DRV_NAME,
>> +               .of_match_table = ti_hecc_dt_ids,
>>         },
>>         .probe = ti_hecc_probe,
>>         .remove = ti_hecc_remove,
>> --
>> 2.1.4
>>
>
> So far the CAN interface will be registered, but I get following issue
> with clock for HECC:
>
> [    6.574174] CAN device driver interface
> [    6.663318] ------------[ cut here ]------------
> [    6.668220] WARNING: CPU: 0 PID: 115 at drivers/clk/clk.c:652
> clk_core_enable+0xdc/0x268
> [    6.676676] Modules linked in: ti_hecc(+) can_dev
> snd_soc_omap_mcbsp omap_wdt snd_soc_omap snd_soc_tlv320aic23_i2c
> snd_soc_tlv320aic23 snd_soc_core ehci_omap snd_pcm_dmaengine snd_pcm
> ehci_hcd snd_timer snd at24 soundcore nvmem_core gpio_pca953x usbcore
> [    6.700227] CPU: 0 PID: 115 Comm: udevd Not tainted 4.10.0-rc3 #2
> [    6.706595] Hardware name: Generic AM3517 (Flattened Device Tree)
> [    6.713000] [<c010fff8>] (unwind_backtrace) from [<c010c178>]
> (show_stack+0x10/0x14)
> [    6.721108] [<c010c178>] (show_stack) from [<c0518fcc>]
> (dump_stack+0xac/0xe0)
> [    6.728682] [<c0518fcc>] (dump_stack) from [<c0136d60>] (__warn+0xd8/0x104)
> [    6.735973] [<c0136d60>] (__warn) from [<c0136e38>]
> (warn_slowpath_null+0x20/0x28)
> [    6.743898] [<c0136e38>] (warn_slowpath_null) from [<c0595530>]
> (clk_core_enable+0xdc/0x268)
> [    6.752730] [<c0595530>] (clk_core_enable) from [<c05966d0>]
> (clk_core_enable_lock+0x18/0x2c)
> [    6.761670] [<c05966d0>] (clk_core_enable_lock) from [<bf17202c>]
> (ti_hecc_probe+0x188/0x3d0 [ti_hecc])
> [    6.771616] [<bf17202c>] (ti_hecc_probe [ti_hecc]) from
> [<c05f5930>] (platform_drv_probe+0x50/0xb0)
> [    6.781095] [<c05f5930>] (platform_drv_probe) from [<c05f3998>]
> (driver_probe_device+0x1f8/0x2cc)
> [    6.790379] [<c05f3998>] (driver_probe_device) from [<c05f3b2c>]
> (__driver_attach+0xc0/0xc4)
> [    6.799208] [<c05f3b2c>] (__driver_attach) from [<c05f1e3c>]
> (bus_for_each_dev+0x6c/0xa0)
> [    6.807763] [<c05f1e3c>] (bus_for_each_dev) from [<c05f2ef8>]
> (bus_add_driver+0x100/0x210)
> [    6.816410] [<c05f2ef8>] (bus_add_driver) from [<c05f4968>]
> (driver_register+0x78/0xf4)
> [    6.824785] [<c05f4968>] (driver_register) from [<c0101874>]
> (do_one_initcall+0x3c/0x170)
> [    6.833342] [<c0101874>] (do_one_initcall) from [<c023b4c8>]
> (do_init_module+0x5c/0x1b8)
> [    6.841818] [<c023b4c8>] (do_init_module) from [<c01d9a08>]
> (load_module+0x1d2c/0x23f8)
> [    6.850194] [<c01d9a08>] (load_module) from [<c01da2f0>]
> (SyS_finit_module+0xa4/0xb8)
> [    6.858392] [<c01da2f0>] (SyS_finit_module) from [<c0107760>]
> (ret_fast_syscall+0x0/0x1c)
> [    6.866939] ---[ end trace b502c707901327dc ]---
> [    6.875799] ti_hecc 5c050000.can: device registered
> (reg_base=d0b1c000, irq=40)
>
> Going to take a closer look at this.

I've fixed this issue. One needs to use clk_prepare_enable() instead
of clk_enable().

Can it be, that earlier all OMAP clocks were "prepared", so that one
could begin with clk_enable() right away?

Will make a separate patch.

Yegor

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-01-17 15:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 14:05 [PATCH v2 0/3] Adding DT support for TI HECC module yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
2017-01-11 14:05 ` [PATCH v2 1/3] ARM: dts: AM35x: Add hecc node yegorslists
2017-01-11 14:05 ` [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation yegorslists
     [not found]   ` <1484143521-4898-3-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
2017-01-13 19:56     ` Rob Herring
2017-01-16 10:59       ` Yegor Yefremov
     [not found] ` <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
2017-01-11 14:05   ` [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
     [not found]     ` <1484143521-4898-4-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
2017-01-11 14:24       ` Yegor Yefremov
2017-01-17 15:31         ` Yegor Yefremov
2017-01-11 21:50   ` [PATCH v2 0/3] Adding " Yegor Yefremov
     [not found]     ` <CAGm1_kvWXfZ_f3PPL1VJj8AhBf59Pax_GFHDVdejuMBRDu9y6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-12  0:47       ` Tony Lindgren
     [not found]         ` <20170112004757.GZ2630-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2017-01-12  7:59           ` Yegor Yefremov
     [not found]             ` <CAGm1_kvNcmpayN-=mMmkCn1=wXaykhENUrHK2-MVmZLC+Cca0Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-12 15:32               ` Yegor Yefremov
     [not found]                 ` <CAGm1_ksOZ591TQHVo5u0MHP_H5fzPX3ip5Jf3eunfT2OOW7fZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-12 15:44                   ` Tony Lindgren
2017-01-12 15:41               ` Tony Lindgren
2017-01-16  9:34                 ` Yegor Yefremov
2017-01-16 17:38                   ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.