Revert commit da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")
and define gf_ioread32()/gf_iowrite32() to be able to use accessors defined by the architecture.
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 }
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);
__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
On Wed, Jan 26, 2022 at 2:41 PM Greg KH greg@kroah.com wrote:
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?
The reverted commit was an incorrect workaround for an old qemu bug:
The goldfish devices are apparently defined as "native" endianness, which in qemu is whatever the architecture maintainers thought it should be at the time (little-endian on arm, big-endian on powerpc, board specific on mips, etc.) but independent of what kernel is actually running on the machine if the CPU supports both.
m68k in qemu picked big-endian here, which is the opposite of what the kernel driver does, presumably because that is the endianness of the CPU itself.
Since we can't fix qemu any more, and runtime detection doesn't work on machines without devicetree, having a sane default (whatever the driver used to do) and an architecture specific override for those that got it wrong is probably our best option.
Arnd
Le 26/01/2022 à 14:41, Greg KH a écrit :
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(-)
...
-- 2.34.1
This feels like a step backwards. Why keep this level of indirection for no good reason?
It was proposed by Arnd on my previous iteration of the series when I wanted to update goldfish-rtc in the same way:
https://lore.kernel.org/all/CAK8P3a1H6-sd_+FqnOq0Zhj=L51EWuW5VCcYeTENcp3+PkT...
Keeping __raw_XXX() functions works on most of the cases except if the current CPU endianness can differ from the architecture one (like a ppc64le kernel (little-endian) running a ppc64 machine (big-endian)).
The best solution would be to update QEMU to set the device as a little-endian one, but google didn't merge its implemention in upstream QEMU and this would break all other OSes using goldfish devices on big-endian architectures.
Thanks, Laurent
linux-stable-mirror@lists.linaro.org
-
Arnd Bergmann -
Greg KH -
Laurent Vivier