* [PATCH 2/3] can: ti_hecc: Add TI HECC DT binding documentation
2015-10-04 19:59 [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module Anton Glukhov
@ 2015-10-04 19:59 ` Anton Glukhov
2015-10-12 9:50 ` Marc Kleine-Budde
2015-10-04 19:59 ` [PATCH 3/3] ARM: dts: AM35x: Add hecc node Anton Glukhov
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Anton Glukhov @ 2015-10-04 19:59 UTC (permalink / raw)
To: mkl, linux-can, bcousson; +Cc: Anton Glukhov
DT binding documentation for TI High End CAN Controller
Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
---
.../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..e29b0ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
@@ -0,0 +1,31 @@
+* Texas Instruments High End CAN Controller
+
+This file provides information, what the device node
+for the hecc interface contains.
+
+Required properties:
+- compatible: "ti,am35x-hecc"
+- 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,am35x-hecc";
+ 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>;
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] can: ti_hecc: Add TI HECC DT binding documentation
2015-10-04 19:59 ` [PATCH 2/3] can: ti_hecc: Add TI HECC DT binding documentation Anton Glukhov
@ 2015-10-12 9:50 ` Marc Kleine-Budde
0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2015-10-12 9:50 UTC (permalink / raw)
To: Anton Glukhov, linux-can, bcousson
[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]
On 10/04/2015 09:59 PM, Anton Glukhov wrote:
> DT binding documentation for TI High End CAN Controller
Please add the device tree binding discussion mailing list on Cc.
>
> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
> ---
> .../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..e29b0ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
> @@ -0,0 +1,31 @@
> +* Texas Instruments High End CAN Controller
> +
> +This file provides information, what the device node
> +for the hecc interface contains.
> +
> +Required properties:
> +- compatible: "ti,am35x-hecc"
> +- 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
Are the offsets a per SoC parameter? I'm not sure if it's better to put
the offsets into the driver.
> +
> +Optional properties:
> +- ti,int-line: interrupt line
Better make it a bool and describe it better.
> +
> +Example:
> +
> +For am3517evm board:
> + hecc: can@0x5c050000 {
> + compatible = "ti,am35x-hecc";
> + 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>;
> + };
>
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: 455 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] ARM: dts: AM35x: Add hecc node
2015-10-04 19:59 [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module Anton Glukhov
2015-10-04 19:59 ` [PATCH 2/3] can: ti_hecc: Add TI HECC DT binding documentation Anton Glukhov
@ 2015-10-04 19:59 ` Anton Glukhov
2015-10-12 9:42 ` [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module Marc Kleine-Budde
2016-03-16 10:53 ` Marc Kleine-Budde
3 siblings, 0 replies; 6+ messages in thread
From: Anton Glukhov @ 2015-10-04 19:59 UTC (permalink / raw)
To: mkl, linux-can, bcousson; +Cc: Anton Glukhov
HECC node description for am35x SOCs
Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
---
arch/arm/boot/dts/am3517.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/am3517.dtsi b/arch/arm/boot/dts/am3517.dtsi
index 5e3f5e8..cbd6d97 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 {
@@ -70,6 +71,17 @@
interrupt-controller;
pinctrl-single,register-width = <16>;
pinctrl-single,function-mask = <0xff1f>;
+
+ hecc: can@0x5c050000 {
+ compatible = "ti,am35x-hecc";
+ 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>;
};
};
};
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module
2015-10-04 19:59 [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module Anton Glukhov
2015-10-04 19:59 ` [PATCH 2/3] can: ti_hecc: Add TI HECC DT binding documentation Anton Glukhov
2015-10-04 19:59 ` [PATCH 3/3] ARM: dts: AM35x: Add hecc node Anton Glukhov
@ 2015-10-12 9:42 ` Marc Kleine-Budde
2016-03-16 10:53 ` Marc Kleine-Budde
3 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2015-10-12 9:42 UTC (permalink / raw)
To: Anton Glukhov, linux-can, bcousson
[-- Attachment #1: Type: text/plain, Size: 3978 bytes --]
On 10/04/2015 09:59 PM, Anton Glukhov wrote:
> These patch set adds device tree support for TI HECC module.
>
> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
> ---
> drivers/net/can/ti_hecc.c | 56 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
> index cf345cb..c1a89fd 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>
> @@ -875,19 +877,64 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
> .ndo_change_mtu = can_change_mtu,
> };
>
> +#if defined(CONFIG_OF)
Have you actually tested to compile without CONFIG_OF? I think the
assignment of "of_match_table = ti_hecc_dt_ids" will fail. Please remove
the ifdef here. If it's still possible to compile without CONFIG_OF,
please use of_match_ptr and mark the struct as __maybe_unused.
> +static const struct of_device_id ti_hecc_dt_ids[] = {
> + {
> + .compatible = "ti,am35x-hecc",
AFAIK it's considered best practise not to use a generic name "am35x"
but to add the oldest SoC that support this IP core.
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
> +#endif
> +
> +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);
> + }
> + if (of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line)) {
bool?
> + pdata->int_line = 0;
This makes no sense, as pdata is initialized as "0" anyways.
> + }
> +
> + 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);
> @@ -1040,6 +1087,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,
>
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: 455 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module
2015-10-04 19:59 [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module Anton Glukhov
` (2 preceding siblings ...)
2015-10-12 9:42 ` [PATCH 1/3] can: ti_hecc: Add DT support for TI HECC module Marc Kleine-Budde
@ 2016-03-16 10:53 ` Marc Kleine-Budde
3 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2016-03-16 10:53 UTC (permalink / raw)
To: Anton Glukhov, linux-can, bcousson
[-- Attachment #1.1: Type: text/plain, Size: 3677 bytes --]
Hello,
just stumbled over these patches.
On 10/04/2015 09:59 PM, Anton Glukhov wrote:
> These patch set adds device tree support for TI HECC module.
> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
> ---
> drivers/net/can/ti_hecc.c | 56 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
> index cf345cb..c1a89fd 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>
> @@ -875,19 +877,64 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
> .ndo_change_mtu = can_change_mtu,
> };
>
> +#if defined(CONFIG_OF)
please remove the ifdef
> +static const struct of_device_id ti_hecc_dt_ids[] = {
> + {
> + .compatible = "ti,am35x-hecc",
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
> +#endif
> +
> +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);
Please don't allocate pdata - better create two functions, one parsing
the pdata and one parse the dt.
> + 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);
> + }
> + if (of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line)) {
> + pdata->int_line = 0;
> + }
> +
> + 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);
> @@ -1040,6 +1087,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,
>
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: 455 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread