Messages in this thread | | | Date | Sun, 4 Jun 2023 11:33:30 +0300 | Subject | Re: [PATCH net-next 26/30] net: dsa: mt7530: properly set MT7530_CPU_PORT | From | Arınç ÜNAL <> |
| |
On 26.05.2023 19:55, Vladimir Oltean wrote: > On Mon, May 22, 2023 at 03:15:28PM +0300, arinc9.unal@gmail.com wrote: >> From: Arınç ÜNAL <arinc.unal@arinc9.com> >> >> The MT7530_CPU_PORT bits represent the CPU port to trap frames to for the >> MT7530 switch. There are two issues with the current way of setting these >> bits. ID_MT7530 which is for the standalone MT7530 switch is not included. > > It's best to say in the commit title what the change does, rather than > the equivalent of "here, this way is proper!". Commit titles should be > uniquely identifiable, and "properly set MT7530_CPU_PORT" doesn't say a > lot about how proper it is. It's enough to imagine a future person > finding something else that's perfectible and writing another "net: dsa: > mt7530: properly set MT7530_CPU_PORT" commit. Try to be less definitive > and at the same time more specific. > > If there are 2 issues, there should be 2 changes with individual titles > which each describes what was wrong and how that was changed.
Got it, this is a bug fix for future devicetrees so I will send a 2-patch patch series to net. First one sets the MT7530_CPU_PORT bit to the active CPU port, the other adds the ID_MT7530 check.
> >> When multiple CPU ports are being used, the trapped frames won't be >> received when the DSA conduit interface, which the frames are supposed to >> be trapped to, is down because it's not affine to any user port. This >> requires the DSA conduit interface to be manually set up for the trapped >> frames to be received. >> >> Address these issues by implementing ds->ops->master_state_change() on this >> subdriver and setting the MT7530_CPU_PORT bits there. Introduce the >> active_cpu_ports field to store the information of active CPU ports. >> Correct the macros, MT7530_CPU_PORT is bits 4 through 6 of the register. >> >> Any frames set for trapping to CPU port will be trapped to the numerically >> smallest CPU port which is affine to the DSA conduit interface that is set >> up. To make the understatement obvious, the frames won't necessarily be >> trapped to the CPU port the user port, which these frames are received >> from, is affine to. This operation is only there to make sure the trapped >> frames always reach the CPU. >> >> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> Co-developed-by: Vladimir Oltean <olteanv@gmail.com> >> Signed-off-by: Vladimir Oltean <olteanv@gmail.com> > > A single Suggested-by: is fine. As a rule of thumb, I would use Co-developed-by > when I'm working with a patch formally pre-formatted or committed by somebody else, > that I've changed in a significant manner. Since all I did was to comment with > a suggestion of how to handle this, and with a code snippet written in the email > client to a patch of yours, I don't believe that's necessary here.
Will do, thanks.
Arınç
| |