Subject: Re: [PATCH] net/fsl: Add format length modifier to avoid negative values

From: Randy Dunlap <[email-masked]>
Date: Tue, 29 Jul 2014 13:29:29 -0700

On 07/29/14 12:53, Shruti Kanetkar wrote:
> Signed-off-by: Shruti Kanetkar <[email-masked]>
> ---
>  drivers/net/ethernet/freescale/xgmac_mdio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> index 8173407..6e7db66 100644
> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> @@ -163,7 +163,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
>  	/* Return all Fs if nothing was there */
>  	if (in_be32(&regs->mdio_stat) & MDIO_STAT_RD_ER) {
>  		dev_err(&bus->dev,
> -			"Error while reading PHY%d reg at %d.%d\n",
> +			"Error while reading PHY%d reg at %d.%hhu\n",

I don't get it.  Please explain better.

regnum is not an unsigned char, but that is what %hhu indicates, isn't it?

>  			phy_id, dev_addr, regnum);

Earlier in the code, dev_addr is set to:
	uint16_t dev_addr = regnum >> 16;

and then we see that the register address is the low 16 bits of regnum:
	/* Set the register address */
	out_be32(&regs->mdio_addr, regnum & 0xffff);

so maybe this printk (dev_err) should just use regnum & 0xffff and %u format?

I'm certainly not sure about that suggestion, but the patch didn't make sense to me.

Thanks.

>  		return 0xffff;
>  	}

-- 
~Randy