Re: [PATCH v2 net-next 1/3] net: dsa: lan9303: Refactor lan9303_xxx_packet_processing()

[Andrew Lunn <andrew@lunn.ch>]

On Tue, Aug 01, 2017 at 03:50:14PM +0200, Egil Hjelmeland wrote:
> On 01. aug. 2017 15:39, Andrew Lunn wrote:
> >>@@ -704,7 +710,7 @@ static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
> >>  	unsigned int u, poff;
> >>  	int ret;
> >>-	poff = port * 0x400;
> >>+	poff = LAN9303_SWITCH_PORT_REG(port, 0);
> >>  	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> >>  		ret = lan9303_read_switch_reg(chip,
> >
> >So the actual code is:
> >
> >	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> >        		ret = lan9303_read_switch_reg(chip,
> >			      		      lan9303_mib[u].offset + poff,
> >					      &reg);
> >
> >Could this be written as
> >
> >	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
> >		ret = lan9303_read_switch_port(chip, port, lan9303_mib[u].offset, &reg);
> >
> >It is then clear you are reading the statistics from a port register.
> >
> >    Andrew
> >
> 
> Yes it can. Since it is (insignificantly) less efficient, I
> chose not to touch it. But I can do it if you like.

I doubt it is less efficient. The compiler has seen
lan9303_read_switch_port() and will probably inline it.  So what the
optimiser gets to see is probably the same in both cases.

Try generating the assembler listing in both cases, and compare them

make drivers/net/dsa/lan9303-core.lst

     Andrew