| Message ID | 20230314121216.413434-3-schnelle@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 3/14/23 21:11, 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. I do not see HAS_IOPORT=y defined anywhere in 6.3-rc. Is that in linux-next ? There is a HAS_IOPORT_MAP, but I guess it is different. > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > drivers/ata/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index b56fba76b43f..e5e67bdc2dff 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -342,6 +342,7 @@ endif # HAS_DMA > > config ATA_SFF > bool "ATA SFF support (for legacy IDE and PATA)" > + depends on HAS_IOPORT > default y > help > This option adds support for ATA controllers with SFF
On Wed, Mar 15, 2023, at 02:23, Damien Le Moal wrote: > On 3/14/23 21:11, 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. > > I do not see HAS_IOPORT=y defined anywhere in 6.3-rc. Is that in linux-next ? > There is a HAS_IOPORT_MAP, but I guess it is different. It's defined in patch 1 of the series, so the later patches can't be applied into subsystem trees without that. We can either merge patch 1 as a preparation first, or keep it all together as a series. Arnd
On 3/15/23 15:46, Arnd Bergmann wrote: > On Wed, Mar 15, 2023, at 02:23, Damien Le Moal wrote: >> On 3/14/23 21:11, 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. >> >> I do not see HAS_IOPORT=y defined anywhere in 6.3-rc. Is that in linux-next ? >> There is a HAS_IOPORT_MAP, but I guess it is different. > > It's defined in patch 1 of the series, so the later patches > can't be applied into subsystem trees without that. > > We can either merge patch 1 as a preparation first, or keep it > all together as a series. Got it. Either is fine with me. To allow option 2, I will send my ack. Thanks ! > > Arnd
On 3/14/23 21:11, 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/ata/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index b56fba76b43f..e5e67bdc2dff 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -342,6 +342,7 @@ endif # HAS_DMA > > config ATA_SFF > bool "ATA SFF support (for legacy IDE and PATA)" > + depends on HAS_IOPORT > default y > help > This option adds support for ATA controllers with SFF Acked-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Hi Niklas, On Tue, Mar 14, 2023 at 1:12 PM Niklas Schnelle <schnelle@linux.ibm.com> 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> Thanks for your patch! > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -342,6 +342,7 @@ endif # HAS_DMA > > config ATA_SFF > bool "ATA SFF support (for legacy IDE and PATA)" > + depends on HAS_IOPORT > default y > help > This option adds support for ATA controllers with SFF ATA_SFF is a dependency for lots of (S)ATA drivers. (at least) The following don't use I/O port access: CONFIG_SATA_RCAR (arm/arm64) CONFIG_PATA_FALCON (m68k/atari and m68k/q40) CONFIG_PATA_GAYLE (m68k/amiga) CONFIG_PATA_BUDDHA (m68k/amiga) (at least) The following can use either MMIO or I/O port accesses: CONFIG_PATA_PLATFORM (m68k/mac) Gr{oetje,eeting}s, Geert
On 3/15/23 17:39, Geert Uytterhoeven wrote: > Hi Niklas, > > On Tue, Mar 14, 2023 at 1:12 PM Niklas Schnelle <schnelle@linux.ibm.com> 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> > > Thanks for your patch! > >> --- a/drivers/ata/Kconfig >> +++ b/drivers/ata/Kconfig >> @@ -342,6 +342,7 @@ endif # HAS_DMA >> >> config ATA_SFF >> bool "ATA SFF support (for legacy IDE and PATA)" >> + depends on HAS_IOPORT >> default y >> help >> This option adds support for ATA controllers with SFF > > ATA_SFF is a dependency for lots of (S)ATA drivers. > (at least) The following don't use I/O port access: > > CONFIG_SATA_RCAR (arm/arm64) > CONFIG_PATA_FALCON (m68k/atari and m68k/q40) > CONFIG_PATA_GAYLE (m68k/amiga) > CONFIG_PATA_BUDDHA (m68k/amiga) > > (at least) The following can use either MMIO or I/O port accesses: > > CONFIG_PATA_PLATFORM (m68k/mac) But for these arch/platforms, would there be any reason to not have HAS_IOPORT ? It is supported right now, so we should have HAS_IOPORT for them. > > Gr{oetje,eeting}s, > > Geert >
Hi Damien, On Wed, Mar 15, 2023 at 10:12 AM Damien Le Moal <damien.lemoal@opensource.wdc.com> wrote: > On 3/15/23 17:39, Geert Uytterhoeven wrote: > > On Tue, Mar 14, 2023 at 1:12 PM Niklas Schnelle <schnelle@linux.ibm.com> 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> > > > > Thanks for your patch! > > > >> --- a/drivers/ata/Kconfig > >> +++ b/drivers/ata/Kconfig > >> @@ -342,6 +342,7 @@ endif # HAS_DMA > >> > >> config ATA_SFF > >> bool "ATA SFF support (for legacy IDE and PATA)" > >> + depends on HAS_IOPORT > >> default y > >> help > >> This option adds support for ATA controllers with SFF > > > > ATA_SFF is a dependency for lots of (S)ATA drivers. > > (at least) The following don't use I/O port access: > > > > CONFIG_SATA_RCAR (arm/arm64) > > CONFIG_PATA_FALCON (m68k/atari and m68k/q40) > > CONFIG_PATA_GAYLE (m68k/amiga) > > CONFIG_PATA_BUDDHA (m68k/amiga) > > > > (at least) The following can use either MMIO or I/O port accesses: > > > > CONFIG_PATA_PLATFORM (m68k/mac) > > But for these arch/platforms, would there be any reason to not have HAS_IOPORT ? > It is supported right now, so we should have HAS_IOPORT for them. That's the point: on Amiga and Atari, HAS_IOPORT is optional, and not related to IDE support. On Mac, it is never present. Gr{oetje,eeting}s, Geert
On 2023/03/15 20:36, Geert Uytterhoeven wrote: > Hi Damien, > > On Wed, Mar 15, 2023 at 10:12 AM Damien Le Moal > <damien.lemoal@opensource.wdc.com> wrote: >> On 3/15/23 17:39, Geert Uytterhoeven wrote: >>> On Tue, Mar 14, 2023 at 1:12 PM Niklas Schnelle <schnelle@linux.ibm.com> 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> >>> >>> Thanks for your patch! >>> >>>> --- a/drivers/ata/Kconfig >>>> +++ b/drivers/ata/Kconfig >>>> @@ -342,6 +342,7 @@ endif # HAS_DMA >>>> >>>> config ATA_SFF >>>> bool "ATA SFF support (for legacy IDE and PATA)" >>>> + depends on HAS_IOPORT >>>> default y >>>> help >>>> This option adds support for ATA controllers with SFF >>> >>> ATA_SFF is a dependency for lots of (S)ATA drivers. >>> (at least) The following don't use I/O port access: >>> >>> CONFIG_SATA_RCAR (arm/arm64) >>> CONFIG_PATA_FALCON (m68k/atari and m68k/q40) >>> CONFIG_PATA_GAYLE (m68k/amiga) >>> CONFIG_PATA_BUDDHA (m68k/amiga) >>> >>> (at least) The following can use either MMIO or I/O port accesses: >>> >>> CONFIG_PATA_PLATFORM (m68k/mac) >> >> But for these arch/platforms, would there be any reason to not have HAS_IOPORT ? >> It is supported right now, so we should have HAS_IOPORT for them. > > That's the point: on Amiga and Atari, HAS_IOPORT is optional, and > not related to IDE support. On Mac, it is never present. Ah. OK. I see now. So indeed, applying the dependency on the entire ATA_SFF group of drivers is very coarse. Niklas, Can you change this to apply the dependency per driver ?
On Thu, Mar 16, 2023, at 00:57, Damien Le Moal wrote: > On 2023/03/15 20:36, Geert Uytterhoeven wrote: > Ah. OK. I see now. So indeed, applying the dependency on the entire ATA_SFF > group of drivers is very coarse. > Can you change this to apply the dependency per driver ? I think that will fail to build because of this function on architectures that drop their non-functional inb/outb helpers: int ata_pci_bmdma_clear_simplex(struct pci_dev *pdev) { unsigned long bmdma = pci_resource_start(pdev, 4); u8 simplex; if (bmdma == 0) return -ENOENT; simplex = inb(bmdma + 0x02); outb(simplex & 0x60, bmdma + 0x02); simplex = inb(bmdma + 0x02); if (simplex & 0x80) return -EOPNOTSUPP; return 0; } This is only called from five pata drivers (ali, amd, cmd64x, netcell, serverworks), so an easy workaround would be to make sure those depend on HAS_IOPORT and enclose the function definition in an #ifdef. Arnd
On Thu, 2023-03-16 at 16:21 +0100, Arnd Bergmann wrote: > On Thu, Mar 16, 2023, at 00:57, Damien Le Moal wrote: > > On 2023/03/15 20:36, Geert Uytterhoeven wrote: > > > Ah. OK. I see now. So indeed, applying the dependency on the entire ATA_SFF > > group of drivers is very coarse. > > > > Can you change this to apply the dependency per driver ? > > I think that will fail to build because of this function > on architectures that drop their non-functional > inb/outb helpers: > > int ata_pci_bmdma_clear_simplex(struct pci_dev *pdev) > { > unsigned long bmdma = pci_resource_start(pdev, 4); > u8 simplex; > > if (bmdma == 0) > return -ENOENT; > > simplex = inb(bmdma + 0x02); > outb(simplex & 0x60, bmdma + 0x02); > simplex = inb(bmdma + 0x02); > if (simplex & 0x80) > return -EOPNOTSUPP; > return 0; > } > > This is only called from five pata drivers (ali, amd, > cmd64x, netcell, serverworks), so an easy workaround > would be to make sure those depend on HAS_IOPORT > and enclose the function definition in an #ifdef. > > Arnd There were a few additional inb()/outb() uses so a few more drivers had to have the dependency added but for v4 it will no longer be on the ATA_SFF option and I used your suggestion above for this function. There was another call to it in ata_generic_init_one() that is only done for PCI_VENDOR_ID_AL so I added an #ifdef CONFIG_PATA_ALI around that. Thanks, Niklas
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index b56fba76b43f..e5e67bdc2dff 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -342,6 +342,7 @@ endif # HAS_DMA config ATA_SFF bool "ATA SFF support (for legacy IDE and PATA)" + depends on HAS_IOPORT default y help This option adds support for ATA controllers with SFF
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/ata/Kconfig | 1 + 1 file changed, 1 insertion(+)