On 14 July 2015 at 13:07, Mark Brown broonie@kernel.org wrote:
On Tue, Jul 14, 2015 at 09:34:22AM +0200, Tomeu Vizoso wrote:
On 13 July 2015 at 17:42, Mark Brown broonie@kernel.org wrote:
No, I'm looking at how we already have all the "did all my dependencies appear" logic in the core based on data provided by the drivers.
Sorry, but I still don't get what you mean.
I'm not sure how I can be clearer here... you're replacing something that is currently pure data with open coding in each device. That seems like a step back in terms of ease of use.
I could understand that if snd_soc_dai_link had a field with the property name, and the core called of_parse_phandle on it, but currently what I'm duplicating is:
tegra_max98090_dai.cpu_of_node = of_parse_phandle(np, "nvidia,i2s-controller", 0);
with:
add_dependency(fwnode, "nvidia,i2s-controller", deps);
Admittedly, we could add a cpu_fw_property field to snd_soc_dai_link and have the core call of_parse_phandle itself.
But even then, the core doesn't know about a device's snd_soc_dai_link until probe() is called and then it's too late for the purposes of this series.
That's why I mentioned devm_probe, as it would add a common way to specify the data needed to acquire resources in each driver, which could be made available before probe() is called.
From the proof of concept that Arnd sent in
https://lkml.kernel.org/g/4742258.TBitC3hVuO@wuerfel :
struct foo_priv { spinlock_t lock; void __iomem *regs; int irq; struct gpio_desc *gpio; struct dma_chan *rxdma; struct dma_chan *txdma; bool oldstyle_dma; };
/* * 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), {}, };
Thanks,
Tomeu
Information about dependencies is currently available only after probe() starts executing, and used to decide whether we want to defer the probe.
The goal of this series is to eliminate most or all of the deferred probes by checking that all dependencies are available before probe() is called.
Right, but the way it does this is by moving code out of the core into the drivers - currently drivers just tell the core what resources to look up and the core then makes sure that they're all present.
I thought you were pointing out that the property names would be duplicated, once in the probe() implementation and also in the implementation of the get_dependencies callback.
Yes, that is another part of issue with this approach - drivers now have to specify things twice, once for this new interface and once for actually looking things up. That doesn't seem awesome and adding the code into the individual drivers and then having to pull it out again when the redundancy is removed is going to be an enormous amount of churn.
A way to consolidate the code and remove that duplication would be having a declarative API for expressing dependencies, which could be used for both fetching dependencies and for preventing deferred probes. That's why I mentioned devm_probe.
Part of what I'm saying here is that in ASoC we already have (at least as far as the individual drivers are concerned) a declarative way of specifying dependencies. This new code should be able to make use of that, if it can't and especially if none of the code can be shared between drivers then that seems like the interface needs another spin.
I've not seen this devm_probe() code but the name sounds worryingly like it might be fixing the wrong problem :/