All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH dtc] C-based DT schema checker integrated into dtc
Date: Tue, 5 Nov 2013 09:39:20 +0100	[thread overview]
Message-ID: <201311050939.21071.arnd@arndb.de> (raw)
In-Reply-To: <20131104212930.GB9638@obsidianresearch.com>

On Monday 04 November 2013, Jason Gunthorpe wrote:
> On Mon, Nov 04, 2013 at 09:43:22PM +0100, Arnd Bergmann wrote:
> 
> > /*
> >  * 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"),
> > 	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),
> > 	{},
> > };
> 
> Drivers are required to gain control of, and disable the device before
> they bind and enable things like DMA or interrupts.
> 
> At the very least the action list above needs an explicit callback to
> do that step..

I was aware of this for the interrupts, my plan for this was to either
leave the interrupt disabled when requesting it and leave it up to the
probe function to call enable_irq(), or to have the irq request function
last in the list, and preceded by a custom per-driver callback. AFAIK
this is only required for some drivers anyway, while other drivers
would function just fine when the irq is enabled early, because they
never raise an IRQ without first having something touch their registers.
Another option would be to have a flag in the driver data that lets
the irq handler know it should ignore the interrupt (or disable the source)
if the probe function has not completed yet.

What is the requirement for DMA channels? I did not expect the
dma_request_slave_channel step to have any ordering requirements.

> > static int foo_probe(struct platform_device *dev)
> > {
> > 	int ret;
> > 
> > 	ret = devm_probe(dev->dev, foo_probe_list);
> 
> Some subsystems (like net) have the core system allocate the private
> data, and some subsystem (like tpm) steal the drvdata for use in the
> core, drivers can't touch it.

I think we can handle the first case by adding a per-subsystem DEV_ALLOC
variant. For the second case, I don't see a solution other than than
changing the subsystem not to steal the driver_data pointer. I need
to find out how common this case is. If a lot of subsystems do it,
we probably want a different solution.

Thanks a lot for your insights!

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	Jon Loeliger <jdl@jdl.com>,
	khilman@linaro.org, Stephen Warren <swarren@nvidia.com>,
	s.nawrocki@samsung.com, pawel.moll@arm.com,
	Stephen Warren <swarren@wwwdotorg.org>,
	Tomasz Figa <t.figa@samsung.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	rob.herring@calxeda.com, Grant Likely <grant.likely@secretlab.ca>,
	fparent@baylibre.com, a.hajda@samsung.com,
	Benoit Cousson <bcousson@baylibre.com>,
	galak@codeaurora.org, olof@lixom.net, Alison_Chaiken@mentor.com,
	linux-arm-kernel@lists.infradead.org,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [RFC PATCH dtc] C-based DT schema checker integrated into dtc
Date: Tue, 5 Nov 2013 09:39:20 +0100	[thread overview]
Message-ID: <201311050939.21071.arnd@arndb.de> (raw)
In-Reply-To: <20131104212930.GB9638@obsidianresearch.com>

On Monday 04 November 2013, Jason Gunthorpe wrote:
> On Mon, Nov 04, 2013 at 09:43:22PM +0100, Arnd Bergmann wrote:
> 
> > /*
> >  * 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"),
> > 	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),
> > 	{},
> > };
> 
> Drivers are required to gain control of, and disable the device before
> they bind and enable things like DMA or interrupts.
> 
> At the very least the action list above needs an explicit callback to
> do that step..

I was aware of this for the interrupts, my plan for this was to either
leave the interrupt disabled when requesting it and leave it up to the
probe function to call enable_irq(), or to have the irq request function
last in the list, and preceded by a custom per-driver callback. AFAIK
this is only required for some drivers anyway, while other drivers
would function just fine when the irq is enabled early, because they
never raise an IRQ without first having something touch their registers.
Another option would be to have a flag in the driver data that lets
the irq handler know it should ignore the interrupt (or disable the source)
if the probe function has not completed yet.

What is the requirement for DMA channels? I did not expect the
dma_request_slave_channel step to have any ordering requirements.

> > static int foo_probe(struct platform_device *dev)
> > {
> > 	int ret;
> > 
> > 	ret = devm_probe(dev->dev, foo_probe_list);
> 
> Some subsystems (like net) have the core system allocate the private
> data, and some subsystem (like tpm) steal the drvdata for use in the
> core, drivers can't touch it.

I think we can handle the first case by adding a per-subsystem DEV_ALLOC
variant. For the second case, I don't see a solution other than than
changing the subsystem not to steal the driver_data pointer. I need
to find out how common this case is. If a lot of subsystems do it,
we probably want a different solution.

Thanks a lot for your insights!

	Arnd

  parent reply	other threads:[~2013-11-05  8:39 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 [this message]
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
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=201311050939.21071.arnd@arndb.de \
    --to=arnd@arndb.de \
    --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.