2023-05-01 22:32:31

by Judith Mendez

[permalink] [raw]
Subject: [PATCH v3 0/4] Enable multiple MCAN on AM62x

On AM62x there is one MCAN in MAIN domain and two in MCU domain.
The MCANs in MCU domain were not enabled since there is no
hardware interrupt routed to A53 GIC interrupt controller.
Therefore A53 Linux cannot be interrupted by MCU MCANs.

This solution instantiates a hrtimer with 1 ms polling interval
for MCAN device when there is no hardware interrupt and there is
poll-interval property in DTB MCAN node. The hrtimer generates a
recurring software interrupt which allows to call the isr. The isr
will check if there is pending transaction by reading a register
and proceed normally if there is.

On AM62x, this series enables two MCU MCAN which will use the hrtimer
implementation. MCANs with hardware interrupt routed to A53 Linux
will continue to use the hardware interrupt as expected.

Timer polling method was tested on both classic CAN and CAN-FD
at 125 KBPS, 250 KBPS, 1 MBPS and 2.5 MBPS with 4 MBPS bitrate
switching.

Letency and CPU load benchmarks were tested on 3x MCAN on AM62x.
1 MBPS timer polling interval is the better timer polling interval
since it has comparable latency to hardware interrupt with the worse
case being 1ms + CAN frame propagation time and CPU load is not
substantial. Latency can be improved further with less than 1 ms
polling intervals, howerver it is at the cost of CPU usage since CPU
load increases at 0.5 ms.

Note that in terms of power, enabling MCU MCANs with timer-polling
implementation might have negative impact since we will have to wake
up every 1 ms whether there are CAN packets pending in the RX FIFO or
not. This might prevent the CPU from entering into deeper idle states
for extended periods of time.

This patch series depends on 'Enable CAN PHY transceiver driver':
Link: https://lore.kernel.org/lkml/[email protected]/T/

v2:
Link: https://lore.kernel.org/linux-can/[email protected]/T/#t

V1:
Link: https://lore.kernel.org/linux-can/[email protected]/T/#t

RFC:
Link: https://lore.kernel.org/linux-can/[email protected]/T/#t

Changes since v2:
- Change binding patch first
- Update binding poll-interval description
- Add oneOf to select either interrupts/interrupt-names or poll-interval
- Sort list of includes
- Create a define for 1 ms polling interval
- Change plarform_get_irq to optional to not print error msg
- Fix indentations, lengths of code lines, and added other style changes

Changes since v1:
- Add poll-interval property to bindings and MCAN DTB node
- Add functionality to check for 'poll-interval' property in MCAN node
- Bindings: add an example using poll-interval
- Add 'polling' flag in driver to check if device is using polling method
- Check for both timer polling and hardware interrupt case, default to
hardware interrupt method
- Change ns_to_ktime() to ms_to_ktime()

Judith Mendez (4):
dt-bindings: net: can: Add poll-interval for MCAN
can: m_can: Add hrtimer to generate software interrupt
arm64: dts: ti: Add AM62x MCAN MAIN domain transceiver overlay
arm64: dts: ti: Enable MCU MCANs for AM62x

.../bindings/net/can/bosch,m_can.yaml | 36 +++++++++++-
arch/arm64/boot/dts/ti/Makefile | 2 +
arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi | 24 ++++++++
.../boot/dts/ti/k3-am625-sk-mcan-main.dtso | 35 ++++++++++++
.../boot/dts/ti/k3-am625-sk-mcan-mcu.dtso | 57 +++++++++++++++++++
drivers/net/can/m_can/m_can.c | 29 +++++++++-
drivers/net/can/m_can/m_can.h | 4 ++
drivers/net/can/m_can/m_can_platform.c | 33 ++++++++++-
8 files changed, 213 insertions(+), 7 deletions(-)
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-sk-mcan-main.dtso
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso


base-commit: 92e815cf07ed24ee1c51b122f24ffcf2964b4b13
--
2.17.1


2023-05-01 22:32:44

by Judith Mendez

[permalink] [raw]
Subject: [PATCH v3 2/4] can: m_can: Add hrtimer to generate software interrupt

Add an hrtimer to MCAN class device. Each MCAN will have its own
hrtimer instantiated if there is no hardware interrupt found and
poll-interval property is defined in device tree M_CAN node.

The hrtimer will generate a software interrupt every 1 ms. In
hrtimer callback, we check if there is a transaction pending by
reading a register, then process by calling the isr if there is.

Signed-off-by: Judith Mendez <[email protected]>
---
Changelog:
v1:
1. Sort list of includes
2. Create a define for HR_TIMER_POLL_INTERVAL
3. Fix indentations and style issues/warnings
4. Change polling variable to type bool
5. Change platform_get_irq to optional so not to print error msg
6. Move error check for addr directly after assignment
7. Print appropriate error msg with dev_err_probe insead of dev_dbg

v2:
1. Add poll-interval to MCAN class device to check if poll-interval propery is
present in MCAN node, this enables timer polling method.
2. Add 'polling' flag to MCAN class device to check if a device is using timer
polling method
3. Check if both timer polling and hardware interrupt are enabled for a MCAN
device, default to hardware interrupt mode if both are enabled.
4. Change ms_to_ktime() to ns_to_ktime()
5. Remove newlines, tabs, and restructure if/else section.

drivers/net/can/m_can/m_can.c | 29 ++++++++++++++++++++--
drivers/net/can/m_can/m_can.h | 4 ++++
drivers/net/can/m_can/m_can_platform.c | 33 +++++++++++++++++++++++---
3 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a5003435802b..e1ac0c1d85a3 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -11,6 +11,7 @@
#include <linux/bitfield.h>
#include <linux/can/dev.h>
#include <linux/ethtool.h>
+#include <linux/hrtimer.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
@@ -308,6 +309,9 @@ enum m_can_reg {
#define TX_EVENT_MM_MASK GENMASK(31, 24)
#define TX_EVENT_TXTS_MASK GENMASK(15, 0)

+/* Hrtimer polling interval */
+#define HRTIMER_POLL_INTERVAL 1
+
/* The ID and DLC registers are adjacent in M_CAN FIFO memory,
* and we can save a (potentially slow) bus round trip by combining
* reads and writes to them.
@@ -1587,6 +1591,11 @@ static int m_can_close(struct net_device *dev)
if (!cdev->is_peripheral)
napi_disable(&cdev->napi);

+ if (cdev->polling) {
+ dev_dbg(cdev->dev, "Disabling the hrtimer\n");
+ hrtimer_cancel(&cdev->hrtimer);
+ }
+
m_can_stop(dev);
m_can_clk_stop(cdev);
free_irq(dev->irq, dev);
@@ -1793,6 +1802,18 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
return NETDEV_TX_OK;
}

+static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer)
+{
+ struct m_can_classdev *cdev = container_of(timer, struct
+ m_can_classdev, hrtimer);
+
+ m_can_isr(0, cdev->net);
+
+ hrtimer_forward_now(timer, ms_to_ktime(HRTIMER_POLL_INTERVAL));
+
+ return HRTIMER_RESTART;
+}
+
static int m_can_open(struct net_device *dev)
{
struct m_can_classdev *cdev = netdev_priv(dev);
@@ -1827,13 +1848,17 @@ static int m_can_open(struct net_device *dev)
}

INIT_WORK(&cdev->tx_work, m_can_tx_work_queue);
-
err = request_threaded_irq(dev->irq, NULL, m_can_isr,
IRQF_ONESHOT,
dev->name, dev);
- } else {
+ } else if (!cdev->polling) {
err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name,
dev);
+ } else {
+ dev_dbg(cdev->dev, "Start hrtimer\n");
+ cdev->hrtimer.function = &hrtimer_callback;
+ hrtimer_start(&cdev->hrtimer, ms_to_ktime(HRTIMER_POLL_INTERVAL),
+ HRTIMER_MODE_REL_PINNED);
}

if (err < 0) {
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index a839dc71dc9b..e9db5cce4e68 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -15,6 +15,7 @@
#include <linux/device.h>
#include <linux/dma-mapping.h>
#include <linux/freezer.h>
+#include <linux/hrtimer.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
@@ -93,6 +94,9 @@ struct m_can_classdev {
int is_peripheral;

struct mram_cfg mcfg[MRAM_CFG_NUM];
+
+ struct hrtimer hrtimer;
+ bool polling;
};

struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, int sizeof_priv);
diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index 9c1dcf838006..ec2277d89c73 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -5,6 +5,7 @@
//
// Copyright (C) 2018-19 Texas Instruments Incorporated - http://www.ti.com/

+#include <linux/hrtimer.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>

@@ -96,12 +97,38 @@ static int m_can_plat_probe(struct platform_device *pdev)
goto probe_fail;

addr = devm_platform_ioremap_resource_byname(pdev, "m_can");
- irq = platform_get_irq_byname(pdev, "int0");
- if (IS_ERR(addr) || irq < 0) {
- ret = -EINVAL;
+ if (IS_ERR(addr)) {
+ ret = PTR_ERR(addr);
goto probe_fail;
}

+ irq = platform_get_irq_byname_optional(pdev, "int0");
+ if (irq == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto probe_fail;
+ }
+
+ if (device_property_present(mcan_class->dev, "poll-interval"))
+ mcan_class->polling = 1;
+
+ if (!mcan_class->polling && irq < 0) {
+ ret = -ENXIO;
+ dev_err_probe(mcan_class->dev, ret, "IRQ int0 not found and polling not
+ activated\n");
+ goto probe_fail;
+ }
+
+ if (mcan_class->polling) {
+ if (irq > 0) {
+ mcan_class->polling = 0;
+ dev_dbg(mcan_class->dev, "Polling enabled and hardware IRQ found, use hardware IRQ\n");
+ } else {
+ dev_dbg(mcan_class->dev, "Polling enabled, initialize hrtimer");
+ hrtimer_init(&mcan_class->hrtimer, CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL_PINNED);
+ }
+ }
+
/* message ram could be shared */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
if (!res) {
--
2.17.1

2023-05-01 22:32:49

by Judith Mendez

[permalink] [raw]
Subject: [PATCH v3 3/4] DO_NOT_MERGE arm64: dts: ti: Add AM62x MCAN MAIN domain transceiver overlay

Add an overlay for main domain MCAN on AM62x SK. The AM62x
SK board does not have on-board CAN transceiver so instead
of changing the DTB permanently, add an overlay to enable
MAIN domain MCAN and support for 1 CAN transceiver.

This DT overlay can be used with the following EVM:
Link: https://www.ti.com/tool/TCAN1042DEVM

Signed-off-by: Judith Mendez <[email protected]>
---
Changelog:
v3:
1. Add link for specific board

arch/arm64/boot/dts/ti/Makefile | 2 ++
.../boot/dts/ti/k3-am625-sk-mcan-main.dtso | 35 +++++++++++++++++++
2 files changed, 37 insertions(+)
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-sk-mcan-main.dtso

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index c83c9d772b81..abe15e76b614 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -9,8 +9,10 @@
# alphabetically.

# Boards with AM62x SoC
+k3-am625-sk-mcan-dtbs := k3-am625-sk.dtb k3-am625-sk-mcan-main.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-sk.dtb
+dtb-$(CONFIG_ARCH_K3) += k3-am625-sk-mcan.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am62-lp-sk.dtb

# Boards with AM62Ax SoC
diff --git a/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-main.dtso b/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-main.dtso
new file mode 100644
index 000000000000..0a7b2f394f87
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-main.dtso
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * DT overlay for MCAN transceiver in main domain on AM625 SK
+ *
+ * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include "k3-pinctrl.h"
+
+&{/} {
+ transceiver1: can-phy0 {
+ compatible = "ti,tcan1042";
+ #phy-cells = <0>;
+ max-bitrate = <5000000>;
+ };
+};
+
+&main_pmx0 {
+ main_mcan0_pins_default: main-mcan0-pins-default {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x1dc, PIN_INPUT, 0) /* (E15) MCAN0_RX */
+ AM62X_IOPAD(0x1d8, PIN_OUTPUT, 0) /* (C15) MCAN0_TX */
+ >;
+ };
+};
+
+&main_mcan0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mcan0_pins_default>;
+ phys = <&transceiver1>;
+};
--
2.17.1

2023-05-01 22:33:22

by Judith Mendez

[permalink] [raw]
Subject: [PATCH v3 1/4] dt-bindings: net: can: Add poll-interval for MCAN

On AM62x SoC, MCANs on MCU domain do not have hardware interrupt
routed to A53 Linux, instead they will use software interrupt by
hrtimer. To enable timer method, interrupts should be optional so
remove interrupts property from required section and introduce
poll-interval property.

Signed-off-by: Judith Mendez <[email protected]>
---
Changelog:
v3:
1. Move binding patch to first in series
2. Update description for poll-interval
3. Add oneOf to specify using interrupts/interrupt-names or poll-interval
4. Fix example property: add comment below 'example'

v2:
1. Add poll-interval property to enable timer polling method
2. Add example using poll-interval property

.../bindings/net/can/bosch,m_can.yaml | 36 +++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
index 67879aab623b..c024ee49962c 100644
--- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
+++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml
@@ -14,6 +14,13 @@ maintainers:
allOf:
- $ref: can-controller.yaml#

+oneOf:
+ - required:
+ - interrupts
+ - interrupt-names
+ - required:
+ - poll-interval
+
properties:
compatible:
const: bosch,m_can
@@ -40,6 +47,14 @@ properties:
- const: int1
minItems: 1

+ poll-interval:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: Enable hrtimer polling method for an M_CAN device.
+ If this property is defined in MCAN node, it tells the driver to
+ enable polling method for an MCAN device. If for an MCAN device,
+ hardware interrupt is found and hrtimer polling method is enabled,
+ the driver will use hardware interrupt method.
+
clocks:
items:
- description: peripheral clock
@@ -122,8 +137,6 @@ required:
- compatible
- reg
- reg-names
- - interrupts
- - interrupt-names
- clocks
- clock-names
- bosch,mram-cfg
@@ -132,6 +145,7 @@ additionalProperties: false

examples:
- |
+ // Example with interrupts
#include <dt-bindings/clock/imx6sx-clock.h>
can@20e8000 {
compatible = "bosch,m_can";
@@ -149,4 +163,22 @@ examples:
};
};

+ - |
+ // Example with timer polling
+ #include <dt-bindings/clock/imx6sx-clock.h>
+ can@20e8000 {
+ compatible = "bosch,m_can";
+ reg = <0x020e8000 0x4000>, <0x02298000 0x4000>;
+ reg-names = "m_can", "message_ram";
+ poll-interval;
+ clocks = <&clks IMX6SX_CLK_CANFD>,
+ <&clks IMX6SX_CLK_CANFD>;
+ clock-names = "hclk", "cclk";
+ bosch,mram-cfg = <0x0 0 0 32 0 0 0 1>;
+
+ can-transceiver {
+ max-bitrate = <5000000>;
+ };
+ };
+
...
--
2.17.1

2023-05-01 22:33:24

by Judith Mendez

[permalink] [raw]
Subject: [PATCH v3 4/4] arm64: dts: ti: Enable MCU MCANs for AM62x

On AM62x there are no hardware interrupts routed to A53 GIC
interrupt controller for MCU MCAN IPs, so MCU MCANs were not
added to the MCU dtsi. In this patch series an hrtimer is introduced
to MCAN driver to generate software interrupts. Now add MCU MCAN
nodes to the MCU dtsi but disable the MCAN devices by default.

AM62x does not carry on-board CAN transceivers, so instead of
changing DTB permanently use an overlay to enable MCU MCANs and to
add CAN transceiver nodes.

If there is no hardware interrupt and timer method is used, remove
interrupt properties and add poll-interval to enable the hrtimer
per MCAN node.

This DT overlay can be used with the following EVM:
Link: https://www.ti.com/tool/TCAN1042DEVM

Signed-off-by: Judith Mendez <[email protected]>
---
Changelog:
v3:
1. Add link for specific board

arch/arm64/boot/dts/ti/Makefile | 2 +-
arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi | 24 ++++++++
.../boot/dts/ti/k3-am625-sk-mcan-mcu.dtso | 57 +++++++++++++++++++
3 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index abe15e76b614..c76be3888e4d 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -9,7 +9,7 @@
# alphabetically.

# Boards with AM62x SoC
-k3-am625-sk-mcan-dtbs := k3-am625-sk.dtb k3-am625-sk-mcan-main.dtbo
+k3-am625-sk-mcan-dtbs := k3-am625-sk.dtb k3-am625-sk-mcan-main.dtbo k3-am625-sk-mcan-mcu.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-sk.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-sk-mcan.dtb
diff --git a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
index 076601a41e84..20462f457643 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
@@ -141,4 +141,28 @@
/* Tightly coupled to M4F */
status = "reserved";
};
+
+ mcu_mcan1: can@4e00000 {
+ compatible = "bosch,m_can";
+ reg = <0x00 0x4e00000 0x00 0x8000>,
+ <0x00 0x4e08000 0x00 0x200>;
+ reg-names = "message_ram", "m_can";
+ power-domains = <&k3_pds 188 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 188 6>, <&k3_clks 188 1>;
+ clock-names = "hclk", "cclk";
+ bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+ status = "disabled";
+ };
+
+ mcu_mcan2: can@4e10000 {
+ compatible = "bosch,m_can";
+ reg = <0x00 0x4e10000 0x00 0x8000>,
+ <0x00 0x4e18000 0x00 0x200>;
+ reg-names = "message_ram", "m_can";
+ power-domains = <&k3_pds 189 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 189 6>, <&k3_clks 189 1>;
+ clock-names = "hclk", "cclk";
+ bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
+ status = "disabled";
+ };
};
diff --git a/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso b/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso
new file mode 100644
index 000000000000..5145b3de4f9b
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * DT overlay for MCAN in MCU domain on AM625 SK
+ *
+ * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include "k3-pinctrl.h"
+
+&{/} {
+ transceiver2: can-phy1 {
+ compatible = "ti,tcan1042";
+ #phy-cells = <0>;
+ max-bitrate = <5000000>;
+ };
+
+ transceiver3: can-phy2 {
+ compatible = "ti,tcan1042";
+ #phy-cells = <0>;
+ max-bitrate = <5000000>;
+ };
+};
+
+&mcu_pmx0 {
+ mcu_mcan1_pins_default: mcu-mcan1-pins-default {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x038, PIN_INPUT, 0) /* (B3) MCU_MCAN0_RX */
+ AM62X_IOPAD(0x034, PIN_OUTPUT, 0) /* (D6) MCU_MCAN0_TX */
+ >;
+ };
+
+ mcu_mcan2_pins_default: mcu-mcan2-pins-default {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x040, PIN_INPUT, 0) /* (D4) MCU_MCAN1_RX */
+ AM62X_IOPAD(0x03C, PIN_OUTPUT, 0) /* (E5) MCU_MCAN1_TX */
+ >;
+ };
+};
+
+&mcu_mcan1 {
+ poll-interval;
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_mcan1_pins_default>;
+ phys = <&transceiver2>;
+ status = "okay";
+};
+
+&mcu_mcan2 {
+ poll-interval;
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_mcan2_pins_default>;
+ phys = <&transceiver3>;
+ status = "okay";
+};
--
2.17.1

2023-05-01 23:33:18

by Judith Mendez

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Enable multiple MCAN on AM62x

Hello all,

On 5/1/23 17:31, Judith Mendez wrote:
> On AM62x there is one MCAN in MAIN domain and two in MCU domain.
> The MCANs in MCU domain were not enabled since there is no
> hardware interrupt routed to A53 GIC interrupt controller.
> Therefore A53 Linux cannot be interrupted by MCU MCANs.
>
> This solution instantiates a hrtimer with 1 ms polling interval
> for MCAN device when there is no hardware interrupt and there is
> poll-interval property in DTB MCAN node. The hrtimer generates a
> recurring software interrupt which allows to call the isr. The isr
> will check if there is pending transaction by reading a register
> and proceed normally if there is.
>
> On AM62x, this series enables two MCU MCAN which will use the hrtimer
> implementation. MCANs with hardware interrupt routed to A53 Linux
> will continue to use the hardware interrupt as expected.
>
> Timer polling method was tested on both classic CAN and CAN-FD
> at 125 KBPS, 250 KBPS, 1 MBPS and 2.5 MBPS with 4 MBPS bitrate
> switching.
>
> Letency and CPU load benchmarks were tested on 3x MCAN on AM62x.
> 1 MBPS timer polling interval is the better timer polling interval
> since it has comparable latency to hardware interrupt with the worse
> case being 1ms + CAN frame propagation time and CPU load is not
> substantial. Latency can be improved further with less than 1 ms
> polling intervals, howerver it is at the cost of CPU usage since CPU
> load increases at 0.5 ms.
>
> Note that in terms of power, enabling MCU MCANs with timer-polling
> implementation might have negative impact since we will have to wake
> up every 1 ms whether there are CAN packets pending in the RX FIFO or
> not. This might prevent the CPU from entering into deeper idle states
> for extended periods of time.
>
> This patch series depends on 'Enable CAN PHY transceiver driver':
> Link: https://lore.kernel.org/lkml/[email protected]/T/
>
> v2:
> Link: https://lore.kernel.org/linux-can/[email protected]/T/#t
>
> V1:
> Link: https://lore.kernel.org/linux-can/[email protected]/T/#t
>
> RFC:
> Link: https://lore.kernel.org/linux-can/[email protected]/T/#t
>
> Changes since v2:
> - Change binding patch first
> - Update binding poll-interval description
> - Add oneOf to select either interrupts/interrupt-names or poll-interval
> - Sort list of includes
> - Create a define for 1 ms polling interval
> - Change plarform_get_irq to optional to not print error msg
> - Fix indentations, lengths of code lines, and added other style changes
>
> Changes since v1:
> - Add poll-interval property to bindings and MCAN DTB node
> - Add functionality to check for 'poll-interval' property in MCAN node
> - Bindings: add an example using poll-interval
> - Add 'polling' flag in driver to check if device is using polling method
> - Check for both timer polling and hardware interrupt case, default to
> hardware interrupt method
> - Change ns_to_ktime() to ms_to_ktime()

Please ignore this version v3, apologies for sending the wrong patches.

regards,
Judith






2023-05-02 00:29:39

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] arm64: dts: ti: Enable MCU MCANs for AM62x

On 17:31-20230501, Judith Mendez wrote:
> On AM62x there are no hardware interrupts routed to A53 GIC
> interrupt controller for MCU MCAN IPs, so MCU MCANs were not
> added to the MCU dtsi. In this patch series an hrtimer is introduced
> to MCAN driver to generate software interrupts. Now add MCU MCAN
> nodes to the MCU dtsi but disable the MCAN devices by default.
>
> AM62x does not carry on-board CAN transceivers, so instead of
> changing DTB permanently use an overlay to enable MCU MCANs and to
> add CAN transceiver nodes.
>
> If there is no hardware interrupt and timer method is used, remove
> interrupt properties and add poll-interval to enable the hrtimer
> per MCAN node.
>
> This DT overlay can be used with the following EVM:
> Link: https://www.ti.com/tool/TCAN1042DEVM
>
> Signed-off-by: Judith Mendez <[email protected]>
> ---
> Changelog:
> v3:
> 1. Add link for specific board
>
> arch/arm64/boot/dts/ti/Makefile | 2 +-
> arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi | 24 ++++++++
> .../boot/dts/ti/k3-am625-sk-mcan-mcu.dtso | 57 +++++++++++++++++++
NAK - I dont see "DO NOT MERGE" in $subject.

please send this patch addressing previous comments with arch maintainer
tree not the mcan tree.

> 3 files changed, 82 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso
>
> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
> index abe15e76b614..c76be3888e4d 100644
> --- a/arch/arm64/boot/dts/ti/Makefile
> +++ b/arch/arm64/boot/dts/ti/Makefile
> @@ -9,7 +9,7 @@
> # alphabetically.
>
> # Boards with AM62x SoC
> -k3-am625-sk-mcan-dtbs := k3-am625-sk.dtb k3-am625-sk-mcan-main.dtbo
> +k3-am625-sk-mcan-dtbs := k3-am625-sk.dtb k3-am625-sk-mcan-main.dtbo k3-am625-sk-mcan-mcu.dtbo
> dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay.dtb
> dtb-$(CONFIG_ARCH_K3) += k3-am625-sk.dtb
> dtb-$(CONFIG_ARCH_K3) += k3-am625-sk-mcan.dtb
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
> index 076601a41e84..20462f457643 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi
> @@ -141,4 +141,28 @@
> /* Tightly coupled to M4F */
> status = "reserved";
> };
> +
> + mcu_mcan1: can@4e00000 {
> + compatible = "bosch,m_can";
> + reg = <0x00 0x4e00000 0x00 0x8000>,
> + <0x00 0x4e08000 0x00 0x200>;
> + reg-names = "message_ram", "m_can";
> + power-domains = <&k3_pds 188 TI_SCI_PD_EXCLUSIVE>;
> + clocks = <&k3_clks 188 6>, <&k3_clks 188 1>;
> + clock-names = "hclk", "cclk";
> + bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
> + status = "disabled";
> + };
> +
> + mcu_mcan2: can@4e10000 {
> + compatible = "bosch,m_can";
> + reg = <0x00 0x4e10000 0x00 0x8000>,
> + <0x00 0x4e18000 0x00 0x200>;
> + reg-names = "message_ram", "m_can";
> + power-domains = <&k3_pds 189 TI_SCI_PD_EXCLUSIVE>;
> + clocks = <&k3_clks 189 6>, <&k3_clks 189 1>;
> + clock-names = "hclk", "cclk";
> + bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
> + status = "disabled";
> + };
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso b/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso
> new file mode 100644
> index 000000000000..5145b3de4f9b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-sk-mcan-mcu.dtso
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * DT overlay for MCAN in MCU domain on AM625 SK
> + *
> + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include "k3-pinctrl.h"
> +
> +&{/} {
> + transceiver2: can-phy1 {
> + compatible = "ti,tcan1042";
> + #phy-cells = <0>;
> + max-bitrate = <5000000>;
> + };
> +
> + transceiver3: can-phy2 {
> + compatible = "ti,tcan1042";
> + #phy-cells = <0>;
> + max-bitrate = <5000000>;
> + };
> +};
> +
> +&mcu_pmx0 {
> + mcu_mcan1_pins_default: mcu-mcan1-pins-default {
> + pinctrl-single,pins = <
> + AM62X_IOPAD(0x038, PIN_INPUT, 0) /* (B3) MCU_MCAN0_RX */
> + AM62X_IOPAD(0x034, PIN_OUTPUT, 0) /* (D6) MCU_MCAN0_TX */
> + >;
> + };
> +
> + mcu_mcan2_pins_default: mcu-mcan2-pins-default {
> + pinctrl-single,pins = <
> + AM62X_IOPAD(0x040, PIN_INPUT, 0) /* (D4) MCU_MCAN1_RX */
> + AM62X_IOPAD(0x03C, PIN_OUTPUT, 0) /* (E5) MCU_MCAN1_TX */
> + >;
> + };
> +};
> +
> +&mcu_mcan1 {
> + poll-interval;
> + pinctrl-names = "default";
> + pinctrl-0 = <&mcu_mcan1_pins_default>;
> + phys = <&transceiver2>;
> + status = "okay";
> +};
> +
> +&mcu_mcan2 {
> + poll-interval;
> + pinctrl-names = "default";
> + pinctrl-0 = <&mcu_mcan2_pins_default>;
> + phys = <&transceiver3>;
> + status = "okay";
> +};
> --
> 2.17.1
>

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D