| Message ID | 20230419223323.20384-2-jm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable multiple MCAN on AM62x | expand |
On 19.04.2023 17:33:20, Judith Mendez wrote: > Add an hrtimer to MCAN struct. Each MCAN will have its own > hrtimer instantiated if there is no hardware interrupt found. > > 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> > --- > drivers/net/can/m_can/m_can.c | 30 ++++++++++++++++++++++++-- > drivers/net/can/m_can/m_can.h | 3 +++ > drivers/net/can/m_can/m_can_platform.c | 13 +++++++++-- > 3 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > index a5003435802b..8784bdea300a 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -23,6 +23,7 @@ > #include <linux/pinctrl/consumer.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > +#include <linux/hrtimer.h> > > #include "m_can.h" > > @@ -1587,6 +1588,11 @@ static int m_can_close(struct net_device *dev) > if (!cdev->is_peripheral) > napi_disable(&cdev->napi); > > + if (dev->irq < 0) { > + 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 +1799,18 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, > return NETDEV_TX_OK; > } > > +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(1)); > + > + return HRTIMER_RESTART; > +} > + > static int m_can_open(struct net_device *dev) > { > struct m_can_classdev *cdev = netdev_priv(dev); > @@ -1827,13 +1845,21 @@ 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); > + nitpick: Please remove these 2 newline changes. > } else { > - err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name, > + if (dev->irq > 0) { Please follow the kernel coding style and use a space not a tab after the closing ")" of the "if". > + err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name, > dev); > + } > + > + else { Please use kernel coding style: "} else {" > + dev_dbg(cdev->dev, "Enabling the hrtimer\n"); > + cdev->hrtimer.function = &hrtimer_callback; > + hrtimer_start(&cdev->hrtimer, ns_to_ktime(0), HRTIMER_MODE_REL_PINNED); > + } I think there's no need to have nested else branches, what about this approach? --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1831,9 +1831,11 @@ static int m_can_open(struct net_device *dev) err = request_threaded_irq(dev->irq, NULL, m_can_isr, IRQF_ONESHOT, dev->name, dev); - } else { + } else if (dev->irq) { err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name, dev); + } else { + // polling } if (err < 0) { > } > > 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..ed046d77fdb9 100644 > --- a/drivers/net/can/m_can/m_can.h > +++ b/drivers/net/can/m_can/m_can.h > @@ -28,6 +28,7 @@ > #include <linux/pm_runtime.h> > #include <linux/slab.h> > #include <linux/uaccess.h> > +#include <linux/hrtimer.h> > > /* m_can lec values */ > enum m_can_lec_type { > @@ -93,6 +94,8 @@ struct m_can_classdev { > int is_peripheral; > > struct mram_cfg mcfg[MRAM_CFG_NUM]; > + > + struct hrtimer hrtimer; > }; > > 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..7540db74b7d0 100644 > --- a/drivers/net/can/m_can/m_can_platform.c > +++ b/drivers/net/can/m_can/m_can_platform.c > @@ -7,6 +7,7 @@ > > #include <linux/phy/phy.h> > #include <linux/platform_device.h> > +#include <linux/hrtimer.h> > > #include "m_can.h" > > @@ -98,8 +99,16 @@ static int m_can_plat_probe(struct platform_device *pdev) > addr = devm_platform_ioremap_resource_byname(pdev, "m_can"); > irq = platform_get_irq_byname(pdev, "int0"); > if (IS_ERR(addr) || irq < 0) { > - ret = -EINVAL; > - goto probe_fail; > + if (irq == -EPROBE_DEFER) { > + ret = -EPROBE_DEFER; > + goto probe_fail; > + } > + if (IS_ERR(addr)) { > + ret = PTR_ERR(addr); > + goto probe_fail; > + } > + dev_dbg(mcan_class->dev, "Failed to get irq, initialize hrtimer\n"); > + hrtimer_init(&mcan_class->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED); Looks better. Please remove the outer "if (IS_ERR(addr) || irq < 0)" and move the error checking directly after "addr = devm_platform_ioremap_resource_byname()". What do you think about introducing the "poll-interval" property and only enable polling if it is set? > } > > /* message ram could be shared */ > -- > 2.17.1 > > regards, Marc
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index a5003435802b..8784bdea300a 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -23,6 +23,7 @@ #include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/hrtimer.h> #include "m_can.h" @@ -1587,6 +1588,11 @@ static int m_can_close(struct net_device *dev) if (!cdev->is_peripheral) napi_disable(&cdev->napi); + if (dev->irq < 0) { + 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 +1799,18 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, return NETDEV_TX_OK; } +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(1)); + + return HRTIMER_RESTART; +} + static int m_can_open(struct net_device *dev) { struct m_can_classdev *cdev = netdev_priv(dev); @@ -1827,13 +1845,21 @@ 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 { - err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name, + if (dev->irq > 0) { + err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name, dev); + } + + else { + dev_dbg(cdev->dev, "Enabling the hrtimer\n"); + cdev->hrtimer.function = &hrtimer_callback; + hrtimer_start(&cdev->hrtimer, ns_to_ktime(0), 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..ed046d77fdb9 100644 --- a/drivers/net/can/m_can/m_can.h +++ b/drivers/net/can/m_can/m_can.h @@ -28,6 +28,7 @@ #include <linux/pm_runtime.h> #include <linux/slab.h> #include <linux/uaccess.h> +#include <linux/hrtimer.h> /* m_can lec values */ enum m_can_lec_type { @@ -93,6 +94,8 @@ struct m_can_classdev { int is_peripheral; struct mram_cfg mcfg[MRAM_CFG_NUM]; + + struct hrtimer hrtimer; }; 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..7540db74b7d0 100644 --- a/drivers/net/can/m_can/m_can_platform.c +++ b/drivers/net/can/m_can/m_can_platform.c @@ -7,6 +7,7 @@ #include <linux/phy/phy.h> #include <linux/platform_device.h> +#include <linux/hrtimer.h> #include "m_can.h" @@ -98,8 +99,16 @@ static int m_can_plat_probe(struct platform_device *pdev) addr = devm_platform_ioremap_resource_byname(pdev, "m_can"); irq = platform_get_irq_byname(pdev, "int0"); if (IS_ERR(addr) || irq < 0) { - ret = -EINVAL; - goto probe_fail; + if (irq == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto probe_fail; + } + if (IS_ERR(addr)) { + ret = PTR_ERR(addr); + goto probe_fail; + } + dev_dbg(mcan_class->dev, "Failed to get irq, initialize hrtimer\n"); + hrtimer_init(&mcan_class->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED); } /* message ram could be shared */
Add an hrtimer to MCAN struct. Each MCAN will have its own hrtimer instantiated if there is no hardware interrupt found. 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> --- drivers/net/can/m_can/m_can.c | 30 ++++++++++++++++++++++++-- drivers/net/can/m_can/m_can.h | 3 +++ drivers/net/can/m_can/m_can_platform.c | 13 +++++++++-- 3 files changed, 42 insertions(+), 4 deletions(-)