Re: [PATCH net-next v3 7/9] net: sunhme: Clean up mac address init

[Sean Anderson]

On 3/18/23 05:03, Simon Horman wrote:
> On Mon, Mar 13, 2023 at 08:36:11PM -0400, Sean Anderson wrote:
> > Clean up some oddities suggested during review.
> >
> > Signed-off-by: Sean Anderson <seanga2@xxxxxxxxx>
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - New
> >
> >   drivers/net/ethernet/sun/sunhme.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
> > index c2737f26afbe..1f27e99abf17 100644
> > --- a/drivers/net/ethernet/sun/sunhme.c
> > +++ b/drivers/net/ethernet/sun/sunhme.c
> > @@ -2348,9 +2348,7 @@ static int find_eth_addr_in_vpd(void __iomem *rom_base, int len, int index, unsi
> >   		p += 6;
> >   
> >   		if (index == 0) {
> > -			int i;
> > -
> > -			for (i = 0; i < 6; i++)
> > +			for (int i = 0; i < 6; i++)
> >   				dev_addr[i] = readb(p + i);
> >   			return 1;
> >   		}
> > @@ -2362,9 +2360,10 @@ static int find_eth_addr_in_vpd(void __iomem *rom_base, int len, int index, unsi
> >   static void __maybe_unused get_hme_mac_nonsparc(struct pci_dev *pdev,
> >   						unsigned char *dev_addr)
> >   {
> > +	void __iomem *p;
> >   	size_t size;
> > -	void __iomem *p = pci_map_rom(pdev, &size);
> >   
> > +	p = pci_map_rom(pdev, &size);
> >   	if (p) {
> >   		int index = 0;
> >   		int found;
> > @@ -2386,7 +2385,7 @@ static void __maybe_unused get_hme_mac_nonsparc(struct pci_dev *pdev,
> >   	dev_addr[2] = 0x20;
> >   	get_random_bytes(&dev_addr[3], 3);
> >   }
> > -#endif /* !(CONFIG_SPARC) */
> > +#endif
>
> Hi Sean,
>
> I think this problem was added by patch 6/9,
> so perhaps best to squash it into that patch.

> Actually, I'd squash all these changes into 6/9.
> But I don't feel strongly about it.

6/9 just moves code around. I am keeping the modifications for other commits.

--Sean

> So in any case,
>
> Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>
>
> I will pause my review here (again!) because I need to go to a football match.