linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Grant Likely <grant.likely@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Aaron Lu <aaron.lu@intel.com>,
	devicetree@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Bryan Wu <cooloney@gmail.com>,
	Darren Hart <dvhart@linux.intel.com>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
Date: Tue, 21 Oct 2014 00:22:09 +0200	[thread overview]
Message-ID: <5981363.HuOrK2kMZG@vostro.rjw.lan> (raw)
In-Reply-To: <3213594.3tCDm06Aee@wuerfel>

On Monday, October 20, 2014 04:19:57 PM Arnd Bergmann wrote:
> On Monday 20 October 2014 01:46:00 Rafael J. Wysocki wrote:
> > 
> > > Something like:
> > > 
> > > #define define_fwnode_accessors(__type, __devprop_type) \
> > > int device_property_read_##__type(struct device *dev, \
> > >                               const char *propname, __type *val) \
> > > { \
> > >       if (IS_ENABLED(CONFIG_OF) && dev->of_node) \
> > >               return of_property_read_##__type(dev->of_node, propname, val); \
> > >       return acpi_dev_prop_read(ACPI_COMPANION(dev), propname, \
> > >                                 __devprop_type, val); \
> > > } \
> > > int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
> > >                               const char *propname, __type *val) \
> > > { \
> > >       if (IS_ENABLED(CONFIG_OF) && is_of_node(fwnode)) \
> > >               return of_property_read_##__type(of_node(fwnode), propname, val); \
> > >       else if (IS_ENABLED(CONFIG_ACPI) && is_acpi_node(fwnode)) \
> > >               return acpi_dev_prop_read(acpi_node(fwnode), propname, \
> > >                                         __devprop_type, val); \
> > >       return -ENXIO; \
> > > }
> > > 
> > > define_fwnode_accessors(u8, DEV_PROP_U8);
> > > define_fwnode_accessors(u16, DEV_PROP_U16);
> > > define_fwnode_accessors(u32, DEV_PROP_U32);
> > > define_fwnode_accessors(u64, DEV_PROP_U64);
> > > 
> > > That significantly reduces the code size for these things.
> > 
> > So I was considering to do that, but eventually decided not to, because (1)
> > adding kerneldoc comments to such things looks odd and (2) (which IMO is
> > more important) this breaks LXR (for example, the thing at lxr.free-electrons.com
> > that some people, including me in particular, occasionally use to check how things
> > are defined).  And even if you used the old good grep to look for a definition of
> > fwnode_property_read_u8, say, this wouldn't work exactly as expected I'm afraid. 
> 
> Agreed, I'd also prefer your proposed code over Grant's macros.
> 
> > I would very much like to retain the headers at least for this reason, if that's
> > not a big deal.
> > 
> > What I can do, however, is to use macros for generating the bodies of those
> > functions.
> 
> Yes, just don't do any concatenation to generate the names of the called
> functions, i.e.
> 
> 	return fwnode_call(of_property_read_u32, acpi_dev_prop_read, DEV_PROP_U32, node, propname, val);
> 
> is better than
> 
> 	return fwnode_call(u32, DEV_PROP_U32, node, propname, val);
> 
> because it's easier to understand the call chain.

There is one concatenation like that in the code I have today, but it was
already present in the $subject series, in the of_dev_prop_read_array macro
in patch #2 (and it actually makes sense to me).

Rafael


  parent reply	other threads:[~2014-10-20 22:01 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-07  0:10 [PATCH v4 00/13] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
2014-10-07  0:12 ` [PATCH 01/13] ACPI: Add support for device specific properties Rafael J. Wysocki
2014-10-13 12:47   ` Grant Likely
2014-10-07  0:12 ` [PATCH 02/13] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
2014-10-07  0:13 ` [PATCH 03/13] ACPI: Allow drivers to match using Device Tree compatible property Rafael J. Wysocki
2014-10-14 13:38   ` Grant Likely
2014-10-07  0:14 ` [PATCH 04/13] ACPI: Document ACPI device specific properties Rafael J. Wysocki
2014-10-13 12:41   ` Grant Likely
2014-10-14  9:42     ` Mika Westerberg
2014-10-07  0:14 ` [PATCH 05/13] misc: at25: Make use of device property API Rafael J. Wysocki
2014-10-07  9:10   ` Geert Uytterhoeven
2014-10-07  9:32     ` Mika Westerberg
2014-10-07  0:15 ` [PATCH 06/13] gpio / ACPI: Add support for _DSD device properties Rafael J. Wysocki
2014-10-14 13:44   ` Grant Likely
2014-10-15  8:46     ` Mika Westerberg
2014-10-07  0:15 ` [PATCH 07/13] gpio: sch: Consolidate core and resume banks Rafael J. Wysocki
2014-10-07  0:16 ` [PATCH 08/13] leds: leds-gpio: Add support for GPIO descriptors Rafael J. Wysocki
2014-10-07  0:16 ` [PATCH 09/12] input: gpio_keys_polled - " Rafael J. Wysocki
2014-10-07 17:29   ` Dmitry Torokhov
2014-10-07  0:17 ` [PATCH 10/13] Driver core: Child node properties for devices Rafael J. Wysocki
2014-10-07  0:18 ` [PATCH 11/13] gpio: Support for unified device properties interface Rafael J. Wysocki
2014-10-07 10:22   ` Alexandre Courbot
2014-10-07 10:40     ` Mika Westerberg
2014-10-07 10:52       ` Alexandre Courbot
2014-10-08  0:09         ` Rafael J. Wysocki
2014-10-08  2:55           ` Alexandre Courbot
2014-10-08 14:01             ` Rafael J. Wysocki
2014-10-07  0:18 ` [PATCH 12/13] leds: leds-gpio: Make use of device property API Rafael J. Wysocki
2014-10-08 14:04   ` Rafael J. Wysocki
2014-10-08 17:47     ` Bryan Wu
2014-10-08 22:02       ` Rafael J. Wysocki
2014-10-07  0:19 ` [PATCH 13/13] input: gpio_keys_polled - " Rafael J. Wysocki
2014-10-07 17:30   ` Dmitry Torokhov
2014-10-07  0:39 ` [PATCH v4 00/13] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
2014-10-07  2:28 ` Greg Kroah-Hartman
2014-10-15 13:04 ` David Woodhouse
2014-10-15 13:15   ` Mark Rutland
2014-10-15 13:28     ` David Woodhouse
2014-10-15 13:42       ` Mark Rutland
2014-10-15 14:08         ` David Woodhouse
2014-10-15 14:46           ` Darren Hart
2014-10-15 15:11             ` David Woodhouse
2014-10-15 15:17             ` Mark Rutland
2014-10-15 15:43               ` Darren Hart
2014-10-16 10:05                 ` Rafael J. Wysocki
2014-10-16 14:55                 ` David Woodhouse
2014-10-18  8:37                   ` Grant Likely
2014-10-18  8:39                   ` Grant Likely
2014-10-18  8:35                 ` Grant Likely
2014-10-21 21:50                   ` [PATCH v4 00/13] Add ACPI _DSD and unified device properties? support Darren Hart
2015-01-14 18:42     ` [PATCH v4 00/13] Add ACPI _DSD and unified device properties support David Woodhouse
2015-01-15  9:12       ` Rafael J. Wysocki
2014-10-17 12:01 ` [PATCH v5 00/12] " Rafael J. Wysocki
2014-10-17 12:03   ` [PATCH v5 01/12] ACPI: Add support for device specific properties Rafael J. Wysocki
2014-10-17 12:04   ` [PATCH v5 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
2014-10-20  0:07     ` [Update][PATCH " Rafael J. Wysocki
2014-10-17 12:05   ` [PATCH v5 03/12] ACPI: Allow drivers to match using Device Tree compatible property Rafael J. Wysocki
2014-10-20 14:05     ` Grant Likely
2014-10-20 22:19       ` Rafael J. Wysocki
2014-10-17 12:07   ` [PATCH v5 04/12] misc: at25: Make use of device property API Rafael J. Wysocki
2014-10-17 12:09   ` [PATCH v5 05/12] gpio / ACPI: Add support for _DSD device properties Rafael J. Wysocki
2014-10-17 12:10   ` [PATCH v5 06/12] gpio: sch: Consolidate core and resume banks Rafael J. Wysocki
2014-10-17 12:11   ` [PATCH v5 07/12] leds: leds-gpio: Add support for GPIO descriptors Rafael J. Wysocki
2014-10-28 15:26     ` Linus Walleij
2014-10-28 21:56       ` Rafael J. Wysocki
2014-10-29  8:53         ` Mika Westerberg
2014-10-30 15:40           ` Linus Walleij
2014-10-30 16:15             ` Mika Westerberg
2014-10-31  9:41               ` Linus Walleij
2014-10-31  9:55                 ` Mika Westerberg
2014-10-30 15:34         ` Linus Walleij
2014-10-17 12:12   ` [PATCH v5 08/12] input: gpio_keys_polled - " Rafael J. Wysocki
2014-10-17 12:14   ` [PATCH v5 09/12] Driver core: Unified interface for firmware node properties Rafael J. Wysocki
2014-10-18  9:35     ` Arnd Bergmann
2014-10-19 23:30       ` Rafael J. Wysocki
2014-10-20 14:14         ` Arnd Bergmann
2014-10-18 14:55     ` Grant Likely
2014-10-19 23:46       ` Rafael J. Wysocki
2014-10-20 14:18         ` Grant Likely
2014-10-20 22:14           ` Rafael J. Wysocki
2014-10-20 14:19         ` Arnd Bergmann
2014-10-20 14:55           ` Grant Likely
2014-10-20 22:22           ` Rafael J. Wysocki [this message]
2014-10-19 22:14     ` Greg Kroah-Hartman
2014-10-19 23:31       ` Rafael J. Wysocki
2014-10-20  0:15     ` [Update][PATCH " Rafael J. Wysocki
2014-10-17 12:16   ` [PATCH v5 10/12] gpio: Support for unified device properties interface Rafael J. Wysocki
2014-10-17 18:09     ` Arnd Bergmann
2014-10-18  9:47       ` Arnd Bergmann
2014-10-19 23:58         ` Rafael J. Wysocki
2014-10-20 14:22           ` Arnd Bergmann
2014-10-20  6:12         ` Alexandre Courbot
2014-10-20 14:26           ` Arnd Bergmann
2014-10-17 12:17   ` [PATCH v5 11/12] leds: leds-gpio: Make use of device property API Rafael J. Wysocki
2014-10-17 12:18   ` [PATCH v5 12/12] input: gpio_keys_polled - " Rafael J. Wysocki
2014-10-17 12:22   ` [PATCH v5 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
2014-10-17 15:40   ` Greg Kroah-Hartman
2014-10-17 19:23     ` Darren Hart
2014-10-17 21:49     ` Rafael J. Wysocki
2014-10-19 22:14       ` Greg Kroah-Hartman
2014-10-17 18:04   ` Arnd Bergmann
2014-10-17 22:50     ` Rafael J. Wysocki
2014-10-18  8:49       ` Grant Likely
2014-10-19 23:32         ` Rafael J. Wysocki

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=5981363.HuOrK2kMZG@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=aaron.lu@intel.com \
    --cc=arnd@arndb.de \
    --cc=cooloney@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=dvhart@linux.intel.com \
    --cc=gnurou@gmail.com \
    --cc=grant.likely@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mika.westerberg@linux.intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).