Re: [PATCH v7 2/2] can: m_can: Add hrtimer to generate software interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Simon,

On 5/23/23 6:09 AM, Simon Horman wrote:
On Mon, May 22, 2023 at 09:37:49PM -0500, Judith Mendez wrote:
Add an hrtimer to MCAN class device. Each MCAN will have its own
hrtimer instantiated if there is no hardware interrupt found 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 <jm@xxxxxx>

...

diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index 94dc82644113..b639c9e645d3 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,30 @@ 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;
  	}
+ if (device_property_present(mcan_class->dev, "interrupts") ||
+	    device_property_present(mcan_class->dev, "interrupt-names")) {
+		irq = platform_get_irq_byname(pdev, "int0");
+		mcan_class->polling = false;
+		if (irq == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto probe_fail;
+		}
+		if (irq < 0) {
+			ret = -ENXIO;
+			goto probe_fail;
+		}
+	} else {
+		mcan_class->polling = true;
+		dev_dbg(mcan_class->dev, "Polling enabled, initialize hrtimer");
+		hrtimer_init(&mcan_class->hrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_PINNED);
+	}

Hi Judith,

it seems that with this change irq is only set in the first arm of
the above conditional. But later on it is used unconditionally.
That is, it may be used uninitialised.

Reported by gcc-12 as:

  drivers/net/can/m_can/m_can_platform.c: In function 'm_can_plat_probe':
  drivers/net/can/m_can/m_can_platform.c:150:30: warning: 'irq' may be used uninitialized [-Wmaybe-uninitialized]
    150 |         mcan_class->net->irq = irq;
        |         ~~~~~~~~~~~~~~~~~~~~~^~~~~
  drivers/net/can/m_can/m_can_platform.c:86:13: note: 'irq' was declared here
     86 |         int irq, ret = 0;
        |             ^~~


Maybe a good solution is to initialize irq=0 here.

+
  	/* message ram could be shared */
  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
  	if (!res) {
--
2.17.1


~Judith



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux