[U-Boot] [PATCH v1] rockchip: video: mipi: Modify variable type for arm32 compatibility

Dr. Philipp Tomsich philipp.tomsich at theobroma-systems.com
Thu Jun 15 13:59:53 UTC 2017


Just a few nitpicks regarding typing the pointers and base-addresses.

> On 14 Jun 2017, at 11:26, Eric Gao <eric.gao at rock-chips.com> wrote:
> 
> Some address relevant varibable is defined originally as u64. To
> compatible with arm32, this patch change them to "void __iomem *" type.
> 
> Signed-off-by: Eric Gao <eric.gao at rock-chips.com>
> 
> ---
> 
> drivers/video/rockchip/rk_mipi.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/rockchip/rk_mipi.c b/drivers/video/rockchip/rk_mipi.c
> index ad00397..af7656e 100644
> --- a/drivers/video/rockchip/rk_mipi.c
> +++ b/drivers/video/rockchip/rk_mipi.c
> @@ -76,13 +76,13 @@ static int rk_mipi_read_timing(struct udevice *dev,
>  *        use define in rk_mipi.h directly for this parameter
>  *  @val: value that will be write to specified bits of register
>  */
> -static void rk_mipi_dsi_write(u32 regs, u32 reg, u32 val)
> +static void rk_mipi_dsi_write(void __iomem *regs, u32 reg, u32 val)

I’d tend towards a uintptr_t here…

> {
> 	u32 dat;
> 	u32 mask;
> 	u32 offset = (reg >> OFFSET_SHIFT) & 0xff;
> 	u32 bits = (reg >> BITS_SHIFT) & 0xff;
> -	u64 addr = (reg >> ADDR_SHIFT) + regs;
> +	void __iomem *addr = (reg >> ADDR_SHIFT) + regs;

The register at that address is a u32, so why not make it ‘u32 __iomem *addr’?

> 
> 	/* Mask for specifiled bits,the corresponding bits will be clear */
> 	mask = ~((0xffffffff << offset) & (0xffffffff >> (32 - offset - bits)));
> @@ -108,7 +108,7 @@ static int rk_mipi_dsi_enable(struct udevice *dev,
> 	int node, timing_node;
> 	int val;
> 	struct rk_mipi_priv *priv = dev_get_priv(dev);
> -	u64 regs = (u64)priv->regs;
> +	void __iomem *regs = priv->regs;
> 	struct display_plat *disp_uc_plat = dev_get_uclass_platdata(dev);
> 	u32 txbyte_clk = priv->txbyte_clk;
> 	u32 txesc_clk = priv->txesc_clk;
> @@ -224,7 +224,7 @@ static int rk_mipi_dsi_enable(struct udevice *dev,
> }
> 
> /* rk mipi dphy write function. It is used to write test data to dphy */
> -static void rk_mipi_phy_write(u32 regs, unsigned char test_code,
> +static void rk_mipi_phy_write(void __iomem *regs, unsigned char test_code,
> 			      unsigned char *test_data, unsigned char size)

Based on a different discussion I had in the USB context a few days back, I’d
tend towards ‘uintptr_t’ when we just want to pass an integer representation
of a base-address around.

> {
> 	int i = 0;
> @@ -253,7 +253,7 @@ static int rk_mipi_phy_enable(struct udevice *dev)
> {
> 	int i;
> 	struct rk_mipi_priv *priv = dev_get_priv(dev);
> -	u64 regs = (u64)priv->regs;
> +	void __iomem *regs = priv->regs;
> 	u64 fbdiv;
> 	u64 prediv = 1;
> 	u32 max_fbdiv = 512;
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot



More information about the U-Boot mailing list