linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 -next 0/4] net: emaclite: support arm64 platform
@ 2020-01-31 11:47 Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 1/4] net: emaclite: Fix coding style Radhey Shyam Pandey
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-01-31 11:47 UTC (permalink / raw)
  To: davem, netdev
  Cc: michal.simek, anirudha.sarangi, john.linn, mchehab+samsung,
	gregkh, nicolas.ferre, linux-arm-kernel, linux-kernel,
	Radhey Shyam Pandey

This patchset fixes the checkpatch warning reported in xilinx emaclite
driver. It also remove obsolete arch dependency in kconfig to support
aarch64 platform and fixes related gcc and sparse warnings.

Changes for v3:
- Fix sparse warnings reported by lkp@intel.com in 
  [PATCH net-next v2 2/3] net: emaclite: In kconfig
  remove arch dependencyarch dependency patch.

Changes for v2:
- Modified description of reset_lock spinlock variable.


Michal Simek (1):
  net: emaclite: Fix arm64 compilation warnings

Radhey Shyam Pandey (3):
  net: emaclite: Fix coding style
  net: emaclite: In kconfig remove arch dependency
  net: emaclite: Fix restricted cast warning of sparse

 drivers/net/ethernet/xilinx/Kconfig           |  2 +-
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 52 +++++++++++++--------------
 2 files changed, 25 insertions(+), 29 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 -next 1/4] net: emaclite: Fix coding style
  2020-01-31 11:47 [PATCH v3 -next 0/4] net: emaclite: support arm64 platform Radhey Shyam Pandey
@ 2020-01-31 11:47 ` Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 2/4] net: emaclite: In kconfig remove arch dependency Radhey Shyam Pandey
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-01-31 11:47 UTC (permalink / raw)
  To: davem, netdev
  Cc: michal.simek, anirudha.sarangi, john.linn, mchehab+samsung,
	gregkh, nicolas.ferre, linux-arm-kernel, linux-kernel,
	Radhey Shyam Pandey

Make coding style changes to fix checkpatch script warnings.
There is no functional change. Fixes below check and warnings-

CHECK: Blank lines aren't necessary after an open brace '{'
CHECK: spinlock_t definition without comment
CHECK: Please don't use multiple blank lines
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
CHECK: braces {} should be used on all arms of this statement
CHECK: Unbalanced braces around else statement
CHECK: Alignment should match open parenthesis
WARNING: Missing a blank line after declarations

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 37 ++++++++++++---------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0c26f5b..7f98728 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Xilinx EmacLite Linux driver for the Xilinx Ethernet MAC Lite device.
+/* Xilinx EmacLite Linux driver for the Xilinx Ethernet MAC Lite device.
  *
  * This is a new flat driver which is based on the original emac_lite
  * driver from John Williams <john.williams@xilinx.com>.
@@ -91,8 +90,6 @@
 #define XEL_ARP_PACKET_SIZE		28	/* Max ARP packet size */
 #define XEL_HEADER_IP_LENGTH_OFFSET	16	/* IP Length Offset */
 
-
-
 #define TX_TIMEOUT		(60 * HZ)	/* Tx timeout is 60 seconds. */
 #define ALIGNMENT		4
 
@@ -115,7 +112,7 @@
  * @next_tx_buf_to_use:	next Tx buffer to write to
  * @next_rx_buf_to_use:	next Rx buffer to read from
  * @base_addr:		base address of the Emaclite device
- * @reset_lock:		lock used for synchronization
+ * @reset_lock:		lock to serialize xmit and tx_timeout execution
  * @deferred_skb:	holds an skb (for transmission at a later time) when the
  *			Tx buffer is not free
  * @phy_dev:		pointer to the PHY device
@@ -124,7 +121,6 @@
  * @last_link:		last link status
  */
 struct net_local {
-
 	struct net_device *ndev;
 
 	bool tx_ping_pong;
@@ -133,7 +129,7 @@ struct net_local {
 	u32 next_rx_buf_to_use;
 	void __iomem *base_addr;
 
-	spinlock_t reset_lock;
+	spinlock_t reset_lock; /* serialize xmit and tx_timeout execution */
 	struct sk_buff *deferred_skb;
 
 	struct phy_device *phy_dev;
@@ -144,7 +140,6 @@ struct net_local {
 	int last_link;
 };
 
-
 /*************************/
 /* EmacLite driver calls */
 /*************************/
@@ -207,7 +202,7 @@ static void xemaclite_disable_interrupts(struct net_local *drvdata)
  * address in the EmacLite device.
  */
 static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
-				    unsigned length)
+				    unsigned int length)
 {
 	u32 align_buffer;
 	u32 *to_u32_ptr;
@@ -264,7 +259,7 @@ static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
  * to a 16-bit aligned buffer.
  */
 static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
-				   unsigned length)
+				   unsigned int length)
 {
 	u16 *to_u16_ptr, *from_u16_ptr;
 	u32 *from_u32_ptr;
@@ -329,7 +324,6 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 	reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
 	if ((reg_data & (XEL_TSR_XMIT_BUSY_MASK |
 	     XEL_TSR_XMIT_ACTIVE_MASK)) == 0) {
-
 		/* Switch to next buffer if configured */
 		if (drvdata->tx_ping_pong != 0)
 			drvdata->next_tx_buf_to_use ^= XEL_BUFFER_OFFSET;
@@ -345,8 +339,9 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 		if ((reg_data & (XEL_TSR_XMIT_BUSY_MASK |
 		     XEL_TSR_XMIT_ACTIVE_MASK)) != 0)
 			return -1; /* Buffers were full, return failure */
-	} else
+	} else {
 		return -1; /* Buffer was full, return failure */
+	}
 
 	/* Write the frame to the buffer */
 	xemaclite_aligned_write(data, (u32 __force *)addr, byte_count);
@@ -421,7 +416,6 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 	 * or an IP packet or an ARP packet
 	 */
 	if (proto_type > ETH_DATA_LEN) {
-
 		if (proto_type == ETH_P_IP) {
 			length = ((ntohl(xemaclite_readl(addr +
 					XEL_HEADER_IP_LENGTH_OFFSET +
@@ -431,23 +425,25 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 			length = min_t(u16, length, ETH_DATA_LEN);
 			length += ETH_HLEN + ETH_FCS_LEN;
 
-		} else if (proto_type == ETH_P_ARP)
+		} else if (proto_type == ETH_P_ARP) {
 			length = XEL_ARP_PACKET_SIZE + ETH_HLEN + ETH_FCS_LEN;
-		else
+		} else {
 			/* Field contains type other than IP or ARP, use max
 			 * frame size and let user parse it
 			 */
 			length = ETH_FRAME_LEN + ETH_FCS_LEN;
-	} else
+		}
+	} else {
 		/* Use the length in the frame, plus the header and trailer */
 		length = proto_type + ETH_HLEN + ETH_FCS_LEN;
+	}
 
 	if (WARN_ON(length > maxlen))
 		length = maxlen;
 
 	/* Read from the EmacLite device */
 	xemaclite_aligned_read((u32 __force *)(addr + XEL_RXBUFF_OFFSET),
-				data, length);
+			       data, length);
 
 	/* Acknowledge the frame */
 	reg_data = xemaclite_readl(addr + XEL_RSR_OFFSET);
@@ -668,8 +664,7 @@ static irqreturn_t xemaclite_interrupt(int irq, void *dev_id)
 	/* Check if the Transmission for the first buffer is completed */
 	tx_status = xemaclite_readl(base_addr + XEL_TSR_OFFSET);
 	if (((tx_status & XEL_TSR_XMIT_BUSY_MASK) == 0) &&
-		(tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) {
-
+	    (tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) {
 		tx_status &= ~XEL_TSR_XMIT_ACTIVE_MASK;
 		xemaclite_writel(tx_status, base_addr + XEL_TSR_OFFSET);
 
@@ -679,8 +674,7 @@ static irqreturn_t xemaclite_interrupt(int irq, void *dev_id)
 	/* Check if the Transmission for the second buffer is completed */
 	tx_status = xemaclite_readl(base_addr + XEL_BUFFER_OFFSET + XEL_TSR_OFFSET);
 	if (((tx_status & XEL_TSR_XMIT_BUSY_MASK) == 0) &&
-		(tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) {
-
+	    (tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) {
 		tx_status &= ~XEL_TSR_XMIT_ACTIVE_MASK;
 		xemaclite_writel(tx_status, base_addr + XEL_BUFFER_OFFSET +
 				 XEL_TSR_OFFSET);
@@ -837,6 +831,7 @@ static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev)
 	of_address_to_resource(npp, 0, &res);
 	if (lp->ndev->mem_start != res.start) {
 		struct phy_device *phydev;
+
 		phydev = of_phy_find_device(lp->phy_node);
 		if (!phydev)
 			dev_info(dev,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 -next 2/4] net: emaclite: In kconfig remove arch dependency
  2020-01-31 11:47 [PATCH v3 -next 0/4] net: emaclite: support arm64 platform Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 1/4] net: emaclite: Fix coding style Radhey Shyam Pandey
@ 2020-01-31 11:47 ` Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse Radhey Shyam Pandey
  3 siblings, 0 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-01-31 11:47 UTC (permalink / raw)
  To: davem, netdev
  Cc: michal.simek, anirudha.sarangi, john.linn, mchehab+samsung,
	gregkh, nicolas.ferre, linux-arm-kernel, linux-kernel,
	Radhey Shyam Pandey

To enable xilinx_emaclite driver support on zynqmp ultrascale platform
(ARCH64) remove the obsolete ARCH dependency list. Also include HAS_IOMEM
dependency to avoid compilation failure on architectures without IOMEM.

Sanity build test done for microblaze, zynq and zynqmp ultrascale platform.

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig
index 6304ebd..0692dd1 100644
--- a/drivers/net/ethernet/xilinx/Kconfig
+++ b/drivers/net/ethernet/xilinx/Kconfig
@@ -18,8 +18,8 @@ if NET_VENDOR_XILINX
 
 config XILINX_EMACLITE
 	tristate "Xilinx 10/100 Ethernet Lite support"
-	depends on PPC32 || MICROBLAZE || ARCH_ZYNQ || MIPS
 	select PHYLIB
+	depends on HAS_IOMEM
 	---help---
 	  This driver supports the 10/100 Ethernet Lite from Xilinx.
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings
  2020-01-31 11:47 [PATCH v3 -next 0/4] net: emaclite: support arm64 platform Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 1/4] net: emaclite: Fix coding style Radhey Shyam Pandey
  2020-01-31 11:47 ` [PATCH v3 -next 2/4] net: emaclite: In kconfig remove arch dependency Radhey Shyam Pandey
@ 2020-01-31 11:47 ` Radhey Shyam Pandey
  2020-01-31 13:37   ` Andrew Lunn
  2020-01-31 11:47 ` [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse Radhey Shyam Pandey
  3 siblings, 1 reply; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-01-31 11:47 UTC (permalink / raw)
  To: davem, netdev
  Cc: michal.simek, anirudha.sarangi, john.linn, mchehab+samsung,
	gregkh, nicolas.ferre, linux-arm-kernel, linux-kernel,
	Radhey Shyam Pandey

From: Michal Simek <michal.simek@xilinx.com>

Recast pointers with ulong instead of u32 for arm64.
This patch fixes these compilation warnings:

drivers/net/ethernet/xilinx/xilinx_emaclite.c:
In function ‘xemaclite_send_data’:
drivers/net/ethernet/xilinx/xilinx_emaclite.c:335:35:
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
   addr = (void __iomem __force *)((u32 __force)addr ^
                                   ^
drivers/net/ethernet/xilinx/xilinx_emaclite.c:335:10:
warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   addr = (void __iomem __force *)((u32 __force)addr ^
          ^
drivers/net/ethernet/xilinx/xilinx_emaclite.c:
In function ‘xemaclite_recv_data’:
drivers/net/ethernet/xilinx/xilinx_emaclite.c:397:36:
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    addr = (void __iomem __force *)((u32 __force)addr ^
                                    ^
drivers/net/ethernet/xilinx/xilinx_emaclite.c:397:11:
warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
    addr = (void __iomem __force *)((u32 __force)addr ^
           ^
drivers/net/ethernet/xilinx/xilinx_emaclite.c:
In function ‘xemaclite_rx_handler’:
drivers/net/ethernet/xilinx/xilinx_emaclite.c:97:42:
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 #define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT)
                                          ^
drivers/net/ethernet/xilinx/xilinx_emaclite.c:612:10:
note: in expansion of macro ‘BUFFER_ALIGN’
  align = BUFFER_ALIGN(skb->data);
          ^~~~~~~~~~~~
In file included from ./include/linux/dma-mapping.h:7,
                 from ./include/linux/skbuff.h:31,
                 from ./include/linux/if_ether.h:19,
                 from ./include/uapi/linux/ethtool.h:19,
                 from ./include/linux/ethtool.h:18,
                 from ./include/linux/netdevice.h:37,
                 from drivers/net/ethernet/xilinx/xilinx_emaclite.c:12:
drivers/net/ethernet/xilinx/xilinx_emaclite.c:
In function ‘xemaclite_of_probe’:
drivers/net/ethernet/xilinx/xilinx_emaclite.c:1191:4:
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    (unsigned int __force)lp->base_addr, ndev->irq);
    ^
./include/linux/device.h:1780:33: note: in definition of macro ‘dev_info’
  _dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
                                 ^~~~~~~~~~~

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 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 7f98728..96e9d21 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -94,7 +94,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 - ((ulong)adr)) % ALIGNMENT)
 
 #ifdef __BIG_ENDIAN
 #define xemaclite_readl		ioread32be
@@ -332,7 +332,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 *)((ulong __force)addr ^
 						 XEL_BUFFER_OFFSET);
 		reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
 
@@ -394,7 +394,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 *)((ulong __force)addr ^
 							 XEL_BUFFER_OFFSET);
 		else
 			return 0;	/* No data was available */
@@ -1186,9 +1186,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:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse
  2020-01-31 11:47 [PATCH v3 -next 0/4] net: emaclite: support arm64 platform Radhey Shyam Pandey
                   ` (2 preceding siblings ...)
  2020-01-31 11:47 ` [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings Radhey Shyam Pandey
@ 2020-01-31 11:47 ` Radhey Shyam Pandey
  2020-01-31 13:48   ` Andrew Lunn
  3 siblings, 1 reply; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-01-31 11:47 UTC (permalink / raw)
  To: davem, netdev
  Cc: michal.simek, anirudha.sarangi, john.linn, mchehab+samsung,
	gregkh, nicolas.ferre, linux-arm-kernel, linux-kernel,
	Radhey Shyam Pandey

Explicitly cast xemaclite_readl return value when it's passed to ntohl.
Fixes below reported sparse warnings:

xilinx_emaclite.c:411:24: sparse: sparse: cast to restricted __be32
xilinx_emaclite.c:420:36: sparse: sparse: cast to restricted __be32

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Reported-by: kbuild test robot <lkp@intel.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 96e9d21..3273d4f 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -408,7 +408,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 
 	/* Get the protocol type of the ethernet frame that arrived
 	 */
-	proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
+	proto_type = ((ntohl((__force __be32)xemaclite_readl(addr +
+			XEL_HEADER_OFFSET +
 			XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
 			XEL_RPLR_LENGTH_MASK);
 
@@ -417,7 +418,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 	 */
 	if (proto_type > ETH_DATA_LEN) {
 		if (proto_type == ETH_P_IP) {
-			length = ((ntohl(xemaclite_readl(addr +
+			length = ((ntohl((__force __be32)xemaclite_readl(addr +
 					XEL_HEADER_IP_LENGTH_OFFSET +
 					XEL_RXBUFF_OFFSET)) >>
 					XEL_HEADER_SHIFT) &
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings
  2020-01-31 11:47 ` [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings Radhey Shyam Pandey
@ 2020-01-31 13:37   ` Andrew Lunn
  2020-02-10 14:26     ` Radhey Shyam Pandey
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2020-01-31 13:37 UTC (permalink / raw)
  To: Radhey Shyam Pandey
  Cc: davem, netdev, linux-kernel, anirudha.sarangi, michal.simek,
	gregkh, mchehab+samsung, john.linn, linux-arm-kernel

On Fri, Jan 31, 2020 at 05:17:49PM +0530, Radhey Shyam Pandey wrote:
>  
>  /* 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 - ((ulong)adr)) % ALIGNMENT)

Hi Radhey

linux/kernel.h has a few interesting macros, like

#define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))

These are more likely to be correct across all architectures than
anything you role yourself.

	 Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse
  2020-01-31 11:47 ` [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse Radhey Shyam Pandey
@ 2020-01-31 13:48   ` Andrew Lunn
  2020-02-10 14:55     ` Radhey Shyam Pandey
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2020-01-31 13:48 UTC (permalink / raw)
  To: Radhey Shyam Pandey
  Cc: davem, netdev, linux-kernel, anirudha.sarangi, michal.simek,
	gregkh, mchehab+samsung, john.linn, linux-arm-kernel

On Fri, Jan 31, 2020 at 05:17:50PM +0530, Radhey Shyam Pandey wrote:
> Explicitly cast xemaclite_readl return value when it's passed to ntohl.
> Fixes below reported sparse warnings:
> 
> xilinx_emaclite.c:411:24: sparse: sparse: cast to restricted __be32
> xilinx_emaclite.c:420:36: sparse: sparse: cast to restricted __be32
> 
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> Reported-by: kbuild test robot <lkp@intel.com>
> ---
>  drivers/net/ethernet/xilinx/xilinx_emaclite.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> index 96e9d21..3273d4f 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> @@ -408,7 +408,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
>  
>  	/* Get the protocol type of the ethernet frame that arrived
>  	 */
> -	proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
> +	proto_type = ((ntohl((__force __be32)xemaclite_readl(addr +
> +			XEL_HEADER_OFFSET +
>  			XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
>  			XEL_RPLR_LENGTH_MASK);
>  
> @@ -417,7 +418,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
>  	 */
>  	if (proto_type > ETH_DATA_LEN) {
>  		if (proto_type == ETH_P_IP) {
> -			length = ((ntohl(xemaclite_readl(addr +
> +			length = ((ntohl((__force __be32)xemaclite_readl(addr +
>  					XEL_HEADER_IP_LENGTH_OFFSET +
>  					XEL_RXBUFF_OFFSET)) >>
>  					XEL_HEADER_SHIFT) &

If i understand this code correctly, you need the ntohl because you
are poking around inside the packet. All the other uses of
xemaclite_readl() are for descriptors etc.

It would be cleaner if you defined a xemaclite_readlbe32. If you use
ioread32be() it will do the endinness swap for you, so you don't need
the ntohl() and the horrible cast.

    Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings
  2020-01-31 13:37   ` Andrew Lunn
@ 2020-02-10 14:26     ` Radhey Shyam Pandey
  0 siblings, 0 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-02-10 14:26 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: davem, netdev, linux-kernel, Anirudha Sarangi, Michal Simek,
	gregkh, mchehab+samsung, John Linn, linux-arm-kernel

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, January 31, 2020 7:08 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Anirudha Sarangi <anirudh@xilinx.com>; Michal Simek
> <michals@xilinx.com>; gregkh@linuxfoundation.org;
> mchehab+samsung@kernel.org; John Linn <linnj@xilinx.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings
> 
> On Fri, Jan 31, 2020 at 05:17:49PM +0530, Radhey Shyam Pandey wrote:
> >
> >  /* 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 - ((ulong)adr)) % ALIGNMENT)
> 
> Hi Radhey
> 
> linux/kernel.h has a few interesting macros, like
> 
> #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
> 
> These are more likely to be correct across all architectures than
> anything you role yourself.
> 
Thanks for the review. I agree using a kernel macro is preferred. However,
as a second thought it seems 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.

> 	 Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse
  2020-01-31 13:48   ` Andrew Lunn
@ 2020-02-10 14:55     ` Radhey Shyam Pandey
  0 siblings, 0 replies; 9+ messages in thread
From: Radhey Shyam Pandey @ 2020-02-10 14:55 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: davem, netdev, linux-kernel, Anirudha Sarangi, Michal Simek,
	gregkh, mchehab+samsung, John Linn, linux-arm-kernel

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, January 31, 2020 7:19 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Anirudha Sarangi <anirudh@xilinx.com>; Michal Simek
> <michals@xilinx.com>; gregkh@linuxfoundation.org;
> mchehab+samsung@kernel.org; John Linn <linnj@xilinx.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of
> sparse
> 
> On Fri, Jan 31, 2020 at 05:17:50PM +0530, Radhey Shyam Pandey wrote:
> > Explicitly cast xemaclite_readl return value when it's passed to ntohl.
> > Fixes below reported sparse warnings:
> >
> > xilinx_emaclite.c:411:24: sparse: sparse: cast to restricted __be32
> > xilinx_emaclite.c:420:36: sparse: sparse: cast to restricted __be32
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > Reported-by: kbuild test robot <lkp@intel.com>
> > ---
> >  drivers/net/ethernet/xilinx/xilinx_emaclite.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > index 96e9d21..3273d4f 100644
> > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > @@ -408,7 +408,8 @@ static u16 xemaclite_recv_data(struct net_local
> *drvdata, u8 *data, int maxlen)
> >
> >  	/* Get the protocol type of the ethernet frame that arrived
> >  	 */
> > -	proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
> > +	proto_type = ((ntohl((__force __be32)xemaclite_readl(addr +
> > +			XEL_HEADER_OFFSET +
> >  			XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
> >  			XEL_RPLR_LENGTH_MASK);
> >
> > @@ -417,7 +418,7 @@ static u16 xemaclite_recv_data(struct net_local
> *drvdata, u8 *data, int maxlen)
> >  	 */
> >  	if (proto_type > ETH_DATA_LEN) {
> >  		if (proto_type == ETH_P_IP) {
> > -			length = ((ntohl(xemaclite_readl(addr +
> > +			length = ((ntohl((__force __be32)xemaclite_readl(addr
> +
> >  					XEL_HEADER_IP_LENGTH_OFFSET +
> >  					XEL_RXBUFF_OFFSET)) >>
> >  					XEL_HEADER_SHIFT) &
> 
> If i understand this code correctly, you need the ntohl because you
> are poking around inside the packet. All the other uses of
> xemaclite_readl() are for descriptors etc.
> 
> It would be cleaner if you defined a xemaclite_readlbe32. If you use
> ioread32be() it will do the endinness swap for you, so you don't need
> the ntohl() and the horrible cast.
Thanks for the review. Yes, defining xemaclite_readlbe32 would eliminate the
cast need.  I will address it in the next version. 

> 
>     Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-10 14:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-31 11:47 [PATCH v3 -next 0/4] net: emaclite: support arm64 platform Radhey Shyam Pandey
2020-01-31 11:47 ` [PATCH v3 -next 1/4] net: emaclite: Fix coding style Radhey Shyam Pandey
2020-01-31 11:47 ` [PATCH v3 -next 2/4] net: emaclite: In kconfig remove arch dependency Radhey Shyam Pandey
2020-01-31 11:47 ` [PATCH v3 -next 3/4] net: emaclite: Fix arm64 compilation warnings Radhey Shyam Pandey
2020-01-31 13:37   ` Andrew Lunn
2020-02-10 14:26     ` Radhey Shyam Pandey
2020-01-31 11:47 ` [PATCH v3 -next 4/4] net: emaclite: Fix restricted cast warning of sparse Radhey Shyam Pandey
2020-01-31 13:48   ` Andrew Lunn
2020-02-10 14:55     ` Radhey Shyam Pandey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).