Re: [RFC PATCH 02/16] ASoC: pcm512x: use "sclk" string to retrieve clock - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Mark Brown <broonie@kernel.org>
Cc: alsa-devel@alsa-project.org, Matthias Reichl <hias@horus.com>,
	tiwai@suse.de, Linus Walleij <linus.walleij@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Daniel Matuschek <daniel@hifiberry.com>,
	linux-clk@vger.kernel.org, Hui Wang <hui.wang@canonical.com>,
	linux-gpio@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Michael Turquette <mturquette@baylibre.com>
Subject: Re: [RFC PATCH 02/16] ASoC: pcm512x: use "sclk" string to retrieve clock
Date: Wed, 15 Apr 2020 15:22:50 -0500	[thread overview]
Message-ID: <b2db6026-c29f-0213-98d1-d8533e3159e1@linux.intel.com> (raw)
In-Reply-To: <20200415195033.GL5265@sirena.org.uk>

> In the case of this driver could you look at registering the link from
> the device for the clocks?  Have it say "I supply SCK on device X" as it
> registers.  That should be fairly straightforward I think, we do that
> for one of the regulators.

When you wrote 'in the case of this driver', were you referring to the 
clock provider, saying 'I support SCK on device i2c-104C5122:00' ?

If you have a pointer on the regulator example, I'd appreciate it, I am 
really way beyond my comfort zone.

> The main thing I want to avoid is having to have the CODEC drivers know
> platform specific strings that they're supposed to look up, or general
> approaches where that ends up being a thing that looks idiomatic.  That
> was something board files did for a while, it didn't work very well and
> we did something better with clkdev instead.  I'm a lot less worried
> about this for cases where it's two devices that are part of the SoC
> talking to each other, that's relatively well controled and doesn't
> affect non-x86 platforms.  When it starts touching the CODECs it's a lot
> more worrying.

I see the nuance, thanks for the clarification.

Maybe an alternate suggestion if you want to avoid hard-coded strings in 
the kernel: what if we added optional properties for the clock lookup 
name in both the codec and clock driver, and set the name in a _DSD 
blob. That would move the platform-specific names to platform firmware, 
and avoid the links described above that are probably ACPI-only.

In this case we can add whatever we want, the DSDT table contains 
absolutely nothing for audio so we can add things as needed, and in case 
another usage of this codec happens in a future device they'd have to 
define their own clock name and store it in platform firmware.

> I think by now there's ample evidence that it's worth investing in
> better firmware descriptions :(

Indeed, and tools to check they are correct! Most of the stuff we 
defined for SoundWire ends-up wrong or undefined, still an uphill battle...