mvpp2: Fix warning over 32bit vs 64bit targets

PATCH New Tom Rini 7 participants 19 messages Jan 27, 2018 – Jan 29, 2018

Subsystems (5)

Network drivers USB Arm i.MX and Layerscape Btrfs file system +1 more subsystems

lore

Share & tools

lore

Tom Rini

author

2018-01-27 19:48 UTC #0 lore raw

PATCH 1/4 mvpp2: Fix warning over 32bit vs 64bit targets

When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.

Fixes 377883f16d36 ("net: mvpp2x: fix phy connected to wrong mdio issue")

Cc Stefan Chulski <[email protected]>

Cc Stefan Roese <[email protected]>

Cc Joe Hershberger <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 drivers/net/mvpp2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/mvpp2.c +1 -1

index 233c98b66c88..e3d31a560d54 100644
@@ -4722,7 +4722,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
 	u32 id;
 	u32 phyaddr = 0;
 	int phy_mode = -1;
-	u64 mdio_addr;
+	phys_addr_t mdio_addr;
 
 	phy_node = fdtdec_lookup_phandle(gd->fdt_blob, port_node, "phy");

Signature · 1 line

2.7.4

Tom Rini

author

2018-01-29 20:20 UTC #1 lore raw

PATCH 1/4 mvpp2: Fix warning over 32bit vs 64bit targets

Quoted 9 lines · Tom Rini

When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.
Fixes 377883f16d36 ("net: mvpp2x: fix phy connected to wrong mdio issue")
Cc Stefan Chulski <[email protected]>
Signed-off-by Tom Rini <[email protected]>
Reviewed-by Stefan Roese <[email protected]>
Reviewed-by Joe Hershberger <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180129/ef599123/attachment.sig>

Tom Rini

author

2018-01-27 19:48 UTC #2 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

In reply to Tom Rini

When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.

Cc Lukasz Majewski <[email protected]>

Cc Marek Vasut <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +1 -1

index b6164afa9245..ec4b1e219ed0 100644
@@ -114,7 +114,7 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
 				(unsigned long) ep->dma_buf +
 				ROUND(ep->len, CONFIG_SYS_CACHELINE_SIZE));
 
-	writel((unsigned int) ep->dma_buf, &reg->out_endp[ep_num].doepdma);
+	writel((phys_addr_t) ep->dma_buf, &reg->out_endp[ep_num].doepdma);
 	writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
 	       &reg->out_endp[ep_num].doeptsiz);
 	writel(DEPCTL_EPENA|DEPCTL_CNAK|ctrl, &reg->out_endp[ep_num].doepctl);

Signature · 1 line

2.7.4

Marek Vasut

2018-01-27 19:52 UTC #3 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

Quoted 3 lines · Tom Rini

When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.

Can someoneone pass in a pointer above 32bit address range ? That might cause some mess ...

Quoted 21 lines

Cc Lukasz Majewski <[email protected]>

Cc Marek Vasut <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +1 -1

index b6164afa9245..ec4b1e219ed0 100644
@@ -114,7 +114,7 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
 				(unsigned long) ep->dma_buf +
 				ROUND(ep->len, CONFIG_SYS_CACHELINE_SIZE));
 
-	writel((unsigned int) ep->dma_buf, &reg->out_endp[ep_num].doepdma);
+	writel((phys_addr_t) ep->dma_buf, &reg->out_endp[ep_num].doepdma);
 	writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
 	       &reg->out_endp[ep_num].doeptsiz);
 	writel(DEPCTL_EPENA|DEPCTL_CNAK|ctrl, &reg->out_endp[ep_num].doepctl);

Signature · 2 lines

Best regards,
Marek Vasut

Tom Rini

author

2018-01-27 19:56 UTC #4 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

Quoted 7 lines · Marek Vasut

Quoted 3 lines · Tom Rini
When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.
Can someoneone pass in a pointer above 32bit address range ? That might cause some mess ...

On rockchip, where this gets used as well, it comes down to writel and a warning about "cast from pointer to integer of different size".

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180127/85da2e73/attachment.sig>

Marek Vasut

2018-01-27 20:09 UTC #5 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

Quoted 11 lines · Tom Rini

Quoted 7 lines · Marek Vasut
Quoted 3 lines · Tom Rini
When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.
Can someoneone pass in a pointer above 32bit address range ? That might cause some mess ...
On rockchip, where this gets used as well, it comes down to writel and a warning about "cast from pointer to integer of different size".

My concern is with passing addresses which the core cannot handle to the core, addresses above 32bit address space. But that's probably not an issue here.

btw it seems that the common practice in that driver is to cast it to unsigned long, so I think you should be consistent and do that. Or even better, adjust drivers/usb/gadget/dwc2_udc_otg_priv.h and change dma_buf in struct dwc2_ep to phys_addr_t there, then you can probably drop all the casts all over the dwc2 gadget and host drivers.

Signature · 2 lines

Best regards,
Marek Vasut

Dr. Philipp Tomsich

2018-01-27 20:17 UTC #6 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

In reply to Tom Rini

Quoted 9 lines · Tom Rini

Quoted 5 lines · Marek Vasut
Quoted 3 lines · Tom Rini
When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.

Quoted 5 lines

Quoted 2 lines
Can someoneone pass in a pointer above 32bit address range ? That might cause some mess ...
On rockchip, where this gets used as well, it comes down to writel and a warning about "cast from pointer to integer of different size”.

I had submitted a patch to resolve this middle of last year

	https://patchwork.ozlabs.org/patch/783541/

and used the rationale that this change would make the truncation explicit.

I don’t recall why did this didn’t move anywhere, but darkly remember the discussion veering towards bounce buffers…

Regards,
Philipp.

Marek Vasut

2018-01-27 20:31 UTC #7 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

Quoted 15 lines · Dr. Philipp Tomsich

Quoted 9 lines · Tom Rini
Quoted 5 lines · Marek Vasut
Quoted 3 lines · Tom Rini
When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.
Quoted 5 lines
Quoted 2 lines
Can someoneone pass in a pointer above 32bit address range ? That might cause some mess ...
On rockchip, where this gets used as well, it comes down to writel and a warning about "cast from pointer to integer of different size”.

Quoted 6 lines

I had submitted a patch to resolve this middle of last year
	https://patchwork.ozlabs.org/patch/783541/
and used the rationale that this change would make the truncation explicit.
I don’t recall why did this didn’t move anywhere, but darkly remember the discussion veering towards bounce buffers…

I'll just bounce this to Lukasz, since he maintains the gadget, but the DWC2 UDC driver looks really dark ... ew ...

Signature · 2 lines

Best regards,
Marek Vasut

Lukasz Majewski

2018-01-28 09:57 UTC #8 lore raw

PATCH 2/4 usb: gadget: dwc2: Fix warning over 32bit vs 64bit targets

Hi Marek, Tom, Philipp,

Quoted 10 lines · Dr. Philipp Tomsich

Quoted 9 lines
Quoted 9 lines · Tom Rini
Quoted 5 lines · Marek Vasut
Quoted 3 lines · Tom Rini
When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Can someoneone pass in a pointer above 32bit address range ? That might cause some mess ...
On rockchip, where this gets used as well, it comes down to writel and a warning about "cast from pointer to integer of different size”.

I'm not sure if DWC2 is prepared to work with 64B pointers. As fair as I remember it works with 32B.

Quoted 3 lines

Quoted 3 lines
I had submitted a patch to resolve this middle of last year
	https://patchwork.ozlabs.org/patch/783541/

Unfortunately, I did not received this patch.

Quoted 9 lines

Quoted 5 lines
and used the rationale that this change would make the truncation explicit.
I don’t recall why did this didn’t move anywhere, but darkly remember the discussion veering towards bounce buffers…
I'll just bounce this to Lukasz, since he maintains the gadget, but the DWC2 UDC driver looks really dark ... ew ...

Best regards,

Lukasz Majewski

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180128/91848388/attachment.sig>

Tom Rini

author

2018-01-27 19:48 UTC #9 lore raw

PATCH 3/4 fs: btrfs: Fix printf format character warning

In reply to Tom Rini

When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.

Cc Marek BehÃºn <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 fs/btrfs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/btrfs/super.c +1 -1

index 2529c2b3b6a6..b0a5cffce31e 100644
@@ -147,7 +147,7 @@ static int btrfs_check_super(struct btrfs_super_block *sb)
 
 	if (sb->sys_chunk_array_size < sizeof(struct btrfs_key) +
 	    sizeof(struct btrfs_chunk)) {
-		printf("%s: system chunk array too small %u < %lu\n", __func__,
+		printf("%s: system chunk array too small %u < %zu\n", __func__,
 		       sb->sys_chunk_array_size, (u32) sizeof(struct btrfs_key)
 		       + sizeof(struct btrfs_chunk));
 		ret = -1;

Signature · 1 line

2.7.4

Marek Behun

2018-01-27 22:17 UTC #10 lore raw

PATCH 3/4 fs: btrfs: Fix printf format character warning

shouldn't in that case the (u32) cast be removed?

Quoted 24 lines · On Sat, 27 Jan 2018 14:48:10 -0500 Tom Rini <[email protected]> wrote:

When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc Marek BehÃºn <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 fs/btrfs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/super.c +1 -1
index 2529c2b3b6a6..b0a5cffce31e 100644
@@ -147,7 +147,7 @@ static int btrfs_check_super(struct
 btrfs_super_block *sb) 
 	if (sb->sys_chunk_array_size < sizeof(struct btrfs_key) +
 	    sizeof(struct btrfs_chunk)) {
-		printf("%s: system chunk array too small %u <
 %lu\n", __func__,
+		printf("%s: system chunk array too small %u <
%zu\n", __func__, sb->sys_chunk_array_size, (u32) sizeof(struct btrfs_key)
fs/btrfs/super.c +0 -0
 		       + sizeof(struct btrfs_chunk));
 		ret = -1;

Tom Rini

author

2018-01-27 22:23 UTC #11 lore raw

PATCH v2 3/4 fs: btrfs: Fix printf format character warning

In reply to Tom Rini

When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.

Cc Marek BehÃºn <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
Changes in v2:
- Drop u32 cast, per Marek.

---
 fs/btrfs/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/btrfs/super.c +2 -2

index 2529c2b3b6a6..7e2528c4fb6f 100644
@@ -147,8 +147,8 @@ static int btrfs_check_super(struct btrfs_super_block *sb)
 
 	if (sb->sys_chunk_array_size < sizeof(struct btrfs_key) +
 	    sizeof(struct btrfs_chunk)) {
-		printf("%s: system chunk array too small %u < %lu\n", __func__,
-		       sb->sys_chunk_array_size, (u32) sizeof(struct btrfs_key)
+		printf("%s: system chunk array too small %u < %zu\n", __func__,
+		       sb->sys_chunk_array_size, sizeof(struct btrfs_key)
 		       + sizeof(struct btrfs_chunk));
 		ret = -1;
 	}

Signature · 1 line

2.7.4

Marek Behun

2018-01-27 22:36 UTC #12 lore raw

PATCH v2 3/4 fs: btrfs: Fix printf format character warning

Reviewed-by Marek Behun <[email protected]>

Quoted 31 lines · On Sat, 27 Jan 2018 17:23:21 -0500 Tom Rini <[email protected]> wrote:

When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.

Cc Marek BehÃºn <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
Changes in v2:
- Drop u32 cast, per Marek.

---
 fs/btrfs/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/btrfs/super.c +2 -2

index 2529c2b3b6a6..7e2528c4fb6f 100644
@@ -147,8 +147,8 @@ static int btrfs_check_super(struct
 btrfs_super_block *sb) 
 	if (sb->sys_chunk_array_size < sizeof(struct btrfs_key) +
 	    sizeof(struct btrfs_chunk)) {
-		printf("%s: system chunk array too small %u <
 %lu\n", __func__,
-		       sb->sys_chunk_array_size, (u32) sizeof(struct
 btrfs_key)
+		printf("%s: system chunk array too small %u <
 %zu\n", __func__,
+		       sb->sys_chunk_array_size, sizeof(struct

btrfs_key)

fs/btrfs/super.c +0 -0

 		       + sizeof(struct btrfs_chunk));
 		ret = -1;
 	}

Tom Rini

author

2018-01-29 20:21 UTC #13 lore raw

PATCH v2 3/4 fs: btrfs: Fix printf format character warning

In reply to Tom Rini

Quoted 6 lines · Tom Rini

When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc Marek BehÃºn <[email protected]>
Signed-off-by Tom Rini <[email protected]>
Reviewed-by Marek Behun <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180129/d4f53f32/attachment.sig>

Stefan Roese

2018-01-29 07:15 UTC #14 lore raw

PATCH 1/4 mvpp2: Fix warning over 32bit vs 64bit targets

In reply to Tom Rini

Quoted 19 lines · Tom Rini

When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.
Fixes 377883f16d36 ("net: mvpp2x: fix phy connected to wrong mdio issue")
Cc Stefan Chulski <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Joe Hershberger <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
  drivers/net/mvpp2.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/mvpp2.c +0 -0
index 233c98b66c88..e3d31a560d54 100644

drivers/net/mvpp2.c static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port) · line 4728

Quoted 8 lines

drivers/net/mvpp2.c +1 -1

@@ -4722,7 +4722,7 @@ static int phy_info_parse(struct udevice *dev, struct mvpp2_port *port)
  	u32 id;
  	u32 phyaddr = 0;
  	int phy_mode = -1;
-	u64 mdio_addr;
+	phys_addr_t mdio_addr;
 
  	phy_node = fdtdec_lookup_phandle(gd->fdt_blob, port_node, "phy");

Reviewed-by Stefan Roese <[email protected]>

Thanks,
Stefan

Joe Hershberger

2018-01-29 17:46 UTC #15 lore raw

PATCH 1/4 mvpp2: Fix warning over 32bit vs 64bit targets

In reply to Tom Rini

Quoted 9 lines · Tom Rini

When we have a driver that is used on both 32bit and 64bit targets and we are talking about address space we cannot use u64 nor u32 and instead need to use phys_addr_t.
Fixes 377883f16d36 ("net: mvpp2x: fix phy connected to wrong mdio issue")
Cc Stefan Chulski <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Joe Hershberger <[email protected]>
Signed-off-by Tom Rini <[email protected]>

Reviewed-by Joe Hershberger <[email protected]>

Tom Rini

author

2018-01-27 19:48 UTC #16 lore raw

PATCH 4/4 usb: ehci-mxs: Fix argument order for ehci_writel()

In reply to Tom Rini

The order of arguments for ehci_writel() is 'dest', 'value' and not 'value', 'dest'.

Cc Stefano Babic <[email protected]>

Cc Marek Vasut <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 drivers/usb/host/ehci-mxs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/ehci-mxs.c +1 -1

index 6b8d969bb000..9872415562c2 100644
@@ -156,7 +156,7 @@ int ehci_hcd_stop(int index)
 
 	tmp = ehci_readl(&hcor->or_usbcmd);
 	tmp &= ~CMD_RUN;
-	ehci_writel(tmp, &hcor->or_usbcmd);
+	ehci_writel(&hcor->or_usbcmd, tmp);
 
 	/* Disable the PHY */
 	tmp = USBPHY_PWD_RXPWDRX | USBPHY_PWD_RXPWDDIFF |

Signature · 1 line

2.7.4

Marek Vasut

2018-01-27 19:53 UTC #17 lore raw

PATCH 4/4 usb: ehci-mxs: Fix argument order for ehci_writel()

Quoted 6 lines · Tom Rini

The order of arguments for ehci_writel() is 'dest', 'value' and not 'value', 'dest'.
Cc Stefano Babic <[email protected]>
Cc Marek Vasut <[email protected]>
Signed-off-by Tom Rini <[email protected]>

Already fixed and in the current USB PR, see:

usb: ehci: mxs: fix swapped argument in ehci_writel()

Quoted 18 lines

---
 drivers/usb/host/ehci-mxs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/ehci-mxs.c +1 -1

index 6b8d969bb000..9872415562c2 100644
@@ -156,7 +156,7 @@ int ehci_hcd_stop(int index)
 
 	tmp = ehci_readl(&hcor->or_usbcmd);
 	tmp &= ~CMD_RUN;
-	ehci_writel(tmp, &hcor->or_usbcmd);
+	ehci_writel(&hcor->or_usbcmd, tmp);
 
 	/* Disable the PHY */
 	tmp = USBPHY_PWD_RXPWDRX | USBPHY_PWD_RXPWDDIFF |

Signature · 2 lines

Best regards,
Marek Vasut

Tom Rini

author

2018-01-27 19:53 UTC #18 lore raw

PATCH 4/4 usb: ehci-mxs: Fix argument order for ehci_writel()

Quoted 11 lines · Marek Vasut

Quoted 6 lines · Tom Rini
The order of arguments for ehci_writel() is 'dest', 'value' and not 'value', 'dest'.
Cc Stefano Babic <[email protected]>
Cc Marek Vasut <[email protected]>
Signed-off-by Tom Rini <[email protected]>
Already fixed and in the current USB PR, see:
usb: ehci: mxs: fix swapped argument in ehci_writel()

Yup, thanks, disregard.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180127/c3dfd5f3/attachment.sig>