| Message ID | 20240529-iep-v1-1-7273c07592d3@siemens.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Enable PTP timestamping/PPS for AM65x SR1.0 devices | expand |
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 <diogo.ivo@siemens.com> > --- > 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 <jacob.e.keller@intel.com>
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 <diogo.ivo@siemens.com> > --- > 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; > + } ...
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 <jacob.e.keller@intel.com> 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
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);
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 <diogo.ivo@siemens.com> --- drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c | 49 +++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)