RE: [PATCH net-next v4 3/3] r8152: add helper functions for PHY OCP registers

[Hayes Wang]

> -----Original Message-----
> From: Chih Kai Hsu <hsu.chih.kai@xxxxxxxxxxx>
> Sent: Tuesday, March 31, 2026 1:46 PM
> To: Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; nic_swsd
> <nic_swsd@xxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> linux-usb@xxxxxxxxxxxxxxx; edumazet@xxxxxxxxxx; bjorn@xxxxxxx;
> pabeni@xxxxxxxxxx
> Subject: RE: [PATCH net-next v4 3/3] r8152: add helper functions for PHY OCP
> registers
> 
> On 31/03/26 10:02 am, Jakub Kicinski wrote:
> >
> > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index
> > > 32a4e8d423115..8747c55e0a484 100644
> > > --- a/drivers/net/usb/r8152.c
> > > +++ b/drivers/net/usb/r8152.c
> > [ ... ]
> > > +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear,
> > > +u16 set) {
> > > +     u16 data;
> > > +
> > > +     data = sram_read(tp, addr);
> > > +     data = (data & ~clear) | set;
> > > +     ocp_reg_write(tp, OCP_SRAM_DATA, data);
> >
> > AI flags:
> >
> >   Does writing directly to OCP_SRAM_DATA here actually write to the
> > intended
> >   address?
> Yes, this actually writes to the intended address.
> 
> >
> >   Because hardware indirect SRAM access ports typically auto-increment
> their
> >   internal address pointer upon access, the sram_read() call will advance
> the
> >   pointer. By writing directly to OCP_SRAM_DATA instead of using
> >   sram_write(tp, addr, data) to re-establish the address, the updated value
> >   would be written to the next address (addr + 1) rather than the original
> addr.
> >
> >   Could this corrupt the PHY configuration memory across the
> > initialization paths
> >   that now use sram_clr_bits() and sram_set_bits()?
> >
> > Can you confirm if this is legit? Not sure whether this device /
> > register auto-increments or where AI got it from.
> The address pointer, OCP_SRAM_ADDR, is not auto-incremented after
> sram_read(tp, addr).
> However, the address pointer is incremented automatically after
> ocp_reg_write(tp, OCP_SRAM_DATA, data).

That's strange - I didn't receive the previous email from Jakub Kicinski.
Also, the answer above is correct.

Best Regards,
Hayes