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

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 4/14/20 2:50 PM, Mark Brown wrote:
> On Tue, Apr 14, 2020 at 02:15:16PM -0500, Pierre-Louis Bossart wrote:
>> On 4/14/20 1:27 PM, Mark Brown wrote:
> 
>>> Wait, so SCLK is in the *global* namespace and the provider has to
>>> register the same name?  That doesn't sound clever.  It might be better
>>> to have the board register the connection from the clock provider to the
>>> device rather than hard code global namespace strings like this, that
>>> sounds like a recipie for misery.
> 
>> I believe this change has zero impact on DT platforms.
> 
>> The 'sclk' is a fallback here. If you find a clock with the NULL string,
>> it's what gets used. Likewise for the clock provider, the 'sclk' is a lookup
>> - an alias in other words. The use of the references and phandles should
>> work just fine for Device Tree.
> 
> It's not just DT platforms that I'm worried about here, it's also ACPI
> systems - all it takes is for a system to have a second device and a
> name collision could happen, especially with such generic names.  We
> tried to avoid doing this for board files for the same reason.

I am on the paranoid side but here I don't see much potential for conflicts:

a) this only works for the Up2 board with a HAT connector
b) this only work with the Hifiberry DAC+ PRO board.

This codec is not used in any traditional client devices.

> 
>>> It is really sad that nobody involved in producing these systems that
>>> don't work with the current limitations in ACPI has been able to make
>>> progress on improving ACPI so it can cope with modern hardware and we're
>>> having to deal with this stuff.
> 
>> I can't disagree but I have to live with what's available to me as an audio
>> guy...I had a solution two years ago where I could set the clock directly
>> from the machine driver. The recommendation at the time was to use the clk
>> framework, but that clk framework is limited for ACPI platforms, so we can
>> only use it with these global names.
> 
> My understanding is that ACPI just doesn't have clock bindings (or audio
> bindings or...) so you're basically using board files here and board
> files can definitely do more than we're seeing here.

I don't understand your definition of board file, sorry. We've never had 
one, the only thing that's board-specific is the machine driver.

>> We had the same problem on Baytrail/Cherrytrail devices some 4 years ago and
>> we had to use an 'mclk' alias. We are going to have the same problem when we
>> expose the SSP MCLK, BLCK and FSYNC clocks - and that's also what the
>> Skylake driver did - we don't have a solution without global names.
> 
> You should be able to register links between devices using the clock
> API, or add that functionality if it's not there but AFAIK clkdev still
> works.

The machine driver has no information whatsoever on who provides the 
clock. I just don't see how I might link stuff without at least some 
amount of information?

All I needed was to toggle 2 gpios to select 44.1 or 48kHz...Looks like 
it's going to take two more years, oh well.