This patch series enables support for PTP in AM65x SR1.0 devices.
This feature relies heavily on the Industrial Ethernet Peripheral
(IEP) hardware module, which implements a hardware counter through
which time is kept. This hardware block is the basis for exposing
a PTP hardware clock to userspace and for issuing timestamps for
incoming/outgoing packets, allowing for time synchronization.
The IEP also has compare registers that fire an interrupt when the
counter reaches the value stored in a compare register. This feature
allows us to support PPS events in the kernel.
The changes are separated into three patches:
- PATCH 01/03: Register SR1.0 devices with the IEP infrastructure to
expose a PHC clock to userspace, allowing time to be
adjusted using standard PTP tools. The code for issuing/
collecting packet timestamps is already present in the
current state of the driver, so only this needs to be
done.
- PATCH 02/03: Add support for IEP compare event/interrupt handling
to enable PPS events.
- PATCH 03/03: Add the interrupts to the IOT2050 device tree.
Currently every compare event generates two interrupts, the first
corresponding to the actual event and the second being a spurious
but otherwise harmless interrupt. The root cause of this has been
identified and has been solved in the platform's SDK. A forward port
of the SDK's patches also fixes the problem in upstream but is not
included here since it's upstreaming is out of the scope of this
series. If someone from TI would be willing to chime in and help
get the interrupt changes upstream that would be great!
Signed-off-by: Diogo Ivo <[email protected]>
---
Diogo Ivo (3):
net: ti: icssg-prueth: Enable PTP timestamping support for SR1.0 devices
net: ti: icss-iep: Enable compare events
arm64: dts: ti: iot2050: Add IEP interrupts for SR1.0 devices
.../boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi | 12 ++++
drivers/net/ethernet/ti/icssg/icss_iep.c | 71 ++++++++++++++++++++++
drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 49 ++++++++++++++-
3 files changed, 131 insertions(+), 1 deletion(-)
---
base-commit: 2f0e3f6a6824dfda2759225326d9c69203c06bc8
change-id: 20240529-iep-8bb4a3cb9068
Best regards,
--
Diogo Ivo <[email protected]>
Enable PTP support for AM65x SR1.0 devices by registering with the IEP
infrastructure in order to expose a PTP clock to userspace.
Signed-off-by: Diogo Ivo <[email protected]>
---
drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 49 +++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
index 7b3304bbd7fc..01cad01965dc 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
@@ -1011,16 +1011,42 @@ static int prueth_probe(struct platform_device *pdev)
dev_dbg(dev, "sram: pa %llx va %p size %zx\n", prueth->msmcram.pa,
prueth->msmcram.va, prueth->msmcram.size);
+ prueth->iep0 = icss_iep_get_idx(np, 0);
+ if (IS_ERR(prueth->iep0)) {
+ ret = dev_err_probe(dev, PTR_ERR(prueth->iep0), "iep0 get failed\n");
+ goto free_pool;
+ }
+
+ prueth->iep1 = icss_iep_get_idx(np, 1);
+ if (IS_ERR(prueth->iep1)) {
+ ret = dev_err_probe(dev, PTR_ERR(prueth->iep1), "iep1 get failed\n");
+ goto put_iep0;
+ }
+
+ ret = icss_iep_init(prueth->iep0, NULL, NULL, 0);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to init iep0\n");
+ goto put_iep;
+ }
+
+ ret = icss_iep_init(prueth->iep1, NULL, NULL, 0);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to init iep1\n");
+ goto exit_iep0;
+ }
+
if (eth0_node) {
ret = prueth_netdev_init(prueth, eth0_node);
if (ret) {
dev_err_probe(dev, ret, "netdev init %s failed\n",
eth0_node->name);
- goto free_pool;
+ goto exit_iep;
}
if (of_find_property(eth0_node, "ti,half-duplex-capable", NULL))
prueth->emac[PRUETH_MAC0]->half_duplex = 1;
+
+ prueth->emac[PRUETH_MAC0]->iep = prueth->iep0;
}
if (eth1_node) {
@@ -1033,6 +1059,8 @@ static int prueth_probe(struct platform_device *pdev)
if (of_find_property(eth1_node, "ti,half-duplex-capable", NULL))
prueth->emac[PRUETH_MAC1]->half_duplex = 1;
+
+ prueth->emac[PRUETH_MAC1]->iep = prueth->iep1;
}
/* register the network devices */
@@ -1091,6 +1119,19 @@ static int prueth_probe(struct platform_device *pdev)
prueth_netdev_exit(prueth, eth_node);
}
+exit_iep:
+ icss_iep_exit(prueth->iep1);
+exit_iep0:
+ icss_iep_exit(prueth->iep0);
+
+put_iep:
+ icss_iep_put(prueth->iep1);
+
+put_iep0:
+ icss_iep_put(prueth->iep0);
+ prueth->iep0 = NULL;
+ prueth->iep1 = NULL;
+
free_pool:
gen_pool_free(prueth->sram_pool,
(unsigned long)prueth->msmcram.va, msmc_ram_size);
@@ -1138,6 +1179,12 @@ static void prueth_remove(struct platform_device *pdev)
prueth_netdev_exit(prueth, eth_node);
}
+ icss_iep_exit(prueth->iep1);
+ icss_iep_exit(prueth->iep0);
+
+ icss_iep_put(prueth->iep1);
+ icss_iep_put(prueth->iep0);
+
gen_pool_free(prueth->sram_pool,
(unsigned long)prueth->msmcram.va,
MSMC_RAM_SIZE_SR1);
--
2.45.1
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 <[email protected]>
---
drivers/net/ethernet/ti/icssg/icss_iep.c | 71 ++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
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);
--
2.45.1
Add the interrupts needed for PTP Hardware Clock support via IEP
in SR1.0 devices.
Signed-off-by: Diogo Ivo <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
index ef7897763ef8..0a29ed172215 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
@@ -73,3 +73,15 @@ &icssg0_eth {
"rx0", "rx1",
"rxmgm0", "rxmgm1";
};
+
+&icssg0_iep0 {
+ interrupt-parent = <&icssg0_intc>;
+ interrupts = <7 7 7>;
+ interrupt-names = "iep_cap_cmp";
+};
+
+&icssg0_iep1 {
+ interrupt-parent = <&icssg0_intc>;
+ interrupts = <56 8 8>;
+ interrupt-names = "iep_cap_cmp";
+};
--
2.45.1
On 5/29/2024 9:05 AM, Diogo Ivo wrote:
> Enable PTP support for AM65x SR1.0 devices by registering with the IEP
> infrastructure in order to expose a PTP clock to userspace.
>
> Signed-off-by: Diogo Ivo <[email protected]>
> ---
> drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 49 +++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> index 7b3304bbd7fc..01cad01965dc 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> @@ -1011,16 +1011,42 @@ static int prueth_probe(struct platform_device *pdev)
> dev_dbg(dev, "sram: pa %llx va %p size %zx\n", prueth->msmcram.pa,
> prueth->msmcram.va, prueth->msmcram.size);
>
> + prueth->iep0 = icss_iep_get_idx(np, 0);
> + if (IS_ERR(prueth->iep0)) {
> + ret = dev_err_probe(dev, PTR_ERR(prueth->iep0), "iep0 get failed\n");
> + goto free_pool;
> + }
> +
> + prueth->iep1 = icss_iep_get_idx(np, 1);
> + if (IS_ERR(prueth->iep1)) {
> + ret = dev_err_probe(dev, PTR_ERR(prueth->iep1), "iep1 get failed\n");
> + goto put_iep0;
> + }
> +
> + ret = icss_iep_init(prueth->iep0, NULL, NULL, 0);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to init iep0\n");
> + goto put_iep;
> + }
> +
> + ret = icss_iep_init(prueth->iep1, NULL, NULL, 0);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to init iep1\n");
> + goto exit_iep0;
> + }
> +
Once initialized, the icss_iep driver logic must implement the actual
PTP clock interfaces?
Neat.
Reviewed-by: Jacob Keller <[email protected]>
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 <[email protected]>
> ---
Reviewed-by: Jacob Keller <[email protected]>
On 5/29/2024 9:05 AM, Diogo Ivo wrote:
> Add the interrupts needed for PTP Hardware Clock support via IEP
> in SR1.0 devices.
>
> Signed-off-by: Diogo Ivo <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
> index ef7897763ef8..0a29ed172215 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi
> @@ -73,3 +73,15 @@ &icssg0_eth {
> "rx0", "rx1",
> "rxmgm0", "rxmgm1";
> };
> +
> +&icssg0_iep0 {
> + interrupt-parent = <&icssg0_intc>;
> + interrupts = <7 7 7>;
> + interrupt-names = "iep_cap_cmp";
> +};
> +
> +&icssg0_iep1 {
> + interrupt-parent = <&icssg0_intc>;
> + interrupts = <56 8 8>;
> + interrupt-names = "iep_cap_cmp";
> +};
>
Reviewed-by: Jacob Keller <[email protected]>
>-----Original Message-----
>From: Diogo Ivo <[email protected]>
>Sent: Wednesday, May 29, 2024 9:35 PM
>To: MD Danish Anwar <[email protected]>; Roger Quadros
><[email protected]>; David S. Miller <[email protected]>; Eric Dumazet
><[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
><[email protected]>; Richard Cochran <[email protected]>;
>Nishanth Menon <[email protected]>; Vignesh Raghavendra <[email protected]>;
>Tero Kristo <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Jan
>Kiszka <[email protected]>
>Cc: [email protected]; [email protected]; linux-
>[email protected]; [email protected]; Diogo Ivo
><[email protected]>
>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 <[email protected]>
>---
> 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);
>+ }
>+
On 29.05.24 18:05, Diogo Ivo wrote:
> This patch series enables support for PTP in AM65x SR1.0 devices.
>
> This feature relies heavily on the Industrial Ethernet Peripheral
> (IEP) hardware module, which implements a hardware counter through
> which time is kept. This hardware block is the basis for exposing
> a PTP hardware clock to userspace and for issuing timestamps for
> incoming/outgoing packets, allowing for time synchronization.
>
> The IEP also has compare registers that fire an interrupt when the
> counter reaches the value stored in a compare register. This feature
> allows us to support PPS events in the kernel.
>
> The changes are separated into three patches:
> - PATCH 01/03: Register SR1.0 devices with the IEP infrastructure to
> expose a PHC clock to userspace, allowing time to be
> adjusted using standard PTP tools. The code for issuing/
> collecting packet timestamps is already present in the
> current state of the driver, so only this needs to be
> done.
> - PATCH 02/03: Add support for IEP compare event/interrupt handling
> to enable PPS events.
> - PATCH 03/03: Add the interrupts to the IOT2050 device tree.
>
> Currently every compare event generates two interrupts, the first
> corresponding to the actual event and the second being a spurious
> but otherwise harmless interrupt. The root cause of this has been
> identified and has been solved in the platform's SDK. A forward port
> of the SDK's patches also fixes the problem in upstream but is not
> included here since it's upstreaming is out of the scope of this
> series. If someone from TI would be willing to chime in and help
> get the interrupt changes upstream that would be great!
>
IIRC, we are talking about this downstream patch:
https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.10.y&id=bbe0ff82f922d368cb7e00c5905f6d4a51635c47
Jan
> Signed-off-by: Diogo Ivo <[email protected]>
> ---
> Diogo Ivo (3):
> net: ti: icssg-prueth: Enable PTP timestamping support for SR1.0 devices
> net: ti: icss-iep: Enable compare events
> arm64: dts: ti: iot2050: Add IEP interrupts for SR1.0 devices
>
> .../boot/dts/ti/k3-am65-iot2050-common-pg1.dtsi | 12 ++++
> drivers/net/ethernet/ti/icssg/icss_iep.c | 71 ++++++++++++++++++++++
> drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 49 ++++++++++++++-
> 3 files changed, 131 insertions(+), 1 deletion(-)
> ---
> base-commit: 2f0e3f6a6824dfda2759225326d9c69203c06bc8
> change-id: 20240529-iep-8bb4a3cb9068
>
> Best regards,
--
Siemens AG, Technology
Linux Expert Center
On Wed, May 29, 2024 at 05:05:10PM +0100, Diogo Ivo wrote:
> Enable PTP support for AM65x SR1.0 devices by registering with the IEP
> infrastructure in order to expose a PTP clock to userspace.
>
> Signed-off-by: Diogo Ivo <[email protected]>
> ---
> drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 49 +++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> index 7b3304bbd7fc..01cad01965dc 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
> @@ -1011,16 +1011,42 @@ static int prueth_probe(struct platform_device *pdev)
> dev_dbg(dev, "sram: pa %llx va %p size %zx\n", prueth->msmcram.pa,
> prueth->msmcram.va, prueth->msmcram.size);
>
> + prueth->iep0 = icss_iep_get_idx(np, 0);
> + if (IS_ERR(prueth->iep0)) {
> + ret = dev_err_probe(dev, PTR_ERR(prueth->iep0), "iep0 get failed\n");
Hi Diogo,
A minor nit from my side.
No need to address this unless there will be a v2 for some other reason.
Networking still prefers code to be 80 columns wide or less.
It looks like that can be trivially achieved here.
Flagged by checkpatch.pl --max-line-length=80
> + goto free_pool;
> + }
> +
> + prueth->iep1 = icss_iep_get_idx(np, 1);
> + if (IS_ERR(prueth->iep1)) {
> + ret = dev_err_probe(dev, PTR_ERR(prueth->iep1), "iep1 get failed\n");
Likewise, here.
> + goto put_iep0;
> + }
...
> >+ 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 Jacob,
On 5/30/24 7:43 PM, Jacob Keller wrote:
>
>
> On 5/29/2024 9:05 AM, Diogo Ivo wrote:
>> + ret = icss_iep_init(prueth->iep0, NULL, NULL, 0);
>> + if (ret) {
>> + dev_err_probe(dev, ret, "failed to init iep0\n");
>> + goto put_iep;
>> + }
>> +
>> + ret = icss_iep_init(prueth->iep1, NULL, NULL, 0);
>> + if (ret) {
>> + dev_err_probe(dev, ret, "failed to init iep1\n");
>> + goto exit_iep0;
>> + }
>> +
>
> Once initialized, the icss_iep driver logic must implement the actual
> PTP clock interfaces?
Yes exactly, the IEP driver then implements the PHC operations.
> Neat.
>
> Reviewed-by: Jacob Keller <[email protected]>
Thank you for the review!
Best regards,
Diogo
Hi Simon,
On 6/1/24 1:03 PM, Simon Horman wrote:
> On Wed, May 29, 2024 at 05:05:10PM +0100, Diogo Ivo wrote:
>> + prueth->iep0 = icss_iep_get_idx(np, 0);
>> + if (IS_ERR(prueth->iep0)) {
>> + ret = dev_err_probe(dev, PTR_ERR(prueth->iep0), "iep0 get failed\n");
>
> Hi Diogo,
>
> A minor nit from my side.
> No need to address this unless there will be a v2 for some other reason.
>
> Networking still prefers code to be 80 columns wide or less.
> It looks like that can be trivially achieved here.
Noted :)
Since I already have to address Sunil's comments on patch 2 I'll change
this one too to comply with the 80 character rule.
Thank you for the review!
Best regards,
Diogo
Hi Sunil,
On 5/31/24 6:12 AM, Sunil Kovvuri Goutham wrote:
>> From: Diogo Ivo <[email protected]>
>> 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