| Message ID | 20241215-v6-13-rc1-net-next-mv88e6xxx-rmu-ops-v1-0-87671db17a65@lunn.ch (mailing list archive) |
|---|---|
| Headers | show |
| Series | dsa: mv88e6xxx: Add RMU enable/disable ops | expand |
On Sun, Dec 15, 2024 at 05:30:02PM +0000, Andrew Lunn wrote: > Add internal APIs for enabling the Remote Management Unit, and > extending the existing implementation to other families. Actually > making use of the RMU is not included here, that will be part of a > later big patch set, which without this preliminary patchset would be > too big. Gerr, forget to use b4 --set-prefixes net-next Andrew
Hi Andrew, On Sun, Dec 15, 2024 at 05:30:02PM +0000, Andrew Lunn wrote: > Add internal APIs for enabling the Remote Management Unit, and > extending the existing implementation to other families. Actually > making use of the RMU is not included here, that will be part of a > later big patch set, which without this preliminary patchset would be > too big. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- How big is the later patch set? Too big to accept even one more patch? There is a risk that the RMU effort gets abandoned before it becomes functional. And in that case, we will have a newly introduced rmu_enable() operation which does nothing. Could you splice the first patch of this set, providing rmu_enable() for some switches but not all, with the set that integrates the RMU with DSA? If the big set is accepted, a trivial follow-up will be necessary to complete the support. If it is not accepted, we don't end up with merged code that we don't need.
On Mon, Dec 16, 2024 at 12:59:10AM +0200, Vladimir Oltean wrote: > Hi Andrew, > > On Sun, Dec 15, 2024 at 05:30:02PM +0000, Andrew Lunn wrote: > > Add internal APIs for enabling the Remote Management Unit, and > > extending the existing implementation to other families. Actually > > making use of the RMU is not included here, that will be part of a > > later big patch set, which without this preliminary patchset would be > > too big. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > How big is the later patch set? Too big to accept even one more patch? The patchset is 21 patches, if i only support one switch family. I can remove a couple of patches, getting statistics via RMU, and timing the RMU vs MDIO and disabling RMU if it is slower. The other way i can slice it is split it into two patchsets: 1) incremental modifications to qca8k to centralise code 2) implement the mv88e6xxx changes to add RMU to it. I did not really want to slice it like this, because the central API is designed around what both QCA8K and Marvell needs, and hopefully is generic enough for other devices. But there might be questions asked when you can only see the qca8k refactor without the Marvell parts. I can maybe squash some of the QCA patches together. Previously i was doing lots of simple changes because i did not have hardware to test on. I do have a QCA8K test system now. > There is a risk that the RMU effort gets abandoned before it becomes > functional. And in that case, we will have a newly introduced rmu_enable() > operation which does nothing. True, but i'm more motivated this time, i'm getting paid for the work :-) And there is one other interested party as well that i know of. This patch series is fully self contained, so it easy to revert, if this ends up going nowhere. Andrew
On Mon, Dec 16, 2024 at 10:50:47AM +0100, Andrew Lunn wrote: > > How big is the later patch set? Too big to accept even one more patch? > > The patchset is 21 patches, if i only support one switch family. > > I can remove a couple of patches, getting statistics via RMU, and > timing the RMU vs MDIO and disabling RMU if it is slower. > > The other way i can slice it is split it into two patchsets: > > 1) incremental modifications to qca8k to centralise code > 2) implement the mv88e6xxx changes to add RMU to it. > > I did not really want to slice it like this, because the central API > is designed around what both QCA8K and Marvell needs, and hopefully is > generic enough for other devices. But there might be questions asked > when you can only see the qca8k refactor without the Marvell parts. > > I can maybe squash some of the QCA patches together. Previously i was > doing lots of simple changes because i did not have hardware to test > on. I do have a QCA8K test system now. > > > There is a risk that the RMU effort gets abandoned before it becomes > > functional. And in that case, we will have a newly introduced rmu_enable() > > operation which does nothing. > > True, but i'm more motivated this time, i'm getting paid for the work :-) > > And there is one other interested party as well that i know of. > > This patch series is fully self contained, so it easy to revert, if > this ends up going nowhere. So what's a no-go is introducing code with no user. Splitting into 2 sets like this should be fine. You could post a link to Github with the complete picture when you post the qca8k refactoring, so that we know what to expect next and where things are going. Hopefully it makes sense on its own and does not leave loose ends hanging. I don't think that squashing multiple logical changes to fit the 15 patch limit is a good idea.
On Mon, 16 Dec 2024 16:59:40 +0200 Vladimir Oltean wrote: > > > There is a risk that the RMU effort gets abandoned before it becomes > > > functional. And in that case, we will have a newly introduced rmu_enable() > > > operation which does nothing. > > > > True, but i'm more motivated this time, i'm getting paid for the work :-) > > > > And there is one other interested party as well that i know of. > > > > This patch series is fully self contained, so it easy to revert, if > > this ends up going nowhere. > > So what's a no-go is introducing code with no user. > > Splitting into 2 sets like this should be fine. You could post a link to > Github with the complete picture when you post the qca8k refactoring, so > that we know what to expect next and where things are going. Hopefully > it makes sense on its own and does not leave loose ends hanging. > > I don't think that squashing multiple logical changes to fit the 15 > patch limit is a good idea. Yes, we're not religious about the 15 patch rule. If you give it an honest try and it doesn't make sense just say so in the cover letter.
Add internal APIs for enabling the Remote Management Unit, and extending the existing implementation to other families. Actually making use of the RMU is not included here, that will be part of a later big patch set, which without this preliminary patchset would be too big. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- Andrew Lunn (2): net: dsa: mv88e6xxx: Enable RMU on 6165 family net: dsa: mv88e6xxx: Enable RMU on 6351 family Mattias Forsblad (1): net: dsa: mv88e6xxx: Add RMU enable for switches that support disable. drivers/net/dsa/mv88e6xxx/chip.c | 29 ++++++++++++ drivers/net/dsa/mv88e6xxx/chip.h | 1 + drivers/net/dsa/mv88e6xxx/global1.c | 89 +++++++++++++++++++++++++++++++++++++ drivers/net/dsa/mv88e6xxx/global1.h | 10 +++++ 4 files changed, 129 insertions(+) --- base-commit: 2c2b61d2138f472e50b5531ec0cb4a1485837e21 change-id: 20241207-v6-13-rc1-net-next-mv88e6xxx-rmu-ops-4d0a84f5f7d1 Best regards,