On Wed, Apr 15, 2020 at 12:26:57PM -0500, Pierre-Louis Bossart wrote:

> > You have the opportunity to run whatever code you want to run at the
> > point where you're registering your drivers with the system on module
> > init, things like DMI quirk tables (which is what you're going to need
> > to do here AFAICT) should work just as well there as they do later on
> > when the driver loads.

> The idea here was to have one single build, and then rely on what the user
> configured with initrd override to probe the right I2C codec driver and
> indirectly the machine driver. It's similar to device tree overlays.

> With the same up2 board, I change the .aml file in
> /lib/firmware/acpi-upgrades, swap one HAT board for another and the new
> board is handled automagically.

> I don't see how I can use hard-coded DMI tables or board-specific things
> without losing the single build?

Ugh, so you change for another machine description and don't update any
of the DMI information?  Perhaps you can't...  That doesn't seem very
ACPI given how reliant it is on doing quirks based on DMI data for
modern systems :/

> > The clkdev stuff can use dev_name() so so long as the devices appear
> > with predictable names you should be fine.  If not IIRC everything in
> > ACPI is named in the AML so clkdev could be extended to be able to find
> > things based on the names it gives.

> I had a discussion with Andy Shevchenko that we should precisely not be
> using dev_name() since we don't control the names that ACPI selects for us.

It's not ideal and you should definitely use something better if it's
available but if it's all you've got...  You could also search for the
device by binding string at runtime, that'd blow up if you've got more
than one of the same device but it's a bit less likely.

> And since I was using the generic PRP0001 thing for the clock device to
> probe using the 'compatible' string it's actually even less reliable and
> unique...

I see, though actually that might be a place to set up links from now I
think about it - if you know what the I2C device is going to be called
you could have the platform device for the clock source register the
links too.

> > If existing usages that have ended up getting merged are going to be
> > used to push for additional adoption then that's not encouraging.

> I wasn't trying to push this against your will, rather I wanted to highlight
> that we should be clear on the direction for all these uses of the clk API
> in an ACPI context. If what I suggested here is not the right path, then how
> do we deal with all the existing cases? This PCM512x use is not a mainstream
> usage, we use this board mainly for validation and for community support,
> the other cases with 'mclk' and 'sspX_fsync' are critical and impact devices
> shipping in large volumes.

The ideal thing would of course be to extend ACPI to encode things like
this in the firmware description so that it is able to reflect the
reality of modern systems, the graph bits of this were already specified
so most of the work has been done.  However it's a bit late for
the shipping systems.

Normal shipping systems are in a lot better position here in that they 
do have some hope of being able to patch up the links after the fact
with DMI based quirks.  It really would be good to see a way of doing
that deployed, especially in cases where you might otherwise have to
modify the CODEC driver.  I *think* that should be doable, assuming
there's some stable or runtime discoverable naming for the ACPI devices.

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.

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 think by now there's ample evidence that it's worth investing in
better firmware descriptions :(