All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
Cc: tglx@linutronix.de, jic23@kernel.org, knaack.h@gmx.de,
	lars@metafoo.de, pmeerw@pmeerw.net, wim@iguana.be,
	linux@roeck-us.net, linus.walleij@linaro.org, gnurou@gmail.com,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	linux-watchdog@vger.kernel.org, linux-gpio@vger.kernel.org
Subject: Re: [PATCH 03/10] Documentation: Add ISA bus driver documentation
Date: Sun, 1 May 2016 14:26:09 -0700	[thread overview]
Message-ID: <20160501212609.GB8337@kroah.com> (raw)
In-Reply-To: <e43db27011750537e86e8176bf532e20757133fd.1460040201.git.vilhelm.gray@gmail.com>

On Thu, Apr 07, 2016 at 10:47:24AM -0400, William Breathitt Gray wrote:
> This is a verbatim copy of the original commit message of the initial
> commit of the ISA bus driver authored by Rene Herman. Descriptions of
> the module_isa_driver macro and max_num_isa_dev macro are provided at
> the end.
> 
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
>  Documentation/isa.txt | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS           |   5 +++
>  2 files changed, 126 insertions(+)
>  create mode 100644 Documentation/isa.txt
> 
> diff --git a/Documentation/isa.txt b/Documentation/isa.txt
> new file mode 100644
> index 0000000..f232c26
> --- /dev/null
> +++ b/Documentation/isa.txt
> @@ -0,0 +1,121 @@
> +ISA Drivers
> +-----------
> +
> +The following text is adapted from the commit message of the initial
> +commit of the ISA bus driver authored by Rene Herman.
> +
> +During the recent "isa drivers using platform devices" discussion it was
> +pointed out that (ALSA) ISA drivers ran into the problem of not having
> +the option to fail driver load (device registration rather) upon not
> +finding their hardware due to a probe() error not being passed up
> +through the driver model. In the course of that, I suggested a separate
> +ISA bus might be best; Russell King agreed and suggested this bus could
> +use the .match() method for the actual device discovery.
> +
> +The attached does this. For this old non (generically) discoverable ISA
> +hardware only the driver itself can do discovery so as a difference with
> +the platform_bus, this isa_bus also distributes match() up to the
> +driver.
> +
> +As another difference: these devices only exist in the driver model due
> +to the driver creating them because it might want to drive them, meaning
> +that all device creation has been made internal as well.
> +
> +The usage model this provides is nice, and has been acked from the ALSA
> +side by Takashi Iwai and Jaroslav Kysela. The ALSA driver module_init's
> +now (for oldisa-only drivers) become:
> +
> +static int __init alsa_card_foo_init(void)
> +{
> +	return isa_register_driver(&snd_foo_isa_driver, SNDRV_CARDS);
> +}
> +
> +static void __exit alsa_card_foo_exit(void)
> +{
> +	isa_unregister_driver(&snd_foo_isa_driver);
> +}
> +
> +Quite like the other bus models therefore. This removes a lot of
> +duplicated init code from the ALSA ISA drivers.
> +
> +The passed in isa_driver struct is the regular driver struct embedding a
> +struct device_driver, the normal probe/remove/shutdown/suspend/resume
> +callbacks, and as indicated that .match callback.
> +
> +The "SNDRV_CARDS" you see being passed in is a "unsigned int ndev"
> +parameter, indicating how many devices to create and call our methods
> +with.
> +
> +The platform_driver callbacks are called with a platform_device param;
> +the isa_driver callbacks are being called with a "struct device *dev,
> +unsigned int id" pair directly -- with the device creation completely
> +internal to the bus it's much cleaner to not leak isa_dev's by passing
> +them in at all. The id is the only thing we ever want other then the
> +struct device * anyways, and it makes for nicer code in the callbacks as
> +well.
> +
> +With this additional .match() callback ISA drivers have all options. If
> +ALSA would want to keep the old non-load behaviour, it could stick all
> +of the old .probe in .match, which would only keep them registered after
> +everything was found to be present and accounted for. If it wanted the
> +behaviour of always loading as it inadvertently did for a bit after the
> +changeover to platform devices, it could just not provide a .match() and
> +do everything in .probe() as before.
> +
> +If it, as Takashi Iwai already suggested earlier as a way of following
> +the model from saner buses more closely, wants to load when a later bind
> +could conceivably succeed, it could use .match() for the prerequisites
> +(such as checking the user wants the card enabled and that port/irq/dma
> +values have been passed in) and .probe() for everything else. This is
> +the nicest model.
> +
> +To the code...
> +
> +This exports only two functions; isa_{,un}register_driver().
> +
> +isa_register_driver() register's the struct device_driver, and then
> +loops over the passed in ndev creating devices and registering them.
> +This causes the bus match method to be called for them, which is:
> +
> +int isa_bus_match(struct device *dev, struct device_driver *driver)
> +{
> +          struct isa_driver *isa_driver = to_isa_driver(driver);
> +
> +          if (dev->platform_data == isa_driver) {
> +                  if (!isa_driver->match ||
> +                          isa_driver->match(dev, to_isa_dev(dev)->id))
> +                          return 1;
> +                  dev->platform_data = NULL;
> +          }
> +          return 0;
> +}
> +
> +The first thing this does is check if this device is in fact one of this
> +driver's devices by seeing if the device's platform_data pointer is set
> +to this driver. Platform devices compare strings, but we don't need to
> +do that with everything being internal, so isa_register_driver() abuses
> +dev->platform_data as a isa_driver pointer which we can then check here.
> +I believe platform_data is available for this, but if rather not, moving
> +the isa_driver pointer to the private struct isa_dev is ofcourse fine as
> +well.
> +
> +Then, if the the driver did not provide a .match, it matches. If it did,
> +the driver match() method is called to determine a match.
> +
> +If it did _not_ match, dev->platform_data is reset to indicate this to
> +isa_register_driver which can then unregister the device again.
> +
> +If during all this, there's any error, or no devices matched at all
> +everything is backed out again and the error, or -ENODEV, is returned.
> +
> +isa_unregister_driver() just unregisters the matched devices and the
> +driver itself.
> +
> +module_isa_driver is a helper macro for ISA drivers which do not do
> +anything special in module init/exit. This eliminates a lot of
> +boilerplate code. Each module may only use this macro once, and calling
> +it replaces module_init and module_exit.
> +
> +max_num_isa_dev is a macro to determine the maximum possible number of
> +ISA devices which may be registered in the I/O port address space given
> +the address extent of the ISA devices.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 03e00c7..3713010 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5994,6 +5994,11 @@ F:	include/linux/irqdomain.h
>  F:	kernel/irq/irqdomain.c
>  F:	kernel/irq/msi.c
>  
> +ISA
> +M:	William Breathitt Gray <vilhelm.gray@gmail.com>
> +S:	Maintained
> +F:	Documentation/isa.txt

You are just in charge of one documentation file?  That seems odd for a
whole subsystem :(

Why not list all of the needed files?

thanks,

greg k-h

  reply	other threads:[~2016-05-01 21:26 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-07 14:47 [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices William Breathitt Gray
2016-04-07 14:47 ` William Breathitt Gray
2016-04-07 14:47 ` [PATCH 01/10] isa: Implement the module_isa_driver macro William Breathitt Gray
2016-04-07 14:47 ` [PATCH 02/10] isa: Implement the max_num_isa_dev macro William Breathitt Gray
2016-04-07 14:47 ` [PATCH 03/10] Documentation: Add ISA bus driver documentation William Breathitt Gray
2016-05-01 21:26   ` Greg KH [this message]
2016-04-07 14:47 ` [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray
     [not found]   ` <783be62acf68b35f3fe4785a2cedfe017624688b.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-04-08  0:45     ` Guenter Roeck
2016-04-08  0:45       ` Guenter Roeck
     [not found]       ` <20160408004503.GB10211-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2016-04-08 12:31         ` William Breathitt Gray
2016-04-08 12:31           ` William Breathitt Gray
2016-04-08 13:18           ` Guenter Roeck
2016-04-08 13:18             ` Guenter Roeck
     [not found]             ` <5707AF91.5010704-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2016-04-08 15:09               ` William Breathitt Gray
2016-04-08 15:09                 ` William Breathitt Gray
2016-04-08 18:28                 ` Guenter Roeck
     [not found]                   ` <20160408182801.GB7083-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2016-04-08 19:27                     ` William Breathitt Gray
2016-04-08 19:27                       ` William Breathitt Gray
2016-04-09 12:58                       ` One Thousand Gnomes
     [not found]                         ` <20160409135814.359e24d6-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2016-04-09 13:50                           ` William Breathitt Gray
2016-04-09 13:50                             ` William Breathitt Gray
2016-04-09 15:51                             ` One Thousand Gnomes
2016-04-09 15:51                               ` One Thousand Gnomes
2016-04-07 14:47 ` [PATCH 05/10] iio: stx104: Utilize the module_isa_driver and max_num_isa_dev macros William Breathitt Gray
2016-04-07 14:47 ` [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver William Breathitt Gray
     [not found]   ` <1f5bf2e21006f0fd4f10ab3948cf69a737c0b039.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-04-08  0:35     ` Guenter Roeck
2016-04-08  0:35       ` Guenter Roeck
2016-04-08 12:03       ` William Breathitt Gray
2016-05-11 17:04     ` Sasha Levin
2016-05-11 17:04       ` Sasha Levin
     [not found]       ` <57336622.9070508-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2016-05-11 19:34         ` William Breathitt Gray
2016-05-11 19:34           ` William Breathitt Gray
2016-04-07 14:47 ` [PATCH 07/10] gpio: 104-dio-48e: " William Breathitt Gray
2016-04-07 14:47 ` [PATCH 08/10] gpio: 104-idi-48: " William Breathitt Gray
2016-04-07 14:47 ` [PATCH 09/10] gpio: 104-idio-16: " William Breathitt Gray
2016-04-07 14:47 ` [PATCH 10/10] gpio: ws16c48: " William Breathitt Gray
2016-04-11  6:59 ` [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices Linus Walleij
     [not found] ` <cover.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-05-01 21:26   ` Greg KH
2016-05-01 21:26     ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160501212609.GB8337@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=gnurou@gmail.com \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=pmeerw@pmeerw.net \
    --cc=tglx@linutronix.de \
    --cc=vilhelm.gray@gmail.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.