All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Sverdlin <alexander.sverdlin@nsn.com>
To: ext Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
	Ioan Nicu <ioan.nicu.ext@nsn.com>
Cc: Grant Likely <grant.likely@secretlab.ca>,
	Rob Herring <robherring2@gmail.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Matt Porter <matt.porter@linaro.org>,
	Koen Kooi <koen@dominion.thruhere.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alison Chaiken <Alison_Chaiken@mentor.com>,
	Dinh Nguyen <dinh.linux@gmail.com>, Jan Lubbe <jluebbe@lasnet.de>,
	Michael Stickel <ms@mycable.de>,
	Guenter Roeck <linux@roeck-us.net>,
	Dirk Behme <dirk.behme@gmail.com>,
	Alan Tull <delicious.quinoa@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Michael Bohan <mbohan@codeaurora.org>,
	Michal Simek <monstr@monstr.eu>,
	Matt Ranostay <mranostay@gmail.com>,
	Joel Becker <jlbec@evilplan.org>,
	devicetree@vger.kernel.org, Wolfram Sang <wsa@the-dreams.de>,
	linux-i2c@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pete Popov <pete.popov@konsulko.com>,
	Dan Malek <dan.malek@konsulko.com>,
	Georgi Vlaev <georgi.vlaev@konsulko.com>
Subject: Re: [PATCH 5/6] OF: Utility helper functions for dynamic nodes
Date: Tue, 24 Jun 2014 10:12:52 +0200	[thread overview]
Message-ID: <53A93304.9000604@nsn.com> (raw)
In-Reply-To: <78ACBAF6-A73E-4272-8D3A-258C4B10858C@konsulko.com>

Hi!

On 23/06/14 21:13, ext Pantelis Antoniou wrote:

[...]

>>> I don't know of any place in the kernel accessing the value if prop->length==0
>>>
>>
>> We have a simple use case. We have an overlay which adds an interrupt controller.
>> If you look in drivers/of/irq.c, in of_irq_parse_raw():
>>
>> [...]
>> 	/* Now start the actual "proper" walk of the interrupt tree */
>> 	while (ipar != NULL) {
>> 		/* Now check if cursor is an interrupt-controller and if it is
>> 		 * then we are done
>> 		 */
>> 		if (of_get_property(ipar, "interrupt-controller", NULL) !=
>> 				NULL) {
>> 			pr_debug(" -> got it !\n");
>> 			return 0;
>> 		}
>> [...]
>>
>> A node is identified as an interrupt controller if it has a zero-length property
>> called "interrupt-controller" but with a non-NULL value.
>>
>> My proposed fix for this was to remove the if () condition. propn->value will be
>> allocated with kmalloc(0) which returns ZERO_SIZE_PTR which is != NULL.
>>
> 
> If that's the case, the code in irq.c is wrong.
> 
> interrupt-controller is a bool property; the correct call to use is of_property_read_bool()
> which returns true or false when the value is defined.

No, it's not bool... It's an existence of a void property. 

> The use of of_get_property is a bug here. It is perfectly valid for a property to have a
> NULL value when length = 0.

of_find_property() would be really correct in this particular case...

-- 
Best regards,
Alexander Sverdlin.

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Sverdlin <alexander.sverdlin-OYasijW0DpE@public.gmane.org>
To: ext Pantelis Antoniou
	<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
	Ioan Nicu <ioan.nicu.ext-OYasijW0DpE@public.gmane.org>
Cc: Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	Matt Porter <matt.porter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Koen Kooi
	<koen-QLwJDigV5abLmq1fohREcCpxlwaOVQ5f@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Alison Chaiken
	<Alison_Chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>,
	Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Jan Lubbe <jluebbe-H4yykcOXDpCzQB+pC5nmwQ@public.gmane.org>,
	Michael Stickel <ms-g5CePrrZ5ROELgA04lAiVw@public.gmane.org>,
	Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>,
	Dirk Behme <dirk.behme-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Alan Tull
	<delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Michael Bohan <mbohan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>,
	Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Joel Becker <jlbec-aKy9MeLSZ9dg9hUCZPvPmw@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pete Popov <pet>
Subject: Re: [PATCH 5/6] OF: Utility helper functions for dynamic nodes
Date: Tue, 24 Jun 2014 10:12:52 +0200	[thread overview]
Message-ID: <53A93304.9000604@nsn.com> (raw)
In-Reply-To: <78ACBAF6-A73E-4272-8D3A-258C4B10858C-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

Hi!

On 23/06/14 21:13, ext Pantelis Antoniou wrote:

[...]

>>> I don't know of any place in the kernel accessing the value if prop->length==0
>>>
>>
>> We have a simple use case. We have an overlay which adds an interrupt controller.
>> If you look in drivers/of/irq.c, in of_irq_parse_raw():
>>
>> [...]
>> 	/* Now start the actual "proper" walk of the interrupt tree */
>> 	while (ipar != NULL) {
>> 		/* Now check if cursor is an interrupt-controller and if it is
>> 		 * then we are done
>> 		 */
>> 		if (of_get_property(ipar, "interrupt-controller", NULL) !=
>> 				NULL) {
>> 			pr_debug(" -> got it !\n");
>> 			return 0;
>> 		}
>> [...]
>>
>> A node is identified as an interrupt controller if it has a zero-length property
>> called "interrupt-controller" but with a non-NULL value.
>>
>> My proposed fix for this was to remove the if () condition. propn->value will be
>> allocated with kmalloc(0) which returns ZERO_SIZE_PTR which is != NULL.
>>
> 
> If that's the case, the code in irq.c is wrong.
> 
> interrupt-controller is a bool property; the correct call to use is of_property_read_bool()
> which returns true or false when the value is defined.

No, it's not bool... It's an existence of a void property. 

> The use of of_get_property is a bug here. It is perfectly valid for a property to have a
> NULL value when length = 0.

of_find_property() would be really correct in this particular case...

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Sverdlin <alexander.sverdlin-OYasijW0DpE@public.gmane.org>
To: ext Pantelis Antoniou
	<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>,
	Ioan Nicu <ioan.nicu.ext-OYasijW0DpE@public.gmane.org>
Cc: Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	Matt Porter <matt.porter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Koen Kooi
	<koen-QLwJDigV5abLmq1fohREcCpxlwaOVQ5f@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Alison Chaiken
	<Alison_Chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>,
	Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Jan Lubbe <jluebbe-H4yykcOXDpCzQB+pC5nmwQ@public.gmane.org>,
	Michael Stickel <ms-g5CePrrZ5ROELgA04lAiVw@public.gmane.org>,
	Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>,
	Dirk Behme <dirk.behme-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Alan Tull
	<delicious.quinoa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Michael Bohan <mbohan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>,
	Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Joel Becker <jlbec-aKy9MeLSZ9dg9hUCZPvPmw@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pete Popov <pet
Subject: Re: [PATCH 5/6] OF: Utility helper functions for dynamic nodes
Date: Tue, 24 Jun 2014 10:12:52 +0200	[thread overview]
Message-ID: <53A93304.9000604@nsn.com> (raw)
In-Reply-To: <78ACBAF6-A73E-4272-8D3A-258C4B10858C-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

Hi!

On 23/06/14 21:13, ext Pantelis Antoniou wrote:

[...]

>>> I don't know of any place in the kernel accessing the value if prop->length==0
>>>
>>
>> We have a simple use case. We have an overlay which adds an interrupt controller.
>> If you look in drivers/of/irq.c, in of_irq_parse_raw():
>>
>> [...]
>> 	/* Now start the actual "proper" walk of the interrupt tree */
>> 	while (ipar != NULL) {
>> 		/* Now check if cursor is an interrupt-controller and if it is
>> 		 * then we are done
>> 		 */
>> 		if (of_get_property(ipar, "interrupt-controller", NULL) !=
>> 				NULL) {
>> 			pr_debug(" -> got it !\n");
>> 			return 0;
>> 		}
>> [...]
>>
>> A node is identified as an interrupt controller if it has a zero-length property
>> called "interrupt-controller" but with a non-NULL value.
>>
>> My proposed fix for this was to remove the if () condition. propn->value will be
>> allocated with kmalloc(0) which returns ZERO_SIZE_PTR which is != NULL.
>>
> 
> If that's the case, the code in irq.c is wrong.
> 
> interrupt-controller is a bool property; the correct call to use is of_property_read_bool()
> which returns true or false when the value is defined.

No, it's not bool... It's an existence of a void property. 

> The use of of_get_property is a bug here. It is perfectly valid for a property to have a
> NULL value when length = 0.

of_find_property() would be really correct in this particular case...

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2014-06-24  8:13 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-22  9:40 [PATCH 0/6] OF: Fixes preparing transactions/overlays Pantelis Antoniou
2014-06-22  9:40 ` [PATCH 1/6] of: Do not free memory at of_node_release Pantelis Antoniou
2014-06-24 14:10   ` Grant Likely
2014-06-24 14:23     ` Pantelis Antoniou
2014-06-24 20:21       ` Grant Likely
2014-06-24 20:23         ` Pantelis Antoniou
2014-06-24 20:33           ` Guenter Roeck
2014-06-24 21:02             ` Pantelis Antoniou
2014-06-24 23:20               ` Guenter Roeck
2014-06-25 19:24           ` Grant Likely
2014-06-24 14:53     ` Guenter Roeck
2014-06-22  9:40 ` [PATCH 2/6] OF: Add [__]of_find_node_by_full_name Pantelis Antoniou
2014-06-23 17:58   ` Guenter Roeck
2014-06-23 18:00     ` Pantelis Antoniou
2014-06-24 13:55       ` Grant Likely
2014-06-24 13:55         ` Grant Likely
2014-06-24 13:55         ` Grant Likely
2014-06-24 14:12   ` Grant Likely
2014-06-24 14:23     ` Pantelis Antoniou
2014-06-22  9:40 ` [PATCH 3/6] of: platform: Fix and export of_platform_device_destroy Pantelis Antoniou
2014-06-24 14:23   ` Grant Likely
2014-06-24 15:13   ` Grant Likely
2014-06-22  9:40 ` [PATCH 4/6] OF: Export a number of __of_* methods Pantelis Antoniou
2014-06-24 19:27   ` Grant Likely
2014-06-24 19:38     ` Pantelis Antoniou
2014-06-24 20:19       ` Grant Likely
2014-06-22  9:40 ` [PATCH 5/6] OF: Utility helper functions for dynamic nodes Pantelis Antoniou
2014-06-23 16:26   ` Alexander Sverdlin
2014-06-23 16:26     ` Alexander Sverdlin
2014-06-23 16:26     ` Alexander Sverdlin
2014-06-23 16:57     ` Pantelis Antoniou
2014-06-23 18:33       ` Ioan Nicu
2014-06-23 19:13         ` Pantelis Antoniou
2014-06-23 19:48           ` Guenter Roeck
2014-06-23 19:48             ` Guenter Roeck
2014-06-23 19:48             ` Guenter Roeck
2014-06-23 20:39             ` Ioan Nicu
2014-06-24  9:08             ` Pantelis Antoniou
2014-06-24 13:46               ` Rob Herring
2014-06-24 13:53                 ` Alexander Sverdlin
2014-06-24 14:49                   ` Rob Herring
2014-06-24 15:43                     ` Alexander Sverdlin
2014-06-24 15:59                       ` Pantelis Antoniou
2014-06-24 18:23                         ` Ioan Nicu
2014-06-24 18:31                           ` Ioan Nicu
2014-06-24 18:43                           ` Pantelis Antoniou
2014-06-24  8:12           ` Alexander Sverdlin [this message]
2014-06-24  8:12             ` Alexander Sverdlin
2014-06-24  8:12             ` Alexander Sverdlin
2014-06-24  8:19             ` Pantelis Antoniou
2014-06-24  8:38               ` Alexander Sverdlin
2014-06-24  8:54                 ` Pantelis Antoniou
2014-06-24  9:00                   ` Alexander Sverdlin
2014-06-24  9:09                     ` Pantelis Antoniou
2014-06-24  8:10         ` Alexander Sverdlin
2014-06-24  8:10           ` Alexander Sverdlin
2014-06-24  8:10           ` Alexander Sverdlin
2014-06-25  9:09           ` Grant Likely
2014-06-25  9:09             ` Grant Likely
2014-06-25  9:09             ` Grant Likely
2014-06-25 11:14   ` Grant Likely
2014-06-25 11:22     ` Pantelis Antoniou
2014-06-22  9:40 ` [PATCH 6/6] of: Introduce tree change __foo_post methods Pantelis Antoniou

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=53A93304.9000604@nsn.com \
    --to=alexander.sverdlin@nsn.com \
    --cc=Alison_Chaiken@mentor.com \
    --cc=broonie@kernel.org \
    --cc=dan.malek@konsulko.com \
    --cc=delicious.quinoa@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dinh.linux@gmail.com \
    --cc=dirk.behme@gmail.com \
    --cc=georgi.vlaev@konsulko.com \
    --cc=grant.likely@secretlab.ca \
    --cc=gregkh@linuxfoundation.org \
    --cc=ioan.nicu.ext@nsn.com \
    --cc=jlbec@evilplan.org \
    --cc=jluebbe@lasnet.de \
    --cc=koen@dominion.thruhere.net \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=matt.porter@linaro.org \
    --cc=mbohan@codeaurora.org \
    --cc=monstr@monstr.eu \
    --cc=mranostay@gmail.com \
    --cc=ms@mycable.de \
    --cc=pantelis.antoniou@konsulko.com \
    --cc=pete.popov@konsulko.com \
    --cc=robherring2@gmail.com \
    --cc=s.hauer@pengutronix.de \
    --cc=swarren@wwwdotorg.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

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

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