| Message ID | 20240529-iep-v1-2-7273c07592d3@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable PTP timestamping/PPS for AM65x SR1.0 devices | expand |
On 5/29/2024 9:05 AM, Diogo Ivo wrote: > The IEP module supports compare events, in which a value is written to a > hardware register and when the IEP counter reaches the written value an > interrupt is generated. Add handling for this interrupt in order to > support PPS events. > > Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> > --- Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>-----Original Message----- >From: Diogo Ivo <diogo.ivo@siemens.com> >Sent: Wednesday, May 29, 2024 9:35 PM >To: MD Danish Anwar <danishanwar@ti.com>; Roger Quadros ><rogerq@kernel.org>; David S. Miller <davem@davemloft.net>; Eric Dumazet ><edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni ><pabeni@redhat.com>; Richard Cochran <richardcochran@gmail.com>; >Nishanth Menon <nm@ti.com>; Vignesh Raghavendra <vigneshr@ti.com>; >Tero Kristo <kristo@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof >Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jan >Kiszka <jan.kiszka@siemens.com> >Cc: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org; linux- >kernel@vger.kernel.org; devicetree@vger.kernel.org; Diogo Ivo ><diogo.ivo@siemens.com> >Subject: [EXTERNAL] [PATCH 2/3] net: ti: icss-iep: Enable compare events > >The IEP module supports compare events, in which a value is written to a >hardware register and when the IEP counter reaches the written value an >interrupt is generated. Add handling for this interrupt in order to support PPS >events. > >Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> >--- > iep = devm_kzalloc(dev, sizeof(*iep), GFP_KERNEL); > if (!iep) >@@ -827,6 +883,21 @@ static int icss_iep_probe(struct platform_device >*pdev) > if (IS_ERR(iep->base)) > return -ENODEV; > >+ iep->cap_cmp_irq = platform_get_irq_byname_optional(pdev, >"iep_cap_cmp"); >+ if (iep->cap_cmp_irq < 0) { >+ if (iep->cap_cmp_irq == -EPROBE_DEFER) >+ return iep->cap_cmp_irq; This info is coming from DT, is PROBE_DIFFER error return value possible ? >+ iep->cap_cmp_irq = 0; >+ } else { >+ ret = devm_request_irq(dev, iep->cap_cmp_irq, >+ icss_iep_cap_cmp_irq, >IRQF_TRIGGER_HIGH, >+ "iep_cap_cmp", iep); >+ if (ret) >+ return dev_err_probe(iep->dev, ret, >+ "Request irq failed for cap_cmp\n"); Can't this driver live without this feature ? >+ INIT_WORK(&iep->work, icss_iep_cap_cmp_work); >+ } >+
> >+ iep->cap_cmp_irq = platform_get_irq_byname_optional(pdev, > >"iep_cap_cmp"); > >+ if (iep->cap_cmp_irq < 0) { > >+ if (iep->cap_cmp_irq == -EPROBE_DEFER) > >+ return iep->cap_cmp_irq; > > This info is coming from DT, is PROBE_DIFFER error return value possible ? static int __platform_get_irq_byname(struct platform_device *dev, const char *name) { struct resource *r; int ret; ret = fwnode_irq_get_byname(dev_fwnode(&dev->dev), name); if (ret > 0 || ret == -EPROBE_DEFER) return ret; This suggests it can happen. Andrew
Hi Sunil, On 5/31/24 6:12 AM, Sunil Kovvuri Goutham wrote: >> From: Diogo Ivo <diogo.ivo@siemens.com> >> Sent: Wednesday, May 29, 2024 9:35 PM >> >> + iep->cap_cmp_irq = platform_get_irq_byname_optional(pdev, >> "iep_cap_cmp"); >> + if (iep->cap_cmp_irq < 0) { >> + if (iep->cap_cmp_irq == -EPROBE_DEFER) >> + return iep->cap_cmp_irq; > > This info is coming from DT, is PROBE_DIFFER error return value possible ? From my understanding -EPROBE_DEFER is a possible error code. platform_get_irq_byname_optional() will eventually call of_irq_get() where we get -EPROBE_DEFER if the IRQ domain still hasn't been initialized. >> + iep->cap_cmp_irq = 0; >> + } else { >> + ret = devm_request_irq(dev, iep->cap_cmp_irq, >> + icss_iep_cap_cmp_irq, >> IRQF_TRIGGER_HIGH, >> + "iep_cap_cmp", iep); >> + if (ret) >> + return dev_err_probe(iep->dev, ret, >> + "Request irq failed for cap_cmp\n"); > > Can't this driver live without this feature ? Yes it can! I'll rework this logic so that we do not fail to probe here and simply continue without PPS event support. Thank you for the review! Best regards, Diogo
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c index 3025e9c18970..8337508ce8f0 100644 --- a/drivers/net/ethernet/ti/icssg/icss_iep.c +++ b/drivers/net/ethernet/ti/icssg/icss_iep.c @@ -17,6 +17,7 @@ #include <linux/timekeeping.h> #include <linux/interrupt.h> #include <linux/of_irq.h> +#include <linux/workqueue.h> #include "icss_iep.h" @@ -122,6 +123,7 @@ struct icss_iep { int cap_cmp_irq; u64 period; u32 latch_enable; + struct work_struct work; }; /** @@ -571,6 +573,55 @@ static int icss_iep_perout_enable(struct icss_iep *iep, return ret; } +static void icss_iep_cap_cmp_work(struct work_struct *work) +{ + struct icss_iep *iep = container_of(work, struct icss_iep, work); + struct ptp_clock_event pevent; + unsigned int val; + u64 ns, ns_next; + + spin_lock(&iep->irq_lock); + + ns = readl(iep->base + iep->plat_data->reg_offs[ICSS_IEP_CMP1_REG0]); + if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT) { + val = readl(iep->base + iep->plat_data->reg_offs[ICSS_IEP_CMP1_REG1]); + ns |= (u64)val << 32; + } + /* set next event */ + ns_next = ns + iep->period; + writel(lower_32_bits(ns_next), + iep->base + iep->plat_data->reg_offs[ICSS_IEP_CMP1_REG0]); + if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT) + writel(upper_32_bits(ns_next), + iep->base + iep->plat_data->reg_offs[ICSS_IEP_CMP1_REG1]); + + pevent.pps_times.ts_real = ns_to_timespec64(ns); + pevent.type = PTP_CLOCK_PPSUSR; + pevent.index = 0; + ptp_clock_event(iep->ptp_clock, &pevent); + dev_dbg(iep->dev, "IEP:pps ts: %llu next:%llu:\n", ns, ns_next); + + spin_unlock(&iep->irq_lock); +} + +static irqreturn_t icss_iep_cap_cmp_irq(int irq, void *dev_id) +{ + struct icss_iep *iep = (struct icss_iep *)dev_id; + unsigned int val; + + val = readl(iep->base + iep->plat_data->reg_offs[ICSS_IEP_CMP_STAT_REG]); + /* The driver only enables CMP1 */ + if (val & BIT(1)) { + /* Clear the event */ + writel(BIT(1), iep->base + iep->plat_data->reg_offs[ICSS_IEP_CMP_STAT_REG]); + if (iep->pps_enabled || iep->perout_enabled) + schedule_work(&iep->work); + return IRQ_HANDLED; + } + + return IRQ_NONE; +} + static int icss_iep_pps_enable(struct icss_iep *iep, int on) { struct ptp_clock_request rq; @@ -602,6 +653,8 @@ static int icss_iep_pps_enable(struct icss_iep *iep, int on) ret = icss_iep_perout_enable_hw(iep, &rq.perout, on); } else { ret = icss_iep_perout_enable_hw(iep, &rq.perout, on); + if (iep->cap_cmp_irq) + cancel_work_sync(&iep->work); } if (!ret) @@ -777,6 +830,8 @@ int icss_iep_init(struct icss_iep *iep, const struct icss_iep_clockops *clkops, if (iep->ops && iep->ops->perout_enable) { iep->ptp_info.n_per_out = 1; iep->ptp_info.pps = 1; + } else if (iep->cap_cmp_irq) { + iep->ptp_info.pps = 1; } if (iep->ops && iep->ops->extts_enable) @@ -817,6 +872,7 @@ static int icss_iep_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct icss_iep *iep; struct clk *iep_clk; + int ret; iep = devm_kzalloc(dev, sizeof(*iep), GFP_KERNEL); if (!iep) @@ -827,6 +883,21 @@ static int icss_iep_probe(struct platform_device *pdev) if (IS_ERR(iep->base)) return -ENODEV; + iep->cap_cmp_irq = platform_get_irq_byname_optional(pdev, "iep_cap_cmp"); + if (iep->cap_cmp_irq < 0) { + if (iep->cap_cmp_irq == -EPROBE_DEFER) + return iep->cap_cmp_irq; + iep->cap_cmp_irq = 0; + } else { + ret = devm_request_irq(dev, iep->cap_cmp_irq, + icss_iep_cap_cmp_irq, IRQF_TRIGGER_HIGH, + "iep_cap_cmp", iep); + if (ret) + return dev_err_probe(iep->dev, ret, + "Request irq failed for cap_cmp\n"); + INIT_WORK(&iep->work, icss_iep_cap_cmp_work); + } + iep_clk = devm_clk_get(dev, NULL); if (IS_ERR(iep_clk)) return PTR_ERR(iep_clk);
The IEP module supports compare events, in which a value is written to a hardware register and when the IEP counter reaches the written value an interrupt is generated. Add handling for this interrupt in order to support PPS events. Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> --- drivers/net/ethernet/ti/icssg/icss_iep.c | 71 ++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+)