Re: [PATCH v12 2/5] tty: goldfish: introduce gf_ioread32()/gf_iowrite32()

[Greg KH <greg@kroah.com>]

On Fri, Jan 21, 2022 at 09:07:35PM +0100, Laurent Vivier wrote:
> Revert
> commit da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")

Why?

> and define gf_ioread32()/gf_iowrite32() to be able to use accessors
> defined by the architecture.

What does this do?

> 
> Cc: stable@vger.kernel.org # v5.11+
> Fixes: da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  drivers/tty/goldfish.c   | 20 ++++++++++----------
>  include/linux/goldfish.h | 15 +++++++++++----
>  2 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
> index 5ed19a9857ad..10c13b93ed52 100644
> --- a/drivers/tty/goldfish.c
> +++ b/drivers/tty/goldfish.c
> @@ -61,13 +61,13 @@ static void do_rw_io(struct goldfish_tty *qtty,
>  	spin_lock_irqsave(&qtty->lock, irq_flags);
>  	gf_write_ptr((void *)address, base + GOLDFISH_TTY_REG_DATA_PTR,
>  		     base + GOLDFISH_TTY_REG_DATA_PTR_HIGH);
> -	__raw_writel(count, base + GOLDFISH_TTY_REG_DATA_LEN);
> +	gf_iowrite32(count, base + GOLDFISH_TTY_REG_DATA_LEN);
>  
>  	if (is_write)
> -		__raw_writel(GOLDFISH_TTY_CMD_WRITE_BUFFER,
> +		gf_iowrite32(GOLDFISH_TTY_CMD_WRITE_BUFFER,
>  		       base + GOLDFISH_TTY_REG_CMD);
>  	else
> -		__raw_writel(GOLDFISH_TTY_CMD_READ_BUFFER,
> +		gf_iowrite32(GOLDFISH_TTY_CMD_READ_BUFFER,
>  		       base + GOLDFISH_TTY_REG_CMD);
>  
>  	spin_unlock_irqrestore(&qtty->lock, irq_flags);
> @@ -142,7 +142,7 @@ static irqreturn_t goldfish_tty_interrupt(int irq, void *dev_id)
>  	unsigned char *buf;
>  	u32 count;
>  
> -	count = __raw_readl(base + GOLDFISH_TTY_REG_BYTES_READY);
> +	count = gf_ioread32(base + GOLDFISH_TTY_REG_BYTES_READY);
>  	if (count == 0)
>  		return IRQ_NONE;
>  
> @@ -159,7 +159,7 @@ static int goldfish_tty_activate(struct tty_port *port, struct tty_struct *tty)
>  {
>  	struct goldfish_tty *qtty = container_of(port, struct goldfish_tty,
>  									port);
> -	__raw_writel(GOLDFISH_TTY_CMD_INT_ENABLE, qtty->base + GOLDFISH_TTY_REG_CMD);
> +	gf_iowrite32(GOLDFISH_TTY_CMD_INT_ENABLE, qtty->base + GOLDFISH_TTY_REG_CMD);
>  	return 0;
>  }
>  
> @@ -167,7 +167,7 @@ static void goldfish_tty_shutdown(struct tty_port *port)
>  {
>  	struct goldfish_tty *qtty = container_of(port, struct goldfish_tty,
>  									port);
> -	__raw_writel(GOLDFISH_TTY_CMD_INT_DISABLE, qtty->base + GOLDFISH_TTY_REG_CMD);
> +	gf_iowrite32(GOLDFISH_TTY_CMD_INT_DISABLE, qtty->base + GOLDFISH_TTY_REG_CMD);
>  }
>  
>  static int goldfish_tty_open(struct tty_struct *tty, struct file *filp)
> @@ -202,7 +202,7 @@ static unsigned int goldfish_tty_chars_in_buffer(struct tty_struct *tty)
>  {
>  	struct goldfish_tty *qtty = &goldfish_ttys[tty->index];
>  	void __iomem *base = qtty->base;
> -	return __raw_readl(base + GOLDFISH_TTY_REG_BYTES_READY);
> +	return gf_ioread32(base + GOLDFISH_TTY_REG_BYTES_READY);
>  }
>  
>  static void goldfish_tty_console_write(struct console *co, const char *b,
> @@ -355,7 +355,7 @@ static int goldfish_tty_probe(struct platform_device *pdev)
>  	 * on Ranchu emulator (qemu2) returns 1 here and
>  	 * driver will use physical addresses.
>  	 */
> -	qtty->version = __raw_readl(base + GOLDFISH_TTY_REG_VERSION);
> +	qtty->version = gf_ioread32(base + GOLDFISH_TTY_REG_VERSION);
>  
>  	/*
>  	 * Goldfish TTY device on Ranchu emulator (qemu2)
> @@ -374,7 +374,7 @@ static int goldfish_tty_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	__raw_writel(GOLDFISH_TTY_CMD_INT_DISABLE, base + GOLDFISH_TTY_REG_CMD);
> +	gf_iowrite32(GOLDFISH_TTY_CMD_INT_DISABLE, base + GOLDFISH_TTY_REG_CMD);
>  
>  	ret = request_irq(irq, goldfish_tty_interrupt, IRQF_SHARED,
>  			  "goldfish_tty", qtty);
> @@ -436,7 +436,7 @@ static int goldfish_tty_remove(struct platform_device *pdev)
>  #ifdef CONFIG_GOLDFISH_TTY_EARLY_CONSOLE
>  static void gf_early_console_putchar(struct uart_port *port, int ch)
>  {
> -	__raw_writel(ch, port->membase);
> +	gf_iowrite32(ch, port->membase);
>  }
>  
>  static void gf_early_write(struct console *con, const char *s, unsigned int n)
> diff --git a/include/linux/goldfish.h b/include/linux/goldfish.h
> index 12be1601fd84..bcc17f95b906 100644
> --- a/include/linux/goldfish.h
> +++ b/include/linux/goldfish.h
> @@ -8,14 +8,21 @@
>  
>  /* Helpers for Goldfish virtual platform */
>  
> +#ifndef gf_ioread32
> +#define gf_ioread32 ioread32
> +#endif
> +#ifndef gf_iowrite32
> +#define gf_iowrite32 iowrite32
> +#endif
> +
>  static inline void gf_write_ptr(const void *ptr, void __iomem *portl,
>  				void __iomem *porth)
>  {
>  	const unsigned long addr = (unsigned long)ptr;
>  
> -	__raw_writel(lower_32_bits(addr), portl);
> +	gf_iowrite32(lower_32_bits(addr), portl);
>  #ifdef CONFIG_64BIT
> -	__raw_writel(upper_32_bits(addr), porth);
> +	gf_iowrite32(upper_32_bits(addr), porth);
>  #endif
>  }
>  
> @@ -23,9 +30,9 @@ static inline void gf_write_dma_addr(const dma_addr_t addr,
>  				     void __iomem *portl,
>  				     void __iomem *porth)
>  {
> -	__raw_writel(lower_32_bits(addr), portl);
> +	gf_iowrite32(lower_32_bits(addr), portl);
>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> -	__raw_writel(upper_32_bits(addr), porth);
> +	gf_iowrite32(upper_32_bits(addr), porth);
>  #endif
>  }
>  
> -- 
> 2.34.1
> 

This feels like a step backwards.  Why keep this level of indirection
for no good reason?

thanks,

greg k-h