[U-Boot] [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 stefanc@marvell.com Cc: Stefan Roese sr@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com --- drivers/net/mvpp2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c index 233c98b66c88..e3d31a560d54 100644 --- a/drivers/net/mvpp2.c +++ b/drivers/net/mvpp2.c @@ -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");
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 lukma@denx.de Cc: Marek Vasut marex@denx.de Signed-off-by: Tom Rini trini@konsulko.com --- drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index b6164afa9245..ec4b1e219ed0 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -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, ®->out_endp[ep_num].doepdma); + writel((phys_addr_t) ep->dma_buf, ®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz); writel(DEPCTL_EPENA|DEPCTL_CNAK|ctrl, ®->out_endp[ep_num].doepctl);
On 01/27/2018 08:48 PM, Tom Rini wrote:
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 ...
Cc: Lukasz Majewski lukma@denx.de Cc: Marek Vasut marex@denx.de Signed-off-by: Tom Rini trini@konsulko.com
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index b6164afa9245..ec4b1e219ed0 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -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, ®->out_endp[ep_num].doepdma);
- writel((phys_addr_t) ep->dma_buf, ®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz); writel(DEPCTL_EPENA|DEPCTL_CNAK|ctrl, ®->out_endp[ep_num].doepctl);
On Sat, Jan 27, 2018 at 08:52:43PM +0100, Marek Vasut wrote:
On 01/27/2018 08:48 PM, Tom Rini wrote:
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".
On 01/27/2018 08:56 PM, Tom Rini wrote:
On Sat, Jan 27, 2018 at 08:52:43PM +0100, Marek Vasut wrote:
On 01/27/2018 08:48 PM, Tom Rini wrote:
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.
On 27 Jan 2018, at 20:56, Tom Rini trini@konsulko.com wrote:
On Sat, Jan 27, 2018 at 08:52:43PM +0100, Marek Vasut wrote:
On 01/27/2018 08:48 PM, Tom Rini wrote:
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”.
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.
On 01/27/2018 09:17 PM, Dr. Philipp Tomsich wrote:
On 27 Jan 2018, at 20:56, Tom Rini trini@konsulko.com wrote:
On Sat, Jan 27, 2018 at 08:52:43PM +0100, Marek Vasut wrote:
On 01/27/2018 08:48 PM, Tom Rini wrote:
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”.
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 ...
Hi Marek, Tom, Philipp,
On 01/27/2018 09:17 PM, Dr. Philipp Tomsich wrote:
On 27 Jan 2018, at 20:56, Tom Rini trini@konsulko.com wrote:
On Sat, Jan 27, 2018 at 08:52:43PM +0100, Marek Vasut wrote:
On 01/27/2018 08:48 PM, Tom Rini wrote:
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”.
I'm not sure if DWC2 is prepared to work with 64B pointers. As fair as I remember it works with 32B.
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.
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@denx.de
When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc: Marek Behún marek.behun@nic.cz Signed-off-by: Tom Rini trini@konsulko.com --- fs/btrfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 2529c2b3b6a6..b0a5cffce31e 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -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;
shouldn't in that case the (u32) cast be removed?
On Sat, 27 Jan 2018 14:48:10 -0500 Tom Rini trini@konsulko.com wrote:
When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc: Marek Behún marek.behun@nic.cz Signed-off-by: Tom Rini trini@konsulko.com
fs/btrfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 2529c2b3b6a6..b0a5cffce31e 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -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;
When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc: Marek Behún marek.behun@nic.cz Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v2: - Drop u32 cast, per Marek. --- fs/btrfs/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 2529c2b3b6a6..7e2528c4fb6f 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -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; }
Reviewed-by: Marek Behun marek.behun@nic.cz
On Sat, 27 Jan 2018 17:23:21 -0500 Tom Rini trini@konsulko.com wrote:
When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc: Marek Behún marek.behun@nic.cz Signed-off-by: Tom Rini trini@konsulko.com
Changes in v2:
- Drop u32 cast, per Marek.
fs/btrfs/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 2529c2b3b6a6..7e2528c4fb6f 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -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(structbtrfs_key)
printf("%s: system chunk array too small %u <%zu\n", __func__,
sb->sys_chunk_array_size, sizeof(structbtrfs_key) + sizeof(struct btrfs_chunk)); ret = -1; }
On Sat, Jan 27, 2018 at 05:23:21PM -0500, Tom Rini wrote:
When printing a size_t value we need to use %zu for portability between 32bit and 64bit targets.
Cc: Marek Behún marek.behun@nic.cz Signed-off-by: Tom Rini trini@konsulko.com Reviewed-by: Marek Behun marek.behun@nic.cz
Applied to u-boot/master, thanks!
The order of arguments for ehci_writel() is 'dest', 'value' and not 'value', 'dest'.
Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Signed-off-by: Tom Rini trini@konsulko.com --- drivers/usb/host/ehci-mxs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c index 6b8d969bb000..9872415562c2 100644 --- a/drivers/usb/host/ehci-mxs.c +++ b/drivers/usb/host/ehci-mxs.c @@ -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 |
On 01/27/2018 08:48 PM, Tom Rini wrote:
The order of arguments for ehci_writel() is 'dest', 'value' and not 'value', 'dest'.
Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Signed-off-by: Tom Rini trini@konsulko.com
Already fixed and in the current USB PR, see:
usb: ehci: mxs: fix swapped argument in ehci_writel()
drivers/usb/host/ehci-mxs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-mxs.c b/drivers/usb/host/ehci-mxs.c index 6b8d969bb000..9872415562c2 100644 --- a/drivers/usb/host/ehci-mxs.c +++ b/drivers/usb/host/ehci-mxs.c @@ -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 |
On Sat, Jan 27, 2018 at 08:53:34PM +0100, Marek Vasut wrote:
On 01/27/2018 08:48 PM, Tom Rini wrote:
The order of arguments for ehci_writel() is 'dest', 'value' and not 'value', 'dest'.
Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Signed-off-by: Tom Rini trini@konsulko.com
Already fixed and in the current USB PR, see:
usb: ehci: mxs: fix swapped argument in ehci_writel()
Yup, thanks, disregard.
On 27.01.2018 20:48, Tom Rini wrote:
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 stefanc@marvell.com Cc: Stefan Roese sr@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com
drivers/net/mvpp2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c index 233c98b66c88..e3d31a560d54 100644 --- a/drivers/net/mvpp2.c +++ b/drivers/net/mvpp2.c @@ -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 sr@denx.de
Thanks, Stefan
On Sat, Jan 27, 2018 at 1:48 PM, Tom Rini trini@konsulko.com wrote:
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 stefanc@marvell.com Cc: Stefan Roese sr@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Signed-off-by: Tom Rini trini@konsulko.com
Reviewed-by: Joe Hershberger joe.hershberger@ni.com
On Sat, Jan 27, 2018 at 02:48:08PM -0500, Tom Rini wrote:
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 stefanc@marvell.com Signed-off-by: Tom Rini trini@konsulko.com Reviewed-by: Stefan Roese sr@denx.de Reviewed-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot/master, thanks!
participants (7)
-
Dr. Philipp Tomsich -
Joe Hershberger -
Lukasz Majewski -
Marek Behun -
Marek Vasut -
Stefan Roese -
Tom Rini