On 14 July 2015 at 13:07, Mark Brown <broonie@xxxxxxxxxx> wrote:
> On Tue, Jul 14, 2015 at 09:34:22AM +0200, Tomeu Vizoso wrote:
>> On 13 July 2015 at 17:42, Mark Brown <broonie@xxxxxxxxxx> 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 :/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
