On 08/27/2018 01:13 AM, Eugeniu Rosca wrote:
> Running "tftp" on R-Car H3 Salvator-X with CONFIG_UBSAN=y results in:
>
> => tftp
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:237:28
> left shift of 10 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:258:44
> left shift of 9 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:263:46
> left shift of 9 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:283:31
> left shift of 9 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:288:49
> left shift of 9 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:293:46
> left shift of 9 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:345:2
> left shift of 222 by 24 places cannot be represented in type 'int'
> =====================================================================
>
> Pinging the host results in:
>
> => ping 192.168.2.11
> Using ethernet@e6800000 device
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:161:21
> left shift of 15 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:182:25
> left shift of 15 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:182:47
> left shift of 12 by 28 places cannot be represented in type 'int'
> =====================================================================
> =====================================================================
> UBSAN: Undefined behaviour in drivers/net/ravb.c:205:20
> left shift of 12 by 28 places cannot be represented in type 'int'
> =====================================================================
> host 192.168.2.11 is alive
>
> There are two issues behind:
> - calculating RAVB_DESC_DT_* bitfields
> - assembling MAC address from its char components
>
> Fix both.
>
> Fixes: 8ae51b6f324e ("net: ravb: Add Renesas Ethernet RAVB driver")
> Signed-off-by: Eugeniu Rosca <ero...@de.adit-jv.com>
> Acked-by: Marek Vasut <marek.va...@gmail.com>
> ---
>
> Changes in v2:
> - Shorten the summary line
> - Add "Acked-by: Marek Vasut <marek.va...@gmail.com>"
> ---
> drivers/net/ravb.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ravb.c b/drivers/net/ravb.c
> index 749562db960e..5baff198889b 100644
> --- a/drivers/net/ravb.c
> +++ b/drivers/net/ravb.c
> @@ -73,12 +73,12 @@
> #define RAVB_RX_QUEUE_OFFSET 4
>
> #define RAVB_DESC_DT(n) ((n) << 28)
What about changing this instead, ((u32)(n) << 28) ?
> -#define RAVB_DESC_DT_FSINGLE RAVB_DESC_DT(0x7)
> -#define RAVB_DESC_DT_LINKFIX RAVB_DESC_DT(0x9)
> -#define RAVB_DESC_DT_EOS RAVB_DESC_DT(0xa)
> -#define RAVB_DESC_DT_FEMPTY RAVB_DESC_DT(0xc)
> -#define RAVB_DESC_DT_EEMPTY RAVB_DESC_DT(0x3)
> -#define RAVB_DESC_DT_MASK RAVB_DESC_DT(0xf)
> +#define RAVB_DESC_DT_FSINGLE RAVB_DESC_DT(0x7UL)
> +#define RAVB_DESC_DT_LINKFIX RAVB_DESC_DT(0x9UL)
> +#define RAVB_DESC_DT_EOS RAVB_DESC_DT(0xaUL)
> +#define RAVB_DESC_DT_FEMPTY RAVB_DESC_DT(0xcUL)
> +#define RAVB_DESC_DT_EEMPTY RAVB_DESC_DT(0x3UL)
> +#define RAVB_DESC_DT_MASK RAVB_DESC_DT(0xfUL)
>
> #define RAVB_DESC_DS(n) (((n) & 0xfff) << 0)
> #define RAVB_DESC_DS_MASK 0xfff
> @@ -342,8 +342,8 @@ static int ravb_write_hwaddr(struct udevice *dev)
> struct eth_pdata *pdata = dev_get_platdata(dev);
> unsigned char *mac = pdata->enetaddr;
>
> - writel((mac[0] << 24) | (mac[1] << 16) | (mac[2] << 8) | mac[3],
> - eth->iobase + RAVB_REG_MAHR);
> + writel(((u32)mac[0] << 24) | ((u32)mac[1] << 16) | ((u32)mac[2] << 8) |
> + mac[3], eth->iobase + RAVB_REG_MAHR);
Not a big fan of the casts here, I wonder if there isn't some more
elegant solution. If not, so be it.
> writel((mac[4] << 8) | mac[5], eth->iobase + RAVB_REG_MALR);
>
>
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
