| Message ID | 20230314121216.413434-29-schnelle@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
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
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 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.
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(-)