| Message ID | 20201031174721.1080756-3-andrew@lunn.ch |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | xilinx_emaclite W=1 fixes | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Clearly marked for net-next |
| jkicinski/subject_prefix | success | Link |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 35 lines checked |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On 31. 10. 20 18:47, Andrew Lunn wrote: > drivers/net/ethernet//xilinx/xilinx_emaclite.c:341:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > 341 | addr = (void __iomem __force *)((u32 __force)addr ^ > > Use long instead of u32 to avoid problems on 64 bit systems. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > index 2c98e4cc07a5..f56c1fd01061 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > @@ -97,7 +97,7 @@ > #define ALIGNMENT 4 > > /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */ > -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT) > +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % ALIGNMENT) I can't see any reason to change unsigned type to signed one. > > #ifdef __BIG_ENDIAN > #define xemaclite_readl ioread32be > @@ -338,7 +338,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data, > * if it is configured in HW > */ > > - addr = (void __iomem __force *)((u32 __force)addr ^ > + addr = (void __iomem __force *)((long __force)addr ^ ditto. > XEL_BUFFER_OFFSET); > reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET); > > @@ -399,7 +399,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen) > * will correct on subsequent calls > */ > if (drvdata->rx_ping_pong != 0) > - addr = (void __iomem __force *)((u32 __force)addr ^ > + addr = (void __iomem __force *)((long __force)addr ^ ditto. > XEL_BUFFER_OFFSET); > else > return 0; /* No data was available */ > @@ -1192,9 +1192,9 @@ static int xemaclite_of_probe(struct platform_device *ofdev) > } > > dev_info(dev, > - "Xilinx EmacLite at 0x%08X mapped to 0x%08X, irq=%d\n", > + "Xilinx EmacLite at 0x%08X mapped to 0x%08lX, irq=%d\n", > (unsigned int __force)ndev->mem_start, > - (unsigned int __force)lp->base_addr, ndev->irq); > + (unsigned long __force)lp->base_addr, ndev->irq); This is different case but I don't think address can be signed type here too. > return 0; > > error: > Thanks, Michal
> -----Original Message----- > From: Michal Simek <michal.simek@xilinx.com> > Sent: Monday, November 2, 2020 12:51 PM > To: Andrew Lunn <andrew@lunn.ch>; Jakub Kicinski <kuba@kernel.org> > Cc: netdev <netdev@vger.kernel.org>; Michal Simek <michals@xilinx.com>; > Radhey Shyam Pandey <radheys@xilinx.com> > Subject: Re: [PATCH net-next 2/3] drivers: net: xilinx_emaclite: Fix -Wpointer- > to-int-cast warnings with W=1 > > > > On 31. 10. 20 18:47, Andrew Lunn wrote: > > drivers/net/ethernet//xilinx/xilinx_emaclite.c:341:35: warning: cast from > pointer to integer of different size [-Wpointer-to-int-cast] > > 341 | addr = (void __iomem __force *)((u32 __force)addr ^ > > > > Use long instead of u32 to avoid problems on 64 bit systems. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > index 2c98e4cc07a5..f56c1fd01061 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > @@ -97,7 +97,7 @@ > > #define ALIGNMENT 4 > > > > /* BUFFER_ALIGN(adr) calculates the number of bytes to the next > > alignment. */ -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % > > ALIGNMENT) > > +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % ALIGNMENT) > > I can't see any reason to change unsigned type to signed one. Agree. Also, I think we can get rid of this custom BUFFER_ALIGN macro and simply calling skb_reserve(skb, NET_IP_ALIGN) will make the protocol header to be aligned on at least a 4-byte boundary? > > > > > #ifdef __BIG_ENDIAN > > #define xemaclite_readl ioread32be > > @@ -338,7 +338,7 @@ static int xemaclite_send_data(struct net_local > *drvdata, u8 *data, > > * if it is configured in HW > > */ > > > > - addr = (void __iomem __force *)((u32 __force)addr ^ > > + addr = (void __iomem __force *)((long __force)addr ^ > > ditto. > > > XEL_BUFFER_OFFSET); > > reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET); > > > > @@ -399,7 +399,7 @@ static u16 xemaclite_recv_data(struct net_local > *drvdata, u8 *data, int maxlen) > > * will correct on subsequent calls > > */ > > if (drvdata->rx_ping_pong != 0) > > - addr = (void __iomem __force *)((u32 __force)addr ^ > > + addr = (void __iomem __force *)((long __force)addr > ^ > > ditto. > > > > XEL_BUFFER_OFFSET); > > else > > return 0; /* No data was available */ > > @@ -1192,9 +1192,9 @@ static int xemaclite_of_probe(struct > platform_device *ofdev) > > } > > > > dev_info(dev, > > - "Xilinx EmacLite at 0x%08X mapped to 0x%08X, irq=%d\n", > > + "Xilinx EmacLite at 0x%08X mapped to 0x%08lX, irq=%d\n", > > (unsigned int __force)ndev->mem_start, > > - (unsigned int __force)lp->base_addr, ndev->irq); > > + (unsigned long __force)lp->base_addr, ndev->irq); > > This is different case but I don't think address can be signed type here too. > > > return 0; > > > > error: > > > > Thanks, > Michal
On Mon, Nov 02, 2020 at 08:20:47AM +0100, Michal Simek wrote: > > > On 31. 10. 20 18:47, Andrew Lunn wrote: > > drivers/net/ethernet//xilinx/xilinx_emaclite.c:341:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > > 341 | addr = (void __iomem __force *)((u32 __force)addr ^ > > > > Use long instead of u32 to avoid problems on 64 bit systems. > > > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > > --- > > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > index 2c98e4cc07a5..f56c1fd01061 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > > @@ -97,7 +97,7 @@ > > #define ALIGNMENT 4 > > > > /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */ > > -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT) > > +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % ALIGNMENT) > > I can't see any reason to change unsigned type to signed one. I just found out that the kernel has uintptr_t. I will change it to that. > > } > > > > dev_info(dev, > > - "Xilinx EmacLite at 0x%08X mapped to 0x%08X, irq=%d\n", > > + "Xilinx EmacLite at 0x%08X mapped to 0x%08lX, irq=%d\n", > > (unsigned int __force)ndev->mem_start, > > - (unsigned int __force)lp->base_addr, ndev->irq); > > + (unsigned long __force)lp->base_addr, ndev->irq); > > This is different case but I don't think address can be signed type here > too. So unsigned long. Andrew
> > > /* BUFFER_ALIGN(adr) calculates the number of bytes to the next > > > alignment. */ -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % > > > ALIGNMENT) > > > +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % ALIGNMENT) > > > > I can't see any reason to change unsigned type to signed one. > Agree. Also, I think we can get rid of this custom BUFFER_ALIGN > macro and simply calling skb_reserve(skb, NET_IP_ALIGN) > will make the protocol header to be aligned on at > least a 4-byte boundary? Hi Radhey I'm just going to replace the long with a uintptr_t. That will fix the warnings. I don't have this hardware, so don't want to risk anything more invasive which i cannot test. Please feel free to add a follow up patch replacing this with skb_reserve(skb, NET_IP_ALIGN). Andrew
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, November 4, 2020 4:58 AM > To: Radhey Shyam Pandey <radheys@xilinx.com> > Cc: Michal Simek <michals@xilinx.com>; Jakub Kicinski <kuba@kernel.org>; > netdev <netdev@vger.kernel.org> > Subject: Re: [PATCH net-next 2/3] drivers: net: xilinx_emaclite: Fix -Wpointer- > to-int-cast warnings with W=1 > > > > > /* BUFFER_ALIGN(adr) calculates the number of bytes to the next > > > > alignment. */ -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) > % > > > > ALIGNMENT) > > > > +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % > ALIGNMENT) > > > > > > I can't see any reason to change unsigned type to signed one. > > > Agree. Also, I think we can get rid of this custom BUFFER_ALIGN > > macro and simply calling skb_reserve(skb, NET_IP_ALIGN) > > will make the protocol header to be aligned on at > > least a 4-byte boundary? > > Hi Radhey > > I'm just going to replace the long with a uintptr_t. That will fix the > warnings. I don't have this hardware, so don't want to risk anything > more invasive which i cannot test. > > Please feel free to add a follow up patch replacing this with > skb_reserve(skb, NET_IP_ALIGN). Ok, thanks. I will send out a patch to remove this buffer align macro. > > Andrew
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c index 2c98e4cc07a5..f56c1fd01061 100644 --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c @@ -97,7 +97,7 @@ #define ALIGNMENT 4 /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */ -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT) +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % ALIGNMENT) #ifdef __BIG_ENDIAN #define xemaclite_readl ioread32be @@ -338,7 +338,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data, * if it is configured in HW */ - addr = (void __iomem __force *)((u32 __force)addr ^ + addr = (void __iomem __force *)((long __force)addr ^ XEL_BUFFER_OFFSET); reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET); @@ -399,7 +399,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen) * will correct on subsequent calls */ if (drvdata->rx_ping_pong != 0) - addr = (void __iomem __force *)((u32 __force)addr ^ + addr = (void __iomem __force *)((long __force)addr ^ XEL_BUFFER_OFFSET); else return 0; /* No data was available */ @@ -1192,9 +1192,9 @@ static int xemaclite_of_probe(struct platform_device *ofdev) } dev_info(dev, - "Xilinx EmacLite at 0x%08X mapped to 0x%08X, irq=%d\n", + "Xilinx EmacLite at 0x%08X mapped to 0x%08lX, irq=%d\n", (unsigned int __force)ndev->mem_start, - (unsigned int __force)lp->base_addr, ndev->irq); + (unsigned long __force)lp->base_addr, ndev->irq); return 0; error:
drivers/net/ethernet//xilinx/xilinx_emaclite.c:341:35: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 341 | addr = (void __iomem __force *)((u32 __force)addr ^ Use long instead of u32 to avoid problems on 64 bit systems. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- drivers/net/ethernet/xilinx/xilinx_emaclite.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)