Re: [PATCH 1/3] resource: shared I/O region support

[Alan Cox <alan@lxorguk.ukuu.org.uk>]

On Thu, 25 Mar 2010 14:17:41 +0100
Giel van Schijndel <me@mortis.eu> wrote:

> From: Alan Cox <alan@lxorguk.ukuu.org.uk>
> 
> This patch was originally written by Alan Cox [1]. I only updated it to
> apply correctly to Linus' current tree and changed IORESOURCE_MUXED's
> value from 0x00200000 to 0x00400000 because the former has already been
> taken in use since.
> 
> [1] https://patchwork.kernel.org/patch/32397/
> 
> Patch after this line:
> ========================================================================
> SuperIO devices share regions and use lock/unlock operations to chip
> select.  We therefore need to be able to request a resource and wait for
> it to be freed by whichever other SuperIO device currently hogs it.
> Right now you have to poll which is horrible.
> 
> Add a MUXED field to IO port resources. If the MUXED field is set on the
> resource and on the request (via request_muxed_region) then we block
> until the previous owner of the muxed resource releases their region.
> 
> This allows us to implement proper resource sharing and locking for
> superio chips using code of the form
> 
> enable_my_superio_dev() {
> 	request_muxed_region(0x44, 0x02, "superio:watchdog");
> 	outb() ..sequence to enable chip
> }
> 
> disable_my_superio_dev() {
> 	outb() .. sequence of disable chip
> 	release_region(0x44, 0x02);
> }
> 
> Signed-off-by: Giel van Schijndel <me@mortis.eu>
> ---
>  include/linux/ioport.h |    4 +++-
>  kernel/resource.c      |   14 +++++++++++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 71ab79d..604fd29 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -52,6 +52,7 @@ struct resource_list {
>  
>  #define IORESOURCE_MEM_64	0x00100000
>  #define IORESOURCE_WINDOW	0x00200000	/* forwarded by bridge */
> +#define IORESOURCE_MUXED	0x00400000	/* Resource is software muxed */
>  
>  #define IORESOURCE_EXCLUSIVE	0x08000000	/* Userland may not map this resource */
>  #define IORESOURCE_DISABLED	0x10000000
> @@ -141,7 +142,8 @@ static inline unsigned long resource_type(const struct resource *res)
>  }
>  
>  /* Convenience shorthand with allocation */
> -#define request_region(start,n,name)	__request_region(&ioport_resource, (start), (n), (name), 0)
> +#define request_region(start,n,name)		__request_region(&ioport_resource, (start), (n), (name), 0)
> +#define request_muxed_region(start,n,name)	__request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
>  #define __request_mem_region(start,n,name, excl) __request_region(&iomem_resource, (start), (n), (name), excl)
>  #define request_mem_region(start,n,name) __request_region(&iomem_resource, (start), (n), (name), 0)
>  #define request_mem_region_exclusive(start,n,name) \
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 2d5be5d..1484180 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -651,6 +651,8 @@ resource_size_t resource_alignment(struct resource *res)
>   * release_region releases a matching busy region.
>   */
>  
> +static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
> +
>  /**
>   * __request_region - create a new busy resource region
>   * @parent: parent resource descriptor
> @@ -663,6 +665,7 @@ struct resource * __request_region(struct resource *parent,
>  				   resource_size_t start, resource_size_t n,
>  				   const char *name, int flags)
>  {
> +	DECLARE_WAITQUEUE(wait, current);
>  	struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
>  
>  	if (!res)
> @@ -687,7 +690,14 @@ struct resource * __request_region(struct resource *parent,
>  			if (!(conflict->flags & IORESOURCE_BUSY))
>  				continue;
>  		}
> -
> +		if (conflict->flags & flags & IORESOURCE_MUXED) {
> +			add_wait_queue(&muxed_resource_wait, &wait);
> +			write_unlock(&resource_lock);

needs a set_current_state(TASK_UNINTERRUPTIBLE); here

(error from my original test patch that I noticed re-reviewing it)