Re: [alsa-devel] [PATCH v2 11/12] ASoC: tegra: register dependency parser for firmware nodes

On Tue, Jul 14, 2015 at 02:47:04PM +0200, Tomeu Vizoso wrote:

On 14 July 2015 at 13:07, Mark Brown [off-list ref] wrote:

quoted

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.

Yes, we could - that's really what should be happening here.  The other
bit of this is that we're doing it twice which isn't success.

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 not a good reason to encourage bad patterns in drivers.  At the
very least the drivers should be able to pass the same struct into both
places, having to open code the same thing in two places is going to be
error prone.

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.

That does avoid the duplication.  However there are issues with the
interface for enumerable buses, it doesn't solve the problem where
embedded systems need you to power up the device manually prior to the
device actually enumerating.  If we're doing early resource acquisition
we probably want to solve that too.