From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Turquette Subject: Re: [RFC PATCH 1/2] platform/x86: add Atom PMC quirk to disable SATA Date: Wed, 13 Dec 2017 07:25:28 -0800 Message-ID: References: <20170906204237.24x6fzlfmq7jmuce@sig21.net> <20170925191749.2oamusbajgs6clcg@sig21.net> <20170925192109.rty2fnm7c4jnj3vx@sig21.net> <34396652.fljU28PShI@aspire.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: Received: from mail-ot0-f195.google.com ([74.125.82.195]:39882 "EHLO mail-ot0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752340AbdLMP0A (ORCPT ); Wed, 13 Dec 2017 10:26:00 -0500 Received: by mail-ot0-f195.google.com with SMTP id v21so2256019oth.6 for ; Wed, 13 Dec 2017 07:25:59 -0800 (PST) In-Reply-To: Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Hans de Goede Cc: "Rafael J. Wysocki" , Johannes Stezenbach , Andy Shevchenko , Mika Westerberg , Pierre-Louis Bossart , linux-clk , Linux PM list , Carlo Caione , Darren Hart , Enric Balletbo i Serra , Takashi Iwai , ACPI Devel Maling List Hell Hans, all, On Wed, Dec 13, 2017 at 12:53 AM, Hans de Goede wrote: > > Hi, > > On 13-12-17 01:00, Rafael J. Wysocki wrote: >> >> On Monday, September 25, 2017 9:21:09 PM CET Johannes Stezenbach wrote: >>> >>> SATA controller is enabled on Asus E200HA even though the >>> machine doesn't use it (it has eMMC storage), however >>> SATA being on blocks S0ix entry so we need to disable it. >>> >>> Signed-off-by: Johannes Stezenbach >> >> >> Mika, Andy, Hans, any comments on this one? > > > Seems sensible to me, I'm afraid we may need the same quirk on > other devices, but I see no way around this. Quirks go directly into the driver? Is there a Device Tree analogue for x86 that could help here? > > Although, maybe we need to have a specialized (derived) > ahci driver for these Atom SoCs and in there if no > disk is detected do this through the clock framework? Yes please. x86 is already modeling some clocks properly through the clock framework. During late init we clean up any clocks that were enabled out of reset or by the firmware/bootloader but not claimed and enabled by any Linux driver. That should ideally disable this particular clock for the case when no SATA drive is present, and require no quirk logic in the driver. Regards, Mike > > That may be better then a long list of quirks. > > Johannes, question how did you test this and figure out > which clocks to disable, a quick howto on this, I think > a patch adding a little howto / README as say > Documentation/power/intel-S0ix-debugging.txt > documenting this would be great. I'm certainly interested > in trying to reproduce this on some of my own Bay Trail and > Cherry Trail devices and add fixes for those if necessary. > > Regards, > > Hans > > > > > >> >>> --- >>> drivers/platform/x86/pmc_atom.c | 33 +++++++++++++++++++++++++++++++++ >>> 1 file changed, 33 insertions(+) >>> >>> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c >>> index 8d13c86cc418..b5dd38712268 100644 >>> --- a/drivers/platform/x86/pmc_atom.c >>> +++ b/drivers/platform/x86/pmc_atom.c >>> @@ -17,6 +17,7 @@ >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -57,6 +58,9 @@ struct pmc_dev { >>> static struct pmc_dev pmc_device; >>> static u32 acpi_base_addr; >>> +static u32 quirks; >>> +#define QUIRK_DISABLE_SATA BIT(0) >>> + >>> static const struct pmc_clk byt_clks[] = { >>> { >>> .name = "xtal", >>> @@ -271,6 +275,15 @@ static void pmc_hw_reg_setup(struct pmc_dev *pmc) >>> * - GPIO_SCORE shared IRQ >>> */ >>> pmc_reg_write(pmc, PMC_S0IX_WAKE_EN, (u32)PMC_WAKE_EN_SETTING); >>> + >>> + if (quirks & QUIRK_DISABLE_SATA) { >>> + u32 func_dis; >>> + >>> + pr_info("pmc: disable SATA IP\n"); >>> + func_dis = pmc_reg_read(pmc, PMC_FUNC_DIS); >>> + func_dis |= BIT_SATA; >>> + pmc_reg_write(pmc, PMC_FUNC_DIS, func_dis); >>> + } >>> } >>> #ifdef CONFIG_DEBUG_FS >>> @@ -500,6 +513,24 @@ static struct pmc_notifier_block pmc_freeze_nb = { >>> }; >>> #endif >>> +static int cht_asus_e200ha_cb(const struct dmi_system_id *id) >>> +{ >>> + pr_info("pmc: Asus E200HA detected\n"); >>> + quirks |= QUIRK_DISABLE_SATA; >>> + return 1; >>> +} >>> + >>> +static const struct dmi_system_id cht_table[] = { >>> + { >>> + .callback = cht_asus_e200ha_cb, >>> + .matches = { >>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."), >>> + DMI_MATCH(DMI_PRODUCT_NAME, "E200HA"), >>> + }, >>> + }, >>> + { } >>> +}; >>> + >>> static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent) >>> { >>> struct pmc_dev *pmc = &pmc_device; >>> @@ -526,6 +557,8 @@ static int pmc_setup_dev(struct pci_dev *pdev, const struct pci_device_id *ent) >>> pmc->map = map; >>> + dmi_check_system(cht_table); >>> + >>> /* PMC hardware registers setup */ >>> pmc_hw_reg_setup(pmc); >>> >> >> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-clk" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html