| Message ID | 20191024213827.144974-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [1/4] ACPI / PMIC: Do not register handlers for unhandled OpRegions | expand |
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. > +++ 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.
On Fri, Oct 25, 2019 at 10:41:54AM +0300, 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. That said you may use mine Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > +++ 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. > > -- > With Best Regards, > Andy Shevchenko > >
Hi, 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: drivers/acpi/pmic/intel_pmic_crc.c drivers/pwm/pwm-crc.c drivers/mfd/intel_soc_pmic_crc.c 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. Regards, Hans
On Fri, Oct 25, 2019 at 10:59:06AM +0200, Hans de Goede wrote: > On 25-10-2019 09:41, Andy Shevchenko wrote: > > On Thu, Oct 24, 2019 at 11:38:25PM +0200, Hans de Goede wrote: > > 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: > drivers/acpi/pmic/intel_pmic_crc.c > drivers/pwm/pwm-crc.c > drivers/mfd/intel_soc_pmic_crc.c > > 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. Okay, let's do an exception here due to the fact the code and name already exists and spreads enough thru sources. It means you may use mine tags. > > 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. True.
On Fri, Oct 25, 2019 at 11:33 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Oct 25, 2019 at 10:59:06AM +0200, Hans de Goede wrote: > > On 25-10-2019 09:41, Andy Shevchenko wrote: > > > On Thu, Oct 24, 2019 at 11:38:25PM +0200, Hans de Goede wrote: > > > > 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: > > drivers/acpi/pmic/intel_pmic_crc.c > > drivers/pwm/pwm-crc.c > > drivers/mfd/intel_soc_pmic_crc.c > > > > 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. > > Okay, let's do an exception here due to the fact the code and name already > exists and spreads enough thru sources. > > It means you may use mine tags. OK, applying the series as 5.5 material, thanks!
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ebe1e9e5fd81..089f7f8e1be7 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -513,11 +513,12 @@ menuconfig PMIC_OPREGION PMIC chip. if PMIC_OPREGION -config CRC_PMIC_OPREGION - bool "ACPI operation region support for CrystalCove PMIC" +config BYTCRC_PMIC_OPREGION + bool "ACPI operation region support for Bay Trail Crystal Cove PMIC" depends on INTEL_SOC_PMIC help - This config adds ACPI operation region support for CrystalCove PMIC. + This config adds ACPI operation region support for the Bay Trail + version of the Crystal Cove PMIC. config XPOWER_PMIC_OPREGION bool "ACPI operation region support for XPower AXP288 PMIC" diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 5d361e4e3405..ee59b1db69a1 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -109,7 +109,7 @@ obj-$(CONFIG_ACPI_APEI) += apei/ obj-$(CONFIG_ACPI_EXTLOG) += acpi_extlog.o obj-$(CONFIG_PMIC_OPREGION) += pmic/intel_pmic.o -obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o +obj-$(CONFIG_BYTCRC_PMIC_OPREGION) += pmic/intel_pmic_bytcrc.o obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o obj-$(CONFIG_BXT_WC_PMIC_OPREGION) += pmic/intel_pmic_bxtwc.o obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o diff --git a/drivers/acpi/pmic/intel_pmic_crc.c b/drivers/acpi/pmic/intel_pmic_bytcrc.c similarity index 98% rename from drivers/acpi/pmic/intel_pmic_crc.c rename to drivers/acpi/pmic/intel_pmic_bytcrc.c index a0f411a6e5ac..2a692cc4b7ae 100644 --- a/drivers/acpi/pmic/intel_pmic_crc.c +++ b/drivers/acpi/pmic/intel_pmic_bytcrc.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Intel CrystalCove PMIC operation region driver + * Intel Bay Trail Crystal Cove PMIC operation region driver * * Copyright (C) 2014 Intel Corporation. All rights reserved. */ @@ -295,7 +295,7 @@ static int intel_crc_pmic_opregion_probe(struct platform_device *pdev) static struct platform_driver intel_crc_pmic_opregion_driver = { .probe = intel_crc_pmic_opregion_probe, .driver = { - .name = "crystal_cove_pmic", + .name = "byt_crystal_cove_pmic", }, }; builtin_platform_driver(intel_crc_pmic_opregion_driver); diff --git a/drivers/mfd/intel_soc_pmic_crc.c b/drivers/mfd/intel_soc_pmic_crc.c index b6ab72fa0569..ab09b8225b76 100644 --- a/drivers/mfd/intel_soc_pmic_crc.c +++ 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",
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). Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/acpi/Kconfig | 7 ++++--- drivers/acpi/Makefile | 2 +- .../acpi/pmic/{intel_pmic_crc.c => intel_pmic_bytcrc.c} | 4 ++-- drivers/mfd/intel_soc_pmic_crc.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) rename drivers/acpi/pmic/{intel_pmic_crc.c => intel_pmic_bytcrc.c} (98%)