Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v5 2/4] resource: Use list_head to link sibling resource
From: kbuild test robot @ 2018-06-12  4:37 UTC (permalink / raw)
  Cc: nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, brijesh.singh-5C7GfCeVMHo,
	thomas.lendacky-5C7GfCeVMHo, airlied-cv59FeDIM0c,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w, tglx-hfZtesqFncYOwBW4kG4KsQ,
	lorenzo.pieralisi-5wv7dgnIgG8, sthemmin-0li6OtcxBFHby3iVrkZq2A,
	Baoquan He, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, bp-l3A5Bk7waGM,
	dyoung-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	josh-iaAMLnmF4UmaiuxdJuQwMA, jglisse-H+wXaHxf7aLQT0dZR+AlfA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWMP3drIcvDWNA
In-Reply-To: <20180612032831.29747-3-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Hi Baoquan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/resource.c: In function 'reparent_resources':
>> kernel/resource.c:1005:26: error: passing argument 2 of 'list_add' from incompatible pointer type [-Werror=incompatible-pointer-types]
     list_add(&res->sibling, &p->sibling.prev);
                             ^
   In file included from include/linux/ioport.h:15:0,
                    from kernel/resource.c:14:
   include/linux/list.h:77:20: note: expected 'struct list_head *' but argument is of type 'struct list_head **'
    static inline void list_add(struct list_head *new, struct list_head *head)
                       ^~~~~~~~
   In file included from include/linux/list.h:9:0,
                    from include/linux/ioport.h:15,
                    from kernel/resource.c:14:
>> kernel/resource.c:1013:26: error: 'new' undeclared (first use in this function); did you mean 'net'?
     list_for_each_entry(p, &new->child, sibling) {
                             ^
   include/linux/kernel.h:963:26: note: in definition of macro 'container_of'
     void *__mptr = (void *)(ptr);     \
                             ^~~
   include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
     list_entry((ptr)->next, type, member)
     ^~~~~~~~~~
   include/linux/list.h:464:13: note: in expansion of macro 'list_first_entry'
     for (pos = list_first_entry(head, typeof(*pos), member); \
                ^~~~~~~~~~~~~~~~
   kernel/resource.c:1013:2: note: in expansion of macro 'list_for_each_entry'
     list_for_each_entry(p, &new->child, sibling) {
     ^~~~~~~~~~~~~~~~~~~
   kernel/resource.c:1013:26: note: each undeclared identifier is reported only once for each function it appears in
     list_for_each_entry(p, &new->child, sibling) {
                             ^
   include/linux/kernel.h:963:26: note: in definition of macro 'container_of'
     void *__mptr = (void *)(ptr);     \
                             ^~~
   include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
     list_entry((ptr)->next, type, member)
     ^~~~~~~~~~
   include/linux/list.h:464:13: note: in expansion of macro 'list_first_entry'
     for (pos = list_first_entry(head, typeof(*pos), member); \
                ^~~~~~~~~~~~~~~~
   kernel/resource.c:1013:2: note: in expansion of macro 'list_for_each_entry'
     list_for_each_entry(p, &new->child, sibling) {
     ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/list_add +1005 kernel/resource.c

   983	
   984	/*
   985	 * Reparent resource children of pr that conflict with res
   986	 * under res, and make res replace those children.
   987	 */
   988	int reparent_resources(struct resource *parent, struct resource *res)
   989	{
   990		struct resource *p, *first = NULL;
   991	
   992		list_for_each_entry(p, &parent->child, sibling) {
   993			if (p->end < res->start)
   994				continue;
   995			if (res->end < p->start)
   996				break;
   997			if (p->start < res->start || p->end > res->end)
   998				return -1;	/* not completely contained */
   999			if (first == NULL)
  1000				first = p;
  1001		}
  1002		if (first == NULL)
  1003			return -1;	/* didn't find any conflicting entries? */
  1004		res->parent = parent;
> 1005		list_add(&res->sibling, &p->sibling.prev);
  1006		INIT_LIST_HEAD(&res->child);
  1007	
  1008		/*
  1009		 * From first to p's previous sibling, they all fall into
  1010		 * res's region, change them as res's children.
  1011		 */
  1012		list_cut_position(&res->child, first->sibling.prev, res->sibling.prev);
> 1013		list_for_each_entry(p, &new->child, sibling) {
  1014	                p->parent = new;
  1015			pr_debug("PCI: Reparented %s %pR under %s\n",
  1016				 p->name, p, res->name);
  1017		}
  1018		return 0;
  1019	}
  1020	EXPORT_SYMBOL(reparent_resources);
  1021	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: kbuild test robot @ 2018-06-12  3:55 UTC (permalink / raw)
  To: Baoquan He
  Cc: kbuild-all, linux-kernel, akpm, robh+dt, dan.j.williams,
	nicolas.pitre, josh, fengguang.wu, bp, patrik.r.jakobsson,
	airlied, kys, haiyangz, sthemmin, dmitry.torokhov, frowand.list,
	keith.busch, jonathan.derrick, lorenzo.pieralisi, bhelgaas, tglx,
	brijesh.singh, jglisse, thomas.lendacky, gregkh, baiyaowei,
	richard.weiyang, devel, linux-input, linux-nvdimm, devicetree,
	linux-pci
In-Reply-To: <20180612032831.29747-2-bhe@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

Hi Baoquan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600 HEAD 5545e79eef6387857faf41cdffa7be6b1f5d4efe builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> kernel/resource.c:990:12: error: static declaration of 'reparent_resources' follows non-static declaration
    static int reparent_resources(struct resource *parent,
               ^~~~~~~~~~~~~~~~~~
   In file included from kernel/resource.c:14:0:
   include/linux/ioport.h:195:5: note: previous declaration of 'reparent_resources' was here
    int reparent_resources(struct resource *parent, struct resource *res);
        ^~~~~~~~~~~~~~~~~~
   kernel/resource.c:990:12: warning: 'reparent_resources' defined but not used [-Wunused-function]
    static int reparent_resources(struct resource *parent,
               ^~~~~~~~~~~~~~~~~~

vim +/reparent_resources +990 kernel/resource.c

   985	
   986	/*
   987	 * Reparent resource children of pr that conflict with res
   988	 * under res, and make res replace those children.
   989	 */
 > 990	static int reparent_resources(struct resource *parent,
   991					     struct resource *res)
   992	{
   993		struct resource *p, **pp;
   994		struct resource **firstpp = NULL;
   995	
   996		for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
   997			if (p->end < res->start)
   998				continue;
   999			if (res->end < p->start)
  1000				break;
  1001			if (p->start < res->start || p->end > res->end)
  1002				return -1;	/* not completely contained */
  1003			if (firstpp == NULL)
  1004				firstpp = pp;
  1005		}
  1006		if (firstpp == NULL)
  1007			return -1;	/* didn't find any conflicting entries? */
  1008		res->parent = parent;
  1009		res->child = *firstpp;
  1010		res->sibling = *pp;
  1011		*firstpp = res;
  1012		*pp = NULL;
  1013		for (p = res->child; p != NULL; p = p->sibling) {
  1014			p->parent = res;
  1015			pr_debug("PCI: Reparented %s %pR under %s\n",
  1016				 p->name, p, res->name);
  1017		}
  1018		return 0;
  1019	}
  1020	EXPORT_SYMBOL(reparent_resources);
  1021	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6347 bytes --]

^ permalink raw reply

* Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: Baoquan He @ 2018-06-12  3:34 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, josh-iaAMLnmF4UmaiuxdJuQwMA,
	fengguang.wu-ral2JQCrhuEAvxtiuMwx3w, bp-l3A5Bk7waGM
  Cc: brijesh.singh-5C7GfCeVMHo, devicetree-u79uwXL29TY76Z2rM5mHXA,
	airlied-cv59FeDIM0c, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w, Paul Mackerras,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	lorenzo.pieralisi-5wv7dgnIgG8, sthemmin-0li6OtcxBFHby3iVrkZq2A,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Michael Ellerman,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, dyoung-H+wXaHxf7aLQT0dZR+AlfA,
	thomas.lendacky-5C7GfCeVMHo, haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	jglisse-H+wXaHxf7aLQT0dZR+AlfA, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Benjamin Herrenschmidt <ben>
In-Reply-To: <20180612032831.29747-2-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On 06/12/18 at 11:28am, Baoquan He wrote:
> reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> so that it's shared. Later its code also need be updated using list_head
> to replace singly linked list.
> 
> Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
> Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> Cc: Paul Mackerras <paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> Cc: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
> ---
> v4->v5:
>   Fix several code bugs reported by test robot on ARCH powerpc and
>   microblaze.

Oops, I mistakenly added the patch change log of the current patch 0002
here. This patch is a newly added one.

> 
> v3->v4:
>   Fix several bugs test robot reported. And change patch log.
> 
> v2->v3:
>   Rename resource functions first_child() and sibling() to
>   resource_first_chils() and resource_sibling(). Dan suggested this.
> 
>   Move resource_first_chils() and resource_sibling() to linux/ioport.h
>   and make them as inline function. Rob suggested this. Accordingly add
>   linux/list.h including in linux/ioport.h, please help review if this
>   bring efficiency degradation or code redundancy.
> 
>   The change on struct resource {} bring two pointers of size increase,
>   mention this in git log to make it more specifically, Rob suggested
>   this.
> 
>  arch/microblaze/pci/pci-common.c | 37 -------------------------------------
>  arch/powerpc/kernel/pci-common.c | 35 -----------------------------------
>  include/linux/ioport.h           |  1 +
>  kernel/resource.c                | 36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 37 insertions(+), 72 deletions(-)
> 
> diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> index f34346d56095..7899bafab064 100644
> --- a/arch/microblaze/pci/pci-common.c
> +++ b/arch/microblaze/pci/pci-common.c
> @@ -619,43 +619,6 @@ int pcibios_add_device(struct pci_dev *dev)
>  EXPORT_SYMBOL(pcibios_add_device);
>  
>  /*
> - * Reparent resource children of pr that conflict with res
> - * under res, and make res replace those children.
> - */
> -static int __init reparent_resources(struct resource *parent,
> -				     struct resource *res)
> -{
> -	struct resource *p, **pp;
> -	struct resource **firstpp = NULL;
> -
> -	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> -		if (p->end < res->start)
> -			continue;
> -		if (res->end < p->start)
> -			break;
> -		if (p->start < res->start || p->end > res->end)
> -			return -1;	/* not completely contained */
> -		if (firstpp == NULL)
> -			firstpp = pp;
> -	}
> -	if (firstpp == NULL)
> -		return -1;	/* didn't find any conflicting entries? */
> -	res->parent = parent;
> -	res->child = *firstpp;
> -	res->sibling = *pp;
> -	*firstpp = res;
> -	*pp = NULL;
> -	for (p = res->child; p != NULL; p = p->sibling) {
> -		p->parent = res;
> -		pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
> -			 p->name,
> -			 (unsigned long long)p->start,
> -			 (unsigned long long)p->end, res->name);
> -	}
> -	return 0;
> -}
> -
> -/*
>   *  Handle resources of PCI devices.  If the world were perfect, we could
>   *  just allocate all the resource regions and do nothing more.  It isn't.
>   *  On the other hand, we cannot just re-allocate all devices, as it would
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index fe9733ffffaa..926035bb378d 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1088,41 +1088,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  EXPORT_SYMBOL(pcibios_align_resource);
>  
>  /*
> - * Reparent resource children of pr that conflict with res
> - * under res, and make res replace those children.
> - */
> -static int reparent_resources(struct resource *parent,
> -				     struct resource *res)
> -{
> -	struct resource *p, **pp;
> -	struct resource **firstpp = NULL;
> -
> -	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> -		if (p->end < res->start)
> -			continue;
> -		if (res->end < p->start)
> -			break;
> -		if (p->start < res->start || p->end > res->end)
> -			return -1;	/* not completely contained */
> -		if (firstpp == NULL)
> -			firstpp = pp;
> -	}
> -	if (firstpp == NULL)
> -		return -1;	/* didn't find any conflicting entries? */
> -	res->parent = parent;
> -	res->child = *firstpp;
> -	res->sibling = *pp;
> -	*firstpp = res;
> -	*pp = NULL;
> -	for (p = res->child; p != NULL; p = p->sibling) {
> -		p->parent = res;
> -		pr_debug("PCI: Reparented %s %pR under %s\n",
> -			 p->name, p, res->name);
> -	}
> -	return 0;
> -}
> -
> -/*
>   *  Handle resources of PCI devices.  If the world were perfect, we could
>   *  just allocate all the resource regions and do nothing more.  It isn't.
>   *  On the other hand, we cannot just re-allocate all devices, as it would
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index da0ebaec25f0..dfdcd0bfe54e 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -192,6 +192,7 @@ extern int allocate_resource(struct resource *root, struct resource *new,
>  struct resource *lookup_resource(struct resource *root, resource_size_t start);
>  int adjust_resource(struct resource *res, resource_size_t start,
>  		    resource_size_t size);
> +int reparent_resources(struct resource *parent, struct resource *res);
>  resource_size_t resource_alignment(struct resource *res);
>  static inline resource_size_t resource_size(const struct resource *res)
>  {
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 30e1bc68503b..5e7c56d5d838 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -983,6 +983,42 @@ int adjust_resource(struct resource *res, resource_size_t start,
>  }
>  EXPORT_SYMBOL(adjust_resource);
>  
> +/*
> + * Reparent resource children of pr that conflict with res
> + * under res, and make res replace those children.
> + */
> +static int reparent_resources(struct resource *parent,
> +				     struct resource *res)
> +{
> +	struct resource *p, **pp;
> +	struct resource **firstpp = NULL;
> +
> +	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> +		if (p->end < res->start)
> +			continue;
> +		if (res->end < p->start)
> +			break;
> +		if (p->start < res->start || p->end > res->end)
> +			return -1;	/* not completely contained */
> +		if (firstpp == NULL)
> +			firstpp = pp;
> +	}
> +	if (firstpp == NULL)
> +		return -1;	/* didn't find any conflicting entries? */
> +	res->parent = parent;
> +	res->child = *firstpp;
> +	res->sibling = *pp;
> +	*firstpp = res;
> +	*pp = NULL;
> +	for (p = res->child; p != NULL; p = p->sibling) {
> +		p->parent = res;
> +		pr_debug("PCI: Reparented %s %pR under %s\n",
> +			 p->name, p, res->name);
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(reparent_resources);
> +
>  static void __init __reserve_region_with_split(struct resource *root,
>  		resource_size_t start, resource_size_t end,
>  		const char *name)
> -- 
> 2.13.6
> 

^ permalink raw reply

* [PATCH v5 4/4] kexec_file: Load kernel at top of system RAM if required
From: Baoquan He @ 2018-06-12  3:28 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, josh-iaAMLnmF4UmaiuxdJuQwMA,
	fengguang.wu-ral2JQCrhuEAvxtiuMwx3w, bp-l3A5Bk7waGM
  Cc: brijesh.singh-5C7GfCeVMHo, devicetree-u79uwXL29TY76Z2rM5mHXA,
	airlied-cv59FeDIM0c, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	lorenzo.pieralisi-5wv7dgnIgG8, sthemmin-0li6OtcxBFHby3iVrkZq2A,
	Baoquan He, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, dyoung-H+wXaHxf7aLQT0dZR+AlfA,
	thomas.lendacky-5C7GfCeVMHo, haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	jglisse-H+wXaHxf7aLQT0dZR+AlfA, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X, linuxppc-dev
In-Reply-To: <20180612032831.29747-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

For kexec_file loading, if kexec_buf.top_down is 'true', the memory which
is used to load kernel/initrd/purgatory is supposed to be allocated from
top to down. This is what we have been doing all along in the old kexec
loading interface and the kexec loading is still default setting in some
distributions. However, the current kexec_file loading interface doesn't
do likt this. The function arch_kexec_walk_mem() it calls ignores checking
kexec_buf.top_down, but calls walk_system_ram_res() directly to go through
all resources of System RAM from bottom to up, to try to find memory region
which can contain the specific kexec buffer, then call locate_mem_hole_callback()
to allocate memory in that found memory region from top to down. This brings
confusion especially when KASLR is widely supported , users have to make clear
why kexec/kdump kernel loading position is different between these two
interfaces in order to exclude unnecessary noises. Hence these two interfaces
need be unified on behaviour.

Here add checking if kexec_buf.top_down is 'true' in arch_kexec_walk_mem(),
if yes, call the newly added walk_system_ram_res_rev() to find memory region
from top to down to load kernel.

Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Eric Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: Vivek Goyal <vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: Yinghai Lu <yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
---
 kernel/kexec_file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 75d8e7cf040e..7a66d9d5a534 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -518,6 +518,8 @@ int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
 					   IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
 					   crashk_res.start, crashk_res.end,
 					   kbuf, func);
+	else if (kbuf->top_down)
+		return walk_system_ram_res_rev(0, ULONG_MAX, kbuf, func);
 	else
 		return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
 }
-- 
2.13.6

^ permalink raw reply related

* [PATCH v5 3/4] resource: add walk_system_ram_res_rev()
From: Baoquan He @ 2018-06-12  3:28 UTC (permalink / raw)
  To: linux-kernel, akpm, robh+dt, dan.j.williams, nicolas.pitre, josh,
	fengguang.wu, bp
  Cc: patrik.r.jakobsson, airlied, kys, haiyangz, sthemmin,
	dmitry.torokhov, frowand.list, keith.busch, jonathan.derrick,
	lorenzo.pieralisi, bhelgaas, tglx, brijesh.singh, jglisse,
	thomas.lendacky, gregkh, baiyaowei, richard.weiyang, devel,
	linux-input, linux-nvdimm, devicetree, linux-pci, ebiederm,
	vgoyal, dyoung, yinghai, kexec, monstr, davem, chris, jcmvbkbc,
	gustavo, maart
In-Reply-To: <20180612032831.29747-1-bhe@redhat.com>

This function, being a variant of walk_system_ram_res() introduced in
commit 8c86e70acead ("resource: provide new functions to walk through
resources"), walks through a list of all the resources of System RAM
in reversed order, i.e., from higher to lower.

It will be used in kexec_file code.

Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
---
 include/linux/ioport.h |  3 +++
 kernel/resource.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index b7456ae889dd..066cc263e2cc 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -279,6 +279,9 @@ extern int
 walk_system_ram_res(u64 start, u64 end, void *arg,
 		    int (*func)(struct resource *, void *));
 extern int
+walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+			int (*func)(struct resource *, void *));
+extern int
 walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
 		    void *arg, int (*func)(struct resource *, void *));
 
diff --git a/kernel/resource.c b/kernel/resource.c
index ef9a20b75234..3128ac938f38 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -23,6 +23,8 @@
 #include <linux/pfn.h>
 #include <linux/mm.h>
 #include <linux/resource_ext.h>
+#include <linux/string.h>
+#include <linux/vmalloc.h>
 #include <asm/io.h>
 
 
@@ -443,6 +445,44 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
 }
 
 /*
+ * This function, being a variant of walk_system_ram_res(), calls the @func
+ * callback against all memory ranges of type System RAM which are marked as
+ * IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY in reversed order, i.e., from
+ * higher to lower.
+ */
+int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
+				int (*func)(struct resource *, void *))
+{
+	unsigned long flags;
+	struct resource *res;
+	int ret = -1;
+
+	flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+
+	read_lock(&resource_lock);
+	list_for_each_entry_reverse(res, &iomem_resource.child, sibling) {
+		if (start >= end)
+			break;
+		if ((res->flags & flags) != flags)
+			continue;
+		if (res->desc != IORES_DESC_NONE)
+			continue;
+		if (res->end < start)
+			break;
+
+		if ((res->end >= start) && (res->start < end)) {
+			ret = (*func)(res, arg);
+			if (ret)
+				break;
+		}
+		end = res->start - 1;
+
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+
+/*
  * This function calls the @func callback against all memory ranges, which
  * are ranges marked as IORESOURCE_MEM and IORESOUCE_BUSY.
  */
-- 
2.13.6

^ permalink raw reply related

* [PATCH v5 2/4] resource: Use list_head to link sibling resource
From: Baoquan He @ 2018-06-12  3:28 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, josh-iaAMLnmF4UmaiuxdJuQwMA,
	fengguang.wu-ral2JQCrhuEAvxtiuMwx3w, bp-l3A5Bk7waGM
  Cc: brijesh.singh-5C7GfCeVMHo, devicetree-u79uwXL29TY76Z2rM5mHXA,
	airlied-cv59FeDIM0c, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	lorenzo.pieralisi-5wv7dgnIgG8, sthemmin-0li6OtcxBFHby3iVrkZq2A,
	Baoquan He, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, dyoung-H+wXaHxf7aLQT0dZR+AlfA,
	thomas.lendacky-5C7GfCeVMHo, haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	jglisse-H+wXaHxf7aLQT0dZR+AlfA, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X, linuxppc-dev
In-Reply-To: <20180612032831.29747-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

The struct resource uses singly linked list to link siblings, implemented
by pointer operation. Replace it with list_head for better code readability.

Based on this list_head replacement, it will be very easy to do reverse
iteration on iomem_resource's sibling list in later patch.

Besides, type of member variables of struct resource, sibling and child, are
changed from 'struct resource *' to 'struct list_head'. This brings two
pointers of size increase.

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: David Airlie <airlied@linux.ie>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Jonathan Derrick <jonathan.derrick@intel.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: devel@linuxdriverproject.org
Cc: linux-input@vger.kernel.org
Cc: linux-nvdimm@lists.01.org
Cc: devicetree@vger.kernel.org
Cc: linux-pci@vger.kernel.org
---
 arch/arm/plat-samsung/pm-check.c            |   6 +-
 arch/microblaze/pci/pci-common.c            |   4 +-
 arch/powerpc/kernel/pci-common.c            |   4 +-
 arch/sparc/kernel/ioport.c                  |   2 +-
 arch/xtensa/include/asm/pci-bridge.h        |   4 +-
 drivers/eisa/eisa-bus.c                     |   2 +
 drivers/gpu/drm/drm_memory.c                |   3 +-
 drivers/gpu/drm/gma500/gtt.c                |   5 +-
 drivers/hv/vmbus_drv.c                      |  52 +++----
 drivers/input/joystick/iforce/iforce-main.c |   4 +-
 drivers/nvdimm/namespace_devs.c             |   6 +-
 drivers/nvdimm/nd.h                         |   5 +-
 drivers/of/address.c                        |   4 +-
 drivers/parisc/lba_pci.c                    |   4 +-
 drivers/pci/host/vmd.c                      |   8 +-
 drivers/pci/probe.c                         |   2 +
 drivers/pci/setup-bus.c                     |   2 +-
 include/linux/ioport.h                      |  17 ++-
 kernel/resource.c                           | 211 ++++++++++++++--------------
 19 files changed, 176 insertions(+), 169 deletions(-)

diff --git a/arch/arm/plat-samsung/pm-check.c b/arch/arm/plat-samsung/pm-check.c
index cd2c02c68bc3..5494355b1c49 100644
--- a/arch/arm/plat-samsung/pm-check.c
+++ b/arch/arm/plat-samsung/pm-check.c
@@ -46,8 +46,8 @@ typedef u32 *(run_fn_t)(struct resource *ptr, u32 *arg);
 static void s3c_pm_run_res(struct resource *ptr, run_fn_t fn, u32 *arg)
 {
 	while (ptr != NULL) {
-		if (ptr->child != NULL)
-			s3c_pm_run_res(ptr->child, fn, arg);
+		if (!list_empty(&ptr->child))
+			s3c_pm_run_res(resource_first_child(&ptr->child), fn, arg);
 
 		if ((ptr->flags & IORESOURCE_SYSTEM_RAM)
 				== IORESOURCE_SYSTEM_RAM) {
@@ -57,7 +57,7 @@ static void s3c_pm_run_res(struct resource *ptr, run_fn_t fn, u32 *arg)
 			arg = (fn)(ptr, arg);
 		}
 
-		ptr = ptr->sibling;
+		ptr = resource_sibling(ptr);
 	}
 }
 
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 7899bafab064..2bf73e27e231 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -533,7 +533,9 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 			res->flags = range.flags;
 			res->start = range.cpu_addr;
 			res->end = range.cpu_addr + range.size - 1;
-			res->parent = res->child = res->sibling = NULL;
+			res->parent = NULL;
+			INIT_LIST_HEAD(&res->child);
+			INIT_LIST_HEAD(&res->sibling);
 		}
 	}
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 926035bb378d..28fbe83c9daf 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -761,7 +761,9 @@ void pci_process_bridge_OF_ranges(struct pci_controller *hose,
 			res->flags = range.flags;
 			res->start = range.cpu_addr;
 			res->end = range.cpu_addr + range.size - 1;
-			res->parent = res->child = res->sibling = NULL;
+			res->parent = NULL;
+			INIT_LIST_HEAD(&res->child);
+			INIT_LIST_HEAD(&res->sibling);
 		}
 	}
 }
diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
index cca9134cfa7d..99efe4e98b16 100644
--- a/arch/sparc/kernel/ioport.c
+++ b/arch/sparc/kernel/ioport.c
@@ -669,7 +669,7 @@ static int sparc_io_proc_show(struct seq_file *m, void *v)
 	struct resource *root = m->private, *r;
 	const char *nm;
 
-	for (r = root->child; r != NULL; r = r->sibling) {
+	list_for_each_entry(r, &root->child, sibling) {
 		if ((nm = r->name) == NULL) nm = "???";
 		seq_printf(m, "%016llx-%016llx: %s\n",
 				(unsigned long long)r->start,
diff --git a/arch/xtensa/include/asm/pci-bridge.h b/arch/xtensa/include/asm/pci-bridge.h
index 0b68c76ec1e6..f487b06817df 100644
--- a/arch/xtensa/include/asm/pci-bridge.h
+++ b/arch/xtensa/include/asm/pci-bridge.h
@@ -71,8 +71,8 @@ static inline void pcibios_init_resource(struct resource *res,
 	res->flags = flags;
 	res->name = name;
 	res->parent = NULL;
-	res->sibling = NULL;
-	res->child = NULL;
+	INIT_LIST_HEAD(&res->child);
+	INIT_LIST_HEAD(&res->sibling);
 }
 
 
diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
index 1e8062f6dbfc..dba78f75fd06 100644
--- a/drivers/eisa/eisa-bus.c
+++ b/drivers/eisa/eisa-bus.c
@@ -408,6 +408,8 @@ static struct resource eisa_root_res = {
 	.start = 0,
 	.end   = 0xffffffff,
 	.flags = IORESOURCE_IO,
+	.sibling = LIST_HEAD_INIT(eisa_root_res.sibling),
+	.child  = LIST_HEAD_INIT(eisa_root_res.child),
 };
 
 static int eisa_bus_count;
diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 3c54044214db..53e300a993dc 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -155,9 +155,8 @@ u64 drm_get_max_iomem(void)
 	struct resource *tmp;
 	resource_size_t max_iomem = 0;
 
-	for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) {
+	list_for_each_entry(tmp, &iomem_resource.child, sibling)
 		max_iomem = max(max_iomem,  tmp->end);
-	}
 
 	return max_iomem;
 }
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 3949b0990916..addd3bc009af 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -565,7 +565,7 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 int psb_gtt_restore(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = dev->dev_private;
-	struct resource *r = dev_priv->gtt_mem->child;
+	struct resource *r;
 	struct gtt_range *range;
 	unsigned int restored = 0, total = 0, size = 0;
 
@@ -573,14 +573,13 @@ int psb_gtt_restore(struct drm_device *dev)
 	mutex_lock(&dev_priv->gtt_mutex);
 	psb_gtt_init(dev, 1);
 
-	while (r != NULL) {
+	list_for_each_entry(r, &dev_priv->gtt_mem->child, sibling) {
 		range = container_of(r, struct gtt_range, resource);
 		if (range->pages) {
 			psb_gtt_insert(dev, range, 1);
 			size += range->resource.end - range->resource.start;
 			restored++;
 		}
-		r = r->sibling;
 		total++;
 	}
 	mutex_unlock(&dev_priv->gtt_mutex);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b10fe26c4891..d87ec5a1bc4c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1412,9 +1412,8 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
 {
 	resource_size_t start = 0;
 	resource_size_t end = 0;
-	struct resource *new_res;
+	struct resource *new_res, *tmp;
 	struct resource **old_res = &hyperv_mmio;
-	struct resource **prev_res = NULL;
 
 	switch (res->type) {
 
@@ -1461,44 +1460,36 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
 	/*
 	 * If two ranges are adjacent, merge them.
 	 */
-	do {
-		if (!*old_res) {
-			*old_res = new_res;
-			break;
-		}
-
-		if (((*old_res)->end + 1) == new_res->start) {
-			(*old_res)->end = new_res->end;
+	if (!*old_res) {
+		*old_res = new_res;
+		return AE_OK;
+	}
+	tmp = *old_res;
+	list_for_each_entry_from(tmp, &tmp->parent->child, sibling) {
+		if ((tmp->end + 1) == new_res->start) {
+			tmp->end = new_res->end;
 			kfree(new_res);
 			break;
 		}
 
-		if ((*old_res)->start == new_res->end + 1) {
-			(*old_res)->start = new_res->start;
+		if (tmp->start == new_res->end + 1) {
+			tmp->start = new_res->start;
 			kfree(new_res);
 			break;
 		}
 
-		if ((*old_res)->start > new_res->end) {
-			new_res->sibling = *old_res;
-			if (prev_res)
-				(*prev_res)->sibling = new_res;
-			*old_res = new_res;
+		if (tmp->start > new_res->end) {
+			list_add(&new_res->sibling, tmp->sibling.prev);
 			break;
 		}
-
-		prev_res = old_res;
-		old_res = &(*old_res)->sibling;
-
-	} while (1);
+	}
 
 	return AE_OK;
 }
 
 static int vmbus_acpi_remove(struct acpi_device *device)
 {
-	struct resource *cur_res;
-	struct resource *next_res;
+	struct resource *res;
 
 	if (hyperv_mmio) {
 		if (fb_mmio) {
@@ -1507,10 +1498,9 @@ static int vmbus_acpi_remove(struct acpi_device *device)
 			fb_mmio = NULL;
 		}
 
-		for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) {
-			next_res = cur_res->sibling;
-			kfree(cur_res);
-		}
+		res = hyperv_mmio;
+		list_for_each_entry_from(res, &res->parent->child, sibling)
+			kfree(res);
 	}
 
 	return 0;
@@ -1596,7 +1586,8 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 		}
 	}
 
-	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+	iter = hyperv_mmio;
+	list_for_each_entry_from(iter, &iter->parent->child, sibling) {
 		if ((iter->start >= max) || (iter->end <= min))
 			continue;
 
@@ -1639,7 +1630,8 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size)
 	struct resource *iter;
 
 	down(&hyperv_mmio_lock);
-	for (iter = hyperv_mmio; iter; iter = iter->sibling) {
+	iter = hyperv_mmio;
+	list_for_each_entry_from(iter, &iter->parent->child, sibling) {
 		if ((iter->start >= start + size) || (iter->end <= start))
 			continue;
 
diff --git a/drivers/input/joystick/iforce/iforce-main.c b/drivers/input/joystick/iforce/iforce-main.c
index daeeb4c7e3b0..5c0be27b33ff 100644
--- a/drivers/input/joystick/iforce/iforce-main.c
+++ b/drivers/input/joystick/iforce/iforce-main.c
@@ -305,8 +305,8 @@ int iforce_init_device(struct iforce *iforce)
 	iforce->device_memory.end = 200;
 	iforce->device_memory.flags = IORESOURCE_MEM;
 	iforce->device_memory.parent = NULL;
-	iforce->device_memory.child = NULL;
-	iforce->device_memory.sibling = NULL;
+	INIT_LIST_HEAD(&iforce->device_memory.child);
+	INIT_LIST_HEAD(&iforce->device_memory.sibling);
 
 /*
  * Wait until device ready - until it sends its first response.
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 28afdd668905..f53d410d9981 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -637,7 +637,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
  retry:
 	first = 0;
 	for_each_dpa_resource(ndd, res) {
-		struct resource *next = res->sibling, *new_res = NULL;
+		struct resource *next = resource_sibling(res), *new_res = NULL;
 		resource_size_t allocate, available = 0;
 		enum alloc_loc loc = ALLOC_ERR;
 		const char *action;
@@ -763,7 +763,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
 	 * an initial "pmem-reserve pass".  Only do an initial BLK allocation
 	 * when none of the DPA space is reserved.
 	 */
-	if ((is_pmem || !ndd->dpa.child) && n == to_allocate)
+	if ((is_pmem || list_empty(&ndd->dpa.child)) && n == to_allocate)
 		return init_dpa_allocation(label_id, nd_region, nd_mapping, n);
 	return n;
 }
@@ -779,7 +779,7 @@ static int merge_dpa(struct nd_region *nd_region,
  retry:
 	for_each_dpa_resource(ndd, res) {
 		int rc;
-		struct resource *next = res->sibling;
+		struct resource *next = resource_sibling(res);
 		resource_size_t end = res->start + resource_size(res);
 
 		if (!next || strcmp(res->name, label_id->id) != 0
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 32e0364b48b9..da7da15e03e7 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -102,11 +102,10 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
 		(unsigned long long) (res ? res->start : 0), ##arg)
 
 #define for_each_dpa_resource(ndd, res) \
-	for (res = (ndd)->dpa.child; res; res = res->sibling)
+	list_for_each_entry(res, &(ndd)->dpa.child, sibling)
 
 #define for_each_dpa_resource_safe(ndd, res, next) \
-	for (res = (ndd)->dpa.child, next = res ? res->sibling : NULL; \
-			res; res = next, next = next ? next->sibling : NULL)
+	list_for_each_entry_safe(res, next, &(ndd)->dpa.child, sibling)
 
 struct nd_percpu_lane {
 	int count;
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 53349912ac75..e2e25719ab52 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -330,7 +330,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
 {
 	int err;
 	res->flags = range->flags;
-	res->parent = res->child = res->sibling = NULL;
+	res->parent = NULL;
+	INIT_LIST_HEAD(&res->child);
+	INIT_LIST_HEAD(&res->sibling);
 	res->name = np->full_name;
 
 	if (res->flags & IORESOURCE_IO) {
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 69bd98421eb1..7482bdfd1959 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -170,8 +170,8 @@ lba_dump_res(struct resource *r, int d)
 	for (i = d; i ; --i) printk(" ");
 	printk(KERN_DEBUG "%p [%lx,%lx]/%lx\n", r,
 		(long)r->start, (long)r->end, r->flags);
-	lba_dump_res(r->child, d+2);
-	lba_dump_res(r->sibling, d);
+	lba_dump_res(resource_first_child(&r->child), d+2);
+	lba_dump_res(resource_sibling(r), d);
 }
 
 
diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
index 942b64fc7f1f..e3ace20345c7 100644
--- a/drivers/pci/host/vmd.c
+++ b/drivers/pci/host/vmd.c
@@ -542,14 +542,14 @@ static struct pci_ops vmd_ops = {
 
 static void vmd_attach_resources(struct vmd_dev *vmd)
 {
-	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[1];
-	vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[2];
+	list_add(&vmd->resources[1].sibling, &vmd->dev->resource[VMD_MEMBAR1].child);
+	list_add(&vmd->resources[2].sibling, &vmd->dev->resource[VMD_MEMBAR2].child);
 }
 
 static void vmd_detach_resources(struct vmd_dev *vmd)
 {
-	vmd->dev->resource[VMD_MEMBAR1].child = NULL;
-	vmd->dev->resource[VMD_MEMBAR2].child = NULL;
+	INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR1].child);
+	INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR2].child);
 }
 
 /*
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..9624dd1dfd49 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -59,6 +59,8 @@ static struct resource *get_pci_domain_busn_res(int domain_nr)
 	r->res.start = 0;
 	r->res.end = 0xff;
 	r->res.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
+	INIT_LIST_HEAD(&r->res.child);
+	INIT_LIST_HEAD(&r->res.sibling);
 
 	list_add_tail(&r->list, &pci_domain_busn_res_list);
 
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 79b1824e83b4..8e685af8938d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2107,7 +2107,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 				continue;
 
 			/* Ignore BARs which are still in use */
-			if (res->child)
+			if (!list_empty(&res->child))
 				continue;
 
 			ret = add_to_list(&saved, bridge, res, 0, 0);
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index dfdcd0bfe54e..b7456ae889dd 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -12,6 +12,7 @@
 #ifndef __ASSEMBLY__
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/list.h>
 /*
  * Resources are tree-like, allowing
  * nesting etc..
@@ -22,7 +23,8 @@ struct resource {
 	const char *name;
 	unsigned long flags;
 	unsigned long desc;
-	struct resource *parent, *sibling, *child;
+	struct list_head child, sibling;
+	struct resource *parent;
 };
 
 /*
@@ -216,7 +218,6 @@ static inline bool resource_contains(struct resource *r1, struct resource *r2)
 	return r1->start <= r2->start && r1->end >= r2->end;
 }
 
-
 /* Convenience shorthand with allocation */
 #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)
@@ -287,6 +288,18 @@ static inline bool resource_overlaps(struct resource *r1, struct resource *r2)
        return (r1->start <= r2->end && r1->end >= r2->start);
 }
 
+static inline struct resource *resource_sibling(struct resource *res)
+{
+	if (res->parent && !list_is_last(&res->sibling, &res->parent->child))
+		return list_next_entry(res, sibling);
+	return NULL;
+}
+
+static inline struct resource *resource_first_child(struct list_head *head)
+{
+	return list_first_entry_or_null(head, struct resource, sibling);
+}
+
 
 #endif /* __ASSEMBLY__ */
 #endif	/* _LINUX_IOPORT_H */
diff --git a/kernel/resource.c b/kernel/resource.c
index 5e7c56d5d838..ef9a20b75234 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -31,6 +31,8 @@ struct resource ioport_resource = {
 	.start	= 0,
 	.end	= IO_SPACE_LIMIT,
 	.flags	= IORESOURCE_IO,
+	.sibling = LIST_HEAD_INIT(ioport_resource.sibling),
+	.child  = LIST_HEAD_INIT(ioport_resource.child),
 };
 EXPORT_SYMBOL(ioport_resource);
 
@@ -39,6 +41,8 @@ struct resource iomem_resource = {
 	.start	= 0,
 	.end	= -1,
 	.flags	= IORESOURCE_MEM,
+	.sibling = LIST_HEAD_INIT(iomem_resource.sibling),
+	.child  = LIST_HEAD_INIT(iomem_resource.child),
 };
 EXPORT_SYMBOL(iomem_resource);
 
@@ -57,20 +61,20 @@ static DEFINE_RWLOCK(resource_lock);
  * by boot mem after the system is up. So for reusing the resource entry
  * we need to remember the resource.
  */
-static struct resource *bootmem_resource_free;
+static struct list_head bootmem_resource_free = LIST_HEAD_INIT(bootmem_resource_free);
 static DEFINE_SPINLOCK(bootmem_resource_lock);
 
 static struct resource *next_resource(struct resource *p, bool sibling_only)
 {
 	/* Caller wants to traverse through siblings only */
 	if (sibling_only)
-		return p->sibling;
+		return resource_sibling(p);
 
-	if (p->child)
-		return p->child;
-	while (!p->sibling && p->parent)
+	if (!list_empty(&p->child))
+		return resource_first_child(&p->child);
+	while (!resource_sibling(p) && p->parent)
 		p = p->parent;
-	return p->sibling;
+	return resource_sibling(p);
 }
 
 static void *r_next(struct seq_file *m, void *v, loff_t *pos)
@@ -90,7 +94,7 @@ static void *r_start(struct seq_file *m, loff_t *pos)
 	struct resource *p = PDE_DATA(file_inode(m->file));
 	loff_t l = 0;
 	read_lock(&resource_lock);
-	for (p = p->child; p && l < *pos; p = r_next(m, p, &l))
+	for (p = resource_first_child(&p->child); p && l < *pos; p = r_next(m, p, &l))
 		;
 	return p;
 }
@@ -153,8 +157,7 @@ static void free_resource(struct resource *res)
 
 	if (!PageSlab(virt_to_head_page(res))) {
 		spin_lock(&bootmem_resource_lock);
-		res->sibling = bootmem_resource_free;
-		bootmem_resource_free = res;
+		list_add(&res->sibling, &bootmem_resource_free);
 		spin_unlock(&bootmem_resource_lock);
 	} else {
 		kfree(res);
@@ -166,10 +169,9 @@ static struct resource *alloc_resource(gfp_t flags)
 	struct resource *res = NULL;
 
 	spin_lock(&bootmem_resource_lock);
-	if (bootmem_resource_free) {
-		res = bootmem_resource_free;
-		bootmem_resource_free = res->sibling;
-	}
+	res = resource_first_child(&bootmem_resource_free);
+	if (res)
+		list_del(&res->sibling);
 	spin_unlock(&bootmem_resource_lock);
 
 	if (res)
@@ -177,6 +179,8 @@ static struct resource *alloc_resource(gfp_t flags)
 	else
 		res = kzalloc(sizeof(struct resource), flags);
 
+	INIT_LIST_HEAD(&res->child);
+	INIT_LIST_HEAD(&res->sibling);
 	return res;
 }
 
@@ -185,7 +189,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
 {
 	resource_size_t start = new->start;
 	resource_size_t end = new->end;
-	struct resource *tmp, **p;
+	struct resource *tmp;
 
 	if (end < start)
 		return root;
@@ -193,64 +197,62 @@ static struct resource * __request_resource(struct resource *root, struct resour
 		return root;
 	if (end > root->end)
 		return root;
-	p = &root->child;
-	for (;;) {
-		tmp = *p;
-		if (!tmp || tmp->start > end) {
-			new->sibling = tmp;
-			*p = new;
+
+	if (list_empty(&root->child)) {
+		list_add(&new->sibling, &root->child);
+		new->parent = root;
+		INIT_LIST_HEAD(&new->child);
+		return NULL;
+	}
+
+	list_for_each_entry(tmp, &root->child, sibling) {
+		if (tmp->start > end) {
+			list_add(&new->sibling, tmp->sibling.prev);
 			new->parent = root;
+			INIT_LIST_HEAD(&new->child);
 			return NULL;
 		}
-		p = &tmp->sibling;
 		if (tmp->end < start)
 			continue;
 		return tmp;
 	}
+
+	list_add_tail(&new->sibling, &root->child);
+	new->parent = root;
+	INIT_LIST_HEAD(&new->child);
+	return NULL;
 }
 
 static int __release_resource(struct resource *old, bool release_child)
 {
-	struct resource *tmp, **p, *chd;
+	struct resource *tmp, *next, *chd;
 
-	p = &old->parent->child;
-	for (;;) {
-		tmp = *p;
-		if (!tmp)
-			break;
+	list_for_each_entry_safe(tmp, next, &old->parent->child, sibling) {
 		if (tmp == old) {
-			if (release_child || !(tmp->child)) {
-				*p = tmp->sibling;
+			if (release_child || list_empty(&tmp->child)) {
+				list_del(&tmp->sibling);
 			} else {
-				for (chd = tmp->child;; chd = chd->sibling) {
+				list_for_each_entry(chd, &tmp->child, sibling)
 					chd->parent = tmp->parent;
-					if (!(chd->sibling))
-						break;
-				}
-				*p = tmp->child;
-				chd->sibling = tmp->sibling;
+				list_splice(&tmp->child, tmp->sibling.prev);
+				list_del(&tmp->sibling);
 			}
+
 			old->parent = NULL;
 			return 0;
 		}
-		p = &tmp->sibling;
 	}
 	return -EINVAL;
 }
 
 static void __release_child_resources(struct resource *r)
 {
-	struct resource *tmp, *p;
+	struct resource *tmp, *next;
 	resource_size_t size;
 
-	p = r->child;
-	r->child = NULL;
-	while (p) {
-		tmp = p;
-		p = p->sibling;
-
+	list_for_each_entry_safe(tmp, next, &r->child, sibling) {
 		tmp->parent = NULL;
-		tmp->sibling = NULL;
+		INIT_LIST_HEAD(&tmp->sibling);
 		__release_child_resources(tmp);
 
 		printk(KERN_DEBUG "release child resource %pR\n", tmp);
@@ -259,6 +261,8 @@ static void __release_child_resources(struct resource *r)
 		tmp->start = 0;
 		tmp->end = size - 1;
 	}
+
+	INIT_LIST_HEAD(&tmp->child);
 }
 
 void release_child_resources(struct resource *r)
@@ -343,7 +347,8 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
 
 	read_lock(&resource_lock);
 
-	for (p = iomem_resource.child; p; p = next_resource(p, sibling_only)) {
+	for (p = resource_first_child(&iomem_resource.child); p;
+			p = next_resource(p, sibling_only)) {
 		if ((p->flags & res->flags) != res->flags)
 			continue;
 		if ((desc != IORES_DESC_NONE) && (desc != p->desc))
@@ -532,7 +537,7 @@ int region_intersects(resource_size_t start, size_t size, unsigned long flags,
 	struct resource *p;
 
 	read_lock(&resource_lock);
-	for (p = iomem_resource.child; p ; p = p->sibling) {
+	list_for_each_entry(p, &iomem_resource.child, sibling) {
 		bool is_type = (((p->flags & flags) == flags) &&
 				((desc == IORES_DESC_NONE) ||
 				 (desc == p->desc)));
@@ -586,7 +591,7 @@ static int __find_resource(struct resource *root, struct resource *old,
 			 resource_size_t  size,
 			 struct resource_constraint *constraint)
 {
-	struct resource *this = root->child;
+	struct resource *this = resource_first_child(&root->child);
 	struct resource tmp = *new, avail, alloc;
 
 	tmp.start = root->start;
@@ -596,7 +601,7 @@ static int __find_resource(struct resource *root, struct resource *old,
 	 */
 	if (this && this->start == root->start) {
 		tmp.start = (this == old) ? old->start : this->end + 1;
-		this = this->sibling;
+		this = resource_sibling(this);
 	}
 	for(;;) {
 		if (this)
@@ -632,7 +637,7 @@ next:		if (!this || this->end == root->end)
 
 		if (this != old)
 			tmp.start = this->end + 1;
-		this = this->sibling;
+		this = resource_sibling(this);
 	}
 	return -EBUSY;
 }
@@ -676,7 +681,7 @@ static int reallocate_resource(struct resource *root, struct resource *old,
 		goto out;
 	}
 
-	if (old->child) {
+	if (!list_empty(&old->child)) {
 		err = -EBUSY;
 		goto out;
 	}
@@ -757,7 +762,7 @@ struct resource *lookup_resource(struct resource *root, resource_size_t start)
 	struct resource *res;
 
 	read_lock(&resource_lock);
-	for (res = root->child; res; res = res->sibling) {
+	list_for_each_entry(res, &root->child, sibling) {
 		if (res->start == start)
 			break;
 	}
@@ -790,32 +795,27 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
 			break;
 	}
 
-	for (next = first; ; next = next->sibling) {
+	for (next = first; ; next = resource_sibling(next)) {
 		/* Partial overlap? Bad, and unfixable */
 		if (next->start < new->start || next->end > new->end)
 			return next;
-		if (!next->sibling)
+		if (!resource_sibling(next))
 			break;
-		if (next->sibling->start > new->end)
+		if (resource_sibling(next)->start > new->end)
 			break;
 	}
-
 	new->parent = parent;
-	new->sibling = next->sibling;
-	new->child = first;
+	list_add(&new->sibling, &next->sibling);
+	INIT_LIST_HEAD(&new->child);
 
-	next->sibling = NULL;
-	for (next = first; next; next = next->sibling)
+	/*
+	 * From first to next, they all fall into new's region, so change them
+	 * as new's children.
+	 */
+	list_cut_position(&new->child, first->sibling.prev, &next->sibling);
+	list_for_each_entry(next, &new->child, sibling)
 		next->parent = new;
 
-	if (parent->child == first) {
-		parent->child = new;
-	} else {
-		next = parent->child;
-		while (next->sibling != first)
-			next = next->sibling;
-		next->sibling = new;
-	}
 	return NULL;
 }
 
@@ -937,19 +937,17 @@ static int __adjust_resource(struct resource *res, resource_size_t start,
 	if ((start < parent->start) || (end > parent->end))
 		goto out;
 
-	if (res->sibling && (res->sibling->start <= end))
+	if (resource_sibling(res) && (resource_sibling(res)->start <= end))
 		goto out;
 
-	tmp = parent->child;
-	if (tmp != res) {
-		while (tmp->sibling != res)
-			tmp = tmp->sibling;
+	if (res->sibling.prev != &parent->child) {
+		tmp = list_prev_entry(res, sibling);
 		if (start <= tmp->end)
 			goto out;
 	}
 
 skip:
-	for (tmp = res->child; tmp; tmp = tmp->sibling)
+	list_for_each_entry(tmp, &res->child, sibling)
 		if ((tmp->start < start) || (tmp->end > end))
 			goto out;
 
@@ -987,31 +985,33 @@ EXPORT_SYMBOL(adjust_resource);
  * Reparent resource children of pr that conflict with res
  * under res, and make res replace those children.
  */
-static int reparent_resources(struct resource *parent,
-				     struct resource *res)
+int reparent_resources(struct resource *parent, struct resource *res)
 {
-	struct resource *p, **pp;
-	struct resource **firstpp = NULL;
+	struct resource *p, *first = NULL;
 
-	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
+	list_for_each_entry(p, &parent->child, sibling) {
 		if (p->end < res->start)
 			continue;
 		if (res->end < p->start)
 			break;
 		if (p->start < res->start || p->end > res->end)
 			return -1;	/* not completely contained */
-		if (firstpp == NULL)
-			firstpp = pp;
+		if (first == NULL)
+			first = p;
 	}
-	if (firstpp == NULL)
+	if (first == NULL)
 		return -1;	/* didn't find any conflicting entries? */
 	res->parent = parent;
-	res->child = *firstpp;
-	res->sibling = *pp;
-	*firstpp = res;
-	*pp = NULL;
-	for (p = res->child; p != NULL; p = p->sibling) {
-		p->parent = res;
+	list_add(&res->sibling, &p->sibling.prev);
+	INIT_LIST_HEAD(&res->child);
+
+	/*
+	 * From first to p's previous sibling, they all fall into
+	 * res's region, change them as res's children.
+	 */
+	list_cut_position(&res->child, first->sibling.prev, res->sibling.prev);
+	list_for_each_entry(p, &new->child, sibling) {
+                p->parent = new;
 		pr_debug("PCI: Reparented %s %pR under %s\n",
 			 p->name, p, res->name);
 	}
@@ -1210,34 +1210,32 @@ EXPORT_SYMBOL(__request_region);
 void __release_region(struct resource *parent, resource_size_t start,
 			resource_size_t n)
 {
-	struct resource **p;
+	struct resource *res;
 	resource_size_t end;
 
-	p = &parent->child;
+	res = resource_first_child(&parent->child);
 	end = start + n - 1;
 
 	write_lock(&resource_lock);
 
 	for (;;) {
-		struct resource *res = *p;
-
 		if (!res)
 			break;
 		if (res->start <= start && res->end >= end) {
 			if (!(res->flags & IORESOURCE_BUSY)) {
-				p = &res->child;
+				res = resource_first_child(&res->child);
 				continue;
 			}
 			if (res->start != start || res->end != end)
 				break;
-			*p = res->sibling;
+			list_del(&res->sibling);
 			write_unlock(&resource_lock);
 			if (res->flags & IORESOURCE_MUXED)
 				wake_up(&muxed_resource_wait);
 			free_resource(res);
 			return;
 		}
-		p = &res->sibling;
+		res = resource_sibling(res);
 	}
 
 	write_unlock(&resource_lock);
@@ -1272,9 +1270,7 @@ EXPORT_SYMBOL(__release_region);
 int release_mem_region_adjustable(struct resource *parent,
 			resource_size_t start, resource_size_t size)
 {
-	struct resource **p;
-	struct resource *res;
-	struct resource *new_res;
+	struct resource *res, *new_res;
 	resource_size_t end;
 	int ret = -EINVAL;
 
@@ -1285,16 +1281,16 @@ int release_mem_region_adjustable(struct resource *parent,
 	/* The alloc_resource() result gets checked later */
 	new_res = alloc_resource(GFP_KERNEL);
 
-	p = &parent->child;
+	res = resource_first_child(&parent->child);
 	write_lock(&resource_lock);
 
-	while ((res = *p)) {
+	while ((res)) {
 		if (res->start >= end)
 			break;
 
 		/* look for the next resource if it does not fit into */
 		if (res->start > start || res->end < end) {
-			p = &res->sibling;
+			res = resource_sibling(res);
 			continue;
 		}
 
@@ -1302,14 +1298,14 @@ int release_mem_region_adjustable(struct resource *parent,
 			break;
 
 		if (!(res->flags & IORESOURCE_BUSY)) {
-			p = &res->child;
+			res = resource_first_child(&res->child);
 			continue;
 		}
 
 		/* found the target resource; let's adjust accordingly */
 		if (res->start == start && res->end == end) {
 			/* free the whole entry */
-			*p = res->sibling;
+			list_del(&res->sibling);
 			free_resource(res);
 			ret = 0;
 		} else if (res->start == start && res->end != end) {
@@ -1332,14 +1328,13 @@ int release_mem_region_adjustable(struct resource *parent,
 			new_res->flags = res->flags;
 			new_res->desc = res->desc;
 			new_res->parent = res->parent;
-			new_res->sibling = res->sibling;
-			new_res->child = NULL;
+			INIT_LIST_HEAD(&new_res->child);
 
 			ret = __adjust_resource(res, res->start,
 						start - res->start);
 			if (ret)
 				break;
-			res->sibling = new_res;
+			list_add(&new_res->sibling, &res->sibling);
 			new_res = NULL;
 		}
 
@@ -1520,7 +1515,7 @@ static int __init reserve_setup(char *str)
 			res->end = io_start + io_num - 1;
 			res->flags |= IORESOURCE_BUSY;
 			res->desc = IORES_DESC_NONE;
-			res->child = NULL;
+			INIT_LIST_HEAD(&res->child);
 			if (request_resource(parent, res) == 0)
 				reserved = x+1;
 		}
@@ -1540,7 +1535,7 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
 	loff_t l;
 
 	read_lock(&resource_lock);
-	for (p = p->child; p ; p = r_next(NULL, p, &l)) {
+	for (p = resource_first_child(&p->child); p; p = r_next(NULL, p, &l)) {
 		/*
 		 * We can probably skip the resources without
 		 * IORESOURCE_IO attribute?
@@ -1596,7 +1591,7 @@ bool iomem_is_exclusive(u64 addr)
 	addr = addr & PAGE_MASK;
 
 	read_lock(&resource_lock);
-	for (p = p->child; p ; p = r_next(NULL, p, &l)) {
+	for (p = resource_first_child(&p->child); p; p = r_next(NULL, p, &l)) {
 		/*
 		 * We can probably skip the resources without
 		 * IORESOURCE_IO attribute?
-- 
2.13.6

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related

* [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: Baoquan He @ 2018-06-12  3:28 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, josh-iaAMLnmF4UmaiuxdJuQwMA,
	fengguang.wu-ral2JQCrhuEAvxtiuMwx3w, bp-l3A5Bk7waGM
  Cc: brijesh.singh-5C7GfCeVMHo, devicetree-u79uwXL29TY76Z2rM5mHXA,
	airlied-cv59FeDIM0c, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w, Paul Mackerras,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	lorenzo.pieralisi-5wv7dgnIgG8, sthemmin-0li6OtcxBFHby3iVrkZq2A,
	Baoquan He, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Michael Ellerman,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, dyoung-H+wXaHxf7aLQT0dZR+AlfA,
	thomas.lendacky-5C7GfCeVMHo, haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	jglisse-H+wXaHxf7aLQT0dZR+AlfA, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180612032831.29747-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
so that it's shared. Later its code also need be updated using list_head
to replace singly linked list.

Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Cc: Paul Mackerras <paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
Cc: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
---
v4->v5:
  Fix several code bugs reported by test robot on ARCH powerpc and
  microblaze.

v3->v4:
  Fix several bugs test robot reported. And change patch log.

v2->v3:
  Rename resource functions first_child() and sibling() to
  resource_first_chils() and resource_sibling(). Dan suggested this.

  Move resource_first_chils() and resource_sibling() to linux/ioport.h
  and make them as inline function. Rob suggested this. Accordingly add
  linux/list.h including in linux/ioport.h, please help review if this
  bring efficiency degradation or code redundancy.

  The change on struct resource {} bring two pointers of size increase,
  mention this in git log to make it more specifically, Rob suggested
  this.

 arch/microblaze/pci/pci-common.c | 37 -------------------------------------
 arch/powerpc/kernel/pci-common.c | 35 -----------------------------------
 include/linux/ioport.h           |  1 +
 kernel/resource.c                | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 37 insertions(+), 72 deletions(-)

diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index f34346d56095..7899bafab064 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -619,43 +619,6 @@ int pcibios_add_device(struct pci_dev *dev)
 EXPORT_SYMBOL(pcibios_add_device);
 
 /*
- * Reparent resource children of pr that conflict with res
- * under res, and make res replace those children.
- */
-static int __init reparent_resources(struct resource *parent,
-				     struct resource *res)
-{
-	struct resource *p, **pp;
-	struct resource **firstpp = NULL;
-
-	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
-		if (p->end < res->start)
-			continue;
-		if (res->end < p->start)
-			break;
-		if (p->start < res->start || p->end > res->end)
-			return -1;	/* not completely contained */
-		if (firstpp == NULL)
-			firstpp = pp;
-	}
-	if (firstpp == NULL)
-		return -1;	/* didn't find any conflicting entries? */
-	res->parent = parent;
-	res->child = *firstpp;
-	res->sibling = *pp;
-	*firstpp = res;
-	*pp = NULL;
-	for (p = res->child; p != NULL; p = p->sibling) {
-		p->parent = res;
-		pr_debug("PCI: Reparented %s [%llx..%llx] under %s\n",
-			 p->name,
-			 (unsigned long long)p->start,
-			 (unsigned long long)p->end, res->name);
-	}
-	return 0;
-}
-
-/*
  *  Handle resources of PCI devices.  If the world were perfect, we could
  *  just allocate all the resource regions and do nothing more.  It isn't.
  *  On the other hand, we cannot just re-allocate all devices, as it would
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index fe9733ffffaa..926035bb378d 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1088,41 +1088,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 EXPORT_SYMBOL(pcibios_align_resource);
 
 /*
- * Reparent resource children of pr that conflict with res
- * under res, and make res replace those children.
- */
-static int reparent_resources(struct resource *parent,
-				     struct resource *res)
-{
-	struct resource *p, **pp;
-	struct resource **firstpp = NULL;
-
-	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
-		if (p->end < res->start)
-			continue;
-		if (res->end < p->start)
-			break;
-		if (p->start < res->start || p->end > res->end)
-			return -1;	/* not completely contained */
-		if (firstpp == NULL)
-			firstpp = pp;
-	}
-	if (firstpp == NULL)
-		return -1;	/* didn't find any conflicting entries? */
-	res->parent = parent;
-	res->child = *firstpp;
-	res->sibling = *pp;
-	*firstpp = res;
-	*pp = NULL;
-	for (p = res->child; p != NULL; p = p->sibling) {
-		p->parent = res;
-		pr_debug("PCI: Reparented %s %pR under %s\n",
-			 p->name, p, res->name);
-	}
-	return 0;
-}
-
-/*
  *  Handle resources of PCI devices.  If the world were perfect, we could
  *  just allocate all the resource regions and do nothing more.  It isn't.
  *  On the other hand, we cannot just re-allocate all devices, as it would
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..dfdcd0bfe54e 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -192,6 +192,7 @@ extern int allocate_resource(struct resource *root, struct resource *new,
 struct resource *lookup_resource(struct resource *root, resource_size_t start);
 int adjust_resource(struct resource *res, resource_size_t start,
 		    resource_size_t size);
+int reparent_resources(struct resource *parent, struct resource *res);
 resource_size_t resource_alignment(struct resource *res);
 static inline resource_size_t resource_size(const struct resource *res)
 {
diff --git a/kernel/resource.c b/kernel/resource.c
index 30e1bc68503b..5e7c56d5d838 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -983,6 +983,42 @@ int adjust_resource(struct resource *res, resource_size_t start,
 }
 EXPORT_SYMBOL(adjust_resource);
 
+/*
+ * Reparent resource children of pr that conflict with res
+ * under res, and make res replace those children.
+ */
+static int reparent_resources(struct resource *parent,
+				     struct resource *res)
+{
+	struct resource *p, **pp;
+	struct resource **firstpp = NULL;
+
+	for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
+		if (p->end < res->start)
+			continue;
+		if (res->end < p->start)
+			break;
+		if (p->start < res->start || p->end > res->end)
+			return -1;	/* not completely contained */
+		if (firstpp == NULL)
+			firstpp = pp;
+	}
+	if (firstpp == NULL)
+		return -1;	/* didn't find any conflicting entries? */
+	res->parent = parent;
+	res->child = *firstpp;
+	res->sibling = *pp;
+	*firstpp = res;
+	*pp = NULL;
+	for (p = res->child; p != NULL; p = p->sibling) {
+		p->parent = res;
+		pr_debug("PCI: Reparented %s %pR under %s\n",
+			 p->name, p, res->name);
+	}
+	return 0;
+}
+EXPORT_SYMBOL(reparent_resources);
+
 static void __init __reserve_region_with_split(struct resource *root,
 		resource_size_t start, resource_size_t end,
 		const char *name)
-- 
2.13.6

^ permalink raw reply related

* [PATCH v5 0/4] resource: Use list_head to link sibling resource
From: Baoquan He @ 2018-06-12  3:28 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, josh-iaAMLnmF4UmaiuxdJuQwMA,
	fengguang.wu-ral2JQCrhuEAvxtiuMwx3w, bp-l3A5Bk7waGM
  Cc: brijesh.singh-5C7GfCeVMHo, devicetree-u79uwXL29TY76Z2rM5mHXA,
	airlied-cv59FeDIM0c, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	lorenzo.pieralisi-5wv7dgnIgG8, sthemmin-0li6OtcxBFHby3iVrkZq2A,
	Baoquan He, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, dyoung-H+wXaHxf7aLQT0dZR+AlfA,
	thomas.lendacky-5C7GfCeVMHo, haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	jglisse-H+wXaHxf7aLQT0dZR+AlfA, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X, linuxppc-dev

This patchset is doing:
1) Replace struct resource's sibling list from singly linked list to
list_head. Clearing out those pointer operation within singly linked
list for better code readability.
2) Based on list_head replacement, add a new function
walk_system_ram_res_rev() which can does reversed iteration on
iomem_resource's siblings.
3) Change kexec_file loading to search system RAM top down for kernel
loadin, using walk_system_ram_res_rev().

Note:
This patchset passed testing on my kvm guest, x86_64 arch with network
enabling. The thing we need pay attetion to is that a root resource's
child member need be initialized specifically with LIST_HEAD_INIT() if
statically defined or INIT_LIST_HEAD() for dynamically definition. Here
Just like we do for iomem_resource/ioport_resource, or the change in
get_pci_domain_busn_res().


Links of the old post (Boris pointed out that we should use
https://lkml.kernel.org/r/Message-ID, while it can't be opened from
my side, so paste all of them here.):
v4:
https://lkml.kernel.org/r/20180507063224.24229-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
https://lkml.org/lkml/2018/5/7/36

v3:
https://lkml.kernel.org/r/20180419001848.3041-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
https://lkml.org/lkml/2018/4/18/767

v2:
https://lkml.kernel.org/r/20180408024724.16812-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
https://lkml.org/lkml/2018/4/7/169

v1:
https://lkml.kernel.org/r/20180322033722.9279-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
https://lkml.org/lkml/2018/3/21/952

Changelog:
v4->v5:
  Add new patch 0001 to move duplicated reparent_resources() to
  kernel/resource.c to make it be shared by different ARCH-es.

  Fix several code bugs reported by test robot on ARCH powerpc and
  microblaze.
v3->v4:
  Fix several bugs test robot reported. Rewrite cover letter and patch
  log according to reviewer's comment.

v2->v3:
  Rename resource functions first_child() and sibling() to
  resource_first_chils() and resource_sibling(). Dan suggested this.

  Move resource_first_chils() and resource_sibling() to linux/ioport.h
  and make them as inline function. Rob suggested this. Accordingly add
  linux/list.h including in linux/ioport.h, please help review if this
  bring efficiency degradation or code redundancy.

  The change on struct resource {} bring two pointers of size increase,
  mention this in git log to make it more specifically, Rob suggested
  this.

v1->v2:
  Use list_head instead to link resource siblings. This is suggested by
  Andrew.

  Rewrite walk_system_ram_res_rev() after list_head is taken to link
  resouce siblings.

Baoquan He (4):
  resource: Move reparent_resources() to kernel/resource.c and make it
    public
  resource: Use list_head to link sibling resource
  resource: add walk_system_ram_res_rev()
  kexec_file: Load kernel at top of system RAM if required

 arch/arm/plat-samsung/pm-check.c            |   6 +-
 arch/microblaze/pci/pci-common.c            |  41 +----
 arch/powerpc/kernel/pci-common.c            |  39 +----
 arch/sparc/kernel/ioport.c                  |   2 +-
 arch/xtensa/include/asm/pci-bridge.h        |   4 +-
 drivers/eisa/eisa-bus.c                     |   2 +
 drivers/gpu/drm/drm_memory.c                |   3 +-
 drivers/gpu/drm/gma500/gtt.c                |   5 +-
 drivers/hv/vmbus_drv.c                      |  52 +++---
 drivers/input/joystick/iforce/iforce-main.c |   4 +-
 drivers/nvdimm/namespace_devs.c             |   6 +-
 drivers/nvdimm/nd.h                         |   5 +-
 drivers/of/address.c                        |   4 +-
 drivers/parisc/lba_pci.c                    |   4 +-
 drivers/pci/host/vmd.c                      |   8 +-
 drivers/pci/probe.c                         |   2 +
 drivers/pci/setup-bus.c                     |   2 +-
 include/linux/ioport.h                      |  21 ++-
 kernel/kexec_file.c                         |   2 +
 kernel/resource.c                           | 259 ++++++++++++++++++----------
 20 files changed, 244 insertions(+), 227 deletions(-)

-- 
2.13.6

^ permalink raw reply

* Re: [PATCH] Input: add error handling for da9052_reg_write
From: Dmitry Torokhov @ 2018-06-11 17:30 UTC (permalink / raw)
  To: Zhouyang Jia; +Cc: Support Opensource, linux-input, linux-kernel
In-Reply-To: <1528694619-31889-1-git-send-email-jiazhouyang09@gmail.com>

Hi Zhouyang,

On Mon, Jun 11, 2018 at 01:23:39PM +0800, Zhouyang Jia wrote:
> When da9052_reg_write fails, the lack of error-handling code may
> cause unexpected results.
> 
> This patch adds error-handling code after calling da9052_reg_write.
> 
> Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com>
> ---
>  drivers/input/touchscreen/da9052_tsi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
> index b5dfd594..60c82a0 100644
> --- a/drivers/input/touchscreen/da9052_tsi.c
> +++ b/drivers/input/touchscreen/da9052_tsi.c
> @@ -319,8 +319,11 @@ static int da9052_ts_probe(struct platform_device *pdev)
>  static int  da9052_ts_remove(struct platform_device *pdev)
>  {
>  	struct da9052_tsi *tsi = platform_get_drvdata(pdev);
> +	int error;
>  
> -	da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19);
> +	error = da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19);
> +	if (error < 0)
> +		return error;

No, this does not help anything. The remove() action must not fail
(really, having it return an int and not void was an API mistake made
long time ago), and thus returning early in and event of error failing
to communicate with the device is a mistake. You really want to release
the interrupts and memory and unregister input device before returning.

>  
>  	da9052_free_irq(tsi->da9052, DA9052_IRQ_TSIREADY, tsi);
>  	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 02/13] input: add MT_TOOL_DIAL
From: Dmitry Torokhov @ 2018-06-11 17:18 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Mario.Limonciello, Peter Hutterer, linux-input,
	linux-kernel
In-Reply-To: <20180607075448.5706-3-benjamin.tissoires@redhat.com>

On Thu, Jun 07, 2018 at 09:54:37AM +0200, Benjamin Tissoires wrote:
> A dial is a tool you place on a multitouch surface which reports its
> orientation or a relative angle of rotation when rotating its knob.
> 
> Some examples are the Dell Totem (on the Canvas 27"), the Microsoft Dial,
> or the Griffin Powermate, though the later can't be put on a touch surface.
> 
> We give some extra space to account for other types of fingers if we need
> (MT_TOOL_THUMB)
> 
> Slightly change the documentation to not make it mandatory to update each
> MT_TOOL we add.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---
> 
> new in v2 (extracted from previous series in its own patch)
> ---
>  Documentation/input/multi-touch-protocol.rst | 12 ++++++------
>  include/uapi/linux/input-event-codes.h       |  3 ++-

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

(modulo input-event-codes.h vs input.h)

>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/input/multi-touch-protocol.rst b/Documentation/input/multi-touch-protocol.rst
> index b51751a0cd5d..6be70342e709 100644
> --- a/Documentation/input/multi-touch-protocol.rst
> +++ b/Documentation/input/multi-touch-protocol.rst
> @@ -310,12 +310,12 @@ ABS_MT_TOOL_Y
>  ABS_MT_TOOL_TYPE
>      The type of approaching tool. A lot of kernel drivers cannot distinguish
>      between different tool types, such as a finger or a pen. In such cases, the
> -    event should be omitted. The protocol currently supports MT_TOOL_FINGER,
> -    MT_TOOL_PEN, and MT_TOOL_PALM [#f2]_. For type B devices, this event is
> -    handled by input core; drivers should instead use
> -    input_mt_report_slot_state(). A contact's ABS_MT_TOOL_TYPE may change over
> -    time while still touching the device, because the firmware may not be able
> -    to determine which tool is being used when it first appears.
> +    event should be omitted. The protocol currently mainly supports
> +    MT_TOOL_FINGER, MT_TOOL_PEN, and MT_TOOL_PALM [#f2]_.
> +    For type B devices, this event is handled by input core; drivers should
> +    instead use input_mt_report_slot_state(). A contact's ABS_MT_TOOL_TYPE may
> +    change over time while still touching the device, because the firmware may
> +    not be able to determine which tool is being used when it first appears.
>  
>  ABS_MT_BLOB_ID
>      The BLOB_ID groups several packets together into one arbitrarily shaped
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index e8841cdb1ebd..a8d55f3909d3 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -854,6 +854,7 @@
>  #define MT_TOOL_FINGER		0
>  #define MT_TOOL_PEN		1
>  #define MT_TOOL_PALM		2
> -#define MT_TOOL_MAX		2
> +#define MT_TOOL_DIAL		10
> +#define MT_TOOL_MAX		10
>  
>  #endif
> -- 
> 2.14.3
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 01/13] input: move MT_TOOL_* to input-event-codes.h
From: Dmitry Torokhov @ 2018-06-11 17:16 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Mario.Limonciello, Peter Hutterer, linux-input,
	linux-kernel
In-Reply-To: <20180607075448.5706-2-benjamin.tissoires@redhat.com>

Hi Benjamin,

On Thu, Jun 07, 2018 at 09:54:36AM +0200, Benjamin Tissoires wrote:
> This is evdev API and should be in this file.

This is a weak argument. include/uapi/linux/input.h is exclusively evdev
API as well. input-event-codes.h is simply evdev API that is usable in
device tree files, and I do not think that at this time we need to
expose tool types to device tree...

Thanks.

> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---
> 
> new in v2
> ---
>  include/uapi/linux/input-event-codes.h | 8 ++++++++
>  include/uapi/linux/input.h             | 8 --------
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 6cba8a21ea19..e8841cdb1ebd 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -848,4 +848,12 @@
>  #define SND_MAX			0x07
>  #define SND_CNT			(SND_MAX+1)
>  
> +/*
> + * MT_TOOL types
> + */
> +#define MT_TOOL_FINGER		0
> +#define MT_TOOL_PEN		1
> +#define MT_TOOL_PALM		2
> +#define MT_TOOL_MAX		2
> +
>  #endif
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 7288a7c573cc..cc55c140dfef 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -267,14 +267,6 @@ struct input_mask {
>  #define BUS_CEC			0x1E
>  #define BUS_INTEL_ISHTP		0x1F
>  
> -/*
> - * MT_TOOL types
> - */
> -#define MT_TOOL_FINGER		0
> -#define MT_TOOL_PEN		1
> -#define MT_TOOL_PALM		2
> -#define MT_TOOL_MAX		2
> -
>  /*
>   * Values describing the status of a force-feedback effect
>   */
> -- 
> 2.14.3
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] HID: hid-ntrig: add error handling for sysfs_create_group
From: Jiri Kosina @ 2018-06-11 12:20 UTC (permalink / raw)
  To: Zhouyang Jia; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <1528707296-35785-1-git-send-email-jiazhouyang09@gmail.com>

On Mon, 11 Jun 2018, Zhouyang Jia wrote:

> When sysfs_create_group fails, the lack of error-handling code may
> cause unexpected results.
> 
> This patch adds error-handling code after calling sysfs_create_group.
> 
> Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com>
> ---
>  drivers/hid/hid-ntrig.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 43b1c72..1e70dae 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -955,6 +955,10 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  
>  	ret = sysfs_create_group(&hdev->dev.kobj,
>  			&ntrig_attribute_group);
> +	if (ret) {
> +		hid_err(hdev, "cannot create sysfs group\n");
> +		goto err_free;
> +	}

Is it really necessarily to bail out completely here and render the device 
dysfunctional? Issuing a warning about sysfs knobs being unavailable might 
be more appropriate.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: google: Add support for whiskers
From: Jiri Kosina @ 2018-06-11 12:11 UTC (permalink / raw)
  To: Nicolas Boichat; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20180608001450.76836-1-drinkcat@chromium.org>

On Fri, 8 Jun 2018, Nicolas Boichat wrote:

> Another device in the hammer class, with USB id 0x5030.
> 
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH] HID: hid-ntrig: add error handling for sysfs_create_group
From: Zhouyang Jia @ 2018-06-11  8:54 UTC (permalink / raw)
  Cc: Zhouyang Jia, Jiri Kosina, Benjamin Tissoires, linux-input,
	linux-kernel

When sysfs_create_group fails, the lack of error-handling code may
cause unexpected results.

This patch adds error-handling code after calling sysfs_create_group.

Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com>
---
 drivers/hid/hid-ntrig.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 43b1c72..1e70dae 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -955,6 +955,10 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	ret = sysfs_create_group(&hdev->dev.kobj,
 			&ntrig_attribute_group);
+	if (ret) {
+		hid_err(hdev, "cannot create sysfs group\n");
+		goto err_free;
+	}
 
 	return 0;
 err_free:
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 2/2] ARM: dts: am437x: make edt-ft5x06 a wakeup source for imx6 boards
From: Tony Lindgren @ 2018-06-11  7:56 UTC (permalink / raw)
  To: Shawn Guo
  Cc: mark.rutland, devicetree, dmitry.torokhov, robh+dt,
	linux-arm-kernel, kernel, linux-input, fabio.estevam, Daniel Mack
In-Reply-To: <20180611074420.GF16091@dragon>

* Shawn Guo <shawnguo@kernel.org> [180611 07:47]:
> On Wed, May 23, 2018 at 10:30:13AM +0200, Daniel Mack wrote:
> > The touchscreen driver no longer configures the device as wakeup source by
> > default. A "wakeup-source" property is needed.
> > 
> > Signed-off-by: Daniel Mack <daniel@zonque.org>
> 
> This is not an imx6 board, and I have to leave it to relevant platform
> maintainer.  The patch subject should be fixed to drop 'imx6'.

Daniel, can you please resend after -rc1?

Thanks,

Tony

^ permalink raw reply

* Re: [PATCH 2/2] ARM: dts: am437x: make edt-ft5x06 a wakeup source for imx6 boards
From: Shawn Guo @ 2018-06-11  7:44 UTC (permalink / raw)
  To: Daniel Mack
  Cc: mark.rutland, devicetree, dmitry.torokhov, robh+dt, kernel,
	linux-input, fabio.estevam, linux-arm-kernel
In-Reply-To: <20180523083013.7570-2-daniel@zonque.org>

On Wed, May 23, 2018 at 10:30:13AM +0200, Daniel Mack wrote:
> The touchscreen driver no longer configures the device as wakeup source by
> default. A "wakeup-source" property is needed.
> 
> Signed-off-by: Daniel Mack <daniel@zonque.org>

This is not an imx6 board, and I have to leave it to relevant platform
maintainer.  The patch subject should be fixed to drop 'imx6'.

Shawn

> ---
>  arch/arm/boot/dts/am437x-sk-evm.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> index 4118802b7fea..f17ed89da06b 100644
> --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> @@ -537,6 +537,8 @@
>  
>  		touchscreen-size-x = <480>;
>  		touchscreen-size-y = <272>;
> +
> +		wakeup-source;
>  	};
>  
>  	tlv320aic3106: tlv320aic3106@1b {
> -- 
> 2.14.3
> 

^ permalink raw reply

* Re: [PATCH 1/2] ARM: dts: imx6: make edt-ft5x06 a wakeup source for imx6 boards
From: Shawn Guo @ 2018-06-11  7:42 UTC (permalink / raw)
  To: Daniel Mack
  Cc: mark.rutland, devicetree, dmitry.torokhov, robh+dt, kernel,
	linux-input, fabio.estevam, linux-arm-kernel
In-Reply-To: <20180523083013.7570-1-daniel@zonque.org>

On Wed, May 23, 2018 at 10:30:12AM +0200, Daniel Mack wrote:
> The touchscreen driver no longer configures the device as wakeup source by
> default. A "wakeup-source" property is needed.
> 
> Signed-off-by: Daniel Mack <daniel@zonque.org>

Applied, thanks.

^ permalink raw reply

* [PATCH] Input: add error handling for da9052_reg_write
From: Zhouyang Jia @ 2018-06-11  5:23 UTC (permalink / raw)
  Cc: Zhouyang Jia, Support Opensource, Dmitry Torokhov, linux-input,
	linux-kernel

When da9052_reg_write fails, the lack of error-handling code may
cause unexpected results.

This patch adds error-handling code after calling da9052_reg_write.

Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com>
---
 drivers/input/touchscreen/da9052_tsi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
index b5dfd594..60c82a0 100644
--- a/drivers/input/touchscreen/da9052_tsi.c
+++ b/drivers/input/touchscreen/da9052_tsi.c
@@ -319,8 +319,11 @@ static int da9052_ts_probe(struct platform_device *pdev)
 static int  da9052_ts_remove(struct platform_device *pdev)
 {
 	struct da9052_tsi *tsi = platform_get_drvdata(pdev);
+	int error;
 
-	da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19);
+	error = da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19);
+	if (error < 0)
+		return error;
 
 	da9052_free_irq(tsi->da9052, DA9052_IRQ_TSIREADY, tsi);
 	da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] hid: intel_ish-hid: ipc: register more pm callbacks to support hibernation
From: Srinivas Pandruvada @ 2018-06-11  1:56 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Even Xu

From: Even Xu <even.xu@intel.com>

Current ish driver only register resume/suspend PM callbacks which
don't support hibernation (suspend to disk). Now use the
SIMPLE_DEV_PM_OPS() MACRO instead of struct dev_pm_ops directly.
The suspend and resume functions will now be used for both suspend
to RAM and hibernation.

If power management is disable, SIMPLE_DEV_PM_OPS will do nothing,
the suspend and resume related functions won't be used, so mark them
as __maybe_unused to clarify that this is intended behavior, and
remove #ifdefs for power management.

Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 582e449be9fe..a2c53ea3b5ed 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -205,8 +205,7 @@ static void ish_remove(struct pci_dev *pdev)
 	kfree(ishtp_dev);
 }
 
-#ifdef CONFIG_PM
-static struct device *ish_resume_device;
+static struct device __maybe_unused *ish_resume_device;
 
 /* 50ms to get resume response */
 #define WAIT_FOR_RESUME_ACK_MS		50
@@ -220,7 +219,7 @@ static struct device *ish_resume_device;
  * in that case a simple resume message is enough, others we need
  * a reset sequence.
  */
-static void ish_resume_handler(struct work_struct *work)
+static void __maybe_unused ish_resume_handler(struct work_struct *work)
 {
 	struct pci_dev *pdev = to_pci_dev(ish_resume_device);
 	struct ishtp_device *dev = pci_get_drvdata(pdev);
@@ -262,7 +261,7 @@ static void ish_resume_handler(struct work_struct *work)
  *
  * Return: 0 to the pm core
  */
-static int ish_suspend(struct device *device)
+static int __maybe_unused ish_suspend(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct ishtp_device *dev = pci_get_drvdata(pdev);
@@ -288,7 +287,7 @@ static int ish_suspend(struct device *device)
 	return 0;
 }
 
-static DECLARE_WORK(resume_work, ish_resume_handler);
+static __maybe_unused DECLARE_WORK(resume_work, ish_resume_handler);
 /**
  * ish_resume() - ISH resume callback
  * @device:	device pointer
@@ -297,7 +296,7 @@ static DECLARE_WORK(resume_work, ish_resume_handler);
  *
  * Return: 0 to the pm core
  */
-static int ish_resume(struct device *device)
+static int __maybe_unused ish_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct ishtp_device *dev = pci_get_drvdata(pdev);
@@ -311,21 +310,14 @@ static int ish_resume(struct device *device)
 	return 0;
 }
 
-static const struct dev_pm_ops ish_pm_ops = {
-	.suspend = ish_suspend,
-	.resume = ish_resume,
-};
-#define ISHTP_ISH_PM_OPS	(&ish_pm_ops)
-#else
-#define ISHTP_ISH_PM_OPS	NULL
-#endif /* CONFIG_PM */
+static SIMPLE_DEV_PM_OPS(ish_pm_ops, ish_suspend, ish_resume);
 
 static struct pci_driver ish_driver = {
 	.name = KBUILD_MODNAME,
 	.id_table = ish_pci_tbl,
 	.probe = ish_probe,
 	.remove = ish_remove,
-	.driver.pm = ISHTP_ISH_PM_OPS,
+	.driver.pm = &ish_pm_ops,
 };
 
 module_pci_driver(ish_driver);
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH v7 9/9] ARM: dts: at91: sama5d2: Add resistive touch device
From: Jonathan Cameron @ 2018-06-10 12:36 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov, robh
In-Reply-To: <1526975559-18966-10-git-send-email-eugen.hristev@microchip.com>

On Tue, 22 May 2018 10:52:39 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> Add generic resistive touch device which is connected to ADC block
> inside the SAMA5D2 SoC
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
As with the previous patch, the driver side of things is working it's
way through the IIO tree, but will be the next merge window, not the
current one.

Thanks,

Jonathan

> ---
> Changes in v5:
>  - renamed touchscreen-threshold-pressure to touchscreen-min-pressure
> 
>  arch/arm/boot/dts/sama5d2.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index f06ba99..a44f325 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -1442,6 +1442,16 @@
>  				status = "disabled";
>  			};
>  
> +			resistive_touch: resistive-touch {
> +				compatible = "resistive-adc-touch";
> +				io-channels = <&adc AT91_SAMA5D2_ADC_X_CHANNEL>,
> +					      <&adc AT91_SAMA5D2_ADC_Y_CHANNEL>,
> +					      <&adc AT91_SAMA5D2_ADC_P_CHANNEL>;
> +				io-channel-names = "x", "y", "pressure";
> +				touchscreen-min-pressure = <50000>;
> +				status = "disabled";
> +			};
> +
>  			pioA: pinctrl@fc038000 {
>  				compatible = "atmel,sama5d2-pinctrl";
>  				reg = <0xfc038000 0x600>;

^ permalink raw reply

* Re: [PATCH v7 8/9] ARM: dts: at91: sama5d2: add channel cells for ADC device
From: Jonathan Cameron @ 2018-06-10 12:35 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov, robh
In-Reply-To: <1526975559-18966-9-git-send-email-eugen.hristev@microchip.com>

On Tue, 22 May 2018 10:52:38 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> Preparing the ADC device to connect channel consumer drivers
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
The driver for the touchscreen is no in the IIO tree, but it wasn't
early enough (due to a block on a fix working it's way in) for this
merge window.  Will be going upstream in the next merge window.

Thanks,

Jonathan

> ---
>  arch/arm/boot/dts/sama5d2.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index 61f68e5..f06ba99 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -47,6 +47,7 @@
>  #include <dt-bindings/dma/at91.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/clock/at91.h>
> +#include <dt-bindings/iio/adc/at91-sama5d2_adc.h>
>  
>  / {
>  	model = "Atmel SAMA5D2 family SoC";
> @@ -1437,6 +1438,7 @@
>  				atmel,max-sample-rate-hz = <20000000>;
>  				atmel,startup-time-ms = <4>;
>  				atmel,trigger-edge-type = <IRQ_TYPE_EDGE_RISING>;
> +				#io-channel-cells = <1>;
>  				status = "disabled";
>  			};
>  

^ permalink raw reply

* Re: [PATCH v7 7/9] dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific consumer info
From: Jonathan Cameron @ 2018-06-10 12:34 UTC (permalink / raw)
  To: Ludovic Desroches
  Cc: Eugen Hristev, alexandre.belloni, linux-arm-kernel, devicetree,
	linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov, robh
In-Reply-To: <20180522122319.GN22971@rfolt0960.corp.atmel.com>

On Tue, 22 May 2018 14:23:19 +0200
Ludovic Desroches <ludovic.desroches@microchip.com> wrote:

> On Tue, May 22, 2018 at 10:52:37AM +0300, Eugen Hristev wrote:
> > Added defines for channel consumer device-tree binding
> > 
> > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>  
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> 
> > ---
> >  .../devicetree/bindings/iio/adc/at91-sama5d2_adc.txt     |  9 +++++++++
> >  include/dt-bindings/iio/adc/at91-sama5d2_adc.h           | 16 ++++++++++++++++
> >  2 files changed, 25 insertions(+)
> >  create mode 100644 include/dt-bindings/iio/adc/at91-sama5d2_adc.h
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt
> > index 6469a4c..4a3c1d4 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt
> > @@ -21,6 +21,14 @@ Optional properties:
> >    - dmas: Phandle to dma channel for the ADC.
> >    - dma-names: Must be "rx" when dmas property is being used.
> >    See ../../dma/dma.txt for details.
> > +  - #io-channel-cells: in case consumer drivers are attached, this must be 1.
> > +  See <Documentation/devicetree/bindings/iio/iio-bindings.txt> for details.
> > +
> > +Properties for consumer drivers:
> > +  - Consumer drivers can be connected to this producer device, as specified
> > +  in <Documentation/devicetree/bindings/iio/iio-bindings.txt>
> > +  - Channels exposed are specified in:
> > +  <dt-bindings/iio/adc/at91-sama5d2_adc.txt>
> >  
> >  Example:
> >  
> > @@ -38,4 +46,5 @@ adc: adc@fc030000 {
> >  	atmel,trigger-edge-type = <IRQ_TYPE_EDGE_BOTH>;
> >  	dmas = <&dma0 (AT91_XDMAC_DT_MEM_IF(0) | AT91_XDMAC_DT_PER_IF(1) | AT91_XDMAC_DT_PERID(25))>;
> >  	dma-names = "rx";
> > +	#io-channel-cells = <1>;
> >  }
> > diff --git a/include/dt-bindings/iio/adc/at91-sama5d2_adc.h b/include/dt-bindings/iio/adc/at91-sama5d2_adc.h
> > new file mode 100644
> > index 0000000..70f99db
> > --- /dev/null
> > +++ b/include/dt-bindings/iio/adc/at91-sama5d2_adc.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * This header provides constants for configuring the AT91 SAMA5D2 ADC
> > + */
> > +
> > +#ifndef _DT_BINDINGS_IIO_ADC_AT91_SAMA5D2_ADC_H
> > +#define _DT_BINDINGS_IIO_ADC_AT91_SAMA5D2_ADC_H
> > +
> > +/* X relative position channel index */
> > +#define AT91_SAMA5D2_ADC_X_CHANNEL		24
> > +/* Y relative position channel index */
> > +#define AT91_SAMA5D2_ADC_Y_CHANNEL		25
> > +/* pressure channel index */
> > +#define AT91_SAMA5D2_ADC_P_CHANNEL		26
> > +
> > +#endif
> > -- 
> > 2.7.4
> >   

^ permalink raw reply

* Re: [PATCH v7 6/9] input: touchscreen: resistive-adc-touch: add generic resistive ADC touchscreen
From: Jonathan Cameron @ 2018-06-10 12:33 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov, robh
In-Reply-To: <1526975559-18966-7-git-send-email-eugen.hristev@microchip.com>

On Tue, 22 May 2018 10:52:36 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> This adds a generic resistive touchscreen (GRTS) driver, which is based
> on an IIO device (an ADC). It must be connected to the channels of an ADC
> to receive touch data. Then it will feed the data into the input subsystem
> where it registers an input device.
> It uses an IIO callback buffer to register to the IIO device
> 
> Some parts of this patch are based on initial original work by
> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
> Changes in v7:
>  - cosmetic fixes as suggested by Dmitry: moved min pressure
> property lookup below and only if pressure channel is available.
> used the error variable in probe at some point, removed it at
> different point.
> 
> Changes in v6:
>  - changed a dev_err to dev_dbg which was forgotten in v5.
> 
> Changes in v5:
>  - return error on probe if failed add_action_or_reset
>  - renamed property touchscreen-threshold-pressure to
> touchscreen-min-pressure, changed variables accordingly
> 
> Changes in v4:
>  - added a small description in file header
>  - changed MAX_POS_MASK to GRTS_MAX_POS_MASK
>  - initialized press with 0, as this value means no touch.
> press has to be initialized because compiler/checkpatch complains
> that can be used uninitialized.
>  - changed of_property_read_u32 to device_*
>  - set_abs_params for pressure will have range according to threshold
>  - changed evbit and keybit with set_capability call
>  - changed variable names to error instead of ret
>  - checked errors for add_action_or_reset
>  - cosmetic cleaning
> 
> Changes in v3:
>  - pressure now reported naturally growing down-up
>  - using helpers from touchscreen.h to parse DT properties
>  - added devm_add_action_or_reset to handle callback buffer clean on exit
>  - changed compatible and threshold property to adapt to changed bindings
> 
> Changes in v2:
>  - this is now a generic resistive adc touchscreen driver
> 
>  drivers/input/touchscreen/Kconfig               |  13 ++
>  drivers/input/touchscreen/Makefile              |   1 +
>  drivers/input/touchscreen/resistive-adc-touch.c | 204 ++++++++++++++++++++++++
>  3 files changed, 218 insertions(+)
>  create mode 100644 drivers/input/touchscreen/resistive-adc-touch.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 4f15496..8f85d3a 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -92,6 +92,19 @@ config TOUCHSCREEN_AD7879_SPI
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad7879-spi.
>  
> +config TOUCHSCREEN_ADC
> +	tristate "Generic ADC based resistive touchscreen"
> +	depends on IIO
> +	select IIO_BUFFER_CB
> +	help
> +	  Say Y here if you want to use the generic ADC
> +	  resistive touchscreen driver.
> +
> +	  If unsure, say N (but it's safe to say "Y").
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called resistive-adc-touch.ko.
> +
>  config TOUCHSCREEN_AR1021_I2C
>  	tristate "Microchip AR1020/1021 i2c touchscreen"
>  	depends on I2C && OF
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index dddae79..843c7f9 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7877)	+= ad7877.o
>  obj-$(CONFIG_TOUCHSCREEN_AD7879)	+= ad7879.o
>  obj-$(CONFIG_TOUCHSCREEN_AD7879_I2C)	+= ad7879-i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI)	+= ad7879-spi.o
> +obj-$(CONFIG_TOUCHSCREEN_ADC)		+= resistive-adc-touch.o
>  obj-$(CONFIG_TOUCHSCREEN_ADS7846)	+= ads7846.o
>  obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C)	+= ar1021_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT)	+= atmel_mxt_ts.o
> diff --git a/drivers/input/touchscreen/resistive-adc-touch.c b/drivers/input/touchscreen/resistive-adc-touch.c
> new file mode 100644
> index 0000000..cfc8bb4
> --- /dev/null
> +++ b/drivers/input/touchscreen/resistive-adc-touch.c
> @@ -0,0 +1,204 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ADC generic resistive touchscreen (GRTS)
> + * This is a generic input driver that connects to an ADC
> + * given the channels in device tree, and reports events to the input
> + * subsystem.
> + *
> + * Copyright (C) 2017,2018 Microchip Technology,
> + * Author: Eugen Hristev <eugen.hristev@microchip.com>
> + *
> + */
> +#include <linux/input.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME					"resistive-adc-touch"
> +#define GRTS_DEFAULT_PRESSURE_MIN			50000
> +#define GRTS_MAX_POS_MASK				GENMASK(11, 0)
> +
> +/**
> + * grts_state - generic resistive touch screen information struct
> + * @pressure_min:	number representing the minimum for the pressure
> + * @pressure:		are we getting pressure info or not
> + * @iio_chans:		list of channels acquired
> + * @iio_cb:		iio_callback buffer for the data
> + * @input:		the input device structure that we register
> + * @prop:		touchscreen properties struct
> + */
> +struct grts_state {
> +	u32				pressure_min;
> +	bool				pressure;
> +	struct iio_channel		*iio_chans;
> +	struct iio_cb_buffer		*iio_cb;
> +	struct input_dev		*input;
> +	struct touchscreen_properties	prop;
> +};
> +
> +static int grts_cb(const void *data, void *private)
> +{
> +	const u16 *touch_info = data;
> +	struct grts_state *st = private;
> +	unsigned int x, y, press = 0x0;
> +
> +	/* channel data coming in buffer in the order below */
> +	x = touch_info[0];
> +	y = touch_info[1];
> +	if (st->pressure)
> +		press = touch_info[2];
> +
> +	if ((!x && !y) || (st->pressure && (press < st->pressure_min))) {
> +		/* report end of touch */
> +		input_report_key(st->input, BTN_TOUCH, 0);
> +		input_sync(st->input);
> +		return 0;
> +	}
> +
> +	/* report proper touch to subsystem*/
> +	touchscreen_report_pos(st->input, &st->prop, x, y, false);
> +	if (st->pressure)
> +		input_report_abs(st->input, ABS_PRESSURE, press);
> +	input_report_key(st->input, BTN_TOUCH, 1);
> +	input_sync(st->input);
> +
> +	return 0;
> +}
> +
> +static int grts_open(struct input_dev *dev)
> +{
> +	int error;
> +	struct grts_state *st = input_get_drvdata(dev);
> +
> +	error = iio_channel_start_all_cb(st->iio_cb);
> +	if (error) {
> +		dev_err(dev->dev.parent, "failed to start callback buffer.\n");
> +		return error;
> +	}
> +	return 0;
> +}
> +
> +static void grts_close(struct input_dev *dev)
> +{
> +	struct grts_state *st = input_get_drvdata(dev);
> +
> +	iio_channel_stop_all_cb(st->iio_cb);
> +}
> +
> +static void grts_disable(void *data)
> +{
> +	iio_channel_release_all_cb(data);
> +}
> +
> +static int grts_probe(struct platform_device *pdev)
> +{
> +	struct grts_state *st;
> +	struct input_dev *input;
> +	struct device *dev = &pdev->dev;
> +	struct iio_channel *chan;
> +	int error;
> +
> +	st = devm_kzalloc(dev, sizeof(struct grts_state), GFP_KERNEL);
> +	if (!st)
> +		return -ENOMEM;
> +
> +	/* get the channels from IIO device */
> +	st->iio_chans = devm_iio_channel_get_all(dev);
> +	if (IS_ERR(st->iio_chans)) {
> +		error = PTR_ERR(st->iio_chans);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(dev, "can't get iio channels.\n");
> +		return error;
> +	}
> +
> +	chan = &st->iio_chans[0];
> +	st->pressure = false;
> +	while (chan && chan->indio_dev) {
> +		if (!strcmp(chan->channel->datasheet_name, "pressure"))
> +			st->pressure = true;
> +		chan++;
> +	}
> +
> +	if (st->pressure) {
> +		error = device_property_read_u32(dev,
> +						 "touchscreen-min-pressure",
> +						 &st->pressure_min);
> +		if (error) {
> +			dev_dbg(dev, "can't get touchscreen-min-pressure property.\n");
> +			st->pressure_min = GRTS_DEFAULT_PRESSURE_MIN;
> +		}
> +	}
> +
> +	input = devm_input_allocate_device(dev);
> +	if (!input) {
> +		dev_err(dev, "failed to allocate input device.\n");
> +		return -ENOMEM;
> +	}
> +
> +	input->name = DRIVER_NAME;
> +	input->id.bustype = BUS_HOST;
> +	input->open = grts_open;
> +	input->close = grts_close;
> +
> +	input_set_abs_params(input, ABS_X, 0, GRTS_MAX_POS_MASK - 1, 0, 0);
> +	input_set_abs_params(input, ABS_Y, 0, GRTS_MAX_POS_MASK - 1, 0, 0);
> +	if (st->pressure)
> +		input_set_abs_params(input, ABS_PRESSURE, st->pressure_min,
> +				     0xffff, 0, 0);
> +
> +	input_set_capability(input, EV_KEY, BTN_TOUCH);
> +
> +	/* parse optional device tree properties */
> +	touchscreen_parse_properties(input, false, &st->prop);
> +
> +	st->input = input;
> +	input_set_drvdata(input, st);
> +
> +	error = input_register_device(input);
> +	if (error) {
> +		dev_err(dev, "failed to register input device.");
> +		return error;
> +	}
> +
> +	st->iio_cb = iio_channel_get_all_cb(dev, grts_cb, st);
> +	if (IS_ERR(st->iio_cb)) {
> +		dev_err(dev, "failed to allocate callback buffer.\n");
> +		return PTR_ERR(st->iio_cb);
> +	}
> +
> +	error = devm_add_action_or_reset(dev, grts_disable, st->iio_cb);
> +	if (error) {
> +		dev_err(dev, "failed to add disable action.\n");
> +		return error;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id grts_of_match[] = {
> +	{
> +		.compatible = "resistive-adc-touch",
> +	}, {
> +		/* sentinel */
> +	},
> +};
> +
> +MODULE_DEVICE_TABLE(of, grts_of_match);
> +
> +static struct platform_driver grts_driver = {
> +	.probe = grts_probe,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.of_match_table = of_match_ptr(grts_of_match),
> +	},
> +};
> +
> +module_platform_driver(grts_driver);
> +
> +MODULE_AUTHOR("Eugen Hristev <eugen.hristev@microchip.com>");
> +MODULE_DESCRIPTION("Generic ADC Resistive Touch Driver");
> +MODULE_LICENSE("GPL v2");

^ permalink raw reply

* Re: [PATCH v7 5/9] iio: adc: at91-sama5d2_adc: add support for position and pressure channels
From: Jonathan Cameron @ 2018-06-10 12:32 UTC (permalink / raw)
  To: Ludovic Desroches
  Cc: Eugen Hristev, alexandre.belloni, linux-arm-kernel, devicetree,
	linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov, robh
In-Reply-To: <20180522122244.GM22971@rfolt0960.corp.atmel.com>

On Tue, 22 May 2018 14:22:44 +0200
Ludovic Desroches <ludovic.desroches@microchip.com> wrote:

> On Tue, May 22, 2018 at 10:52:35AM +0300, Eugen Hristev wrote:
> > This implements the support for position and pressure for the included
> > touchscreen support in the SAMA5D2 SOC ADC block.
> > Two position channels are added and one for pressure.
> > They can be read in raw format, or through a buffer.
> > A normal use case is for a consumer driver to register a callback buffer
> > for these channels.
> > When the touchscreen channels are in the active scan mask,
> > the driver will start the touchscreen sampling and push the data to the
> > buffer.
> > 
> > Some parts of this patch are based on initial original work by
> > Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
> > 
> > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>  
> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

Applied, thanks.

Jonathan
> 
> > ---
> > Changes in v6:
> >  - fixed a crash when issuing buffer enable from sysfs, if no trigger was
> > previously configured. This is because now the driver can work in software
> > buffer mode (to connect the callback buffer). So, when trying to enable the
> > buffer, check if we are going indeed to a triggered mode or not. If not, do
> > not allow buffer to be started (we do not have the right trigger).
> > It's in buffer_postenable and predisable.
> > 
> > Changes in v4:
> >  - use return value of at91_adc_configure_touch
> >  - rewrote some part of the read_info_raw according to Jonathan's
> > suggestion
> > 
> > Changes in v3:
> >  - prefix macros with AT91_SAMA5D2
> >  - reworked the x_pos and y_pos functions into a single one with two
> > additional wrappers
> >  - reworked pressure report to have it grow naturally and not top down
> >  - fixed some checks regarding IIO_VOLTAGE as suggested
> >  - added a comment explaining some code in trigger handling
> >  - reworked the frequency get handler to use the saved value instead of
> > reading it from the hardware.
> >  - added comment on deffered work queueing
> >  - pulled out INFO_RAW function into a separate utility function as suggested
> >  - added iio_dev ops structure at all times . The functions are needed in
> > case we do not have a hardware trigger attached, but we want to use the
> > consumer touchscreen driver, thus a callback buffer is attached. Then we still
> > need to have buffer preenable and postdisable to configure the touch IRQs (etc.)
> > 
> > Changes in v2:
> >  - the support is now based on callback buffer.
> > 
> >  drivers/iio/adc/at91-sama5d2_adc.c | 609 +++++++++++++++++++++++++++++++++----
> >  1 file changed, 551 insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> > index 8729d65..58c4c2b 100644
> > --- a/drivers/iio/adc/at91-sama5d2_adc.c
> > +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> > @@ -102,14 +102,26 @@
> >  #define AT91_SAMA5D2_LCDR	0x20
> >  /* Interrupt Enable Register */
> >  #define AT91_SAMA5D2_IER	0x24
> > +/* Interrupt Enable Register - TS X measurement ready */
> > +#define AT91_SAMA5D2_IER_XRDY   BIT(20)
> > +/* Interrupt Enable Register - TS Y measurement ready */
> > +#define AT91_SAMA5D2_IER_YRDY   BIT(21)
> > +/* Interrupt Enable Register - TS pressure measurement ready */
> > +#define AT91_SAMA5D2_IER_PRDY   BIT(22)
> >  /* Interrupt Enable Register - general overrun error */
> >  #define AT91_SAMA5D2_IER_GOVRE BIT(25)
> > +/* Interrupt Enable Register - Pen detect */
> > +#define AT91_SAMA5D2_IER_PEN    BIT(29)
> > +/* Interrupt Enable Register - No pen detect */
> > +#define AT91_SAMA5D2_IER_NOPEN  BIT(30)
> >  /* Interrupt Disable Register */
> >  #define AT91_SAMA5D2_IDR	0x28
> >  /* Interrupt Mask Register */
> >  #define AT91_SAMA5D2_IMR	0x2c
> >  /* Interrupt Status Register */
> >  #define AT91_SAMA5D2_ISR	0x30
> > +/* Interrupt Status Register - Pen touching sense status */
> > +#define AT91_SAMA5D2_ISR_PENS   BIT(31)
> >  /* Last Channel Trigger Mode Register */
> >  #define AT91_SAMA5D2_LCTMR	0x34
> >  /* Last Channel Compare Window Register */
> > @@ -131,8 +143,38 @@
> >  #define AT91_SAMA5D2_CDR0	0x50
> >  /* Analog Control Register */
> >  #define AT91_SAMA5D2_ACR	0x94
> > +/* Analog Control Register - Pen detect sensitivity mask */
> > +#define AT91_SAMA5D2_ACR_PENDETSENS_MASK        GENMASK(1, 0)
> > +
> >  /* Touchscreen Mode Register */
> >  #define AT91_SAMA5D2_TSMR	0xb0
> > +/* Touchscreen Mode Register - No touch mode */
> > +#define AT91_SAMA5D2_TSMR_TSMODE_NONE           0
> > +/* Touchscreen Mode Register - 4 wire screen, no pressure measurement */
> > +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_NO_PRESS 1
> > +/* Touchscreen Mode Register - 4 wire screen, pressure measurement */
> > +#define AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS    2
> > +/* Touchscreen Mode Register - 5 wire screen */
> > +#define AT91_SAMA5D2_TSMR_TSMODE_5WIRE          3
> > +/* Touchscreen Mode Register - Average samples mask */
> > +#define AT91_SAMA5D2_TSMR_TSAV_MASK             GENMASK(5, 4)
> > +/* Touchscreen Mode Register - Average samples */
> > +#define AT91_SAMA5D2_TSMR_TSAV(x)               ((x) << 4)
> > +/* Touchscreen Mode Register - Touch/trigger frequency ratio mask */
> > +#define AT91_SAMA5D2_TSMR_TSFREQ_MASK           GENMASK(11, 8)
> > +/* Touchscreen Mode Register - Touch/trigger frequency ratio */
> > +#define AT91_SAMA5D2_TSMR_TSFREQ(x)             ((x) << 8)
> > +/* Touchscreen Mode Register - Pen Debounce Time mask */
> > +#define AT91_SAMA5D2_TSMR_PENDBC_MASK           GENMASK(31, 28)
> > +/* Touchscreen Mode Register - Pen Debounce Time */
> > +#define AT91_SAMA5D2_TSMR_PENDBC(x)            ((x) << 28)
> > +/* Touchscreen Mode Register - No DMA for touch measurements */
> > +#define AT91_SAMA5D2_TSMR_NOTSDMA               BIT(22)
> > +/* Touchscreen Mode Register - Disable pen detection */
> > +#define AT91_SAMA5D2_TSMR_PENDET_DIS            (0 << 24)
> > +/* Touchscreen Mode Register - Enable pen detection */
> > +#define AT91_SAMA5D2_TSMR_PENDET_ENA            BIT(24)
> > +
> >  /* Touchscreen X Position Register */
> >  #define AT91_SAMA5D2_XPOSR	0xb4
> >  /* Touchscreen Y Position Register */
> > @@ -151,6 +193,12 @@
> >  #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_FALL 2
> >  /* Trigger Mode external trigger any edge */
> >  #define AT91_SAMA5D2_TRGR_TRGMOD_EXT_TRIG_ANY 3
> > +/* Trigger Mode internal periodic */
> > +#define AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC 5
> > +/* Trigger Mode - trigger period mask */
> > +#define AT91_SAMA5D2_TRGR_TRGPER_MASK           GENMASK(31, 16)
> > +/* Trigger Mode - trigger period */
> > +#define AT91_SAMA5D2_TRGR_TRGPER(x)             ((x) << 16)
> >  
> >  /* Correction Select Register */
> >  #define AT91_SAMA5D2_COSR	0xd0
> > @@ -169,6 +217,22 @@
> >  #define AT91_SAMA5D2_SINGLE_CHAN_CNT 12
> >  #define AT91_SAMA5D2_DIFF_CHAN_CNT 6
> >  
> > +#define AT91_SAMA5D2_TIMESTAMP_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> > +					 AT91_SAMA5D2_DIFF_CHAN_CNT + 1)
> > +
> > +#define AT91_SAMA5D2_TOUCH_X_CHAN_IDX (AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> > +					 AT91_SAMA5D2_DIFF_CHAN_CNT * 2)
> > +#define AT91_SAMA5D2_TOUCH_Y_CHAN_IDX   (AT91_SAMA5D2_TOUCH_X_CHAN_IDX + 1)
> > +#define AT91_SAMA5D2_TOUCH_P_CHAN_IDX   (AT91_SAMA5D2_TOUCH_Y_CHAN_IDX + 1)
> > +#define AT91_SAMA5D2_MAX_CHAN_IDX	AT91_SAMA5D2_TOUCH_P_CHAN_IDX
> > +
> > +#define AT91_SAMA5D2_TOUCH_SAMPLE_PERIOD_US          2000    /* 2ms */
> > +#define AT91_SAMA5D2_TOUCH_PEN_DETECT_DEBOUNCE_US    200
> > +
> > +#define AT91_SAMA5D2_XYZ_MASK		GENMASK(11, 0)
> > +
> > +#define AT91_SAMA5D2_MAX_POS_BITS			12
> > +
> >  /*
> >   * Maximum number of bytes to hold conversion from all channels
> >   * without the timestamp.
> > @@ -222,6 +286,37 @@
> >  		.indexed = 1,						\
> >  	}
> >  
> > +#define AT91_SAMA5D2_CHAN_TOUCH(num, name, mod)				\
> > +	{								\
> > +		.type = IIO_POSITIONRELATIVE,				\
> > +		.modified = 1,						\
> > +		.channel = num,						\
> > +		.channel2 = mod,					\
> > +		.scan_index = num,					\
> > +		.scan_type = {						\
> > +			.sign = 'u',					\
> > +			.realbits = 12,					\
> > +			.storagebits = 16,				\
> > +		},							\
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> > +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> > +		.datasheet_name = name,					\
> > +	}
> > +#define AT91_SAMA5D2_CHAN_PRESSURE(num, name)				\
> > +	{								\
> > +		.type = IIO_PRESSURE,					\
> > +		.channel = num,						\
> > +		.scan_index = num,					\
> > +		.scan_type = {						\
> > +			.sign = 'u',					\
> > +			.realbits = 12,					\
> > +			.storagebits = 16,				\
> > +		},							\
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> > +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> > +		.datasheet_name = name,					\
> > +	}
> > +
> >  #define at91_adc_readl(st, reg)		readl_relaxed(st->base + reg)
> >  #define at91_adc_writel(st, reg, val)	writel_relaxed(val, st->base + reg)
> >  
> > @@ -260,6 +355,22 @@ struct at91_adc_dma {
> >  	s64				dma_ts;
> >  };
> >  
> > +/**
> > + * at91_adc_touch - at91-sama5d2 touchscreen information struct
> > + * @sample_period_val:		the value for periodic trigger interval
> > + * @touching:			is the pen touching the screen or not
> > + * @x_pos:			temporary placeholder for pressure computation
> > + * @channels_bitmask:		bitmask with the touchscreen channels enabled
> > + * @workq:			workqueue for buffer data pushing
> > + */
> > +struct at91_adc_touch {
> > +	u16				sample_period_val;
> > +	bool				touching;
> > +	u16				x_pos;
> > +	unsigned long			channels_bitmask;
> > +	struct work_struct		workq;
> > +};
> > +
> >  struct at91_adc_state {
> >  	void __iomem			*base;
> >  	int				irq;
> > @@ -267,6 +378,7 @@ struct at91_adc_state {
> >  	struct regulator		*reg;
> >  	struct regulator		*vref;
> >  	int				vref_uv;
> > +	unsigned int			current_sample_rate;
> >  	struct iio_trigger		*trig;
> >  	const struct at91_adc_trigger	*selected_trig;
> >  	const struct iio_chan_spec	*chan;
> > @@ -275,6 +387,7 @@ struct at91_adc_state {
> >  	struct at91_adc_soc_info	soc_info;
> >  	wait_queue_head_t		wq_data_available;
> >  	struct at91_adc_dma		dma_st;
> > +	struct at91_adc_touch		touch_st;
> >  	u16				buffer[AT91_BUFFER_MAX_HWORDS];
> >  	/*
> >  	 * lock to prevent concurrent 'single conversion' requests through
> > @@ -329,8 +442,10 @@ static const struct iio_chan_spec at91_adc_channels[] = {
> >  	AT91_SAMA5D2_CHAN_DIFF(6, 7, 0x68),
> >  	AT91_SAMA5D2_CHAN_DIFF(8, 9, 0x70),
> >  	AT91_SAMA5D2_CHAN_DIFF(10, 11, 0x78),
> > -	IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_SINGLE_CHAN_CNT
> > -				+ AT91_SAMA5D2_DIFF_CHAN_CNT + 1),
> > +	IIO_CHAN_SOFT_TIMESTAMP(AT91_SAMA5D2_TIMESTAMP_CHAN_IDX),
> > +	AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_X_CHAN_IDX, "x", IIO_MOD_X),
> > +	AT91_SAMA5D2_CHAN_TOUCH(AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, "y", IIO_MOD_Y),
> > +	AT91_SAMA5D2_CHAN_PRESSURE(AT91_SAMA5D2_TOUCH_P_CHAN_IDX, "pressure"),
> >  };
> >  
> >  static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan)
> > @@ -354,6 +469,160 @@ at91_adc_chan_get(struct iio_dev *indio_dev, int chan)
> >  	return indio_dev->channels + index;
> >  }
> >  
> > +static inline int at91_adc_of_xlate(struct iio_dev *indio_dev,
> > +				    const struct of_phandle_args *iiospec)
> > +{
> > +	return at91_adc_chan_xlate(indio_dev, iiospec->args[0]);
> > +}
> > +
> > +static int at91_adc_configure_touch(struct at91_adc_state *st, bool state)
> > +{
> > +	u32 clk_khz = st->current_sample_rate / 1000;
> > +	int i = 0;
> > +	u16 pendbc;
> > +	u32 tsmr, acr;
> > +
> > +	if (!state) {
> > +		/* disabling touch IRQs and setting mode to no touch enabled */
> > +		at91_adc_writel(st, AT91_SAMA5D2_IDR,
> > +				AT91_SAMA5D2_IER_PEN | AT91_SAMA5D2_IER_NOPEN);
> > +		at91_adc_writel(st, AT91_SAMA5D2_TSMR, 0);
> > +		return 0;
> > +	}
> > +	/*
> > +	 * debounce time is in microseconds, we need it in milliseconds to
> > +	 * multiply with kilohertz, so, divide by 1000, but after the multiply.
> > +	 * round up to make sure pendbc is at least 1
> > +	 */
> > +	pendbc = round_up(AT91_SAMA5D2_TOUCH_PEN_DETECT_DEBOUNCE_US *
> > +			  clk_khz / 1000, 1);
> > +
> > +	/* get the required exponent */
> > +	while (pendbc >> i++)
> > +		;
> > +
> > +	pendbc = i;
> > +
> > +	tsmr = AT91_SAMA5D2_TSMR_TSMODE_4WIRE_PRESS;
> > +
> > +	tsmr |= AT91_SAMA5D2_TSMR_TSAV(2) & AT91_SAMA5D2_TSMR_TSAV_MASK;
> > +	tsmr |= AT91_SAMA5D2_TSMR_PENDBC(pendbc) &
> > +		AT91_SAMA5D2_TSMR_PENDBC_MASK;
> > +	tsmr |= AT91_SAMA5D2_TSMR_NOTSDMA;
> > +	tsmr |= AT91_SAMA5D2_TSMR_PENDET_ENA;
> > +	tsmr |= AT91_SAMA5D2_TSMR_TSFREQ(2) & AT91_SAMA5D2_TSMR_TSFREQ_MASK;
> > +
> > +	at91_adc_writel(st, AT91_SAMA5D2_TSMR, tsmr);
> > +
> > +	acr =  at91_adc_readl(st, AT91_SAMA5D2_ACR);
> > +	acr &= ~AT91_SAMA5D2_ACR_PENDETSENS_MASK;
> > +	acr |= 0x02 & AT91_SAMA5D2_ACR_PENDETSENS_MASK;
> > +	at91_adc_writel(st, AT91_SAMA5D2_ACR, acr);
> > +
> > +	/* Sample Period Time = (TRGPER + 1) / ADCClock */
> > +	st->touch_st.sample_period_val =
> > +				 round_up((AT91_SAMA5D2_TOUCH_SAMPLE_PERIOD_US *
> > +				 clk_khz / 1000) - 1, 1);
> > +	/* enable pen detect IRQ */
> > +	at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
> > +
> > +	return 0;
> > +}
> > +
> > +static u16 at91_adc_touch_pos(struct at91_adc_state *st, int reg)
> > +{
> > +	u32 val;
> > +	u32 scale, result, pos;
> > +
> > +	/*
> > +	 * to obtain the actual position we must divide by scale
> > +	 * and multiply with max, where
> > +	 * max = 2^AT91_SAMA5D2_MAX_POS_BITS - 1
> > +	 */
> > +	/* first half of register is the x or y, second half is the scale */
> > +	val = at91_adc_readl(st, reg);
> > +	if (!val)
> > +		dev_dbg(&iio_priv_to_dev(st)->dev, "pos is 0\n");
> > +
> > +	pos = val & AT91_SAMA5D2_XYZ_MASK;
> > +	result = (pos << AT91_SAMA5D2_MAX_POS_BITS) - pos;
> > +	scale = (val >> 16) & AT91_SAMA5D2_XYZ_MASK;
> > +	if (scale == 0) {
> > +		dev_err(&iio_priv_to_dev(st)->dev, "scale is 0\n");
> > +		return 0;
> > +	}
> > +	result /= scale;
> > +
> > +	return result;
> > +}
> > +
> > +static u16 at91_adc_touch_x_pos(struct at91_adc_state *st)
> > +{
> > +	st->touch_st.x_pos = at91_adc_touch_pos(st, AT91_SAMA5D2_XPOSR);
> > +	return st->touch_st.x_pos;
> > +}
> > +
> > +static u16 at91_adc_touch_y_pos(struct at91_adc_state *st)
> > +{
> > +	return at91_adc_touch_pos(st, AT91_SAMA5D2_YPOSR);
> > +}
> > +
> > +static u16 at91_adc_touch_pressure(struct at91_adc_state *st)
> > +{
> > +	u32 val;
> > +	u32 z1, z2;
> > +	u32 pres;
> > +	u32 rxp = 1;
> > +	u32 factor = 1000;
> > +
> > +	/* calculate the pressure */
> > +	val = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
> > +	z1 = val & AT91_SAMA5D2_XYZ_MASK;
> > +	z2 = (val >> 16) & AT91_SAMA5D2_XYZ_MASK;
> > +
> > +	if (z1 != 0)
> > +		pres = rxp * (st->touch_st.x_pos * factor / 1024) *
> > +			(z2 * factor / z1 - factor) /
> > +			factor;
> > +	else
> > +		pres = 0xFFFF;       /* no pen contact */
> > +
> > +	/*
> > +	 * The pressure from device grows down, minimum is 0xFFFF, maximum 0x0.
> > +	 * We compute it this way, but let's return it in the expected way,
> > +	 * growing from 0 to 0xFFFF.
> > +	 */
> > +	return 0xFFFF - pres;
> > +}
> > +
> > +static int at91_adc_read_position(struct at91_adc_state *st, int chan, u16 *val)
> > +{
> > +	*val = 0;
> > +	if (!st->touch_st.touching)
> > +		return -ENODATA;
> > +	if (chan == AT91_SAMA5D2_TOUCH_X_CHAN_IDX)
> > +		*val = at91_adc_touch_x_pos(st);
> > +	else if (chan == AT91_SAMA5D2_TOUCH_Y_CHAN_IDX)
> > +		*val = at91_adc_touch_y_pos(st);
> > +	else
> > +		return -ENODATA;
> > +
> > +	return IIO_VAL_INT;
> > +}
> > +
> > +static int at91_adc_read_pressure(struct at91_adc_state *st, int chan, u16 *val)
> > +{
> > +	*val = 0;
> > +	if (!st->touch_st.touching)
> > +		return -ENODATA;
> > +	if (chan == AT91_SAMA5D2_TOUCH_P_CHAN_IDX)
> > +		*val = at91_adc_touch_pressure(st);
> > +	else
> > +		return -ENODATA;
> > +
> > +	return IIO_VAL_INT;
> > +}
> > +
> >  static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> >  {
> >  	struct iio_dev *indio = iio_trigger_get_drvdata(trig);
> > @@ -375,6 +644,11 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> >  
> >  		if (!chan)
> >  			continue;
> > +		/* these channel types cannot be handled by this trigger */
> > +		if (chan->type == IIO_POSITIONRELATIVE ||
> > +		    chan->type == IIO_PRESSURE)
> > +			continue;
> > +
> >  		if (state) {
> >  			at91_adc_writel(st, AT91_SAMA5D2_CHER,
> >  					BIT(chan->channel));
> > @@ -520,7 +794,20 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev)
> >  static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> >  {
> >  	int ret;
> > +	struct at91_adc_state *st = iio_priv(indio_dev);
> >  
> > +	/* check if we are enabling triggered buffer or the touchscreen */
> > +	if (bitmap_subset(indio_dev->active_scan_mask,
> > +			  &st->touch_st.channels_bitmask,
> > +			  AT91_SAMA5D2_MAX_CHAN_IDX + 1)) {
> > +		/* touchscreen enabling */
> > +		return at91_adc_configure_touch(st, true);
> > +	}
> > +	/* if we are not in triggered mode, we cannot enable the buffer. */
> > +	if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
> > +		return -EINVAL;
> > +
> > +	/* we continue with the triggered buffer */
> >  	ret = at91_adc_dma_start(indio_dev);
> >  	if (ret) {
> >  		dev_err(&indio_dev->dev, "buffer postenable failed\n");
> > @@ -536,6 +823,18 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> >  	int ret;
> >  	u8 bit;
> >  
> > +	/* check if we are disabling triggered buffer or the touchscreen */
> > +	if (bitmap_subset(indio_dev->active_scan_mask,
> > +			  &st->touch_st.channels_bitmask,
> > +			  AT91_SAMA5D2_MAX_CHAN_IDX + 1)) {
> > +		/* touchscreen disable */
> > +		return at91_adc_configure_touch(st, false);
> > +	}
> > +	/* if we are not in triggered mode, nothing to do here */
> > +	if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
> > +		return -EINVAL;
> > +
> > +	/* continue with the triggered buffer */
> >  	ret = iio_triggered_buffer_predisable(indio_dev);
> >  	if (ret < 0)
> >  		dev_err(&indio_dev->dev, "buffer predisable failed\n");
> > @@ -558,6 +857,10 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> >  
> >  		if (!chan)
> >  			continue;
> > +		/* these channel types are virtual, no need to do anything */
> > +		if (chan->type == IIO_POSITIONRELATIVE ||
> > +		    chan->type == IIO_PRESSURE)
> > +			continue;
> >  		if (st->dma_st.dma_chan)
> >  			at91_adc_readl(st, chan->address);
> >  	}
> > @@ -622,7 +925,22 @@ static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
> >  
> >  		if (!chan)
> >  			continue;
> > -		st->buffer[i] = at91_adc_readl(st, chan->address);
> > +		/*
> > +		 * Our external trigger only supports the voltage channels.
> > +		 * In case someone requested a different type of channel
> > +		 * just put zeroes to buffer.
> > +		 * This should not happen because we check the scan mode
> > +		 * and scan mask when we enable the buffer, and we don't allow
> > +		 * the buffer to start with a mixed mask (voltage and something
> > +		 * else).
> > +		 * Thus, emit a warning.
> > +		 */
> > +		if (chan->type == IIO_VOLTAGE) {
> > +			st->buffer[i] = at91_adc_readl(st, chan->address);
> > +		} else {
> > +			st->buffer[i] = 0;
> > +			WARN(true, "This trigger cannot handle this type of channel");
> > +		}
> >  		i++;
> >  	}
> >  	iio_push_to_buffers_with_timestamp(indio_dev, st->buffer,
> > @@ -688,9 +1006,20 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
> >  
> >  static int at91_adc_buffer_init(struct iio_dev *indio)
> >  {
> > -	return devm_iio_triggered_buffer_setup(&indio->dev, indio,
> > +	struct at91_adc_state *st = iio_priv(indio);
> > +
> > +	if (st->selected_trig->hw_trig) {
> > +		return devm_iio_triggered_buffer_setup(&indio->dev, indio,
> >  			&iio_pollfunc_store_time,
> >  			&at91_adc_trigger_handler, &at91_buffer_setup_ops);
> > +	}
> > +	/*
> > +	 * we need to prepare the buffer ops in case we will get
> > +	 * another buffer attached (like a callback buffer for the touchscreen)
> > +	 */
> > +	indio->setup_ops = &at91_buffer_setup_ops;
> > +
> > +	return 0;
> >  }
> >  
> >  static unsigned at91_adc_startup_time(unsigned startup_time_min,
> > @@ -736,19 +1065,83 @@ static void at91_adc_setup_samp_freq(struct at91_adc_state *st, unsigned freq)
> >  
> >  	dev_dbg(&indio_dev->dev, "freq: %u, startup: %u, prescal: %u\n",
> >  		freq, startup, prescal);
> > +	st->current_sample_rate = freq;
> >  }
> >  
> > -static unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
> > +static inline unsigned at91_adc_get_sample_freq(struct at91_adc_state *st)
> >  {
> > -	unsigned f_adc, f_per = clk_get_rate(st->per_clk);
> > -	unsigned mr, prescal;
> > +	return st->current_sample_rate;
> > +}
> >  
> > -	mr = at91_adc_readl(st, AT91_SAMA5D2_MR);
> > -	prescal = (mr >> AT91_SAMA5D2_MR_PRESCAL_OFFSET)
> > -		  & AT91_SAMA5D2_MR_PRESCAL_MAX;
> > -	f_adc = f_per / (2 * (prescal + 1));
> > +static void at91_adc_touch_data_handler(struct iio_dev *indio_dev)
> > +{
> > +	struct at91_adc_state *st = iio_priv(indio_dev);
> > +	u8 bit;
> > +	u16 val;
> > +	int i = 0;
> >  
> > -	return f_adc;
> > +	for_each_set_bit(bit, indio_dev->active_scan_mask,
> > +			 AT91_SAMA5D2_MAX_CHAN_IDX + 1) {
> > +		struct iio_chan_spec const *chan =
> > +					 at91_adc_chan_get(indio_dev, bit);
> > +
> > +		if (chan->type == IIO_POSITIONRELATIVE)
> > +			at91_adc_read_position(st, chan->channel, &val);
> > +		else if (chan->type == IIO_PRESSURE)
> > +			at91_adc_read_pressure(st, chan->channel, &val);
> > +		else
> > +			continue;
> > +		st->buffer[i] = val;
> > +		i++;
> > +	}
> > +	/*
> > +	 * Schedule work to push to buffers.
> > +	 * This is intended to push to the callback buffer that another driver
> > +	 * registered. We are still in a handler from our IRQ. If we push
> > +	 * directly, it means the other driver has it's callback called
> > +	 * from our IRQ context. Which is something we better avoid.
> > +	 * Let's schedule it after our IRQ is completed.
> > +	 */
> > +	schedule_work(&st->touch_st.workq);
> > +}
> > +
> > +static void at91_adc_pen_detect_interrupt(struct at91_adc_state *st)
> > +{
> > +	at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_PEN);
> > +	at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_NOPEN |
> > +			AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> > +			AT91_SAMA5D2_IER_PRDY);
> > +	at91_adc_writel(st, AT91_SAMA5D2_TRGR,
> > +			AT91_SAMA5D2_TRGR_TRGMOD_PERIODIC |
> > +			AT91_SAMA5D2_TRGR_TRGPER(st->touch_st.sample_period_val));
> > +	st->touch_st.touching = true;
> > +}
> > +
> > +static void at91_adc_no_pen_detect_interrupt(struct at91_adc_state *st)
> > +{
> > +	struct iio_dev *indio_dev = iio_priv_to_dev(st);
> > +
> > +	at91_adc_writel(st, AT91_SAMA5D2_TRGR,
> > +			AT91_SAMA5D2_TRGR_TRGMOD_NO_TRIGGER);
> > +	at91_adc_writel(st, AT91_SAMA5D2_IDR, AT91_SAMA5D2_IER_NOPEN |
> > +			AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> > +			AT91_SAMA5D2_IER_PRDY);
> > +	st->touch_st.touching = false;
> > +
> > +	at91_adc_touch_data_handler(indio_dev);
> > +
> > +	at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_PEN);
> > +}
> > +
> > +static void at91_adc_workq_handler(struct work_struct *workq)
> > +{
> > +	struct at91_adc_touch *touch_st = container_of(workq,
> > +					struct at91_adc_touch, workq);
> > +	struct at91_adc_state *st = container_of(touch_st,
> > +					struct at91_adc_state, touch_st);
> > +	struct iio_dev *indio_dev = iio_priv_to_dev(st);
> > +
> > +	iio_push_to_buffers(indio_dev, st->buffer);
> >  }
> >  
> >  static irqreturn_t at91_adc_interrupt(int irq, void *private)
> > @@ -757,17 +1150,39 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
> >  	struct at91_adc_state *st = iio_priv(indio);
> >  	u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR);
> >  	u32 imr = at91_adc_readl(st, AT91_SAMA5D2_IMR);
> > +	u32 rdy_mask = AT91_SAMA5D2_IER_XRDY | AT91_SAMA5D2_IER_YRDY |
> > +			AT91_SAMA5D2_IER_PRDY;
> >  
> >  	if (!(status & imr))
> >  		return IRQ_NONE;
> > -
> > -	if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> > +	if (status & AT91_SAMA5D2_IER_PEN) {
> > +		/* pen detected IRQ */
> > +		at91_adc_pen_detect_interrupt(st);
> > +	} else if ((status & AT91_SAMA5D2_IER_NOPEN)) {
> > +		/* nopen detected IRQ */
> > +		at91_adc_no_pen_detect_interrupt(st);
> > +	} else if ((status & AT91_SAMA5D2_ISR_PENS) &&
> > +		   ((status & rdy_mask) == rdy_mask)) {
> > +		/* periodic trigger IRQ - during pen sense */
> > +		at91_adc_touch_data_handler(indio);
> > +	} else if (status & AT91_SAMA5D2_ISR_PENS) {
> > +		/*
> > +		 * touching, but the measurements are not ready yet.
> > +		 * read and ignore.
> > +		 */
> > +		status = at91_adc_readl(st, AT91_SAMA5D2_XPOSR);
> > +		status = at91_adc_readl(st, AT91_SAMA5D2_YPOSR);
> > +		status = at91_adc_readl(st, AT91_SAMA5D2_PRESSR);
> > +	} else if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> > +		/* triggered buffer without DMA */
> >  		disable_irq_nosync(irq);
> >  		iio_trigger_poll(indio->trig);
> >  	} else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) {
> > +		/* triggered buffer with DMA - should not happen */
> >  		disable_irq_nosync(irq);
> >  		WARN(true, "Unexpected irq occurred\n");
> >  	} else if (!iio_buffer_enabled(indio)) {
> > +		/* software requested conversion */
> >  		st->conversion_value = at91_adc_readl(st, st->chan->address);
> >  		st->conversion_done = true;
> >  		wake_up_interruptible(&st->wq_data_available);
> > @@ -775,58 +1190,97 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static int at91_adc_read_raw(struct iio_dev *indio_dev,
> > -			     struct iio_chan_spec const *chan,
> > -			     int *val, int *val2, long mask)
> > +static int at91_adc_read_info_raw(struct iio_dev *indio_dev,
> > +				  struct iio_chan_spec const *chan, int *val)
> >  {
> >  	struct at91_adc_state *st = iio_priv(indio_dev);
> >  	u32 cor = 0;
> >  	int ret;
> >  
> > -	switch (mask) {
> > -	case IIO_CHAN_INFO_RAW:
> > -		/* we cannot use software trigger if hw trigger enabled */
> > +	/*
> > +	 * Keep in mind that we cannot use software trigger or touchscreen
> > +	 * if external trigger is enabled
> > +	 */
> > +	if (chan->type == IIO_POSITIONRELATIVE) {
> >  		ret = iio_device_claim_direct_mode(indio_dev);
> >  		if (ret)
> >  			return ret;
> >  		mutex_lock(&st->lock);
> >  
> > -		st->chan = chan;
> > +		ret = at91_adc_read_position(st, chan->channel,
> > +					     (u16 *)val);
> > +		mutex_unlock(&st->lock);
> > +		iio_device_release_direct_mode(indio_dev);
> >  
> > -		if (chan->differential)
> > -			cor = (BIT(chan->channel) | BIT(chan->channel2)) <<
> > -			      AT91_SAMA5D2_COR_DIFF_OFFSET;
> > -
> > -		at91_adc_writel(st, AT91_SAMA5D2_COR, cor);
> > -		at91_adc_writel(st, AT91_SAMA5D2_CHER, BIT(chan->channel));
> > -		at91_adc_writel(st, AT91_SAMA5D2_IER, BIT(chan->channel));
> > -		at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_START);
> > -
> > -		ret = wait_event_interruptible_timeout(st->wq_data_available,
> > -						       st->conversion_done,
> > -						       msecs_to_jiffies(1000));
> > -		if (ret == 0)
> > -			ret = -ETIMEDOUT;
> > -
> > -		if (ret > 0) {
> > -			*val = st->conversion_value;
> > -			if (chan->scan_type.sign == 's')
> > -				*val = sign_extend32(*val, 11);
> > -			ret = IIO_VAL_INT;
> > -			st->conversion_done = false;
> > -		}
> > +		return ret;
> > +	}
> > +	if (chan->type == IIO_PRESSURE) {
> > +		ret = iio_device_claim_direct_mode(indio_dev);
> > +		if (ret)
> > +			return ret;
> > +		mutex_lock(&st->lock);
> >  
> > -		at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
> > -		at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
> > +		ret = at91_adc_read_pressure(st, chan->channel,
> > +					     (u16 *)val);
> > +		mutex_unlock(&st->lock);
> > +		iio_device_release_direct_mode(indio_dev);
> >  
> > -		/* Needed to ACK the DRDY interruption */
> > -		at91_adc_readl(st, AT91_SAMA5D2_LCDR);
> > +		return ret;
> > +	}
> >  
> > -		mutex_unlock(&st->lock);
> > +	/* in this case we have a voltage channel */
> >  
> > -		iio_device_release_direct_mode(indio_dev);
> > +	ret = iio_device_claim_direct_mode(indio_dev);
> > +	if (ret)
> >  		return ret;
> > +	mutex_lock(&st->lock);
> > +
> > +	st->chan = chan;
> > +
> > +	if (chan->differential)
> > +		cor = (BIT(chan->channel) | BIT(chan->channel2)) <<
> > +		      AT91_SAMA5D2_COR_DIFF_OFFSET;
> > +
> > +	at91_adc_writel(st, AT91_SAMA5D2_COR, cor);
> > +	at91_adc_writel(st, AT91_SAMA5D2_CHER, BIT(chan->channel));
> > +	at91_adc_writel(st, AT91_SAMA5D2_IER, BIT(chan->channel));
> > +	at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_START);
> > +
> > +	ret = wait_event_interruptible_timeout(st->wq_data_available,
> > +					       st->conversion_done,
> > +					       msecs_to_jiffies(1000));
> > +	if (ret == 0)
> > +		ret = -ETIMEDOUT;
> > +
> > +	if (ret > 0) {
> > +		*val = st->conversion_value;
> > +		if (chan->scan_type.sign == 's')
> > +			*val = sign_extend32(*val, 11);
> > +		ret = IIO_VAL_INT;
> > +		st->conversion_done = false;
> > +	}
> > +
> > +	at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
> > +	at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
> > +
> > +	/* Needed to ACK the DRDY interruption */
> > +	at91_adc_readl(st, AT91_SAMA5D2_LCDR);
> > +
> > +	mutex_unlock(&st->lock);
> > +
> > +	iio_device_release_direct_mode(indio_dev);
> > +	return ret;
> > +}
> > +
> > +static int at91_adc_read_raw(struct iio_dev *indio_dev,
> > +			     struct iio_chan_spec const *chan,
> > +			     int *val, int *val2, long mask)
> > +{
> > +	struct at91_adc_state *st = iio_priv(indio_dev);
> >  
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		return at91_adc_read_info_raw(indio_dev, chan, val);
> >  	case IIO_CHAN_INFO_SCALE:
> >  		*val = st->vref_uv / 1000;
> >  		if (chan->differential)
> > @@ -974,9 +1428,29 @@ static int at91_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> >  	return 0;
> >  }
> >  
> > +static int at91_adc_update_scan_mode(struct iio_dev *indio_dev,
> > +				     const unsigned long *scan_mask)
> > +{
> > +	struct at91_adc_state *st = iio_priv(indio_dev);
> > +
> > +	if (bitmap_subset(scan_mask, &st->touch_st.channels_bitmask,
> > +			  AT91_SAMA5D2_MAX_CHAN_IDX + 1))
> > +		return 0;
> > +	/*
> > +	 * if the new bitmap is a combination of touchscreen and regular
> > +	 * channels, then we are not fine
> > +	 */
> > +	if (bitmap_intersects(&st->touch_st.channels_bitmask, scan_mask,
> > +			      AT91_SAMA5D2_MAX_CHAN_IDX + 1))
> > +		return -EINVAL;
> > +	return 0;
> > +}
> > +
> >  static const struct iio_info at91_adc_info = {
> >  	.read_raw = &at91_adc_read_raw,
> >  	.write_raw = &at91_adc_write_raw,
> > +	.update_scan_mode = &at91_adc_update_scan_mode,
> > +	.of_xlate = &at91_adc_of_xlate,
> >  	.hwfifo_set_watermark = &at91_adc_set_watermark,
> >  };
> >  
> > @@ -1044,13 +1518,20 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  
> >  	indio_dev->dev.parent = &pdev->dev;
> >  	indio_dev->name = dev_name(&pdev->dev);
> > -	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> >  	indio_dev->info = &at91_adc_info;
> >  	indio_dev->channels = at91_adc_channels;
> >  	indio_dev->num_channels = ARRAY_SIZE(at91_adc_channels);
> >  
> >  	st = iio_priv(indio_dev);
> >  
> > +	bitmap_set(&st->touch_st.channels_bitmask,
> > +		   AT91_SAMA5D2_TOUCH_X_CHAN_IDX, 1);
> > +	bitmap_set(&st->touch_st.channels_bitmask,
> > +		   AT91_SAMA5D2_TOUCH_Y_CHAN_IDX, 1);
> > +	bitmap_set(&st->touch_st.channels_bitmask,
> > +		   AT91_SAMA5D2_TOUCH_P_CHAN_IDX, 1);
> > +
> >  	ret = of_property_read_u32(pdev->dev.of_node,
> >  				   "atmel,min-sample-rate-hz",
> >  				   &st->soc_info.min_sample_rate);
> > @@ -1100,6 +1581,7 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  
> >  	init_waitqueue_head(&st->wq_data_available);
> >  	mutex_init(&st->lock);
> > +	INIT_WORK(&st->touch_st.workq, at91_adc_workq_handler);
> >  
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res)
> > @@ -1159,13 +1641,13 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, indio_dev);
> >  
> > -	if (st->selected_trig->hw_trig) {
> > -		ret = at91_adc_buffer_init(indio_dev);
> > -		if (ret < 0) {
> > -			dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
> > -			goto per_clk_disable_unprepare;
> > -		}
> > +	ret = at91_adc_buffer_init(indio_dev);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
> > +		goto per_clk_disable_unprepare;
> > +	}
> >  
> > +	if (st->selected_trig->hw_trig) {
> >  		ret = at91_adc_trigger_init(indio_dev);
> >  		if (ret < 0) {
> >  			dev_err(&pdev->dev, "couldn't setup the triggers.\n");
> > @@ -1272,9 +1754,20 @@ static __maybe_unused int at91_adc_resume(struct device *dev)
> >  	at91_adc_hw_init(st);
> >  
> >  	/* reconfiguring trigger hardware state */
> > -	if (iio_buffer_enabled(indio_dev))
> > -		at91_adc_configure_trigger(st->trig, true);
> > +	if (!iio_buffer_enabled(indio_dev))
> > +		return 0;
> > +
> > +	/* check if we are enabling triggered buffer or the touchscreen */
> > +	if (bitmap_subset(indio_dev->active_scan_mask,
> > +			  &st->touch_st.channels_bitmask,
> > +			  AT91_SAMA5D2_MAX_CHAN_IDX + 1)) {
> > +		/* touchscreen enabling */
> > +		return at91_adc_configure_touch(st, true);
> > +	} else {
> > +		return at91_adc_configure_trigger(st->trig, true);
> > +	}
> >  
> > +	/* not needed but more explicit */
> >  	return 0;
> >  
> >  vref_disable_resume:
> > -- 
> > 2.7.4
> >   

^ permalink raw reply

* Re: [PATCH v7 4/9] dt-bindings: input: touchscreen: resistive-adc-touch: create bindings
From: Jonathan Cameron @ 2018-06-10 12:32 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: ludovic.desroches, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov, robh
In-Reply-To: <1526975559-18966-5-git-send-email-eugen.hristev@microchip.com>

On Tue, 22 May 2018 10:52:34 +0300
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> Added bindings for generic resistive touchscreen ADC.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Applied, thanks

Jonathan
> ---
> Changes in v5:
>  - changed property name touchscreen-threshold-pressure to
> touchscreen-min-pressure
> 
> Changes in v3:
>  - renamed file and compatible to exclude "generic" keyword
>  - removed the pressure threshold property, added it as a common
> touchscreen property in the touchscreen common bindings in a separate
> commit.
> 
> Changes in v2:
>  - modified bindings to have a generic resistive touchscreen adc driver
> instead of specific architecture one.
> 
>  .../input/touchscreen/resistive-adc-touch.txt      | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt b/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
> new file mode 100644
> index 0000000..51456c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
> @@ -0,0 +1,30 @@
> +Generic resistive touchscreen ADC
> +
> +Required properties:
> +
> + - compatible: must be "resistive-adc-touch"
> +The device must be connected to an ADC device that provides channels for
> +position measurement and optional pressure.
> +Refer to ../iio/iio-bindings.txt for details
> + - iio-channels: must have at least two channels connected to an ADC device.
> +These should correspond to the channels exposed by the ADC device and should
> +have the right index as the ADC device registers them. These channels
> +represent the relative position on the "x" and "y" axes.
> + - iio-channel-names: must have all the channels' names. Mandatory channels
> +are "x" and "y".
> +
> +Optional properties:
> + - iio-channels: The third channel named "pressure" is optional and can be
> +used if the ADC device also measures pressure besides position.
> +If this channel is missing, pressure will be ignored and the touchscreen
> +will only report position.
> + - iio-channel-names: optional channel named "pressure".
> +
> +Example:
> +
> +	resistive_touch: resistive_touch {
> +		compatible = "resistive-adc-touch";
> +		touchscreen-min-pressure = <50000>;
> +		io-channels = <&adc 24>, <&adc 25>, <&adc 26>;
> +		io-channel-names = "x", "y", "pressure";
> +	};

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox