Messages in this thread | | | Date | Wed, 24 May 2023 16:29:01 -0500 | Subject | Re: [PATCH v7 2/2] can: m_can: Add hrtimer to generate software interrupt | From | Judith Mendez <> |
| |
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@ti.com> > > ... > >> 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
| |