[PATCH v2 0/2] Enable Goldfish RTC for RISC-V

[PATCH v2 0/2] Enable Goldfish RTC for RISC-V

[Anup Patel]

We will be using Goldfish RTC device real date-time on QEMU RISC-V virt
machine so this series:
1. Allows GOLDFISH kconfig option to be enabled for RISC-V
2. Enables GOLDFISH RTC driver in RISC-V defconfigs

This series can be found in goldfish_rtc_v2 branch at:
https//github.com/avpatel/linux.git

For the QEMU patches adding Goldfish RTC to virt machine refer:
https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg05465.html

Changes since v1:
 - Updated PATCH1 to allow goldfish drivers for all archs with IOMEM
   and DMA support

Anup Patel (2):
  platform: goldfish: Allow goldfish drivers for archs with IOMEM and
    DMA
  RISC-V: defconfig: Enable Goldfish RTC driver

 arch/riscv/configs/defconfig      | 3 +++
 arch/riscv/configs/rv32_defconfig | 3 +++
 drivers/platform/goldfish/Kconfig | 3 +--
 3 files changed, 7 insertions(+), 2 deletions(-)

--
2.17.1

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/2] platform: goldfish: Allow goldfish drivers for archs with IOMEM and DMA

[Anup Patel]

We don't need explicit dependency of Goldfish kconfig option on various
architectures. Instead, the Goldfish kconfig option should only depend
on HAS_IOMEM and HAS_DMA which is sufficient for all Goldfish devices.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 drivers/platform/goldfish/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/goldfish/Kconfig b/drivers/platform/goldfish/Kconfig
index 77b35df3a801..f3d09b1631e3 100644
--- a/drivers/platform/goldfish/Kconfig
+++ b/drivers/platform/goldfish/Kconfig
@@ -1,8 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig GOLDFISH
 	bool "Platform support for Goldfish virtual devices"
-	depends on X86_32 || X86_64 || ARM || ARM64 || MIPS
-	depends on HAS_IOMEM
+	depends on HAS_IOMEM && HAS_DMA
 	help
 	  Say Y here to get to see options for the Goldfish virtual platform.
 	  This option alone does not add any kernel code.
-- 
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Anup Patel]

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 <anup.patel@wdc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Palmer Dabbelt]

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 <anup.patel@wdc.com>
> ---
>  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 <palmer@sifive.com>

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.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

RE: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Anup Patel]

> -----Original Message-----
> From: Palmer Dabbelt <palmer@sifive.com>
> Sent: Saturday, October 12, 2019 11:09 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>; aou@eecs.berkeley.edu;
> Greg KH <gregkh@linuxfoundation.org>; rkir@google.com; Atish Patra
> <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>;
> Christoph Hellwig <hch@infradead.org>; anup@brainfault.org; linux-
> riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Anup Patel
> <Anup.Patel@wdc.com>
> 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 <anup.patel@wdc.com>
> > ---
> >  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 <palmer@sifive.com>
> 
> 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

RE: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Paul Walmsley]

On Mon, 14 Oct 2019, Anup Patel wrote:

> > -----Original Message-----
> > From: Palmer Dabbelt <palmer@sifive.com>
> > Sent: Saturday, October 12, 2019 11:09 PM
> > To: Anup Patel <Anup.Patel@wdc.com>
> > Cc: Paul Walmsley <paul.walmsley@sifive.com>; aou@eecs.berkeley.edu;
> > Greg KH <gregkh@linuxfoundation.org>; rkir@google.com; Atish Patra
> > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>;
> > Christoph Hellwig <hch@infradead.org>; anup@brainfault.org; linux-
> > riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Anup Patel
> > <Anup.Patel@wdc.com>
> > 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 <palmer@sifive.com>
> > 
> > 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Alistair Francis]

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 <palmer@sifive.com>
> > > Sent: Saturday, October 12, 2019 11:09 PM
> > > To: Anup Patel <Anup.Patel@wdc.com>
> > > Cc: Paul Walmsley <paul.walmsley@sifive.com>; 
> > > aou@eecs.berkeley.edu;
> > > Greg KH <gregkh@linuxfoundation.org>; rkir@google.com; Atish
> > > Patra
> > > <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com
> > > >;
> > > Christoph Hellwig <hch@infradead.org>; anup@brainfault.org;
> > > linux-
> > > riscv@lists.infradead.org; linux-kernel@vger.kernel.org; Anup
> > > Patel
> > > <Anup.Patel@wdc.com>
> > > 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 <palmer@sifive.com>
> > > 
> > > 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Paul Walmsley]

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/20190925063706.56175-2-anup.patel@wdc.com/

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Anup Patel]

On Wed, Oct 23, 2019 at 6:37 AM Paul Walmsley <paul.walmsley@sifive.com> 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/20190925063706.56175-2-anup.patel@wdc.com/
>
> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Paul Walmsley]

On Wed, 23 Oct 2019, Anup Patel wrote:

> On Wed, Oct 23, 2019 at 6:37 AM Paul Walmsley <paul.walmsley@sifive.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Anup Patel]

On Wed, Oct 23, 2019 at 11:30 AM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> On Wed, 23 Oct 2019, Anup Patel wrote:
>
> > On Wed, Oct 23, 2019 at 6:37 AM Paul Walmsley <paul.walmsley@sifive.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Paul Walmsley]

On Wed, 23 Oct 2019, Anup Patel wrote:

> On Wed, Oct 23, 2019 at 11:30 AM Paul Walmsley <paul.walmsley@sifive.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Alistair Francis]

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/20190925063706.56175-2-anup.patel@wdc.com/

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
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] RISC-V: defconfig: Enable Goldfish RTC driver

[Paul Walmsley]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv