Re: [PATCH 2/4] ACPI / PMIC: Add byt prefix to Crystal Cove PMIC OpRegion driver

From: Hans de Goede
Date: Fri Oct 25 2019 - 04:59:16 EST


On 25-10-2019 09:41, Andy Shevchenko wrote:
On Thu, Oct 24, 2019 at 11:38:25PM +0200, Hans de Goede wrote:
Our current Crystal Cove OpRegion driver is only valid for the
Crystal Cove PMIC variant found on Bay Trail (BYT) boards,
Cherry Trail (CHT) based boards use another variant.

At least the regulator registers are different on CHT and these registers
are one of the things controlled by the custom PMIC OpRegion.

Commit 4d9ed62ab142 ("mfd: intel_soc_pmic: Export separate mfd-cell
configs for BYT and CHT") has disabled the intel_pmic_crc.c code for CHT
devices by removing the "crystal_cove_pmic" MFD cell on CHT devices.

This commit renames the intel_pmic_crc.c driver and the cell to be
prefixed with "byt" to indicate that this code is for BYT devices only.

This is a preparation patch for adding a separate PMIC OpRegion
driver for the CHT variant of the Crystal Cove PMIC (sometimes called
Crystal Cove Plus in Android kernel sources).

.../acpi/pmic/{intel_pmic_crc.c => intel_pmic_bytcrc.c} | 4 ++--
drivers/mfd/intel_soc_pmic_crc.c | 2 +-

I would go with previously established pattern, i.e. intel_pmic_bytcc.c.

Well that would be consistent with the chtwc for the Whiskey Cove, but
Crystal Cove related files are shortened to crc in many places already:

Filenames before this patch:

And to me "cc" stands for the Type-C cc lines, or for Cc: from email,
so IMHO it is best to stick with crc here.

+++ b/drivers/mfd/intel_soc_pmic_crc.c
@@ -75,7 +75,7 @@ static struct mfd_cell crystal_cove_byt_dev[] = {
.resources = gpio_resources,
- .name = "crystal_cove_pmic",
+ .name = "byt_crystal_cove_pmic",
.name = "crystal_cove_pwm",

I'm wondering shouldn't we rename the PWM and GPIO for the sake of consistency?
Yes, if a driver is used on both CHT and BYT, let it provide two names.

I believe it is fine to keep the blocks which are identical between
the 2 versions as just "crystal_cove_foo", but renaming them is fine with me
too, but that follows outside the scope of this series and should be
done in a follow-up series IMHO.