Re: [PATCH] net: phy: micrel: Remove unnecessary comparison in lan8814_handle_interrupt

[Andrew Lunn]

> I write the coccicheck and find these reports:

Nice!

> For directly call __phy_read():
> 
> ./drivers/net/phy/micrel.c:1969:59-60: WARNING: __phy_read() assigned to an
> unsigned int 'data'

This one we know about.

> ./drivers/net/phy/mscc/mscc_macsec.c:49:50-51: WARNING: __phy_read()
> assigned to an unsigned int 'val'
> ./drivers/net/phy/mscc/mscc_macsec.c:52:51-52: WARNING: __phy_read()
> assigned to an unsigned int 'val_l'
> ./drivers/net/phy/mscc/mscc_macsec.c:53:51-52: WARNING: __phy_read()
> assigned to an unsigned int 'val_h'
> ./drivers/net/phy/mscc/mscc_macsec.c:89:50-51: WARNING: __phy_read()
> assigned to an unsigned int 'val'

These are all in the same function. Looking at the first check, it has
been decided that if something goes wrong, return 0. Not the best
solution, but better than returning something random. So the do/while
loop can goto failed: val_l and val_h are a bit more messy, but a
check would be nice, return 0 on error.

Actually returning an error code is not going to be easy. The rest of
the code assumes vsc8584_macsec_phy_read() never fails :-(

> ./drivers/net/phy/mscc/mscc_main.c:1511:50-51: WARNING: __phy_read()
> assigned to an unsigned int 'addr'
> ./drivers/net/phy/mscc/mscc_main.c:1514:47-48: WARNING: __phy_read()
> assigned to an unsigned int 'val'

More code which assumes a read can never fail. vsc8514_probe() calls
this, so it should be reasonable easy to catch the error, return it,
and make the probe fail.

> ./drivers/net/phy/mscc/mscc_main.c:366:54-55: WARNING: __phy_read() assigned
> to an unsigned int 'reg_val'
> ./drivers/net/phy/mscc/mscc_main.c:370:55-56: WARNING: __phy_read() assigned
> to an unsigned int 'pwd [ 0 ]'
> ./drivers/net/phy/mscc/mscc_main.c:371:53-54: WARNING: __phy_read() assigned
> to an unsigned int 'pwd [ 1 ]'
> ./drivers/net/phy/mscc/mscc_main.c:372:55-56: WARNING: __phy_read() assigned
> to an unsigned int 'pwd [ 2 ]'
> ./drivers/net/phy/mscc/mscc_main.c:317:54-55: WARNING: __phy_read() assigned
> to an unsigned int 'reg_val'

Another void function which assumes it cannot fail. Error checks would
be nice, don't return random data in the wol structure.

Thanks
	Andrew