Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

From: Guenter Roeck
Date: Fri Apr 08 2016 - 09:18:16 EST


On 04/08/2016 05:31 AM, William Breathitt Gray wrote:
On Thu, Apr 07, 2016 at 05:45:03PM -0700, Guenter Roeck wrote:
This means for this and other similar drivers that the driver is no longer
supported on architectures which support ISA but not the newly introduced
ISA_BUS. Affected architectures are alpha, arm, m32r, m68k, mips, powerpc,
and parisc.

A typical example is SCSI_AHA1542, which is no longer supported on those
architectures. It builds, but isa_register_driver() will be a dummy and fail.
Actually, this is true for _all_ drivers calling isa_register_driver().

I hope this is understood and doesn't cause any problems.

Thanks,
Guenter

That's a good catch. I overlooked this when I submitted the ISA_BUS
patch; I had improperly assumed the ISA option to have a dependency on
X86_32 based on arch/x86/Kconfig. The intention of the ISA_BUS is to
allow the proper definition of the isa_register_driver and
isa_unregister_driver functions without the dependency on X86_32 (e.g.
on X86_64 systems). How can this be resolved without ending support for
ISA on these other architectures? Would it be appropriate to add the
ISA_BUS dependency to every "config ISA" block for the other
architectures?

From the context, arm and mips use "select ISA". For those, adding and
auto-selecting ISA_BUS would make sense. For the remaining architectures
you could simply add "config ISA_BUS". I would suggest to update default
configurations, though.

There is also "um", for which you effectively disabled ISA support
as far as I can see. You might want to look into that as well.

My avoidance of making ISA a selection of ISA_BUS is the possibility of
an invalid configuration: a user may initially enable ISA_BUS, then
later disable ISA, resulting in ISA_BUS remaining enabled without ISA
selected.

Does that even make sense ? Not sure I understand why you don't just
select ISA_BUS if ISA is selected. That would also be backward compatible
and avoid the problem I was concerned about.

As a side note, should the dummy isa_register_driver return 0? Would it
be more appropriate for it to return an error code to indicate lack of
support for ISA, rather than silently fail?

One should think so.

Thanks,
Guenter