On Mon, Apr 8, 2019 at 4:34 PM David Müller <dave.mueller@xxxxxx> wrote:
>
> Since commit 648e921888ad ("clk: x86: Stop marking clocks as
> CLK_IS_CRITICAL"), the pmc_plt_clocks of the Bay Trail SoC are
> unconditionally gated off. Unfortunately this will break systems where these
> clocks are used for external purposes beyond the kernel's knowledge. Fix it
> by implementing a system specific quirk to mark the necessary pmc_plt_clks as
> critical.
>
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
supposed to go via CLK tree.
> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> Signed-off-by: David Müller <dave.mueller@xxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> Changes in v4:
> - add '.' to comments / rename field as suggested by Andy Shevchenko
> Changes in v3:
> - add missing sentinel entry to critclk_systems table
> Changes in v2:
> - restore previous clk detection logic as suggested by Hans de Goede
>
> drivers/clk/x86/clk-pmc-atom.c | 14 ++++++++++---
> drivers/platform/x86/pmc_atom.c | 21 +++++++++++++++++++
> .../linux/platform_data/x86/clk-pmc-atom.h | 3 +++
> 3 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
> index d977193842df..19174835693b 100644
> --- a/drivers/clk/x86/clk-pmc-atom.c
> +++ b/drivers/clk/x86/clk-pmc-atom.c
> @@ -165,7 +165,7 @@ static const struct clk_ops plt_clk_ops = {
> };
>
> static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
> - void __iomem *base,
> + const struct pmc_clk_data *pmc_data,
> const char **parent_names,
> int num_parents)
> {
> @@ -184,9 +184,17 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
> init.num_parents = num_parents;
>
> pclk->hw.init = &init;
> - pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
> + pclk->reg = pmc_data->base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
> spin_lock_init(&pclk->lock);
>
> + /*
> + * On some systems, the pmc_plt_clocks already enabled by the
> + * firmware are being marked as critical to avoid them being
> + * gated by the clock framework.
> + */
> + if (pmc_data->critical && plt_clk_is_enabled(&pclk->hw))
> + init.flags |= CLK_IS_CRITICAL;
> +
> ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
> if (ret) {
> pclk = ERR_PTR(ret);
> @@ -332,7 +340,7 @@ static int plt_clk_probe(struct platform_device *pdev)
> return PTR_ERR(parent_names);
>
> for (i = 0; i < PMC_CLK_NUM; i++) {
> - data->clks[i] = plt_clk_register(pdev, i, pmc_data->base,
> + data->clks[i] = plt_clk_register(pdev, i, pmc_data,
> parent_names, data->nparents);
> if (IS_ERR(data->clks[i])) {
> err = PTR_ERR(data->clks[i]);
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index 8f018b3f3cd4..eaec2d306481 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -17,6 +17,7 @@
>
> #include <linux/debugfs.h>
> #include <linux/device.h>
> +#include <linux/dmi.h>
> #include <linux/init.h>
> #include <linux/io.h>
> #include <linux/platform_data/x86/clk-pmc-atom.h>
> @@ -391,11 +392,27 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc)
> }
> #endif /* CONFIG_DEBUG_FS */
>
> +/*
> + * Some systems need one or more of their pmc_plt_clks to be
> + * marked as critical.
> + */
> +static const struct dmi_system_id critclk_systems[] __initconst = {
> + {
> + .ident = "MPL CEC1x",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
> + },
> + },
> + { /*sentinel*/ }
> +};
> +
> static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
> const struct pmc_data *pmc_data)
> {
> struct platform_device *clkdev;
> struct pmc_clk_data *clk_data;
> + const struct dmi_system_id *d = dmi_first_match(critclk_systems);
>
> clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> if (!clk_data)
> @@ -403,6 +420,10 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
>
> clk_data->base = pmc_regmap; /* offset is added by client */
> clk_data->clks = pmc_data->clks;
> + if (d) {
> + clk_data->critical = true;
> + pr_info("%s critclks quirk enabled\n", d->ident);
> + }
>
> clkdev = platform_device_register_data(&pdev->dev, "clk-pmc-atom",
> PLATFORM_DEVID_NONE,
> diff --git a/include/linux/platform_data/x86/clk-pmc-atom.h b/include/linux/platform_data/x86/clk-pmc-atom.h
> index 3ab892208343..7a37ac27d0fb 100644
> --- a/include/linux/platform_data/x86/clk-pmc-atom.h
> +++ b/include/linux/platform_data/x86/clk-pmc-atom.h
> @@ -35,10 +35,13 @@ struct pmc_clk {
> *
> * @base: PMC clock register base offset
> * @clks: pointer to set of registered clocks, typically 0..5
> + * @critical: flag to indicate if firmware enabled pmc_plt_clks
> + * should be marked as critial or not
> */
> struct pmc_clk_data {
> void __iomem *base;
> const struct pmc_clk *clks;
> + bool critical;
> };
>
> #endif /* __PLATFORM_DATA_X86_CLK_PMC_ATOM_H */
> --
> 2.20.1
>
--
With Best Regards,
Andy Shevchenko
