On 08/27/2018 10:24 PM, Eugeniu Rosca wrote:
Hi Marek,
Hi,
On Mon, Aug 27, 2018 at 01:22:54AM +0200, Marek Vasut wrote:
On 08/27/2018 01:13 AM, Eugeniu Rosca wrote:
[...]
#define RAVB_DESC_DT(n) ((n) << 28)
What about changing this instead, ((u32)(n) << 28) ?
This works too.
[...]
- 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.
Actually one cast is enough to fix the UB. Let me know if below patch looks better to you.
I guess, it's less intrusive. What do you think ?
diff --git a/drivers/net/ravb.c b/drivers/net/ravb.c index 749562db960e..2190811c53bb 100644 --- a/drivers/net/ravb.c +++ b/drivers/net/ravb.c @@ -72,7 +72,7 @@ #define RAVB_TX_QUEUE_OFFSET 0 #define RAVB_RX_QUEUE_OFFSET 4
-#define RAVB_DESC_DT(n) ((n) << 28) +#define RAVB_DESC_DT(n) ((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) @@ -342,7 +342,7 @@ 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],
writel(((u32)mac[0] << 24) | (mac[1] << 16) | (mac[2] << 8) | mac[3], eth->iobase + RAVB_REG_MAHR);
writel((mac[4] << 8) | mac[5], eth->iobase + RAVB_REG_MALR);
Thanks, Eugeniu.