On Wed, May 24, 2023 at 04:29:01PM -0500, Judith Mendez wrote: > 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. If it is valid for mcan_class->net->irq to be 0 in this case, then, yes, I agree. > > > + > > > /* message ram could be shared */ > > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram"); > > > if (!res) { > > > -- > > > 2.17.1 > > > ~Judith
