2022-01-21 11:53:41

by Laurent Vivier

[permalink] [raw]
Subject: [PATCH v10 0/5] m68k: Add Virtual M68k Machine

The most powerful m68k machine emulated by QEMU is a Quadra 800,

but this machine is very limited: only 1 GiB of memory and only some

specific interfaces, with no DMA.



The Virtual M68k Machine is based on Goldfish interfaces defined by Google

for Android simulator. It uses Goldfish-rtc (timer and RTC),

Goldfish-pic (PIC) and Goldfish-tty (for early tty).



The machine is created with 128 virtio-mmio buses, and they can

be used to add serial console, GPU, disk, NIC, HID, hwrng, 9PFS...



The virtual m68k machine has been merged in QEMU and will be available

with the release 6.0.



This series introduces the support of this new machine in the linux kernel.



If you want to try:



- Configure and build latest QEMU with (or download qemu 6.0 binary):



.../configure --target-list=m68k-softmmu --enable-virglrenderer

make



- Configure and build linux with:



make virt_defconfig

make vmlinux



A pre-installed qcow2 disk image is available at:



http://vivier.eu/debian-10.0.qcow2



You can run the machine with something like:



qemu-system-m68k -M virt \

-m 3G \

-chardev stdio,signal=off,mux=on,id=char0 \

-mon chardev=char0,mode=readline \

-kernel vmlinux \

-append "console=hvc0 root=/dev/vda2" \

-blockdev node-name=system,driver=file,filename=debian-10.0.qcow2 \

-blockdev node-name=drive0,driver=qcow2,file=system \

-device virtio-blk-device,drive=drive0 \

-serial chardev:char0 \

-device virtio-net-device,netdev=hostnet0 \

-netdev bridge,id=hostnet0,br=virbr0,helper=/usr/libexec/qemu-bridge-helper \

-device virtio-serial-device \

-device virtio-gpu-device \

-device virtconsole,chardev=char0 \

-device virtio-keyboard-device \

-device virtio-mouse-device



You can watch a presentation about the machine on the Planet m68k channel:



https://youtu.be/s_ve0bCC9q4

[Demo at 38:00]



v10:

- move goldfish_ioread32()/goldfish_iowrite32() to io.h

- also update goldfish-tty

- use READ_ONCE()/WRITE_ONCE() for in_nmi



v9:

- include <linux/memblock.h> to declare min_low_pfn

- goldfish accessors: s/CONFIG_CPU_BIG_ENDIAN/CONFIG_M68K/



v8:

- GF_PUT_CHAR is a 32bit register (in arch/m68k/kernel/head.S)

- rework goldfish-pic and virt_ctrl



v7:

- add "#include <linux/slab.h>" in timer-goldfish.c for kzalloc()

- update timer-goldfish Kconfig

- remove EXPORT_SYMBOL()

- introduce goldfish_ioread32()/goldfish_iowrite32()



v6:

- fix goldfish-rtc endianness

- move goldfish-timer code to drivers/clocksource

- remove LEGACY_TIMER_TICK and use directly goldfish-timer



v5:

- add GENERIC_CLOCKEVENTS in Kconfig.machine



v4:

- update PATCH 1: comments and parameter names

- add a patch to move to generic clockevents

(I prefer to have a separate patch as it can be used as an example to

move from legacy timer tick to generic clockevents)



v3:

- introduce config.h to export prototypes to arch/m68k/kernel/setup_mm.c

- define virt_nmi_handler as static



v2:

- Remove VIRTO_MENU set y

- sort the selects

- add CONFIG_LOCALVERSION="-virt"

- generate virt_defconfig using "make savedefconfig"

- rename MACH_VIRTONLY to MACH_VIRT_ONLY

- add a test_notvirt label in head.S

- rework iounmap() to use two separate #ifdefs

- use %u in virt_get_model()

- drop show_registers() in config.c

- drop pr_err() from config_virt()

- sort includes in ints.c

- call virt_irq_enable() in virt_irq_startup()

- drop virt_irq_shutdown() and use virt_irq_disable()

- move in_nmi into virt_nmi_handler()

- use pr_warn() in virt_nmi_handler()

- rework goldfish_pic_irq() IRQ scan

- copy goldfish-pic IRQs related information from QEMU hw/m68k/virt

- add a comment to "min_low_pfn = 0"

- use platform_device_register_simple()

- use goldfish_timer_read(), upper_32_bits() and lower_32_bits()



Thanks,

Laurent



Laurent Vivier (5):

m68k: add asm/config.h

rtc: goldfish: introduce goldfish_ioread32()/goldfish_iowrite32()

tty: goldfish: use goldfish_ioread32()/goldfish_iowrite32()

clocksource/drivers: Add a goldfish-timer clocksource

m68k: introduce a virtual m68k machine



arch/m68k/Kbuild | 1 +

arch/m68k/Kconfig.machine | 17 +++

arch/m68k/amiga/config.c | 1 +

arch/m68k/apollo/config.c | 1 +

arch/m68k/atari/config.c | 1 +

arch/m68k/bvme6000/config.c | 1 +

arch/m68k/configs/virt_defconfig | 65 ++++++++++

arch/m68k/hp300/config.c | 1 +

arch/m68k/include/asm/config.h | 35 ++++++

arch/m68k/include/asm/io.h | 3 +

arch/m68k/include/asm/irq.h | 3 +-

arch/m68k/include/asm/pgtable_mm.h | 7 ++

arch/m68k/include/asm/setup.h | 44 +++++--

arch/m68k/include/asm/virt.h | 25 ++++

arch/m68k/include/uapi/asm/bootinfo-virt.h | 18 +++

arch/m68k/include/uapi/asm/bootinfo.h | 1 +

arch/m68k/kernel/Makefile | 1 +

arch/m68k/kernel/head.S | 31 +++++

arch/m68k/kernel/setup_mm.c | 30 ++---

arch/m68k/mac/config.c | 1 +

arch/m68k/mm/kmap.c | 23 ++--

arch/m68k/mvme147/config.c | 1 +

arch/m68k/mvme16x/config.c | 1 +

arch/m68k/q40/config.c | 1 +

arch/m68k/virt/Makefile | 6 +

arch/m68k/virt/config.c | 119 ++++++++++++++++++

arch/m68k/virt/ints.c | 133 +++++++++++++++++++++

arch/m68k/virt/platform.c | 72 +++++++++++

drivers/clocksource/Kconfig | 7 ++

drivers/clocksource/Makefile | 1 +

drivers/clocksource/timer-goldfish.c | 130 ++++++++++++++++++++

drivers/rtc/rtc-goldfish.c | 30 ++---

drivers/tty/goldfish.c | 20 ++--

include/asm-generic/io.h | 7 ++

include/clocksource/timer-goldfish.h | 11 ++

35 files changed, 784 insertions(+), 65 deletions(-)

create mode 100644 arch/m68k/configs/virt_defconfig

create mode 100644 arch/m68k/include/asm/config.h

create mode 100644 arch/m68k/include/asm/virt.h

create mode 100644 arch/m68k/include/uapi/asm/bootinfo-virt.h

create mode 100644 arch/m68k/virt/Makefile

create mode 100644 arch/m68k/virt/config.c

create mode 100644 arch/m68k/virt/ints.c

create mode 100644 arch/m68k/virt/platform.c

create mode 100644 drivers/clocksource/timer-goldfish.c

create mode 100644 include/clocksource/timer-goldfish.h



--

2.34.1




2022-01-21 11:53:41

by Laurent Vivier

[permalink] [raw]
Subject: [PATCH v10 2/5] rtc: goldfish: introduce goldfish_ioread32()/goldfish_iowrite32()

The goldfish device always uses the same endianness as the architecture
using it:
https://android.googlesource.com/platform/external/qemu/+/refs/heads/emu-master-dev/hw/timer/goldfish_timer.c#177

On a big-endian machine, the device is also big-endian, on a
little-endian machine the device is little-endian.

So we need to use the right accessor to read/write values to the goldfish
registers: ioread32()/iowrite32() on a little-endian machine,
ioread32be()/iowrite32be() on a big-endian machine.

This patch introduces goldfish_ioread32()/goldfish_iowrite32() to allow
architectures to define them accordlingly to their endianness.

We define them by default in asm-generic/io.h to access the device using
little-endian access as it is the current use, but we will be able to define
big-endian version when new architectures will use them.

Signed-off-by: Laurent Vivier <[email protected]>
---
drivers/rtc/rtc-goldfish.c | 30 +++++++++++++++---------------
include/asm-generic/io.h | 7 +++++++
2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/rtc/rtc-goldfish.c b/drivers/rtc/rtc-goldfish.c
index 7ab95d052644..1b2180ccfcdd 100644
--- a/drivers/rtc/rtc-goldfish.c
+++ b/drivers/rtc/rtc-goldfish.c
@@ -41,8 +41,8 @@ static int goldfish_rtc_read_alarm(struct device *dev,
rtcdrv = dev_get_drvdata(dev);
base = rtcdrv->base;

- rtc_alarm_low = readl(base + TIMER_ALARM_LOW);
- rtc_alarm_high = readl(base + TIMER_ALARM_HIGH);
+ rtc_alarm_low = goldfish_ioread32(base + TIMER_ALARM_LOW);
+ rtc_alarm_high = goldfish_ioread32(base + TIMER_ALARM_HIGH);
rtc_alarm = (rtc_alarm_high << 32) | rtc_alarm_low;

do_div(rtc_alarm, NSEC_PER_SEC);
@@ -50,7 +50,7 @@ static int goldfish_rtc_read_alarm(struct device *dev,

rtc_time64_to_tm(rtc_alarm, &alrm->time);

- if (readl(base + TIMER_ALARM_STATUS))
+ if (goldfish_ioread32(base + TIMER_ALARM_STATUS))
alrm->enabled = 1;
else
alrm->enabled = 0;
@@ -71,18 +71,18 @@ static int goldfish_rtc_set_alarm(struct device *dev,

if (alrm->enabled) {
rtc_alarm64 = rtc_tm_to_time64(&alrm->time) * NSEC_PER_SEC;
- writel((rtc_alarm64 >> 32), base + TIMER_ALARM_HIGH);
- writel(rtc_alarm64, base + TIMER_ALARM_LOW);
- writel(1, base + TIMER_IRQ_ENABLED);
+ goldfish_iowrite32((rtc_alarm64 >> 32), base + TIMER_ALARM_HIGH);
+ goldfish_iowrite32(rtc_alarm64, base + TIMER_ALARM_LOW);
+ goldfish_iowrite32(1, base + TIMER_IRQ_ENABLED);
} else {
/*
* if this function was called with enabled=0
* then it could mean that the application is
* trying to cancel an ongoing alarm
*/
- rtc_status_reg = readl(base + TIMER_ALARM_STATUS);
+ rtc_status_reg = goldfish_ioread32(base + TIMER_ALARM_STATUS);
if (rtc_status_reg)
- writel(1, base + TIMER_CLEAR_ALARM);
+ goldfish_iowrite32(1, base + TIMER_CLEAR_ALARM);
}

return 0;
@@ -98,9 +98,9 @@ static int goldfish_rtc_alarm_irq_enable(struct device *dev,
base = rtcdrv->base;

if (enabled)
- writel(1, base + TIMER_IRQ_ENABLED);
+ goldfish_iowrite32(1, base + TIMER_IRQ_ENABLED);
else
- writel(0, base + TIMER_IRQ_ENABLED);
+ goldfish_iowrite32(0, base + TIMER_IRQ_ENABLED);

return 0;
}
@@ -110,7 +110,7 @@ static irqreturn_t goldfish_rtc_interrupt(int irq, void *dev_id)
struct goldfish_rtc *rtcdrv = dev_id;
void __iomem *base = rtcdrv->base;

- writel(1, base + TIMER_CLEAR_INTERRUPT);
+ goldfish_iowrite32(1, base + TIMER_CLEAR_INTERRUPT);

rtc_update_irq(rtcdrv->rtc, 1, RTC_IRQF | RTC_AF);

@@ -128,8 +128,8 @@ static int goldfish_rtc_read_time(struct device *dev, struct rtc_time *tm)
rtcdrv = dev_get_drvdata(dev);
base = rtcdrv->base;

- time_low = readl(base + TIMER_TIME_LOW);
- time_high = readl(base + TIMER_TIME_HIGH);
+ time_low = goldfish_ioread32(base + TIMER_TIME_LOW);
+ time_high = goldfish_ioread32(base + TIMER_TIME_HIGH);
time = (time_high << 32) | time_low;

do_div(time, NSEC_PER_SEC);
@@ -149,8 +149,8 @@ static int goldfish_rtc_set_time(struct device *dev, struct rtc_time *tm)
base = rtcdrv->base;

now64 = rtc_tm_to_time64(tm) * NSEC_PER_SEC;
- writel((now64 >> 32), base + TIMER_TIME_HIGH);
- writel(now64, base + TIMER_TIME_LOW);
+ goldfish_iowrite32((now64 >> 32), base + TIMER_TIME_HIGH);
+ goldfish_iowrite32(now64, base + TIMER_TIME_LOW);

return 0;
}
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 7ce93aaf69f8..7f7e9d8c2eef 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -906,6 +906,13 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
#endif /* CONFIG_64BIT */
#endif /* CONFIG_GENERIC_IOMAP */

+#ifndef goldfish_ioread32
+#define goldfish_ioread32 ioread32
+#endif
+#ifndef goldfish_iowrite32
+#define goldfish_iowrite32 iowrite32
+#endif
+
#ifdef __KERNEL__

#include <linux/vmalloc.h>
--
2.34.1

2022-01-21 11:53:53

by Laurent Vivier

[permalink] [raw]
Subject: [PATCH v10 4/5] clocksource/drivers: Add a goldfish-timer clocksource

Add a clocksource based on the goldfish-rtc device.

Signed-off-by: Laurent Vivier <[email protected]>
---
drivers/clocksource/Kconfig | 7 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-goldfish.c | 130 +++++++++++++++++++++++++++
include/clocksource/timer-goldfish.h | 11 +++
4 files changed, 149 insertions(+)
create mode 100644 drivers/clocksource/timer-goldfish.c
create mode 100644 include/clocksource/timer-goldfish.h

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index f65e31bab9ae..fa66fb7156da 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -711,4 +711,11 @@ config MICROCHIP_PIT64B
modes and high resolution. It is used as a clocksource
and a clockevent.

+config GOLDFISH_TIMER
+ bool "Clocksource using goldfish-rtc"
+ depends on M68K || COMPILE_TEST
+ depends on RTC_DRV_GOLDFISH
+ help
+ Support for the timer/counter of goldfish-rtc
+
endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c17ee32a7151..e624a1a27027 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -88,3 +88,4 @@ obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o
obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o
obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o
obj-$(CONFIG_MICROCHIP_PIT64B) += timer-microchip-pit64b.o
+obj-$(CONFIG_GOLDFISH_TIMER) += timer-goldfish.o
diff --git a/drivers/clocksource/timer-goldfish.c b/drivers/clocksource/timer-goldfish.c
new file mode 100644
index 000000000000..e446f9d3cc49
--- /dev/null
+++ b/drivers/clocksource/timer-goldfish.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <clocksource/timer-goldfish.h>
+
+#define TIMER_TIME_LOW 0x00 /* get low bits of current time */
+ /* and update TIMER_TIME_HIGH */
+#define TIMER_TIME_HIGH 0x04 /* get high bits of time at last */
+ /* TIMER_TIME_LOW read */
+#define TIMER_ALARM_LOW 0x08 /* set low bits of alarm and */
+ /* activate it */
+#define TIMER_ALARM_HIGH 0x0c /* set high bits of next alarm */
+#define TIMER_IRQ_ENABLED 0x10
+#define TIMER_CLEAR_ALARM 0x14
+#define TIMER_ALARM_STATUS 0x18
+#define TIMER_CLEAR_INTERRUPT 0x1c
+
+struct goldfish_timer {
+ struct clock_event_device ced;
+ struct resource res;
+ void __iomem *base;
+ int irq;
+};
+
+static struct goldfish_timer *ced_to_gf(struct clock_event_device *ced)
+{
+ return container_of(ced, struct goldfish_timer, ced);
+}
+
+static int goldfish_timer_set_oneshot(struct clock_event_device *evt)
+{
+ struct goldfish_timer *timerdrv = ced_to_gf(evt);
+ void __iomem *base = timerdrv->base;
+
+ goldfish_iowrite32(0, base + TIMER_ALARM_HIGH);
+ goldfish_iowrite32(0, base + TIMER_ALARM_LOW);
+ goldfish_iowrite32(1, base + TIMER_IRQ_ENABLED);
+
+ return 0;
+}
+
+static int goldfish_timer_shutdown(struct clock_event_device *evt)
+{
+ struct goldfish_timer *timerdrv = ced_to_gf(evt);
+ void __iomem *base = timerdrv->base;
+
+ goldfish_iowrite32(0, base + TIMER_IRQ_ENABLED);
+
+ return 0;
+}
+
+static int goldfish_timer_next_event(unsigned long delta,
+ struct clock_event_device *evt)
+{
+ struct goldfish_timer *timerdrv = ced_to_gf(evt);
+ void __iomem *base = timerdrv->base;
+ u64 now;
+
+ goldfish_iowrite32(1, base + TIMER_CLEAR_INTERRUPT);
+
+ /*
+ * time_low: get low bits of current time and update time_high
+ * time_high: get high bits of time at last time_low read
+ */
+ now = goldfish_ioread32(base + TIMER_TIME_LOW);
+ now += (u64)goldfish_ioread32(base + TIMER_TIME_HIGH) << 32;
+
+ now += delta;
+
+ goldfish_iowrite32(upper_32_bits(now), base + TIMER_ALARM_HIGH);
+ goldfish_iowrite32(lower_32_bits(now), base + TIMER_ALARM_LOW);
+
+ return 0;
+}
+
+static irqreturn_t golfish_timer_tick(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = dev_id;
+
+ evt->event_handler(evt);
+
+ return IRQ_HANDLED;
+}
+
+void __init goldfish_timer_init(int irq, void __iomem *base)
+{
+ struct goldfish_timer *timerdrv;
+ int ret;
+
+ timerdrv = kzalloc(sizeof(*timerdrv), GFP_KERNEL);
+ if (!timerdrv)
+ return;
+
+ timerdrv->base = base;
+ timerdrv->irq = irq;
+
+ timerdrv->ced = (struct clock_event_device){
+ .name = "goldfish_timer",
+ .features = CLOCK_EVT_FEAT_ONESHOT,
+ .set_state_shutdown = goldfish_timer_shutdown,
+ .set_state_oneshot = goldfish_timer_set_oneshot,
+ .set_next_event = goldfish_timer_next_event,
+ .shift = 32,
+ };
+ timerdrv->res = (struct resource){
+ .name = "goldfish_timer",
+ .start = (unsigned long)base,
+ .end = (unsigned long)base + 0xfff,
+ };
+
+ if (request_resource(&iomem_resource, &timerdrv->res)) {
+ pr_err("Cannot allocate goldfish-timer resource\n");
+ return;
+ }
+
+ ret = request_irq(timerdrv->irq, golfish_timer_tick, IRQF_TIMER,
+ "goldfish_timer", &timerdrv->ced);
+ if (ret) {
+ pr_err("Couldn't register goldfish-timer interrupt\n");
+ return;
+ }
+
+ clockevents_config_and_register(&timerdrv->ced, NSEC_PER_SEC,
+ 1, 0xffffffff);
+}
diff --git a/include/clocksource/timer-goldfish.h b/include/clocksource/timer-goldfish.h
new file mode 100644
index 000000000000..b237267844f1
--- /dev/null
+++ b/include/clocksource/timer-goldfish.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * goldfish-timer clocksource
+ */
+
+#ifndef _CLOCKSOURCE_GOLDFISH_TIMER_H
+#define _CLOCKSOURCE_GOLDFISH_TIMER_H
+
+extern void goldfish_timer_init(int irq, void __iomem *base);
+
+#endif /* _CLOCKSOURCE_GOLDFISH_TIMER_H */
--
2.34.1

2022-01-21 18:58:19

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v10 2/5] rtc: goldfish: introduce goldfish_ioread32()/goldfish_iowrite32()

Hi Laurent,

On Wed, Jan 19, 2022 at 1:05 AM Laurent Vivier <[email protected]> wrote:
> The goldfish device always uses the same endianness as the architecture
> using it:
> https://android.googlesource.com/platform/external/qemu/+/refs/heads/emu-master-dev/hw/timer/goldfish_timer.c#177
>
> On a big-endian machine, the device is also big-endian, on a
> little-endian machine the device is little-endian.
>
> So we need to use the right accessor to read/write values to the goldfish
> registers: ioread32()/iowrite32() on a little-endian machine,
> ioread32be()/iowrite32be() on a big-endian machine.
>
> This patch introduces goldfish_ioread32()/goldfish_iowrite32() to allow
> architectures to define them accordlingly to their endianness.
>
> We define them by default in asm-generic/io.h to access the device using
> little-endian access as it is the current use, but we will be able to define
> big-endian version when new architectures will use them.
>
> Signed-off-by: Laurent Vivier <[email protected]>

Thanks for your patch!

> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -906,6 +906,13 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
> #endif /* CONFIG_64BIT */
> #endif /* CONFIG_GENERIC_IOMAP */
>
> +#ifndef goldfish_ioread32
> +#define goldfish_ioread32 ioread32
> +#endif
> +#ifndef goldfish_iowrite32
> +#define goldfish_iowrite32 iowrite32
> +#endif
> +
> #ifdef __KERNEL__

I've just discovered include/linux/goldfish.h, which already has gf_*()
accessors for 64-bit, so it'd make sense to move the above there,
and adjust the names.

Arnd: note that the existing ones do use __raw_writel().

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-01-21 19:00:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v10 2/5] rtc: goldfish: introduce goldfish_ioread32()/goldfish_iowrite32()

On Wed, Jan 19, 2022 at 9:21 AM Geert Uytterhoeven <[email protected]> wrote:
> On Wed, Jan 19, 2022 at 1:05 AM Laurent Vivier <[email protected]> wrote:
>
> I've just discovered include/linux/goldfish.h, which already has gf_*()
> accessors for 64-bit, so it'd make sense to move the above there,
> and adjust the names.

Yes, good idea.

>
> Arnd: note that the existing ones do use __raw_writel().

It looks like Laurent introduced that bug in da31de35cd2f ("tty: goldfish: use
__raw_writel()/__raw_readl()") and could fix it up here. Laurent, was the intent
of this earlier patch also to make the driver usabel for m68k, or are there
any other targets you looked at that had mixed up endianness?

Arnd

2022-01-21 19:03:01

by Laurent Vivier

[permalink] [raw]
Subject: Re: [PATCH v10 2/5] rtc: goldfish: introduce goldfish_ioread32()/goldfish_iowrite32()

Le 19/01/2022 à 09:49, Arnd Bergmann a écrit :
> On Wed, Jan 19, 2022 at 9:21 AM Geert Uytterhoeven <[email protected]> wrote:
>> On Wed, Jan 19, 2022 at 1:05 AM Laurent Vivier <[email protected]> wrote:
>>
>> I've just discovered include/linux/goldfish.h, which already has gf_*()
>> accessors for 64-bit, so it'd make sense to move the above there,
>> and adjust the names.
>
> Yes, good idea.

So the idea is to put goldfish accessors inside a "#ifdef CONFIG_M68K ... #else ... #endif" in
include/linux/goldfish.h and not in generic-asm/io.h for the generic version and
m68k/include/ams/io.h for the m68k version?

>
>>
>> Arnd: note that the existing ones do use __raw_writel().
>
> It looks like Laurent introduced that bug in da31de35cd2f ("tty: goldfish: use
> __raw_writel()/__raw_readl()") and could fix it up here. Laurent, was the intent

The idea was to use the native endianness of the CPU, I missed it can differ from the one of the
architecture.

> of this earlier patch also to make the driver usabel for m68k, or are there
> any other targets you looked at that had mixed up endianness?
>

Yes, the intent was to make it usable for m68k.
I think all the targets that use goldfish are little-endian, it's why there was no problem until now.

Let me know which solution you prefer, I will update the series accordingly.

Thanks,
Laurent

2022-01-21 19:06:18

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v10 2/5] rtc: goldfish: introduce goldfish_ioread32()/goldfish_iowrite32()

Hi Laurent,

On Wed, Jan 19, 2022 at 10:11 AM Laurent Vivier <[email protected]> wrote:
> Le 19/01/2022 à 09:49, Arnd Bergmann a écrit :
> > On Wed, Jan 19, 2022 at 9:21 AM Geert Uytterhoeven <[email protected]> wrote:
> >> On Wed, Jan 19, 2022 at 1:05 AM Laurent Vivier <[email protected]> wrote:
> >>
> >> I've just discovered include/linux/goldfish.h, which already has gf_*()
> >> accessors for 64-bit, so it'd make sense to move the above there,
> >> and adjust the names.
> >
> > Yes, good idea.
>
> So the idea is to put goldfish accessors inside a "#ifdef CONFIG_M68K ... #else ... #endif" in
> include/linux/goldfish.h and not in generic-asm/io.h for the generic version and
> m68k/include/ams/io.h for the m68k version?

No, just move

+#ifndef goldfish_ioread32
+#define goldfish_ioread32 ioread32
+#endif
+#ifndef goldfish_iowrite32
+#define goldfish_iowrite32 iowrite32
+#endif

to <linux/goldfish.h>, and rename them to gf_*().

Architectures can still override them in their own <asm/io.h>
(<linux/goldfish.h> includes <linux/io.h>).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds