Messages in this thread | | | Date | Fri, 08 Jul 2022 09:12:46 +0200 | From | netdev@kapio-te ... | Subject | Re: [PATCH net-next 1/1] net: dsa: mv88e6xxx: allow reading FID when handling ATU violations |
| |
On 2022-07-07 12:28, Vladimir Oltean wrote: > On Wed, Jul 06, 2022 at 02:25:02PM +0200, Hans Schultz wrote: >> For convenience the function mv88e6xxx_g1_atu_op() has been used to >> read >> ATU violations, but the function has other purposes and does not >> enable >> the possibility to read the FID when reading ATU violations. >> >> The FID is needed to get hold of which VID was involved in the >> violation, >> thus the need for future purposes to be able to read the FID. > > Make no mistake, the existing code doesn't disallow reading back the > FID > during an ATU Get/Clear Violation operation, and your patch isn't > "allowing" something that wasn't disallowed.
It would only read 0 the way it worked. And I don't understand why mv88e6xxx_g1_atu_op() writes the FID?
> > The documentation for the ATU FID register says that its contents is > ignored before the operation starts, and it contains the returned ATU > entry's FID after the operation completes. > > So the change simply says: don't bother to write the ATU FID register > with zero, it doesn't matter what this contains. This is probably true, > but the patch needs to do what's written on the box.
Writing 0 to the ATU fID register resulted in a read giving zero of course.
> > Please note that this only even matters at all for switches with > mv88e6xxx_num_databases(chip) > 256, where MV88E6352_G1_ATU_FID is a > dedicated register which this patch avoids writing. For other switches, > the FID is embedded within MV88E6XXX_G1_ATU_CTL or MV88E6XXX_G1_ATU_OP. > So _practically_, for those switches, you are still emitting the > GET_CLR_VIOLATION ATU op with a FID of 0 whether you like it or not, > and > this patch introduces a (most likely irrelevant) discrepancy between > the > access methods for various switches. > > Please note that this observation is relevant for your future changes > to > read back the FID too. As I said here: > https://patchwork.kernel.org/project/netdevbpf/patch/20220524152144.40527-4-schultz.hans+netdev@gmail.com/#24912482 > you can't just assume that the FID lies within the MV88E6352_G1_ATU_FID > register, just look at the way it is packed within > mv88e6xxx_g1_atu_op(). > You'll need to unpack it in the same way.
So I need a new function to read the FID that mimics mv88e6xxx_g1_atu_op() as far as I understand?
| |