Re: [PATCH net-next v2 3/3] net: eth: fbnic: Add pma read and write access

[Mike Marciniszyn <mike.marciniszyn@gmail.com>]

On Tue, May 05, 2026 at 02:40:18PM +0200, Paolo Abeni wrote:
> On 4/30/26 5:08 PM, mike.marciniszyn@gmail.com wrote:
> > From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>
> >
> > Document the MDIO interface topology with an ASCII diagram
> > showing the MAC, PCS (MMD 3), FEC, Separated PMA (MMD 8), and PMD
> > (MMD 1) blocks and their interconnects. The diagram illustrates how
> > 4 lanes connect the MAC through PCS, FEC, and PMA, then narrow to
> > 2 lanes at the PMD.
> >
> > The c45 read and write routines are enhanced to support
> > read and write of the separated PMA for the fbnic.
> >
> > Co-developed-by: Alexander Duyck <alexanderduyck@fb.com>
> > Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
> > Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
> > ---
> > v2:
> >   - no changes
> > v1: https://lore.kernel.org/all/20260428172810.175077-5-mike.marciniszyn@gmail.com/
> >
> >  drivers/net/ethernet/meta/fbnic/fbnic_csr.h  |  1 +
> >  drivers/net/ethernet/meta/fbnic/fbnic_mdio.c | 71 ++++++++++++++++++++
> >  2 files changed, 72 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> > index 81794bd326e1..64b958df7774 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
> > @@ -805,6 +805,7 @@ enum {
> >  #define FBNIC_CSR_END_PCS		0x10668 /* CSR section delimiter */
> >
> >  #define FBNIC_CSR_START_RSFEC		0x10800 /* CSR section delimiter */
> > +#define FBNIC_RSFEC_CONTROL(n)	(0x10800 + 8 * (n))	/* 0x42000 + 32*n */
>
> Sashiko says:
>
> ---
> With the introduction of the RSFEC memory space at 0x10800, does the bounds
> check in fbnic_mdio_write_pcs() need to be updated to prevent clobbering
> these new registers?
> While not introduced by this patch, fbnic_mdio_write_pcs() currently
> contains:
>     /* Allow access to both halves of PCS for 50R2 config */
>     if (addr > 2)
>         return;
> When addr is 2, FBNIC_PCS_PAGE(2) evaluates to 0x10000 + 0x400 * 2, which
> is 0x10800.
> Since this overlaps with FBNIC_CSR_START_RSFEC, an MDIO write to MMD 3
> (PCS) at address 2 could silently overwrite the newly managed RSFEC
> control registers.
> The read function fbnic_mdio_read_pcs() correctly uses if (addr >= 2) to
> prevent this. Should the write function be updated to match?
> ---
>
> Does not look blocking to me, but a follow-up could be needed.
>
> /P
>

The patch that fixes this issue should have preceded the three patches.

Here is the patchwork link: https://patchwork.kernel.org/project/netdevbpf/patch/20260504135815.44226-2-mike.marciniszyn@gmail.com/

I don't see that you actually pulled that patch or that it has been
accepted?

Mike