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

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

On 4/15/20 6:36 AM, Mark Brown wrote:
> On Tue, Apr 14, 2020 at 03:13:01PM -0500, Pierre-Louis Bossart wrote:
>> On 4/14/20 2:50 PM, Mark Brown wrote:
> 
>>> 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.
> 
> That's what you're doing right now but someone else can use the same
> devices, or adopt the same approaches on something like a Chromebook.
> 
>>> 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.
> 
> Architectures that don't have firmware bindings use straight C code to
> register and set things up.  Machine drivers are essentially board
> files, they're just audio specific bits of board file that use audio
> APIs and so are in the sound directory.

Humm, we may have a conceptual disconnect here. In the ACPI world, there 
is no support for the machine driver - unlike Device Tree. It is probed 
when the SST/SOF driver creates a platform device using the codec _HID 
as a key to hard-coded lookup tables in 
sound/soc/intel/common/soc-acpi*.c - it will be probed *after* the codec 
driver probes. I really don't see how to use the machine driver as 
currently implemented to establish board-level connections that would 
influence the codec driver probe and its use of a clock.

> 
>>> 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?
> 
> The machine driver must have this information, it knows exactly what
> hardware it runs on.  The whole point of a machine driver is that it's
> board specific.
> 
>> 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.
> 
> I think you're giving up way too easily here.  The kernel has really
> good support for systems that don't have any firmware description at
> all, this shouldn't be complex or breaking new ground.

See above, I don't think the machine driver can do what you had in mind?

I don't see how to proceed unless we remove all support for ACPI, both 
for codec and clock driver, and trigger their probe "manually" with a 
board-level initialization.

And btw there's already a precedent for using global names, it's what 
the Skylake driver does for the mclk and ssp clocks. To the best of my 
knowledge the device specific namespacing does not exist on any ACPI 
platform. We have a request from Dialog to implement the same thing for 
SOF to solve dependencies on the clock being stable before turning on 
the codec, so if global names are not acceptable we have a real problem.