[U-Boot] [PATCH] ahci: Fix compiling warnings under 64bit platforms

Simon Glass sjg at chromium.org
Fri Jul 3 16:41:16 CEST 2015


Hi,

On 3 July 2015 at 00:51,  <Yuantian.Tang at freescale.com> wrote:
> From: Tang Yuantian <Yuantian.Tang at freescale.com>
>
> When compling under 64bit platforms, there are lots of warnings,
> like:
>
> drivers/block/ahci.c:114:18: warning: cast to pointer from integer
>  of different size [-Wint-to-pointer-cast]
>   u8 *port_mmio = (u8 *)probe_ent->port[port].port_mmio;
>                   ^
> drivers/block/ahci.c: In function ?.hci_host_init?.
> drivers/block/ahci.c:218:49: warning: cast from pointer to integer
>  of different size [-Wpointer-to-int-cast]
>    probe_ent->port[i].port_mmio = ahci_port_base((u32) mmio, i);
>
> ......
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie at freescale.com>
> Signed-off-by: Tang Yuantian <Yuantian.Tang at freescale.com>
> ---
>  drivers/block/ahci.c | 46 ++++++++++++++++++++++++----------------------
>  include/ahci.h       |  6 +++---
>  2 files changed, 27 insertions(+), 25 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

See some ideas below.

>
> diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c
> index 4fb846a..090beed 100644
> --- a/drivers/block/ahci.c
> +++ b/drivers/block/ahci.c
> @@ -43,13 +43,13 @@ u16 *ataid[AHCI_MAX_PORTS];
>  #define WAIT_MS_FLUSH  5000
>  #define WAIT_MS_LINKUP 200
>
> -static inline u32 ahci_port_base(u32 base, u32 port)
> +static inline void __iomem *ahci_port_base(void __iomem *base, u32 port)
>  {
>         return base + 0x100 + (port * 0x80);
>  }
>
>
> -static void ahci_setup_port(struct ahci_ioports *port, unsigned long base,
> +static void ahci_setup_port(struct ahci_ioports *port, void __iomem *base,
>                             unsigned int port_idx)
>  {
>         base = ahci_port_base(base, port_idx);
> @@ -61,7 +61,7 @@ static void ahci_setup_port(struct ahci_ioports *port, unsigned long base,
>
>  #define msleep(a) udelay(a * 1000)
>
> -static void ahci_dcache_flush_range(unsigned begin, unsigned len)
> +static void ahci_dcache_flush_range(unsigned long begin, unsigned len)
>  {
>         const unsigned long start = begin;
>         const unsigned long end = start + len;
> @@ -75,7 +75,7 @@ static void ahci_dcache_flush_range(unsigned begin, unsigned len)
>   * controller is invalidated from dcache; next access comes from
>   * physical RAM.
>   */
> -static void ahci_dcache_invalidate_range(unsigned begin, unsigned len)
> +static void ahci_dcache_invalidate_range(unsigned long begin, unsigned len)

May as well make @len unsigned long also.

>  {
>         const unsigned long start = begin;
>         const unsigned long end = start + len;
> @@ -111,7 +111,7 @@ int __weak ahci_link_up(struct ahci_probe_ent *probe_ent, u8 port)
>  {
>         u32 tmp;
>         int j = 0;
> -       u8 *port_mmio = (u8 *)probe_ent->port[port].port_mmio;
> +       void __iomem *port_mmio = probe_ent->port[port].port_mmio;
>
>         /*
>          * Bring up SATA link.
> @@ -171,10 +171,10 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
>         u16 tmp16;
>         unsigned short vendor;
>  #endif
> -       volatile u8 *mmio = (volatile u8 *)probe_ent->mmio_base;
> +       void __iomem *mmio = probe_ent->mmio_base;
>         u32 tmp, cap_save, cmd;
>         int i, j, ret;
> -       volatile u8 *port_mmio;
> +       void __iomem *port_mmio;
>         u32 port_map;
>
>         debug("ahci_host_init: start\n");
> @@ -215,9 +215,9 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent)
>         for (i = 0; i < probe_ent->n_ports; i++) {
>                 if (!(port_map & (1 << i)))
>                         continue;
> -               probe_ent->port[i].port_mmio = ahci_port_base((u32) mmio, i);
> +               probe_ent->port[i].port_mmio = ahci_port_base(mmio, i);
>                 port_mmio = (u8 *) probe_ent->port[i].port_mmio;
> -               ahci_setup_port(&probe_ent->port[i], (unsigned long)mmio, i);
> +               ahci_setup_port(&probe_ent->port[i], mmio, i);
>
>                 /* make sure port is not active */
>                 tmp = readl(port_mmio + PORT_CMD);
> @@ -462,7 +462,7 @@ static int ahci_fill_sg(u8 port, unsigned char *buf, int buf_len)
>
>         for (i = 0; i < sg_count; i++) {
>                 ahci_sg->addr =
> -                   cpu_to_le32((u32) buf + i * MAX_DATA_BYTE_COUNT);
> +                   cpu_to_le32((unsigned long) buf + i * MAX_DATA_BYTE_COUNT);
>                 ahci_sg->addr_hi = 0;
>                 ahci_sg->flags_size = cpu_to_le32(0x3fffff &
>                                           (buf_len < MAX_DATA_BYTE_COUNT
> @@ -501,7 +501,7 @@ static void ahci_set_feature(u8 port)
>         fis[3] = SETFEATURES_XFER;
>         fis[12] = __ilog2(probe_ent->udma_mask + 1) + 0x40 - 0x01;
>
> -       memcpy((unsigned char *)pp->cmd_tbl, fis, sizeof(fis));
> +       memcpy((void *)(unsigned long)pp->cmd_tbl, fis, sizeof(fis));

Thjis cast looks dodgy to me because it is turning a 32-bit type into
a 64-bit address.

Perhaps cmd_tbl should be a ulong also? Or use phys_addr_t?

>         ahci_fill_cmd_slot(pp, cmd_fis_len);
>         ahci_dcache_flush_sata_cmd(pp);
>         writel(1, port_mmio + PORT_CMD_ISSUE);
> @@ -532,9 +532,9 @@ static int wait_spinup(volatile u8 *port_mmio)
>  static int ahci_port_start(u8 port)
>  {
>         struct ahci_ioports *pp = &(probe_ent->port[port]);
> -       volatile u8 *port_mmio = (volatile u8 *)pp->port_mmio;
> +       void __iomem *port_mmio = pp->port_mmio;
>         u32 port_status;
> -       u32 mem;
> +       void __iomem *mem;
>
>         debug("Enter start port: %d\n", port);
>         port_status = readl(port_mmio + PORT_SCR_STAT);
> @@ -544,15 +544,16 @@ static int ahci_port_start(u8 port)
>                 return -1;
>         }
>
> -       mem = (u32) malloc(AHCI_PORT_PRIV_DMA_SZ + 2048);
> +       mem = malloc(AHCI_PORT_PRIV_DMA_SZ + 2048);
>         if (!mem) {
>                 free(pp);
>                 printf("%s: No mem for table!\n", __func__);
>                 return -ENOMEM;
>         }
>
> -       mem = (mem + 0x800) & (~0x7ff); /* Aligned to 2048-bytes */
> -       memset((u8 *) mem, 0, AHCI_PORT_PRIV_DMA_SZ);
> +       /* Aligned to 2048-bytes */
> +       mem = memalign(2048, AHCI_PORT_PRIV_DMA_SZ);
> +       memset(mem, 0, AHCI_PORT_PRIV_DMA_SZ);
>
>         /*
>          * First item in chunk of DMA memory: 32-slot command table,
> @@ -560,7 +561,7 @@ static int ahci_port_start(u8 port)
>          */
>         pp->cmd_slot =
>                 (struct ahci_cmd_hdr *)(uintptr_t)virt_to_phys((void *)mem);
> -       debug("cmd_slot = 0x%x\n", (unsigned)pp->cmd_slot);
> +       debug("cmd_slot = %p\n", pp->cmd_slot);
>         mem += (AHCI_CMD_SLOT_SZ + 224);
>
>         /*
> @@ -580,7 +581,8 @@ static int ahci_port_start(u8 port)
>         pp->cmd_tbl_sg =
>                         (struct ahci_sg *)(uintptr_t)virt_to_phys((void *)mem);
>
> -       writel_with_flush((u32) pp->cmd_slot, port_mmio + PORT_LST_ADDR);
> +       writel_with_flush((unsigned long)pp->cmd_slot,
> +                         port_mmio + PORT_LST_ADDR);
>
>         writel_with_flush(pp->rx_fis, port_mmio + PORT_FIS_ADDR);
>
> @@ -625,14 +627,14 @@ static int ahci_device_data_io(u8 port, u8 *fis, int fis_len, u8 *buf,
>                 return -1;
>         }
>
> -       memcpy((unsigned char *)pp->cmd_tbl, fis, fis_len);
> +       memcpy((void *)(unsigned long)pp->cmd_tbl, fis, fis_len);
>
>         sg_count = ahci_fill_sg(port, buf, buf_len);
>         opts = (fis_len >> 2) | (sg_count << 16) | (is_write << 6);
>         ahci_fill_cmd_slot(pp, opts);
>
>         ahci_dcache_flush_sata_cmd(pp);
> -       ahci_dcache_flush_range((unsigned)buf, (unsigned)buf_len);
> +       ahci_dcache_flush_range((unsigned long)buf, (unsigned)buf_len);
>
>         writel_with_flush(1, port_mmio + PORT_CMD_ISSUE);
>
> @@ -642,7 +644,7 @@ static int ahci_device_data_io(u8 port, u8 *fis, int fis_len, u8 *buf,
>                 return -1;
>         }
>
> -       ahci_dcache_invalidate_range((unsigned)buf, (unsigned)buf_len);
> +       ahci_dcache_invalidate_range((unsigned long)buf, (unsigned)buf_len);
>         debug("%s: %d byte transferred.\n", __func__, pp->cmd_slot->status);
>
>         return 0;
> @@ -1035,7 +1037,7 @@ static int ata_io_flush(u8 port)
>         fis[1] = 1 << 7;         /* Command FIS. */
>         fis[2] = ATA_CMD_FLUSH_EXT;
>
> -       memcpy((unsigned char *)pp->cmd_tbl, fis, 20);
> +       memcpy((void *)(unsigned long)pp->cmd_tbl, fis, 20);

AND HERE

>         ahci_fill_cmd_slot(pp, cmd_fis_len);
>         writel_with_flush(1, port_mmio + PORT_CMD_ISSUE);
>
> diff --git a/include/ahci.h b/include/ahci.h
> index 6d91712..bee6d52 100644
> --- a/include/ahci.h
> +++ b/include/ahci.h
> @@ -135,9 +135,9 @@ struct ahci_sg {
>  };
>
>  struct ahci_ioports {
> -       u32     cmd_addr;
> -       u32     scr_addr;
> -       u32     port_mmio;
> +       void __iomem    *cmd_addr;
> +       void __iomem    *scr_addr;
> +       void __iomem    *port_mmio;

You could change those to ulong instead of pointers. Also there is
map_sysmem() which converts a physical address (which can be defined
as 32-bit even on a 64-bit machine if so-decided) into a pointer.

>         struct ahci_cmd_hdr     *cmd_slot;
>         struct ahci_sg          *cmd_tbl_sg;
>         u32     cmd_tbl;
> --
> 2.1.0.27.g96db324
>

Regards,
Simon


More information about the U-Boot mailing list