[v3,28/38] rtc: add HAS_IOPORT dependencies

  In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/rtc/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Hello,

On 14/03/2023 13:12:06+0100, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. We thus need to add HAS_IOPORT as dependency for
> those drivers using them.
> 
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/rtc/Kconfig | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 5a71579af0a1..20aa77bf0a9f 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -956,6 +956,7 @@ comment "Platform RTC drivers"
>  config RTC_DRV_CMOS
>  	tristate "PC-style 'CMOS'"
>  	depends on X86 || ARM || PPC || MIPS || SPARC64
> +	depends on HAS_IOPORT

Did you check that this will not break platforms that doesn't have RTC_PORT defined?

>  	default y if X86
>  	select RTC_MC146818_LIB
>  	help
> @@ -976,6 +977,7 @@ config RTC_DRV_CMOS
>  config RTC_DRV_ALPHA
>  	bool "Alpha PC-style CMOS"
>  	depends on ALPHA
> +	depends on HAS_IOPORT
>  	select RTC_MC146818_LIB
>  	default y
>  	help
> @@ -1193,7 +1195,7 @@ config RTC_DRV_MSM6242
>  
>  config RTC_DRV_BQ4802
>  	tristate "TI BQ4802"
> -	depends on HAS_IOMEM
> +	depends on HAS_IOMEM && HAS_IOPORT
>  	help
>  	  If you say Y here you will get support for the TI
>  	  BQ4802 RTC chip.
> -- 
> 2.37.2
>
  

On Tue, 2023-03-14 at 13:52 +0100, Alexandre Belloni wrote:
> Hello,
> 
> On 14/03/2023 13:12:06+0100, Niklas Schnelle wrote:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. We thus need to add HAS_IOPORT as dependency for
> > those drivers using them.
> > 
> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> >  drivers/rtc/Kconfig | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index 5a71579af0a1..20aa77bf0a9f 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -956,6 +956,7 @@ comment "Platform RTC drivers"
> >  config RTC_DRV_CMOS
> >  	tristate "PC-style 'CMOS'"
> >  	depends on X86 || ARM || PPC || MIPS || SPARC64
> > +	depends on HAS_IOPORT
> 
> Did you check that this will not break platforms that doesn't have RTC_PORT defined?
> > 

From what I can tell the CMOS_READ() macro this driver relies on uses
some form of inb() style I/O port access in all its definitions. So my
understanding is that this device is always accessed via I/O ports even
if the variants differ slightly and would make no sense on a platform
without any way of accessing I/O ports which is what lack of HAS_IOPORT
means. From what I can see even without RTC_PORT being defined the
CMOS_READ is still used. Hope that answers your question?
  

On Mon, May 8, 2023, at 17:36, Niklas Schnelle wrote:
> On Tue, 2023-03-14 at 13:52 +0100, Alexandre Belloni wrote:
>> Hello,
>> 
>> On 14/03/2023 13:12:06+0100, Niklas Schnelle wrote:
>> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
>> > not being declared. We thus need to add HAS_IOPORT as dependency for
>> > those drivers using them.
>> > 
>> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
>> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
>> > ---
>> >  drivers/rtc/Kconfig | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> > index 5a71579af0a1..20aa77bf0a9f 100644
>> > --- a/drivers/rtc/Kconfig
>> > +++ b/drivers/rtc/Kconfig
>> > @@ -956,6 +956,7 @@ comment "Platform RTC drivers"
>> >  config RTC_DRV_CMOS
>> >  	tristate "PC-style 'CMOS'"
>> >  	depends on X86 || ARM || PPC || MIPS || SPARC64
>> > +	depends on HAS_IOPORT
>> 
>> Did you check that this will not break platforms that doesn't have RTC_PORT defined?
>> > 
>
> From what I can tell the CMOS_READ() macro this driver relies on uses
> some form of inb() style I/O port access in all its definitions. So my
> understanding is that this device is always accessed via I/O ports even
> if the variants differ slightly and would make no sense on a platform
> without any way of accessing I/O ports which is what lack of HAS_IOPORT
> means. From what I can see even without RTC_PORT being defined the
> CMOS_READ is still used. Hope that answers your question?

I think the m68k/atari and mips/dec variants don't necessarily
qualify as PIO, those are really just pointer dereferences, and
they don't use the actual inb/outb functions.

On atari, it looks like HAS_IOPORT may be set if ATARI_ROM_ISA
is, but on dec it's never enabled.

    Arnd
  

Hi Arnd,

On Mon, May 8, 2023 at 10:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, May 8, 2023, at 17:36, Niklas Schnelle wrote:
> > On Tue, 2023-03-14 at 13:52 +0100, Alexandre Belloni wrote:
> >> On 14/03/2023 13:12:06+0100, Niklas Schnelle wrote:
> >> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> >> > not being declared. We thus need to add HAS_IOPORT as dependency for
> >> > those drivers using them.
> >> >
> >> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> >> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> >> > ---
> >> >  drivers/rtc/Kconfig | 4 +++-
> >> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> >> > index 5a71579af0a1..20aa77bf0a9f 100644
> >> > --- a/drivers/rtc/Kconfig
> >> > +++ b/drivers/rtc/Kconfig
> >> > @@ -956,6 +956,7 @@ comment "Platform RTC drivers"
> >> >  config RTC_DRV_CMOS
> >> >    tristate "PC-style 'CMOS'"
> >> >    depends on X86 || ARM || PPC || MIPS || SPARC64
> >> > +  depends on HAS_IOPORT
> >>
> >> Did you check that this will not break platforms that doesn't have RTC_PORT defined?
> >> >
> >
> > From what I can tell the CMOS_READ() macro this driver relies on uses
> > some form of inb() style I/O port access in all its definitions. So my
> > understanding is that this device is always accessed via I/O ports even
> > if the variants differ slightly and would make no sense on a platform
> > without any way of accessing I/O ports which is what lack of HAS_IOPORT
> > means. From what I can see even without RTC_PORT being defined the
> > CMOS_READ is still used. Hope that answers your question?
>
> I think the m68k/atari and mips/dec variants don't necessarily
> qualify as PIO, those are really just pointer dereferences, and
> they don't use the actual inb/outb functions.
>
> On atari, it looks like HAS_IOPORT may be set if ATARI_ROM_ISA
> is, but on dec it's never enabled.

Atari does not use RTC_DRV_CMOS, but still relies on generic RTC
instead.

Last time (in 2013?) I tried converting to RTC_DRV_CMOS by registering
an "rtc_cmos" platform device, I couldn't get it to work.

Gr{oetje,eeting}s,

                        Geert
  

On Tue, May 9, 2023, at 08:38, Geert Uytterhoeven wrote:
> On Mon, May 8, 2023 at 10:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, May 8, 2023, at 17:36, Niklas Schnelle wrote:
>>
>> I think the m68k/atari and mips/dec variants don't necessarily
>> qualify as PIO, those are really just pointer dereferences, and
>> they don't use the actual inb/outb functions.
>>
>> On atari, it looks like HAS_IOPORT may be set if ATARI_ROM_ISA
>> is, but on dec it's never enabled.
>
> Atari does not use RTC_DRV_CMOS, but still relies on generic RTC
> instead.

Ah right, I now remember working on that code, so we're good on
m68k then. I think it should work for everyone using

       depends on HAS_IOPORT || ARCH_DECSTATION

in that case, as that is the only exception.

> Last time (in 2013?) I tried converting to RTC_DRV_CMOS by registering
> an "rtc_cmos" platform device, I couldn't get it to work.

If you ever want to revisit this, I suspect the harder part here
is to detach arch/m68k/ from the RTC_DRV_GENERIC code first, pushing
the device registration into the individual machine specific time.c
code. It's probably not even worth trying to share the rtc-cmos
driver, but it might be useful to share the library code like
RTC_DRV_ALPHA does.

   Arnd
  

Hi Arnd,

On Tue, May 9, 2023 at 10:23 AM Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, May 9, 2023, at 08:38, Geert Uytterhoeven wrote:
> > On Mon, May 8, 2023 at 10:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Mon, May 8, 2023, at 17:36, Niklas Schnelle wrote:
> >>
> >> I think the m68k/atari and mips/dec variants don't necessarily
> >> qualify as PIO, those are really just pointer dereferences, and
> >> they don't use the actual inb/outb functions.
> >>
> >> On atari, it looks like HAS_IOPORT may be set if ATARI_ROM_ISA
> >> is, but on dec it's never enabled.
> >
> > Atari does not use RTC_DRV_CMOS, but still relies on generic RTC
> > instead.
>
> Ah right, I now remember working on that code, so we're good on
> m68k then. I think it should work for everyone using
>
>        depends on HAS_IOPORT || ARCH_DECSTATION
>
> in that case, as that is the only exception.
>
> > Last time (in 2013?) I tried converting to RTC_DRV_CMOS by registering
> > an "rtc_cmos" platform device, I couldn't get it to work.
>
> If you ever want to revisit this, I suspect the harder part here
> is to detach arch/m68k/ from the RTC_DRV_GENERIC code first, pushing
> the device registration into the individual machine specific time.c
> code. It's probably not even worth trying to share the rtc-cmos
> driver, but it might be useful to share the library code like
> RTC_DRV_ALPHA does.

Arch/m68k is not that entangled with RTC_DRV_GENERIC, as amiga_defconfig
does not enable it, but enables CONFIG_RTC_DRV_MSM6242 and
CONFIG_RTC_DRV_RP5C01 instead.

Gr{oetje,eeting}s,

                        Geert
  

On Tue, May 9, 2023, at 10:51, Geert Uytterhoeven wrote:
> On Tue, May 9, 2023 at 10:23 AM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Tue, May 9, 2023, at 08:38, Geert Uytterhoeven wrote:
>> If you ever want to revisit this, I suspect the harder part here
>> is to detach arch/m68k/ from the RTC_DRV_GENERIC code first, pushing
>> the device registration into the individual machine specific time.c
>> code. It's probably not even worth trying to share the rtc-cmos
>> driver, but it might be useful to share the library code like
>> RTC_DRV_ALPHA does.
>
> Arch/m68k is not that entangled with RTC_DRV_GENERIC, as amiga_defconfig
> does not enable it, but enables CONFIG_RTC_DRV_MSM6242 and
> CONFIG_RTC_DRV_RP5C01 instead.

Ah, I see now, I misremembered this part. The only bit that
is shared on m68k is the mach_hwclk() function that is used
for either read_persistent_clock64() or rtc_generic on platforms
that have one. So any platform could indeed be converted from
mach_hwclk() to a custom rtc driver without affecting the others.

    Arnd