On Mon, Oct 28, 2019 at 2:26 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
On 28-10-2019 13:45, Mark Brown wrote:
On Mon, Oct 28, 2019 at 02:41:46PM +0200, Adrian Hunter wrote:
On 25/10/19 10:55 AM, Andy Shevchenko wrote:
Since it's about UHS/SD, Cc to Adrian as well.
My only concern is that the driver might conflict with ACPI methods trying
to do the same thing, e.g. there is one ACPI SDHC instance from GPDWin DSDT
with code like this:
Oh, right that is a very good point.
That's certainly what's idiomatic for ACPI (though machine specific
quirks are too!). The safe thing to do would be to only register the
supply on systems where we know there's no ACPI method.
Right, so as I mentioned before Andrey told me about the evaluation
board he is using I was aware of only 3 Cherry Trail devices using
the Whiskey Cove PMIC. The GPD win, the GPD pocket and the Lenovo
Yoga book. I've checked the DSDT of all 3 and all 3 of them offer
voltage control through the Intel _DSM method for voltage control.
I've also actually tested this on the GPD win and 1.8V signalling
works fine there without needing Andrey's patch.
Thanks a lot for checking this one out! At least this proves now that
the only platform affected is in fact Intel Aero board, and the patch
as it is might not be necessary to accommodate for all CHT-based
products with Whiskey Cove.
So it seems that Andrey's patch should only be active on his
dev-board, as actual production hardware ships with the _DSM method.
I believe that the best solution is for the Whiskey Cove MFD driver:
To only register the new cell on Andrey's evaluation board model
(based in a DMI match I guess). Another option would be to do
the DMI check in the regulator driver, but that would mean
udev will needlessly modprobe the regulator driver on production
hardware, so doing it in the MFD driver and not registering the cell
I tend to lean to a solution to perform a DMI check in MFD rather than
in the regulator driver, since this would keep the regulator
more-or-less agnostic to the where it is running on.
Or maybe it would even make more sense to create a board-specific hook
(like it was suggested for vqmmc voltage and sdmmc ACPI d of
consumer), and then only register a cell for Aero match? This would
actually keep the regulator consumer and mfd cell together and would
not allow the device-specific code to leak into generic driver
implementation. In this case I'd go with mfd_add_cell() if I get a DMI
match and register a vqmmc consumer in there.
In that case, can you please tell me what you think about it and if
the drivers/acpi/acpi_lpss.c would still be an appropriate location to
put this code to?