Re: [PATCH] parport_pc: Also enable driver for PCI systems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 14 Feb 2022, Niklas Schnelle wrote:

> > While older versions of the driver did have to be explicitly configured 
> > for MMIO rather than port I/O, a feature added with commit e89a2cfb7d7b 
> > ("[TC] defxx: TURBOchannel support"), the driver has been improved with 
> > commit 795e272e5474 ("FDDI: defxx: Implement dynamic CSR I/O address space 
> > selection") and the selection of the I/O space to use now fully automatic.
> 
> Very interesting and thanks for the input! On s390 we really only have
> very few different PCI devices and I can only test another hand full
> with my private x86 and ARM systems.

 Note that for TURBOchannel support, which is likewise MMIO only, the 
driver has this:

#if defined(CONFIG_EISA) || defined(CONFIG_PCI)
#define dfx_use_mmio bp->mmio
#else
#define dfx_use_mmio true
#endif

so if your proposal to add HAS_IOPORT goes forward it'll be enough if we 
update the condition to:

#if defined(CONFIG_HAS_IOPORT)

or maybe even rewrite the entire piece as:

#define dfx_use_mmio (!IS_ENABLED(CONFIG_HAS_IOPORT) || bp->mmio)

and all the port I/O stuff will be optimised away by the compiler.  The 
only part of the driver that actually cannot do without port I/O is EISA 
support, which uses the EISA slot port I/O space for BAR accesses even 
if the actual CSR block has been set up to be decoded in the MMIO space.

> >  Then what about the other FDDI driver there, SKFP?  It's not marked as
> > LEGACY_PCI, although it's not selectable anyway due to the dependency of 
> > FDDI on LEGACY_PCI.
> > 
> >  Niklas, what was the criterion for placing the LEGACY_PCI dependency?
> 
> Hmm, honestly I haven't really worked on this recently. There were some
> open questions from Bjorn towards Arnd and I was waiting for his reply
> but I guess he missed those. I think what you noticed was the main
> problem, there wasn't really a clear set of criteria for LEGACY_PCI and
> even for HAS_IOPORT we missed some uses if they were not compiled on
> s390's allyesconfig due to other dependencies.

 A dynamic boolean variable might be good having for platforms which may 
or may not have PCI port I/O available depending on the specific system 
model in addition to a compile-time constant of HAS_IOPORT.  I looked 
into it briefly in the context of the POWER9 system when I got it back 
in 2020, but figured out it wasn't straightforward enough and decided I 
could not afford the time for a proper investigation.

> >  Also do you plan to post an updated series anytime soon?  I'm asking 
> > because like with the m68k port also the MIPS one needs a more finegrained 
> > approach and I suspect there may be other corner cases and I'd rather look 
> > at the most recent version of your series.  Otherwise I'll have a look 
> > through your original submission, but it may have to wait until the next 
> > weekend due to my other commitments.
> 
> That sounds like you do see a need for something like HAS_IOPORT too,
> correct? Maybe with some input what you need and possibly stripping the
> LEGACY_PCI option it might make sense to do a new version. Rather than
> possibly getting in your way could directly work in your input.

 Yes, it does seem to me like a good direction, but will surely require 
some coordination from platform and driver maintainers, as it's not 
always easy for someone not familiar with a specific piece what the 
context is (such as with the defxx driver as I noted above).

  Maciej



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux