| Message ID | 20220304093418.31645-1-Divya.Koppera@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for 1588 in LAN8814 | expand |
Hello: This series was applied to netdev/net-next.git (master) by David S. Miller <davem@davemloft.net>: On Fri, 4 Mar 2022 15:04:15 +0530 you wrote: > The following patch series contains: > - Fix for concurrent register access, which provides > atomic access to extended page register reads/writes. > - Provides dt-bindings related to latency and timestamping > that are required for LAN8814 phy. > - 1588 hardware timestamping support in LAN8814 phy. > > [...] Here is the summary with links: - [net-next,1/3] net: phy: micrel: Fix concurrent register access https://git.kernel.org/netdev/net-next/c/4488f6b61480 - [net-next,2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy https://git.kernel.org/netdev/net-next/c/2358dd3fd325 - [net-next,3/3] net: phy: micrel: 1588 support for LAN8814 phy https://git.kernel.org/netdev/net-next/c/ece19502834d You are awesome, thank you!
On Fri, Mar 04, 2022 at 12:50:11PM +0000, patchwork-bot+netdevbpf@kernel.org wrote: > Hello: > > This series was applied to netdev/net-next.git (master) > by David S. Miller <davem@davemloft.net>: Hi David Why was this merged? Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: Fri, 4 Mar 2022 14:06:54 +0100 > On Fri, Mar 04, 2022 at 12:50:11PM +0000, patchwork-bot+netdevbpf@kernel.org wrote: >> Hello: >> >> This series was applied to netdev/net-next.git (master) >> by David S. Miller <davem@davemloft.net>: > > Hi David > > Why was this merged? Sorry, it seemed satraightforward to me, and I try to get the backlog under 40 patches before I hand over to Jakub for the day. If you want to review, reply to the thread immediately saying so, don't wait until you haver time for the full review. Thank you.
On Fri, Mar 04, 2022 at 01:21:21PM +0000, David Miller wrote: > From: Andrew Lunn <andrew@lunn.ch> > Date: Fri, 4 Mar 2022 14:06:54 +0100 > > > On Fri, Mar 04, 2022 at 12:50:11PM +0000, patchwork-bot+netdevbpf@kernel.org wrote: > >> Hello: > >> > >> This series was applied to netdev/net-next.git (master) > >> by David S. Miller <davem@davemloft.net>: > > > > Hi David > > > > Why was this merged? > > Sorry, it seemed satraightforward to me, and I try to get the backlog under 40 patches before > I hand over to Jakub for the day. > > If you want to review, reply to the thread immediately saying so, don't wait until you haver time for the > full review. This patchset was on the list for less than 5 hours before it got merged. I tend to sleep for 8 to 10 hours. Making it impossible for me to react any faster. At an absolute minimum, you need to wait 12 hours, if you expect anybody to have a fair chance of being able to say, hold on, i want to comment on this patchset. I also don't like the metric of 40 patches backlog. Is the size of backlog more important than the quality of the patches? Don't we care about the quality of the code any more? Don't we care about getting code reviewed any more? Andrew
On Fri, Mar 04, 2022 at 01:21:21PM +0000, David Miller wrote: > Sorry, it seemed satraightforward to me, and I try to get the backlog under 40 patches before > I hand over to Jakub for the day. Day by day, it seems like there is more and more of this PTP driver stuff. Maybe it is time for me to manage a separate PTP driver tree. I could get the reviews and acks, then place a PR to netdev or lkml when ready. Thoughts? Thanks, Richard
On Fri, Mar 04, 2022 at 06:06:28AM -0800, Richard Cochran wrote: > On Fri, Mar 04, 2022 at 01:21:21PM +0000, David Miller wrote: > > > Sorry, it seemed satraightforward to me, and I try to get the backlog under 40 patches before > > I hand over to Jakub for the day. > > Day by day, it seems like there is more and more of this PTP driver > stuff. Maybe it is time for me to manage a separate PTP driver tree. > I could get the reviews and acks, then place a PR to netdev or lkml > when ready. > > Thoughts? Hi Richard My perception is that you also like to sleep at nights, and cannot keep up with David rapid pace. So setting up your own tree, collecting reviews and acks yourself will help you and the quality of the PTP code. So yes, go for it. I just think it is wrong you have to do this. Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: Fri, 4 Mar 2022 14:47:48 +0100 > On Fri, Mar 04, 2022 at 01:21:21PM +0000, David Miller wrote: >> From: Andrew Lunn <andrew@lunn.ch> >> Date: Fri, 4 Mar 2022 14:06:54 +0100 >> >> > On Fri, Mar 04, 2022 at 12:50:11PM +0000, patchwork-bot+netdevbpf@kernel.org wrote: >> >> Hello: >> >> >> >> This series was applied to netdev/net-next.git (master) >> >> by David S. Miller <davem@davemloft.net>: >> > >> > Hi David >> > >> > Why was this merged? >> >> Sorry, it seemed satraightforward to me, and I try to get the backlog under 40 patches before >> I hand over to Jakub for the day. >> >> If you want to review, reply to the thread immediately saying so, don't wait until you haver time for the >> full review. > > This patchset was on the list for less than 5 hours before it got > merged. I tend to sleep for 8 to 10 hours. Making it impossible for me > to react any faster. At an absolute minimum, you need to wait 12 > hours, if you expect anybody to have a fair chance of being able to > say, hold on, i want to comment on this patchset. > > I also don't like the metric of 40 patches backlog. Is the size of > backlog more important than the quality of the patches? Don't we care > about the quality of the code any more? Don't we care about getting > code reviewed any more? Ok, message received, I'll apply things less aggressively. Thank you.
On Fri, Mar 04, 2022 at 03:17:19PM +0100, Andrew Lunn wrote: > My perception is that you also like to sleep at nights, and cannot > keep up with David rapid pace. So setting up your own tree, collecting > reviews and acks yourself will help you and the quality of the PTP > code. So yes, go for it. I just think it is wrong you have to do > this. I agree that PTP drivers are being merged too quickly without enough review, and my commitment to maintaining a PTP tree will both allow better review and reduce davem's burden. So maybe it is right for me to do this after all. Thanks, Richard
From: patchwork-bot+netdevbpf@kernel.org > Here is the summary with links: > - [net-next,1/3] net: phy: micrel: Fix concurrent register access > https://git.kernel.org/netdev/net-next/c/4488f6b61480 > - [net-next,2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy > https://git.kernel.org/netdev/net-next/c/2358dd3fd325 > - [net-next,3/3] net: phy: micrel: 1588 support for LAN8814 phy > https://git.kernel.org/netdev/net-next/c/ece19502834d I'm almost afraid to ask.. but will this series be reverted (or the device tree bindings patch)? There were quite a few remarks, even about the naming of the properties. So, will it be part of the next kernel release or will it be reverted? -michael
Hi, On Thu, Mar 17, 2022 at 01:16:50PM +0100, Michael Walle wrote: > From: patchwork-bot+netdevbpf@kernel.org > > Here is the summary with links: > > - [net-next,1/3] net: phy: micrel: Fix concurrent register access > > https://git.kernel.org/netdev/net-next/c/4488f6b61480 > > - [net-next,2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy > > https://git.kernel.org/netdev/net-next/c/2358dd3fd325 > > - [net-next,3/3] net: phy: micrel: 1588 support for LAN8814 phy > > https://git.kernel.org/netdev/net-next/c/ece19502834d > > I'm almost afraid to ask.. but will this series be reverted (or > the device tree bindings patch)? There were quite a few remarks, even > about the naming of the properties. So, will it be part of the next > kernel release or will it be reverted? Thanks for bringing this up - was about to ask myself. Not sure what is the normal procedure here. If not reverted, we can do a patch to remove the dt-bindings (and also the code in the driver using them). Also, a few other minor comments was given and we can fix those. The elefant in the room is the 'lan8814_latencies' structure containing the default latency values in the driver, which Richard is unhappy with. Russell indicated that he prefere having these numbers in the driver rather than hiding them in firmware (lan8814 does not have firmware, so not an option). Andrew sugegsted adding additional APIs to let ptp4l control if time-stamps should be calibrated in HW/Kernel or in userspace. Likely something like that can be done - but I did not get the impression that this is what Richard would like to see either. Also, I would like drivers to come with default latency numbers which are good enough for most (and the rest will need to calibrate and compensate further using the hooks and handles in userspace). What would you like to see - believe you will also be a user of this?
On Thu, Mar 17, 2022 at 03:05:59PM +0100, Allan W. Nielsen wrote: > Hi, > > On Thu, Mar 17, 2022 at 01:16:50PM +0100, Michael Walle wrote: > > From: patchwork-bot+netdevbpf@kernel.org > > > Here is the summary with links: > > > - [net-next,1/3] net: phy: micrel: Fix concurrent register access > > > https://git.kernel.org/netdev/net-next/c/4488f6b61480 > > > - [net-next,2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy > > > https://git.kernel.org/netdev/net-next/c/2358dd3fd325 > > > - [net-next,3/3] net: phy: micrel: 1588 support for LAN8814 phy > > > https://git.kernel.org/netdev/net-next/c/ece19502834d > > > > I'm almost afraid to ask.. but will this series be reverted (or > > the device tree bindings patch)? There were quite a few remarks, even > > about the naming of the properties. So, will it be part of the next > > kernel release or will it be reverted? > Thanks for bringing this up - was about to ask myself. > > Not sure what is the normal procedure here. I assume this is in net-next. So we have two weeks of the merge window followed by around 7 weeks of the -rc in order to clean this up. It is only when the code is released in a final kernel does it become an ABI. > If not reverted, we can do a patch to remove the dt-bindings (and also > the code in the driver using them). Also, a few other minor comments was > given and we can fix those. Patches would be good. Ideally the patches would be posted in the next couple of weeks, even if we do have a lot longer. > The elefant in the room is the 'lan8814_latencies' structure containing > the default latency values in the driver, which Richard is unhappy with. The important thing is getting the ABI fixed. So the DT properties need to be removed, etc. To some extend the corrections are ABI. If the corrections change the user space configuration also needs to change when trying to get the best out of the hardware. So depending on how long the elefant is around, it might make sense to actually do a revert, or at minimum disabling PTP, so time can be spent implementing new APIs or whatever is decided. So i would suggest a two pronged attach: Fixup patchs Try to bring the discussion to a close and implement whatever is decided. Andrew
On Thu, Mar 17, 2022 at 03:38:50PM +0100, Andrew Lunn wrote: > On Thu, Mar 17, 2022 at 03:05:59PM +0100, Allan W. Nielsen wrote: > > Hi, > > > > On Thu, Mar 17, 2022 at 01:16:50PM +0100, Michael Walle wrote: > > > From: patchwork-bot+netdevbpf@kernel.org > > > > Here is the summary with links: > > > > - [net-next,1/3] net: phy: micrel: Fix concurrent register access > > > > https://git.kernel.org/netdev/net-next/c/4488f6b61480 > > > > - [net-next,2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy > > > > https://git.kernel.org/netdev/net-next/c/2358dd3fd325 > > > > - [net-next,3/3] net: phy: micrel: 1588 support for LAN8814 phy > > > > https://git.kernel.org/netdev/net-next/c/ece19502834d > > > > > > I'm almost afraid to ask.. but will this series be reverted (or > > > the device tree bindings patch)? There were quite a few remarks, even > > > about the naming of the properties. So, will it be part of the next > > > kernel release or will it be reverted? > > Thanks for bringing this up - was about to ask myself. > > > > Not sure what is the normal procedure here. > > I assume this is in net-next. So we have two weeks of the merge window > followed by around 7 weeks of the -rc in order to clean this up. It is > only when the code is released in a final kernel does it become an > ABI. > > > If not reverted, we can do a patch to remove the dt-bindings (and also > > the code in the driver using them). Also, a few other minor comments was > > given and we can fix those. > > Patches would be good. Ideally the patches would be posted in the next > couple of weeks, even if we do have a lot longer. > > > The elefant in the room is the 'lan8814_latencies' structure containing > > the default latency values in the driver, which Richard is unhappy with. > > The important thing is getting the ABI fixed. So the DT properties > need to be removed, etc. We will do that. > To some extend the corrections are ABI. If the corrections change the > user space configuration also needs to change when trying to get the > best out of the hardware. So depending on how long the elefant is > around, it might make sense to actually do a revert, or at minimum > disabling PTP, so time can be spent implementing new APIs or whatever > is decided. ACK. > So i would suggest a two pronged attach: > > Fixup patchs > Try to bring the discussion to a close and implement whatever is decided. Make sense - we will do the fix-ups and try restart the dicussion.
The following patch series contains: - Fix for concurrent register access, which provides atomic access to extended page register reads/writes. - Provides dt-bindings related to latency and timestamping that are required for LAN8814 phy. - 1588 hardware timestamping support in LAN8814 phy. Divya Koppera (3): net: phy: micrel: Fix concurrent register access dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy net: phy: micrel: 1588 support for LAN8814 phy .../devicetree/bindings/net/micrel.txt | 17 + drivers/net/phy/micrel.c | 1114 ++++++++++++++++- 2 files changed, 1097 insertions(+), 34 deletions(-)