All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH dtc] C-based DT schema checker integrated into dtc
Date: Mon, 04 Nov 2013 14:50:38 -0700	[thread overview]
Message-ID: <527816AE.1080508@wwwdotorg.org> (raw)
In-Reply-To: <1704730.RnIqE1USnv@wuerfel>

On 11/04/2013 01:43 PM, Arnd Bergmann wrote:
> On Monday 04 November 2013 09:37:07 Stephen Warren wrote:
>>> The basic idea is to extend 'devres' to automatically register
>>> all the resources (registers, irq, dma, gpio, pinctrl, clk, regulator, ...)
>>> and simple properties before the ->probe() callback is even called,
>>> based on a per-driver data structure that describes them, and that
>>> can be easily parsed by an external tool.
>>
>> I had suggested that while talking to someone at the kernel summit,
>> basically each driver supplies functions like:
>>
>> 1) ACPI -> platform data or resources converter
>> 2) DT -> platform or resources data converter
>> 3) anything else -> platform or resources data converter
>> 4) probe()
> 
> FWIW, here is a very early draft of the interfaces I have in mind.
> At the moment the implementation is DT focused, but that should
> be extensible to ACPI if necessary.
> 
> At the end, you can see how a probe function could end up looking.
> I'm sure this is full of bugs at the moment, incomplete and needs
> to be moved into actual header and C files, but it should be enough
> to get across where I'm getting with this, and to see if anyone
> thinks it's a really bad idea or won't actually work.

This looks interesting. The simple cases end up quite simple. I wonder
what's the best way to handle the more complex cases; more on that below.

> int devm_probe_gpio(struct device *dev, const struct devm_probe *probe)
...
> #define DEVM_GPIO(_struct, _member, _index) {				 \
...
> #define DEVM_GPIO_NAMED(_struct, _member, _name) { 			 \

Should those save the GPIO flags too? Or, do we assume that gpiod-based
GPIOs already handle the flags inside the gpiod API?

> /*
>  * this lists all properties we access from the driver. The list
>  * is interpreted by devm_probe() and can be programmatically
>  * verified to match the binding.
>  */
> static const struct devm_probe foo_probe_list[] = {
> 	DEVM_ALLOC(foo_priv),
> 	DEVM_IOMAP(foo_priv, regs, 0, 0),
> 	DEVM_PROP_BOOL(foo_priv, oldstyle_dma, "foo,oldstyle-dma"),

I wonder if it makes sense to handle pure data here, or whether this
only makes sense for resources/services that are provided by some other
device?

I guess it's nice for all resources and configuration to come through to
probe() in the same way, but I rather wonder how we'd handle bindings
that have large custom (driver-specific) tables of data. Would be simply
defer such things to custom code in probe()? If so, it feels slightly
inconsistent to handle some data in the probe_list[] and some in code in
probe(). Still, I guess there's something to be said for keeping the
simple cases simple.

> 	DEVM_DMA_SLAVE(foo_priv, rxdma, "rx");
> 	DEVM_DMA_SLAVE(foo_priv, txdma, "tx");
> 	DEVM_GPIO(foo_priv, gpio, 0);
> 	DEVM_IRQ_NAMED(foo_priv, irq, foo_irq_handler, "fifo", IRQF_SHARED),

What about the case where some resources are optional, or only required
based on the values of certain other properties? I suppose that probe()
could call devm_probe() multiple times, with different probe_list[]s,
based on whatever algorithm they need.

> 	{},
> };
> 
> static int foo_probe(struct platform_device *dev)
> {
> 	int ret;
> 
> 	ret = devm_probe(dev->dev, foo_probe_list);
> 	if (ret)
> 		return ret;
> 
> 	return bar_subsystem_register(&foo_bar_ops, dev);
> }

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>,
	khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Benoit Cousson <bcousson-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	fparent-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
	Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org,
	Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
	s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org,
	Alison_Chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org,
	David Gibson
	<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Subject: Re: [RFC PATCH dtc] C-based DT schema checker integrated into dtc
Date: Mon, 04 Nov 2013 14:50:38 -0700	[thread overview]
Message-ID: <527816AE.1080508@wwwdotorg.org> (raw)
In-Reply-To: <1704730.RnIqE1USnv@wuerfel>

On 11/04/2013 01:43 PM, Arnd Bergmann wrote:
> On Monday 04 November 2013 09:37:07 Stephen Warren wrote:
>>> The basic idea is to extend 'devres' to automatically register
>>> all the resources (registers, irq, dma, gpio, pinctrl, clk, regulator, ...)
>>> and simple properties before the ->probe() callback is even called,
>>> based on a per-driver data structure that describes them, and that
>>> can be easily parsed by an external tool.
>>
>> I had suggested that while talking to someone at the kernel summit,
>> basically each driver supplies functions like:
>>
>> 1) ACPI -> platform data or resources converter
>> 2) DT -> platform or resources data converter
>> 3) anything else -> platform or resources data converter
>> 4) probe()
> 
> FWIW, here is a very early draft of the interfaces I have in mind.
> At the moment the implementation is DT focused, but that should
> be extensible to ACPI if necessary.
> 
> At the end, you can see how a probe function could end up looking.
> I'm sure this is full of bugs at the moment, incomplete and needs
> to be moved into actual header and C files, but it should be enough
> to get across where I'm getting with this, and to see if anyone
> thinks it's a really bad idea or won't actually work.

This looks interesting. The simple cases end up quite simple. I wonder
what's the best way to handle the more complex cases; more on that below.

> int devm_probe_gpio(struct device *dev, const struct devm_probe *probe)
...
> #define DEVM_GPIO(_struct, _member, _index) {				 \
...
> #define DEVM_GPIO_NAMED(_struct, _member, _name) { 			 \

Should those save the GPIO flags too? Or, do we assume that gpiod-based
GPIOs already handle the flags inside the gpiod API?

> /*
>  * this lists all properties we access from the driver. The list
>  * is interpreted by devm_probe() and can be programmatically
>  * verified to match the binding.
>  */
> static const struct devm_probe foo_probe_list[] = {
> 	DEVM_ALLOC(foo_priv),
> 	DEVM_IOMAP(foo_priv, regs, 0, 0),
> 	DEVM_PROP_BOOL(foo_priv, oldstyle_dma, "foo,oldstyle-dma"),

I wonder if it makes sense to handle pure data here, or whether this
only makes sense for resources/services that are provided by some other
device?

I guess it's nice for all resources and configuration to come through to
probe() in the same way, but I rather wonder how we'd handle bindings
that have large custom (driver-specific) tables of data. Would be simply
defer such things to custom code in probe()? If so, it feels slightly
inconsistent to handle some data in the probe_list[] and some in code in
probe(). Still, I guess there's something to be said for keeping the
simple cases simple.

> 	DEVM_DMA_SLAVE(foo_priv, rxdma, "rx");
> 	DEVM_DMA_SLAVE(foo_priv, txdma, "tx");
> 	DEVM_GPIO(foo_priv, gpio, 0);
> 	DEVM_IRQ_NAMED(foo_priv, irq, foo_irq_handler, "fifo", IRQF_SHARED),

What about the case where some resources are optional, or only required
based on the values of certain other properties? I suppose that probe()
could call devm_probe() multiple times, with different probe_list[]s,
based on whatever algorithm they need.

> 	{},
> };
> 
> static int foo_probe(struct platform_device *dev)
> {
> 	int ret;
> 
> 	ret = devm_probe(dev->dev, foo_probe_list);
> 	if (ret)
> 		return ret;
> 
> 	return bar_subsystem_register(&foo_bar_ops, dev);
> }

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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:[~2013-11-04 21:50 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-24 21:51 [RFC PATCH dtc] C-based DT schema checker integrated into dtc Stephen Warren
2013-10-24 21:51 ` Stephen Warren
2013-10-24 23:43 ` Grant Likely
2013-10-24 23:43   ` Grant Likely
2013-10-25  4:00   ` Kumar Gala
2013-10-25  4:00     ` Kumar Gala
2013-10-25 14:44   ` Stephen Warren
2013-10-25 14:44     ` Stephen Warren
2013-10-25 15:21     ` Jon Loeliger
2013-10-25 15:21       ` Jon Loeliger
2013-10-25 17:38       ` Rob Herring
2013-10-25 17:38         ` Rob Herring
2013-10-25 23:11       ` David Gibson
2013-10-25 23:11         ` David Gibson
2013-11-03 23:15         ` Tomasz Figa
2013-11-03 23:15           ` Tomasz Figa
2013-11-03 23:26           ` Tomasz Figa
2013-11-03 23:26             ` Tomasz Figa
2013-11-04  9:28             ` Arnd Bergmann
2013-11-04  9:28               ` Arnd Bergmann
2013-11-04 12:31               ` Tomasz Figa
2013-11-04 12:31                 ` Tomasz Figa
2013-11-04 16:37               ` Stephen Warren
2013-11-04 16:37                 ` Stephen Warren
2013-11-04 18:57                 ` Olof Johansson
2013-11-04 18:57                   ` Olof Johansson
2013-11-04 20:43                 ` Arnd Bergmann
2013-11-04 20:43                   ` Arnd Bergmann
2013-11-04 21:29                   ` Jason Gunthorpe
2013-11-04 21:29                     ` Jason Gunthorpe
2013-11-04 21:43                     ` Stephen Warren
2013-11-04 21:43                       ` Stephen Warren
2013-11-04 22:21                       ` Jason Gunthorpe
2013-11-04 22:21                         ` Jason Gunthorpe
2013-11-05 12:14                         ` Arnd Bergmann
2013-11-05 12:14                           ` Arnd Bergmann
2013-11-05  8:39                     ` Arnd Bergmann
2013-11-05  8:39                       ` Arnd Bergmann
2013-11-05 18:03                       ` Jason Gunthorpe
2013-11-05 18:03                         ` Jason Gunthorpe
2013-11-05 18:48                         ` Arnd Bergmann
2013-11-05 18:48                           ` Arnd Bergmann
2013-11-05 19:12                           ` Jason Gunthorpe
2013-11-05 19:12                             ` Jason Gunthorpe
2013-11-05 19:34                             ` Arnd Bergmann
2013-11-05 19:34                               ` Arnd Bergmann
2013-11-05 19:58                               ` Jason Gunthorpe
2013-11-05 19:58                                 ` Jason Gunthorpe
2013-11-05 20:17                                 ` Arnd Bergmann
2013-11-05 20:17                                   ` Arnd Bergmann
2013-11-05 20:36                                   ` Jason Gunthorpe
2013-11-05 20:36                                     ` Jason Gunthorpe
2013-11-04 21:50                   ` Stephen Warren [this message]
2013-11-04 21:50                     ` Stephen Warren
2013-11-05  8:22                     ` Arnd Bergmann
2013-11-05  8:22                       ` Arnd Bergmann
2013-11-06 12:17                   ` Thierry Reding
2013-11-06 12:17                     ` Thierry Reding
2013-11-04 14:28           ` David Gibson
2013-11-04 14:28             ` David Gibson
2013-11-04 16:42           ` Stephen Warren
2013-11-04 16:42             ` Stephen Warren
2013-10-28 10:17     ` David Gibson
2013-10-28 10:17       ` David Gibson
2013-10-31 21:13       ` Stephen Warren
2013-10-31 21:13         ` Stephen Warren
2013-11-01 13:24         ` David Gibson
2013-11-01 13:24           ` David Gibson
2013-10-25 23:29 ` David Gibson
2013-10-25 23:29   ` David Gibson
2013-10-31 21:11   ` Stephen Warren
2013-10-31 21:11     ` Stephen Warren
2013-11-10 11:00     ` David Gibson
2013-11-10 11:00       ` David Gibson
2013-11-12 22:06       ` Stephen Warren
2013-11-12 22:06         ` Stephen Warren
2013-11-13  0:33         ` David Gibson
2013-11-13  0:33           ` David Gibson

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=527816AE.1080508@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.