[PATCH v2 1/1] timer: dw-apb: fix compiler warnings
readl() and writel() expect void *. Do not pass an integer value.
Remove unused include asm/arch/timer.h.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Bin Meng bin.meng@windriver.com --- v2: correct typo in commit message --- drivers/timer/dw-apb-timer.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 35271b20c8..c8be4417fd 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -14,7 +14,6 @@ #include <dm/device_compat.h>
#include <asm/io.h> -#include <asm/arch/timer.h>
#define DW_APB_LOAD_VAL 0x0 #define DW_APB_CURR_VAL 0x4 @@ -34,7 +33,7 @@ static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) * requires the count to be incrementing. Invert the * result. */ - *count = timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL)); + *count = timer_conv_64(~readl((void *)(priv->regs + DW_APB_CURR_VAL)));
return 0; } @@ -61,8 +60,8 @@ static int dw_apb_timer_probe(struct udevice *dev) clk_free(&clk);
/* init timer */ - writel(0xffffffff, priv->regs + DW_APB_LOAD_VAL); - writel(0xffffffff, priv->regs + DW_APB_CURR_VAL); + writel(0xffffffff, (void *)(priv->regs + DW_APB_LOAD_VAL)); + writel(0xffffffff, (void *)(priv->regs + DW_APB_CURR_VAL)); setbits_le32(priv->regs + DW_APB_CTRL, 0x3);
return 0; -- 2.27.0
On 8/26/20 4:29 PM, Heinrich Schuchardt wrote:
readl() and writel() expect void *. Do not pass an integer value.
Remove unused include asm/arch/timer.h.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Bin Meng bin.meng@windriver.com
v2: correct typo in commit message
drivers/timer/dw-apb-timer.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 35271b20c8..c8be4417fd 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -14,7 +14,6 @@ #include <dm/device_compat.h>
#include <asm/io.h> -#include <asm/arch/timer.h>
#define DW_APB_LOAD_VAL 0x0 #define DW_APB_CURR_VAL 0x4 @@ -34,7 +33,7 @@ static int dw_apb_timer_get_count(struct udevice *dev, u64 *count) * requires the count to be incrementing. Invert the * result. */
- *count = timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
- *count = timer_conv_64(~readl((void *)(priv->regs + DW_APB_CURR_VAL)));
How about changing the type of priv.regs to void * and using dev_read_addr_ptr instead of dev_read_addr in probe?
return 0; } @@ -61,8 +60,8 @@ static int dw_apb_timer_probe(struct udevice *dev) clk_free(&clk);
/* init timer */
- writel(0xffffffff, priv->regs + DW_APB_LOAD_VAL);
- writel(0xffffffff, priv->regs + DW_APB_CURR_VAL);
writel(0xffffffff, (void *)(priv->regs + DW_APB_LOAD_VAL));
writel(0xffffffff, (void *)(priv->regs + DW_APB_CURR_VAL)); setbits_le32(priv->regs + DW_APB_CTRL, 0x3);
return 0;
-- 2.27.0
Am 26. August 2020 23:13:01 MESZ schrieb Sean Anderson seanga2@gmail.com:
On 8/26/20 4:29 PM, Heinrich Schuchardt wrote:
readl() and writel() expect void *. Do not pass an integer value.
Remove unused include asm/arch/timer.h.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Bin Meng bin.meng@windriver.com
v2: correct typo in commit message
drivers/timer/dw-apb-timer.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/timer/dw-apb-timer.c
b/drivers/timer/dw-apb-timer.c
index 35271b20c8..c8be4417fd 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -14,7 +14,6 @@ #include <dm/device_compat.h>
#include <asm/io.h> -#include <asm/arch/timer.h>
#define DW_APB_LOAD_VAL 0x0 #define DW_APB_CURR_VAL 0x4 @@ -34,7 +33,7 @@ static int dw_apb_timer_get_count(struct udevice
*dev, u64 *count)
* requires the count to be incrementing. Invert the * result. */
- *count = timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
- *count = timer_conv_64(~readl((void *)(priv->regs +
DW_APB_CURR_VAL)));
How about changing the type of priv.regs to void * and using dev_read_addr_ptr instead of dev_read_addr in probe?
Adding to void* results undefined behavior in C. I know gcc currently supports it, but that does not make it recommendable.
If you wanted to change the type, you could use u8*.
A cleaner way would be to change regs to a pointer to a structure with all registers and to eliminate the offsets.
@Marek Which way do you want it?
Best regards
Heinrich
return 0; } @@ -61,8 +60,8 @@ static int dw_apb_timer_probe(struct udevice *dev) clk_free(&clk);
/* init timer */
- writel(0xffffffff, priv->regs + DW_APB_LOAD_VAL);
- writel(0xffffffff, priv->regs + DW_APB_CURR_VAL);
writel(0xffffffff, (void *)(priv->regs + DW_APB_LOAD_VAL));
writel(0xffffffff, (void *)(priv->regs + DW_APB_CURR_VAL)); setbits_le32(priv->regs + DW_APB_CTRL, 0x3);
return 0;
-- 2.27.0
On 27.08.20 05:35, Heinrich Schuchardt wrote:
Am 26. August 2020 23:13:01 MESZ schrieb Sean Anderson seanga2@gmail.com:
On 8/26/20 4:29 PM, Heinrich Schuchardt wrote:
readl() and writel() expect void *. Do not pass an integer value.
Remove unused include asm/arch/timer.h.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Bin Meng bin.meng@windriver.com
v2: correct typo in commit message
drivers/timer/dw-apb-timer.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/timer/dw-apb-timer.c
b/drivers/timer/dw-apb-timer.c
index 35271b20c8..c8be4417fd 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -14,7 +14,6 @@ #include <dm/device_compat.h>
#include <asm/io.h> -#include <asm/arch/timer.h>
#define DW_APB_LOAD_VAL 0x0 #define DW_APB_CURR_VAL 0x4 @@ -34,7 +33,7 @@ static int dw_apb_timer_get_count(struct udevice
*dev, u64 *count)
* requires the count to be incrementing. Invert the * result. */
- *count = timer_conv_64(~readl(priv->regs + DW_APB_CURR_VAL));
- *count = timer_conv_64(~readl((void *)(priv->regs +
DW_APB_CURR_VAL)));
How about changing the type of priv.regs to void * and using dev_read_addr_ptr instead of dev_read_addr in probe?
Adding to void* results in undefined behavior in C. I know gcc currently supports it, but that does not make it recommendable.
If you wanted to change the type, you could use u8*.
A cleaner way would be to change regs to a pointer to a structure with all registers and to eliminate the offsets.
@Marek Which way do you want it?
Hello Marek,
What are your thoughts?
Best regards
Heinrich
return 0; } @@ -61,8 +60,8 @@ static int dw_apb_timer_probe(struct udevice *dev) clk_free(&clk);
/* init timer */
- writel(0xffffffff, priv->regs + DW_APB_LOAD_VAL);
- writel(0xffffffff, priv->regs + DW_APB_CURR_VAL);
writel(0xffffffff, (void *)(priv->regs + DW_APB_LOAD_VAL));
writel(0xffffffff, (void *)(priv->regs + DW_APB_CURR_VAL)); setbits_le32(priv->regs + DW_APB_CTRL, 0x3);
return 0;
-- 2.27.0
participants (2)
-
Heinrich Schuchardt -
Sean Anderson