| Message ID | 20240815083814.4273-1-foss@martin-whitaker.me.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: dsa: microchip: fix PTP config failure when using multiple ports | expand |
On Thu, Aug 15, 2024 at 09:38:14AM +0100, Martin Whitaker wrote: > When performing the port_hwtstamp_set operation, ptp_schedule_worker() > will be called if hardware timestamoing is enabled on any of the ports. > When using multiple ports for PTP, port_hwtstamp_set is executed for > each port. When called for the first time ptp_schedule_worker() returns > 0. On subsequent calls it returns 1, indicating the worker is already > scheduled. Currently the ksz driver treats 1 as an error and fails to > complete the port_hwtstamp_set operation, thus leaving the timestamping > configuration for those ports unchanged. > > This patch fixes this by ignoring the ptp_schedule_worker() return > value. Hi Martin Is this your first patch to netdev? Nicely done. A few minor improvements. You have the correct tree, net, since this is a fix. You should add a Fixes: tag indicating the patch which added the bug. And also Cc: stable@stable@vger.kernel.org Thanks Andrew --- pw-bot: cr
On 15/08/2024 15:38, Andrew Lunn wrote: > On Thu, Aug 15, 2024 at 09:38:14AM +0100, Martin Whitaker wrote: >> When performing the port_hwtstamp_set operation, ptp_schedule_worker() >> will be called if hardware timestamoing is enabled on any of the ports. >> When using multiple ports for PTP, port_hwtstamp_set is executed for >> each port. When called for the first time ptp_schedule_worker() returns >> 0. On subsequent calls it returns 1, indicating the worker is already >> scheduled. Currently the ksz driver treats 1 as an error and fails to >> complete the port_hwtstamp_set operation, thus leaving the timestamping >> configuration for those ports unchanged. >> >> This patch fixes this by ignoring the ptp_schedule_worker() return >> value. > > Hi Martin > > Is this your first patch to netdev? Nicely done. A few minor > improvements. You have the correct tree, net, since this is a fix. You > should add a Fixes: tag indicating the patch which added the bug. And > also Cc: stable@stable@vger.kernel.org > > Thanks > Andrew Hi Andrew, It's my second patch. Yes, I missed the Fixes: tag. It should be Fixes: bb01ad30570b0 ("net: dsa: microchip: ptp: manipulating absolute time using ptp hw clock") Will you insert this, or do you need me to resend the patch? Martin
On Fri, Aug 16, 2024 at 06:18:46PM +0100, Martin Whitaker wrote: > On 15/08/2024 15:38, Andrew Lunn wrote: > > On Thu, Aug 15, 2024 at 09:38:14AM +0100, Martin Whitaker wrote: > > > When performing the port_hwtstamp_set operation, ptp_schedule_worker() > > > will be called if hardware timestamoing is enabled on any of the ports. > > > When using multiple ports for PTP, port_hwtstamp_set is executed for > > > each port. When called for the first time ptp_schedule_worker() returns > > > 0. On subsequent calls it returns 1, indicating the worker is already > > > scheduled. Currently the ksz driver treats 1 as an error and fails to > > > complete the port_hwtstamp_set operation, thus leaving the timestamping > > > configuration for those ports unchanged. > > > > > > This patch fixes this by ignoring the ptp_schedule_worker() return > > > value. > > > > Hi Martin > > > > Is this your first patch to netdev? Nicely done. A few minor > > improvements. You have the correct tree, net, since this is a fix. You > > should add a Fixes: tag indicating the patch which added the bug. And > > also Cc: stable@stable@vger.kernel.org > > > > Thanks > > Andrew > > Hi Andrew, > > It's my second patch. Yes, I missed the Fixes: tag. It should be > > Fixes: bb01ad30570b0 ("net: dsa: microchip: ptp: manipulating absolute > time using ptp hw clock") > > Will you insert this, or do you need me to resend the patch? Ideally, please resend it. patchwork might pickup the Fixes: tag from your reply, but since it was wrapped, it might get it wrong. Sometimes Jakub fixes things like this when merging patches, but it is easier for him if there is a new correct version. And it is a good learning exercise for somebody who i hope will submit more patches in the future.. Andrew
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c index f0bd46e5d4ec..050f17c43ef6 100644 --- a/drivers/net/dsa/microchip/ksz_ptp.c +++ b/drivers/net/dsa/microchip/ksz_ptp.c @@ -266,7 +266,6 @@ static int ksz_ptp_enable_mode(struct ksz_device *dev) struct ksz_port *prt; struct dsa_port *dp; bool tag_en = false; - int ret; dsa_switch_for_each_user_port(dp, dev->ds) { prt = &dev->ports[dp->index]; @@ -277,9 +276,7 @@ static int ksz_ptp_enable_mode(struct ksz_device *dev) } if (tag_en) { - ret = ptp_schedule_worker(ptp_data->clock, 0); - if (ret) - return ret; + ptp_schedule_worker(ptp_data->clock, 0); } else { ptp_cancel_worker_sync(ptp_data->clock); }
When performing the port_hwtstamp_set operation, ptp_schedule_worker() will be called if hardware timestamoing is enabled on any of the ports. When using multiple ports for PTP, port_hwtstamp_set is executed for each port. When called for the first time ptp_schedule_worker() returns 0. On subsequent calls it returns 1, indicating the worker is already scheduled. Currently the ksz driver treats 1 as an error and fails to complete the port_hwtstamp_set operation, thus leaving the timestamping configuration for those ports unchanged. This patch fixes this by ignoring the ptp_schedule_worker() return value. Link: https://lore.kernel.org/netdev/7aae307a-35ca-4209-a850-7b2749d40f90@martin-whitaker.me.uk/ Signed-off-by: Martin Whitaker <foss@martin-whitaker.me.uk> --- drivers/net/dsa/microchip/ksz_ptp.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) -- 2.41.1