We have Goldfish RTC device available on QEMU RISC-V virt machine
hence enable required driver in RV32 and RV64 defconfigs.
Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/configs/defconfig | 3 +++
arch/riscv/configs/rv32_defconfig | 3 +++
2 files changed, 6 insertions(+)
diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 3efff552a261..57b4f67b0c0b 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -73,7 +73,10 @@ CONFIG_USB_STORAGE=y
CONFIG_USB_UAS=y
CONFIG_MMC=y
CONFIG_MMC_SPI=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_GOLDFISH=y
CONFIG_VIRTIO_MMIO=y
+CONFIG_GOLDFISH=y
CONFIG_EXT4_FS=y
CONFIG_EXT4_FS_POSIX_ACL=y
CONFIG_AUTOFS4_FS=y
diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
index 7da93e494445..50716c1395aa 100644
--- a/arch/riscv/configs/rv32_defconfig
+++ b/arch/riscv/configs/rv32_defconfig
@@ -69,7 +69,10 @@ CONFIG_USB_OHCI_HCD=y
CONFIG_USB_OHCI_HCD_PLATFORM=y
CONFIG_USB_STORAGE=y
CONFIG_USB_UAS=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_GOLDFISH=y
CONFIG_VIRTIO_MMIO=y
+CONFIG_GOLDFISH=y
CONFIG_SIFIVE_PLIC=y
CONFIG_EXT4_FS=y
CONFIG_EXT4_FS_POSIX_ACL=y
--
2.17.1
On Tue, 24 Sep 2019 23:38:08 PDT (-0700), Anup Patel wrote:
> We have Goldfish RTC device available on QEMU RISC-V virt machine
> hence enable required driver in RV32 and RV64 defconfigs.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/configs/defconfig | 3 +++
> arch/riscv/configs/rv32_defconfig | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index 3efff552a261..57b4f67b0c0b 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -73,7 +73,10 @@ CONFIG_USB_STORAGE=y
> CONFIG_USB_UAS=y
> CONFIG_MMC=y
> CONFIG_MMC_SPI=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_GOLDFISH=y
> CONFIG_VIRTIO_MMIO=y
> +CONFIG_GOLDFISH=y
> CONFIG_EXT4_FS=y
> CONFIG_EXT4_FS_POSIX_ACL=y
> CONFIG_AUTOFS4_FS=y
> diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
> index 7da93e494445..50716c1395aa 100644
> --- a/arch/riscv/configs/rv32_defconfig
> +++ b/arch/riscv/configs/rv32_defconfig
> @@ -69,7 +69,10 @@ CONFIG_USB_OHCI_HCD=y
> CONFIG_USB_OHCI_HCD_PLATFORM=y
> CONFIG_USB_STORAGE=y
> CONFIG_USB_UAS=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_GOLDFISH=y
> CONFIG_VIRTIO_MMIO=y
> +CONFIG_GOLDFISH=y
> CONFIG_SIFIVE_PLIC=y
> CONFIG_EXT4_FS=y
> CONFIG_EXT4_FS_POSIX_ACL=y
> --
> 2.17.1
Reviewed-by: Palmer Dabbelt <[email protected]>
IIRC there was supposed to be a follow-up to your QEMU patch set to rebase it
on top of a refactoring of their RTC code, but I don't see it in my inbox. LMK
if I missed it, as QEMU's soft freeze is in a few weeks and I'd like to make
sure I get everything in.
Additionally: we should refactor our Kconfig to have some sort of
CONFIG_SOC_VIRT that selects this stuff, like we have the CONFIG_SOC_SIFIVE.
This will explicitly document why devices are in the defconfig, avoid
duplicating a bunch of stuff between defconfigs, and provide an example of how
we support multiple SOCs in a single image.
I don't see why either of these should block merging the patch, though.
> -----Original Message-----
> From: Palmer Dabbelt <[email protected]>
> Sent: Saturday, October 12, 2019 11:09 PM
> To: Anup Patel <[email protected]>
> Cc: Paul Walmsley <[email protected]>; [email protected];
> Greg KH <[email protected]>; [email protected]; Atish Patra
> <[email protected]>; Alistair Francis <[email protected]>;
> Christoph Hellwig <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Anup Patel
> <[email protected]>
> Subject: Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver
>
> On Tue, 24 Sep 2019 23:38:08 PDT (-0700), Anup Patel wrote:
> > We have Goldfish RTC device available on QEMU RISC-V virt machine
> > hence enable required driver in RV32 and RV64 defconfigs.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/configs/defconfig | 3 +++
> > arch/riscv/configs/rv32_defconfig | 3 +++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/arch/riscv/configs/defconfig
> > b/arch/riscv/configs/defconfig index 3efff552a261..57b4f67b0c0b 100644
> > --- a/arch/riscv/configs/defconfig
> > +++ b/arch/riscv/configs/defconfig
> > @@ -73,7 +73,10 @@ CONFIG_USB_STORAGE=y CONFIG_USB_UAS=y
> > CONFIG_MMC=y CONFIG_MMC_SPI=y
> > +CONFIG_RTC_CLASS=y
> > +CONFIG_RTC_DRV_GOLDFISH=y
> > CONFIG_VIRTIO_MMIO=y
> > +CONFIG_GOLDFISH=y
> > CONFIG_EXT4_FS=y
> > CONFIG_EXT4_FS_POSIX_ACL=y
> > CONFIG_AUTOFS4_FS=y
> > diff --git a/arch/riscv/configs/rv32_defconfig
> > b/arch/riscv/configs/rv32_defconfig
> > index 7da93e494445..50716c1395aa 100644
> > --- a/arch/riscv/configs/rv32_defconfig
> > +++ b/arch/riscv/configs/rv32_defconfig
> > @@ -69,7 +69,10 @@ CONFIG_USB_OHCI_HCD=y
> > CONFIG_USB_OHCI_HCD_PLATFORM=y CONFIG_USB_STORAGE=y
> CONFIG_USB_UAS=y
> > +CONFIG_RTC_CLASS=y
> > +CONFIG_RTC_DRV_GOLDFISH=y
> > CONFIG_VIRTIO_MMIO=y
> > +CONFIG_GOLDFISH=y
> > CONFIG_SIFIVE_PLIC=y
> > CONFIG_EXT4_FS=y
> > CONFIG_EXT4_FS_POSIX_ACL=y
> > --
> > 2.17.1
>
> Reviewed-by: Palmer Dabbelt <[email protected]>
>
> IIRC there was supposed to be a follow-up to your QEMU patch set to rebase
> it on top of a refactoring of their RTC code, but I don't see it in my inbox. LMK
> if I missed it, as QEMU's soft freeze is in a few weeks and I'd like to make
> sure I get everything in.
I was hoping for QEMU RTC refactoring to be merged soon but it has not
happened so far. I will wait couple of more days then send v3 of QEMU
patches.
>
> Additionally: we should refactor our Kconfig to have some sort of
> CONFIG_SOC_VIRT that selects this stuff, like we have the
> CONFIG_SOC_SIFIVE.
> This will explicitly document why devices are in the defconfig, avoid
> duplicating a bunch of stuff between defconfigs, and provide an example of
> how we support multiple SOCs in a single image.
Yes, indeed we need CONFIG_SOC_VIRT but this will be a separate patch.
>
> I don't see why either of these should block merging the patch, though.
Thanks,
Anup
On Mon, 14 Oct 2019, Anup Patel wrote:
> > -----Original Message-----
> > From: Palmer Dabbelt <[email protected]>
> > Sent: Saturday, October 12, 2019 11:09 PM
> > To: Anup Patel <[email protected]>
> > Cc: Paul Walmsley <[email protected]>; [email protected];
> > Greg KH <[email protected]>; [email protected]; Atish Patra
> > <[email protected]>; Alistair Francis <[email protected]>;
> > Christoph Hellwig <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected]; Anup Patel
> > <[email protected]>
> > Subject: Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver
> >
> > On Tue, 24 Sep 2019 23:38:08 PDT (-0700), Anup Patel wrote:
> > > We have Goldfish RTC device available on QEMU RISC-V virt machine
> > > hence enable required driver in RV32 and RV64 defconfigs.
My understanding is that the Goldfish support is still under
discussion on the QEMU side and isn't merged yet - is that accurate?
https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04904.html
> >
> > Reviewed-by: Palmer Dabbelt <[email protected]>
> >
> > IIRC there was supposed to be a follow-up to your QEMU patch set to rebase
> > it on top of a refactoring of their RTC code, but I don't see it in my inbox. LMK
> > if I missed it, as QEMU's soft freeze is in a few weeks and I'd like to make
> > sure I get everything in.
>
> I was hoping for QEMU RTC refactoring to be merged soon but it has not
> happened so far. I will wait couple of more days then send v3 of QEMU
> patches.
The patch looks fine to me, but let's wait until the underlying support
actually appears on the QEMU "hardware". Could you resend once that's
happened?
thanks,
- Paul
On Tue, 2019-10-22 at 12:23 -0700, Paul Walmsley wrote:
> On Mon, 14 Oct 2019, Anup Patel wrote:
>
> > > -----Original Message-----
> > > From: Palmer Dabbelt <[email protected]>
> > > Sent: Saturday, October 12, 2019 11:09 PM
> > > To: Anup Patel <[email protected]>
> > > Cc: Paul Walmsley <[email protected]>;
> > > [email protected];
> > > Greg KH <[email protected]>; [email protected]; Atish
> > > Patra
> > > <[email protected]>; Alistair Francis <[email protected]
> > > >;
> > > Christoph Hellwig <[email protected]>; [email protected];
> > > linux-
> > > [email protected]; [email protected]; Anup
> > > Patel
> > > <[email protected]>
> > > Subject: Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish
> > > RTC driver
> > >
> > > On Tue, 24 Sep 2019 23:38:08 PDT (-0700), Anup Patel wrote:
> > > > We have Goldfish RTC device available on QEMU RISC-V virt
> > > > machine
> > > > hence enable required driver in RV32 and RV64 defconfigs.
>
> My understanding is that the Goldfish support is still under
> discussion on the QEMU side and isn't merged yet - is that accurate?
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04904.html
>
> > > Reviewed-by: Palmer Dabbelt <[email protected]>
> > >
> > > IIRC there was supposed to be a follow-up to your QEMU patch set
> > > to rebase
> > > it on top of a refactoring of their RTC code, but I don't see it
> > > in my inbox. LMK
> > > if I missed it, as QEMU's soft freeze is in a few weeks and I'd
> > > like to make
> > > sure I get everything in.
> >
> > I was hoping for QEMU RTC refactoring to be merged soon but it has
> > not
> > happened so far. I will wait couple of more days then send v3 of
> > QEMU
> > patches.
>
> The patch looks fine to me, but let's wait until the underlying
> support
> actually appears on the QEMU "hardware". Could you resend once
> that's
> happened?
I think it makese sense for this to go into Linux first.
The QEMU patches are going to be accepted, just some nit picking to do
first :)
After that we have to wait for a PR and then a QEMU release until most
people will see the change in QEMU. In that time Linux 5.4 will be
released, if this can make it into 5.4 then everyone using 5.4 will get
the new RTC as soon as they upgrade QEMU (QEMU provides the device
tree). If this has to wait until QEMU has support then it won't be
supported for users until even later.
Users are generally slow to update kernels (buildroot is still using
5.1 by default for example) so the sooner changes like this go in the
better.
Alistair
>
> thanks,
>
> - Paul
On Tue, 22 Oct 2019, Alistair Francis wrote:
> I think it makese sense for this to go into Linux first.
>
> The QEMU patches are going to be accepted, just some nit picking to do
> first :)
>
> After that we have to wait for a PR and then a QEMU release until most
> people will see the change in QEMU. In that time Linux 5.4 will be
> released, if this can make it into 5.4 then everyone using 5.4 will get
> the new RTC as soon as they upgrade QEMU (QEMU provides the device
> tree). If this has to wait until QEMU has support then it won't be
> supported for users until even later.
>
> Users are generally slow to update kernels (buildroot is still using
> 5.1 by default for example) so the sooner changes like this go in the
> better.
The defconfigs are really just for kernel developers. We expect users to
define their own Kconfigs for their own needs.
If using the Goldfish code really is what we all want to do (see below),
then the kernel patch that should go in right away -- which also has no
dependence on what QEMU does -- would be the first patch of this series:
https://lore.kernel.org/linux-riscv/[email protected]/
And that should go in via whoever is maintaining the Goldfish driver, not
the RISC-V tree. (It looks like drivers/platform/goldfish is completely
unmaintained - a red flag! - so probably someone needs to persuade Greg or
Andrew to take it.)
Incidentally, just looking at drivers/platform/goldfish, that driver seems
to be some sort of Google-specific RPC driver. Are you all really sure
you want to enable that just for an RTC? Seems like overkill - there are
much simpler RTCs out there.
- Paul
On Wed, Oct 23, 2019 at 6:37 AM Paul Walmsley <[email protected]> wrote:
>
> On Tue, 22 Oct 2019, Alistair Francis wrote:
>
> > I think it makese sense for this to go into Linux first.
> >
> > The QEMU patches are going to be accepted, just some nit picking to do
> > first :)
> >
> > After that we have to wait for a PR and then a QEMU release until most
> > people will see the change in QEMU. In that time Linux 5.4 will be
> > released, if this can make it into 5.4 then everyone using 5.4 will get
> > the new RTC as soon as they upgrade QEMU (QEMU provides the device
> > tree). If this has to wait until QEMU has support then it won't be
> > supported for users until even later.
> >
> > Users are generally slow to update kernels (buildroot is still using
> > 5.1 by default for example) so the sooner changes like this go in the
> > better.
>
> The defconfigs are really just for kernel developers. We expect users to
> define their own Kconfigs for their own needs.
>
> If using the Goldfish code really is what we all want to do (see below),
> then the kernel patch that should go in right away -- which also has no
> dependence on what QEMU does -- would be the first patch of this series:
>
> https://lore.kernel.org/linux-riscv/[email protected]/
>
> And that should go in via whoever is maintaining the Goldfish driver, not
> the RISC-V tree. (It looks like drivers/platform/goldfish is completely
> unmaintained - a red flag! - so probably someone needs to persuade Greg or
> Andrew to take it.)
GregKH has already queued this for Linux-5.5 and you can see this
commit present in linux-next tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/drivers/platform/goldfish?h=next-20191022
>
> Incidentally, just looking at drivers/platform/goldfish, that driver seems
> to be some sort of Google-specific RPC driver. Are you all really sure
Nopes, it's not RPC driver. In fact, all Goldfish virtual platform devices
are MMIO devices.
> you want to enable that just for an RTC? Seems like overkill - there are
> much simpler RTCs out there.
No, it's not overkill. All Goldfish virtual platform devices are quite simple
MMIO devices having bare minimum registers required for device
functioning.
The problem is VirtIO spec does not define any RTC device so instead of
inventing our own virtual RTC device we re-use RTC device defined in
Goldfish virtual platform for QEMU virt machine. This way we can re-use
the Linux Goldfish RTC driver.
BTW, I will send-out QEMU Goldfish RTC patches today or tomorrow
addressing nit comments from Alistair.
Regards,
Anup
On Wed, 23 Oct 2019, Anup Patel wrote:
> On Wed, Oct 23, 2019 at 11:30 AM Paul Walmsley <[email protected]> wrote:
>
> > Is drivers/platform/goldfish/goldfish_pipe.c required for the Goldfish RTC
> > driver or not?
>
> No, it's not required.
>
> > If not, then the first patch that was sent isn't the right fix. It would
> > be better to remove the Kbuild dependency between the code in
> > drivers/platform/goldfish and the Goldfish RTC.
>
> The common GOLDFISH kconfig option is there to specify the
> common expectations of all GOLDFISH drivers from Linux ARCH
> support.
OK, then in that case the Goldfish RTC sounds reasonable to me.
- Paul
On Wed, 23 Oct 2019, Anup Patel wrote:
> On Wed, Oct 23, 2019 at 6:37 AM Paul Walmsley <[email protected]> wrote:
>
> > Incidentally, just looking at drivers/platform/goldfish, that driver seems
> > to be some sort of Google-specific RPC driver. Are you all really sure
>
> Nopes, it's not RPC driver. In fact, all Goldfish virtual platform
> devices are MMIO devices.
Is drivers/platform/goldfish/goldfish_pipe.c required for the Goldfish RTC
driver or not?
If not, then the first patch that was sent isn't the right fix. It would
be better to remove the Kbuild dependency between the code in
drivers/platform/goldfish and the Goldfish RTC.
If it is required, then surely there must be a simpler RTC implementation
available.
> The problem is VirtIO spec does not define any RTC device so instead of
> inventing our own virtual RTC device we re-use RTC device defined in
> Goldfish virtual platform for QEMU virt machine. This way we can re-use
> the Linux Goldfish RTC driver.
With 160+ RTC drivers in the kernel tree already, we certainly agree that
it doesn't make sense to invent a new RTC.
- Paul
On Wed, Oct 23, 2019 at 11:30 AM Paul Walmsley <[email protected]> wrote:
>
> On Wed, 23 Oct 2019, Anup Patel wrote:
>
> > On Wed, Oct 23, 2019 at 6:37 AM Paul Walmsley <[email protected]> wrote:
> >
> > > Incidentally, just looking at drivers/platform/goldfish, that driver seems
> > > to be some sort of Google-specific RPC driver. Are you all really sure
> >
> > Nopes, it's not RPC driver. In fact, all Goldfish virtual platform
> > devices are MMIO devices.
>
> Is drivers/platform/goldfish/goldfish_pipe.c required for the Goldfish RTC
> driver or not?
No, it's not required.
>
> If not, then the first patch that was sent isn't the right fix. It would
> be better to remove the Kbuild dependency between the code in
> drivers/platform/goldfish and the Goldfish RTC.
The common GOLDFISH kconfig option is there to specify the
common expectations of all GOLDFISH drivers from Linux ARCH
support.
Currently, all GOLDFISH drivers require HAS_IOMEM and
HAS_DMA support from underlying arch.
If you still think that common GOLDFISH kconfig option is not
required then please go ahead and send patch.
>
> If it is required, then surely there must be a simpler RTC implementation
> available.
GOLDFISH pipe is not required so GOLDFISH RTC is certainly
a simple RTC implementation.
>
> > The problem is VirtIO spec does not define any RTC device so instead of
> > inventing our own virtual RTC device we re-use RTC device defined in
> > Goldfish virtual platform for QEMU virt machine. This way we can re-use
> > the Linux Goldfish RTC driver.
>
> With 160+ RTC drivers in the kernel tree already, we certainly agree that
> it doesn't make sense to invent a new RTC.
>
>
> - Paul
Regards,
Anup
On Tue, 2019-10-22 at 18:06 -0700, Paul Walmsley wrote:
> On Tue, 22 Oct 2019, Alistair Francis wrote:
>
> > I think it makese sense for this to go into Linux first.
> >
> > The QEMU patches are going to be accepted, just some nit picking to
> > do
> > first :)
> >
> > After that we have to wait for a PR and then a QEMU release until
> > most
> > people will see the change in QEMU. In that time Linux 5.4 will be
> > released, if this can make it into 5.4 then everyone using 5.4 will
> > get
> > the new RTC as soon as they upgrade QEMU (QEMU provides the device
> > tree). If this has to wait until QEMU has support then it won't be
> > supported for users until even later.
> >
> > Users are generally slow to update kernels (buildroot is still
> > using
> > 5.1 by default for example) so the sooner changes like this go in
> > the
> > better.
>
> The defconfigs are really just for kernel developers. We expect
> users to
> define their own Kconfigs for their own needs.
From experience most people use the defconfig, at least as a starting
point.
>
> If using the Goldfish code really is what we all want to do (see
> below),
> then the kernel patch that should go in right away -- which also has
> no
> dependence on what QEMU does -- would be the first patch of this
> series:
>
> https://lore.kernel.org/linux-riscv/[email protected]/
Ok, so it looks like this patch will be a 5.5 patch not a 5.4 patch. It
looks like that can't be helped. I just don't want the defconfig change
waiting on QEMU as I think that just slows everything down.
>
> And that should go in via whoever is maintaining the Goldfish driver,
> not
> the RISC-V tree. (It looks like drivers/platform/goldfish is
> completely
> unmaintained - a red flag! - so probably someone needs to persuade
> Greg or
> Andrew to take it.)
>
> Incidentally, just looking at drivers/platform/goldfish, that driver
> seems
> to be some sort of Google-specific RPC driver. Are you all really
> sure
> you want to enable that just for an RTC? Seems like overkill - there
> are
> much simpler RTCs out there.
I was under the impression that everyone was on board with this going
in. In QEMU land it doesn't make sense to add it if the kernel isn't
going to, so we need to be on the same page here.
From the other discussions it looks like you are happy with this change
overall right?
Alistair
>
>
> - Paul
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, 23 Oct 2019, Alistair Francis wrote:
> On Tue, 2019-10-22 at 18:06 -0700, Paul Walmsley wrote:
> > On Tue, 22 Oct 2019, Alistair Francis wrote:
> >
> > > I think it makese sense for this to go into Linux first.
> > >
> > > The QEMU patches are going to be accepted, just some nit picking to
> > > do first :)
> > >
> > > After that we have to wait for a PR and then a QEMU release until
> > > most people will see the change in QEMU. In that time Linux 5.4 will
> > > be released, if this can make it into 5.4 then everyone using 5.4
> > > will get the new RTC as soon as they upgrade QEMU (QEMU provides the
> > > device tree). If this has to wait until QEMU has support then it
> > > won't be supported for users until even later.
> > >
> > > Users are generally slow to update kernels (buildroot is still using
> > > 5.1 by default for example) so the sooner changes like this go in
> > > the better.
> >
> > The defconfigs are really just for kernel developers. We expect users
> > to define their own Kconfigs for their own needs.
>
> From experience most people use the defconfig, at least as a starting
> point.
We'll definitely add it to the defconfigs, but I think it makes sense to
do that once the patches hit the QEMU master branch. (No need to wait for
a QEMU release.)
That roughly matches what I understand the Linux kernel's approach is to
adding hardware support: no point in adding hardware support until it
looks likely that it will actually exist. Otherwise it just adds churn
and maintenance burden.
> I was under the impression that everyone was on board with this going
> in. In QEMU land it doesn't make sense to add it if the kernel isn't
> going to, so we need to be on the same page here.
Whatever RTC gets added into QEMU, we'll take defconfig patches for. I
don't care which one it is. Based on the patches that hit the kernel
lists, it initially looked like the Goldfish RTC was more complicated than
it needed to be; but it turned out I just didn't look deeply enough.
> From the other discussions it looks like you are happy with this change
> overall right?
Yes
- Paul