| Message ID | 20220818161649.2058728-1-sean.anderson@seco.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: dpaa: Cleanups in preparation for phylink conversion | expand |
On Thu, 18 Aug 2022 12:16:24 -0400 Sean Anderson wrote: > This series contains several cleanup patches for dpaa/fman. While they > are intended to prepare for a phylink conversion, they stand on their > own. This series was originally submitted as part of [1]. Still over the limit of patches in a patch series, and looks pretty easy to chunk up. We review and apply patches in netdev in 1-3 days, it really is more efficient to post smaller series. And with the other series you sent to the list we have nearly 50 patches from you queued for review. I don't think this is reasonable, people reviewing this code are all volunteers trying to get their work done as well :(
Hi Jakub, On 8/18/22 2:20 PM, Jakub Kicinski wrote: > On Thu, 18 Aug 2022 12:16:24 -0400 Sean Anderson wrote: >> This series contains several cleanup patches for dpaa/fman. While they >> are intended to prepare for a phylink conversion, they stand on their >> own. This series was originally submitted as part of [1]. > > Still over the limit of patches in a patch series, and looks pretty > easy to chunk up. We review and apply patches in netdev in 1-3 days, > it really is more efficient to post smaller series. Last time I offered to arbitrarily chunk things [1], but I did not receive a response for 3 weeks. > And with the other series you sent to the list we have nearly 50 > patches from you queued for review. I don't think this is reasonable, > people reviewing this code are all volunteers trying to get their > work done as well :( These patches have been sent to the list in one form or another since I first sent out an RFC for DPAA conversion back in June [2]. I have not substantially modified any of them (although I did add a few more since v2). It's not like I came up with these just now; I have been seeking feedback on these series for 2-3 months so far. The only reviews I have gotten were from Camelia Groza, who has provided some helpful feedback. --Sean [1] https://lore.kernel.org/netdev/51c26e19-e4ea-7596-1147-7d95eec9e6a9@seco.com/ [2] https://lore.kernel.org/netdev/20220617203312.3799646-1-sean.anderson@seco.com/
On Thu, 18 Aug 2022 14:37:23 -0400 Sean Anderson wrote: > On 8/18/22 2:20 PM, Jakub Kicinski wrote: > > On Thu, 18 Aug 2022 12:16:24 -0400 Sean Anderson wrote: > >> This series contains several cleanup patches for dpaa/fman. While they > >> are intended to prepare for a phylink conversion, they stand on their > >> own. This series was originally submitted as part of [1]. > > > > Still over the limit of patches in a patch series, and looks pretty > > easy to chunk up. We review and apply patches in netdev in 1-3 days, > > it really is more efficient to post smaller series. > > Last time I offered to arbitrarily chunk things [1], but I did not receive > a response for 3 weeks. I sent you the link to the rules. Admittedly not the most polite and clear feedback to put it mildly but that was the reason. > > And with the other series you sent to the list we have nearly 50 > > patches from you queued for review. I don't think this is reasonable, > > people reviewing this code are all volunteers trying to get their > > work done as well :( > > These patches have been sent to the list in one form or another since > I first sent out an RFC for DPAA conversion back in June [2]. I have not > substantially modified any of them (although I did add a few more > since v2). It's not like I came up with these just now; I have been > seeking feedback on these series for 2-3 months so far. The only > reviews I have gotten were from Camelia Groza, who has provided some > helpful feedback. Ack, no question. I'm trying to tell you got to actually get stuff in. It's the first week after the merge window and people are dumping code the had written over the dead time on the list, while some reviewers and maintainers are still on their summer vacation. So being considerate is even more important than normally.
On 8/18/22 2:58 PM, Jakub Kicinski wrote: > On Thu, 18 Aug 2022 14:37:23 -0400 Sean Anderson wrote: >> On 8/18/22 2:20 PM, Jakub Kicinski wrote: >> > On Thu, 18 Aug 2022 12:16:24 -0400 Sean Anderson wrote: >> >> This series contains several cleanup patches for dpaa/fman. While they >> >> are intended to prepare for a phylink conversion, they stand on their >> >> own. This series was originally submitted as part of [1]. >> > >> > Still over the limit of patches in a patch series, and looks pretty >> > easy to chunk up. We review and apply patches in netdev in 1-3 days, >> > it really is more efficient to post smaller series. >> >> Last time I offered to arbitrarily chunk things [1], but I did not receive >> a response for 3 weeks. > > I sent you the link to the rules. Admittedly not the most polite and > clear feedback to put it mildly but that was the reason. It would have helped if you'd clarified; I thought the primary thing was the missing net-next. >> > And with the other series you sent to the list we have nearly 50 >> > patches from you queued for review. I don't think this is reasonable, >> > people reviewing this code are all volunteers trying to get their >> > work done as well :( >> >> These patches have been sent to the list in one form or another since >> I first sent out an RFC for DPAA conversion back in June [2]. I have not >> substantially modified any of them (although I did add a few more >> since v2). It's not like I came up with these just now; I have been >> seeking feedback on these series for 2-3 months so far. The only >> reviews I have gotten were from Camelia Groza, who has provided some >> helpful feedback. > > Ack, no question. I'm trying to tell you got to actually get stuff in. > It's the first week after the merge window and people are dumping code > the had written over the dead time on the list, while some reviewers > and maintainers are still on their summer vacation. So being > considerate is even more important than normally. OK, so perhaps a nice place to split the series is after patch 11. If you would like to review/apply a set of <15 patches, that is the place to break things. I can of course resend again with just those, if that's what I need to do to get these applied. That said, I specifically broke this series up into many small patches to make it easier to review. Each patch does exactly one thing. Had I known about these limits based on patch size, I would have just squashed everything into 15 patches. I think an arbitrary limit such as this has a perverse incentive. --Sean
On Thu, 18 Aug 2022 15:14:04 -0400 Sean Anderson wrote: > > Ack, no question. I'm trying to tell you got to actually get stuff in. > > It's the first week after the merge window and people are dumping code > > the had written over the dead time on the list, while some reviewers > > and maintainers are still on their summer vacation. So being > > considerate is even more important than normally. > > OK, so perhaps a nice place to split the series is after patch 11. If > you would like to review/apply a set of <15 patches, that is the place > to break things. I can of course resend again with just those, if that's > what I need to do to get these applied. Mm, okay, let's give folks the customary 24h to object, otherwise I'll pull in the first 11 tomorrow. > That said, I specifically broke this series up into many small patches > to make it easier to review. Each patch does exactly one thing. Had I > known about these limits based on patch size, I would have just squashed > everything into 15 patches. I think an arbitrary limit such as this has > a perverse incentive. Practically speaking I think it works out fairly nicely, IDK. There's trade offs like always. Takes a bit of getting used to.
Hello: This series was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Thu, 18 Aug 2022 12:16:24 -0400 you wrote: > This series contains several cleanup patches for dpaa/fman. While they > are intended to prepare for a phylink conversion, they stand on their > own. This series was originally submitted as part of [1]. > > [1] https://lore.kernel.org/netdev/20220715215954.1449214-1-sean.anderson@seco.com > > Changes in v4: > - Clarify commit message > - weer -> were > - tricy -> tricky > - Use mac_dev for calling change_addr > - qman_cgr_create -> qman_create_cgr > > [...] Here is the summary with links: - [RESEND,net-next,v4,01/25] dt-bindings: net: Convert FMan MAC bindings to yaml https://git.kernel.org/netdev/net-next/c/ee8433da085e - [RESEND,net-next,v4,02/25] net: fman: Convert to SPDX identifiers https://git.kernel.org/netdev/net-next/c/8585bdadc247 - [RESEND,net-next,v4,03/25] net: fman: Don't pass comm_mode to enable/disable https://git.kernel.org/netdev/net-next/c/b7d852566a52 - [RESEND,net-next,v4,04/25] net: fman: Store en/disable in mac_device instead of mac_priv_s https://git.kernel.org/netdev/net-next/c/e61406a1955e - [RESEND,net-next,v4,05/25] net: fman: dtsec: Always gracefully stop/start https://git.kernel.org/netdev/net-next/c/aae73fde7eb3 - [RESEND,net-next,v4,06/25] net: fman: Get PCS node in per-mac init https://git.kernel.org/netdev/net-next/c/478eb957ced6 - [RESEND,net-next,v4,07/25] net: fman: Store initialization function in match data https://git.kernel.org/netdev/net-next/c/28c3948a018d - [RESEND,net-next,v4,08/25] net: fman: Move struct dev to mac_device https://git.kernel.org/netdev/net-next/c/7bd63966f0cc - [RESEND,net-next,v4,09/25] net: fman: Configure fixed link in memac_initialization https://git.kernel.org/netdev/net-next/c/9ea4742a55ca - [RESEND,net-next,v4,10/25] net: fman: Export/rename some common functions https://git.kernel.org/netdev/net-next/c/c496e4d686aa - [RESEND,net-next,v4,11/25] net: fman: memac: Use params instead of priv for max_speed https://git.kernel.org/netdev/net-next/c/c0e36be156c2 - [RESEND,net-next,v4,12/25] net: fman: Move initialization to mac-specific files (no matching commit) - [RESEND,net-next,v4,13/25] net: fman: Mark mac methods static (no matching commit) - [RESEND,net-next,v4,14/25] net: fman: Inline several functions into initialization (no matching commit) - [RESEND,net-next,v4,15/25] net: fman: Remove internal_phy_node from params (no matching commit) - [RESEND,net-next,v4,16/25] net: fman: Map the base address once (no matching commit) - [RESEND,net-next,v4,17/25] net: fman: Pass params directly to mac init (no matching commit) - [RESEND,net-next,v4,18/25] net: fman: Use mac_dev for some params (no matching commit) - [RESEND,net-next,v4,19/25] net: fman: Specify type of mac_dev for exception_cb (no matching commit) - [RESEND,net-next,v4,20/25] net: fman: Clean up error handling (no matching commit) - [RESEND,net-next,v4,21/25] net: fman: Change return type of disable to void (no matching commit) - [RESEND,net-next,v4,22/25] net: dpaa: Use mac_dev variable in dpaa_netdev_init (no matching commit) - [RESEND,net-next,v4,23/25] soc: fsl: qbman: Add helper for sanity checking cgr ops (no matching commit) - [RESEND,net-next,v4,24/25] soc: fsl: qbman: Add CGR update function (no matching commit) - [RESEND,net-next,v4,25/25] net: dpaa: Adjust queue depth on rate change (no matching commit) You are awesome, thank you!
Hi Jakub, On 8/18/22 3:28 PM, Jakub Kicinski wrote: > On Thu, 18 Aug 2022 15:14:04 -0400 Sean Anderson wrote: >> > Ack, no question. I'm trying to tell you got to actually get stuff in. >> > It's the first week after the merge window and people are dumping code >> > the had written over the dead time on the list, while some reviewers >> > and maintainers are still on their summer vacation. So being >> > considerate is even more important than normally. >> >> OK, so perhaps a nice place to split the series is after patch 11. If >> you would like to review/apply a set of <15 patches, that is the place >> to break things. I can of course resend again with just those, if that's >> what I need to do to get these applied. > > Mm, okay, let's give folks the customary 24h to object, otherwise I'll > pull in the first 11 tomorrow. OK, it's been around two weeks. Aside from one bugfix (thanks Dan), there has been no further feedback on this series. Can we apply the second half? --Sean
On 9/2/22 5:32 PM, Sean Anderson wrote: > Hi Jakub, > > On 8/18/22 3:28 PM, Jakub Kicinski wrote: >> On Thu, 18 Aug 2022 15:14:04 -0400 Sean Anderson wrote: >>> > Ack, no question. I'm trying to tell you got to actually get stuff in. >>> > It's the first week after the merge window and people are dumping code >>> > the had written over the dead time on the list, while some reviewers >>> > and maintainers are still on their summer vacation. So being >>> > considerate is even more important than normally. >>> >>> OK, so perhaps a nice place to split the series is after patch 11. If >>> you would like to review/apply a set of <15 patches, that is the place >>> to break things. I can of course resend again with just those, if that's >>> what I need to do to get these applied. >> >> Mm, okay, let's give folks the customary 24h to object, otherwise I'll >> pull in the first 11 tomorrow. > > OK, it's been around two weeks. Aside from one bugfix (thanks Dan), there > has been no further feedback on this series. Can we apply the second half? Actually, the mentioned bugfix causes this series not to apply. I'm resending the second half. --Sean