Re: [PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper

[Philippe Mathieu-Daudé]

On 13/11/24 15:23, Edgar E. Iglesias wrote:
> On Tue, Nov 12, 2024 at 07:10:32PM +0100, Philippe Mathieu-Daudé wrote:
> > For a particular physical address within the EthLite MMIO range,
> > addr_to_port_index() returns which port is accessed.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/net/xilinx_ethlite.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> > index 20919b4f54..fe91891310 100644
> > --- a/hw/net/xilinx_ethlite.c
> > +++ b/hw/net/xilinx_ethlite.c
> > @@ -24,6 +24,7 @@
> >   
> >   #include "qemu/osdep.h"
> >   #include "qemu/module.h"
> > +#include "qemu/bitops.h"
> >   #include "qom/object.h"
> >   #include "qapi/error.h"
> >   #include "exec/tswap.h"
> > @@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
> >       }
> >   }
> >   
> > +__attribute__((unused))
> > +static unsigned addr_to_port_index(hwaddr addr)
> > +{
> > +    return extract64(addr, 11, 1);
> > +}
> > +
>
> Shouldn't you add addr_to_port_index in the following patch and avoid
> the temporary unused attribute?

OK.

> >   static uint64_t
> >   eth_read(void *opaque, hwaddr addr, unsigned int size)
> >   {
> > @@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
> >   static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
> >   {
> >       XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> > -    unsigned int rxbase = s->port_index * (0x800 / 4);
> > +    unsigned int port_index = s->port_index;
> > +    unsigned int rxbase = port_index * (0x800 / 4);
>
>
> Hmm, AFAICT s->port_index is an unsigned int, what is the purpose of this 
> change?

Likely a rebase mistake, this belongs to the next patch indeed.