Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] thermal: intel: intel_tcc: Add model checks for temperature registers
From: Ricardo Neri @ 2024-04-08 14:23 UTC (permalink / raw)
  To: Zhang, Rui
  Cc: linux@roeck-us.net, Wysocki, Rafael J, jdelvare@suse.com,
	srinivas.pandruvada@linux.intel.com, lukasz.luba@arm.com,
	linux-pm@vger.kernel.org, linux-hwmon@vger.kernel.org,
	daniel.lezcano@linaro.org, linux-kernel@vger.kernel.org,
	Neri, Ricardo
In-Reply-To: <5e86524413ec2cfeb1096f49851bf18837c7e50b.camel@intel.com>

On Sun, Apr 07, 2024 at 08:13:28AM +0000, Zhang, Rui wrote:
> > +
> > +#define TCC_FAM6_MODEL_TEMP_MASKS

Thank your your review, Rui!

> 
> Future non FAM6 processors can still use this macro, right?
> So I'd prefer to remove FAM6_MODEL in the macro name.

Yes, it is true, FAM6_MODEL it is restrictive and also not needed here.
I will update accodingly.
 
> [...]
> > 
> > +
> > +/**
> > + * get_tcc_offset_mask() - Returns the model-specific bitmask for
> > TCC offset
> > + *
> > + * Get the model-specific bitmask to extract TCC_OFFSET from the
> > MSR_TEMPERATURE_
> > + * TARGET register. If the mask is 0, it means the processor does
> > not support TCC offset.
> > + *
> > + * Return: The model-specific bitmask for TCC offset.
> > + */
> > +u32 get_tcc_offset_mask(void)
> > +{
> > +       return intel_tcc_temp_masks.tcc_offset;
> > +}
> > +EXPORT_SYMBOL_NS(get_tcc_offset_mask, INTEL_TCC);
> 
> the name is not consistent with the other intel_tcc APIs.
> 
> how about intel_tcc_get_offset_mask()?

Sure. I can make this change.

> 
> [...]
> 
> > diff --git a/include/linux/intel_tcc.h b/include/linux/intel_tcc.h
> > index 8ff8eabb4a98..e281cf06aeab 100644
> > --- a/include/linux/intel_tcc.h
> > +++ b/include/linux/intel_tcc.h
> > @@ -14,5 +14,13 @@ int intel_tcc_get_tjmax(int cpu);
> >  int intel_tcc_get_offset(int cpu);
> >  int intel_tcc_set_offset(int cpu, int offset);
> >  int intel_tcc_get_temp(int cpu, int *temp, bool pkg);
> > +#ifdef CONFIG_INTEL_TCC
> > +u32 get_tcc_offset_mask(void);
> > +u32 intel_tcc_get_temp_mask(bool pkg);
> > +#else
> > +static inline u32 get_tcc_offset_mask(void) { return 0; }
> > +/* Use the architectural bitmask of the temperature readout. No
> > model checks. */
> > +static inline u32 intel_tcc_get_temp_mask(bool pkg) { return 0x7f; }
> > +#endif
> 
> for intel_tcc_get_temp_mask()
>    1. with CONFIG_INTEL_TCC
>       a) for a platform in the model list, return the hardcoded value
>       b) for a platform not in the model list, return 0xff
>    2. without CONFIG_INTEL_TCC, return 0x7f
> 
> This is a bit confusing. IMO, at least we should leave a comment about
> this difference.

If we don't do model checks, I think we should rely on what is architectural
as per the SDM. Hence the 0x7f value.

Perhaps I can expand the comment in this hunk to detail what we do when we
do model checks.


^ permalink raw reply

* Re: [PATCH] ACPI: DPTF: Add Lunar Lake support
From: Rafael J. Wysocki @ 2024-04-08 14:11 UTC (permalink / raw)
  To: Sumeet Pawnikar
  Cc: rafael, srinivas.pandruvada, linux-acpi, linux-kernel, linux-pm
In-Reply-To: <20240405121819.31331-1-sumeet.r.pawnikar@intel.com>

On Fri, Apr 5, 2024 at 2:31 PM Sumeet Pawnikar
<sumeet.r.pawnikar@intel.com> wrote:
>
> Add Lunar Lake ACPI IDs for DPTF devices.
>
> Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
> ---
>  drivers/acpi/dptf/dptf_pch_fivr.c                       | 1 +
>  drivers/acpi/dptf/dptf_power.c                          | 2 ++
>  drivers/acpi/dptf/int340x_thermal.c                     | 6 ++++++
>  drivers/acpi/fan.h                                      | 1 +
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 1 +
>  drivers/thermal/intel/int340x_thermal/int3403_thermal.c | 1 +
>  6 files changed, 12 insertions(+)
>
> diff --git a/drivers/acpi/dptf/dptf_pch_fivr.c b/drivers/acpi/dptf/dptf_pch_fivr.c
> index 654aaa53c67f..d202730fafd8 100644
> --- a/drivers/acpi/dptf/dptf_pch_fivr.c
> +++ b/drivers/acpi/dptf/dptf_pch_fivr.c
> @@ -150,6 +150,7 @@ static const struct acpi_device_id pch_fivr_device_ids[] = {
>         {"INTC1045", 0},
>         {"INTC1049", 0},
>         {"INTC1064", 0},
> +       {"INTC106B", 0},
>         {"INTC10A3", 0},
>         {"", 0},
>  };
> diff --git a/drivers/acpi/dptf/dptf_power.c b/drivers/acpi/dptf/dptf_power.c
> index b8187babbbbb..8023b3e23315 100644
> --- a/drivers/acpi/dptf/dptf_power.c
> +++ b/drivers/acpi/dptf/dptf_power.c
> @@ -232,6 +232,8 @@ static const struct acpi_device_id int3407_device_ids[] = {
>         {"INTC1061", 0},
>         {"INTC1065", 0},
>         {"INTC1066", 0},
> +       {"INTC106C", 0},
> +       {"INTC106D", 0},
>         {"INTC10A4", 0},
>         {"INTC10A5", 0},
>         {"", 0},
> diff --git a/drivers/acpi/dptf/int340x_thermal.c b/drivers/acpi/dptf/int340x_thermal.c
> index b7113fa92fa6..014ada759954 100644
> --- a/drivers/acpi/dptf/int340x_thermal.c
> +++ b/drivers/acpi/dptf/int340x_thermal.c
> @@ -43,6 +43,12 @@ static const struct acpi_device_id int340x_thermal_device_ids[] = {
>         {"INTC1064"},
>         {"INTC1065"},
>         {"INTC1066"},
> +       {"INTC1068"},
> +       {"INTC1069"},
> +       {"INTC106A"},
> +       {"INTC106B"},
> +       {"INTC106C"},
> +       {"INTC106D"},
>         {"INTC10A0"},
>         {"INTC10A1"},
>         {"INTC10A2"},
> diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h
> index e7b4b4e4a55e..f89d19c922dc 100644
> --- a/drivers/acpi/fan.h
> +++ b/drivers/acpi/fan.h
> @@ -15,6 +15,7 @@
>         {"INTC1044", }, /* Fan for Tiger Lake generation */ \
>         {"INTC1048", }, /* Fan for Alder Lake generation */ \
>         {"INTC1063", }, /* Fan for Meteor Lake generation */ \
> +       {"INTC106A", }, /* Fan for Lunar Lake generation */ \
>         {"INTC10A2", }, /* Fan for Raptor Lake generation */ \
>         {"PNP0C0B", } /* Generic ACPI fan */
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 427d370648d5..f8ebdd19d340 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -705,6 +705,7 @@ static const struct acpi_device_id int3400_thermal_match[] = {
>         {"INTC1040", 0},
>         {"INTC1041", 0},
>         {"INTC1042", 0},
> +       {"INTC1068", 0},
>         {"INTC10A0", 0},
>         {}
>  };
> diff --git a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
> index 9b33fd3a66da..86901f9f54d8 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
> @@ -284,6 +284,7 @@ static const struct acpi_device_id int3403_device_ids[] = {
>         {"INTC1043", 0},
>         {"INTC1046", 0},
>         {"INTC1062", 0},
> +       {"INTC1069", 0},
>         {"INTC10A1", 0},
>         {"", 0},
>  };
> --

Applied as 6.10 material, thanks!

^ permalink raw reply

* Re: [PATCH v5 0/4] Update Energy Model after chip binning adjusted voltages
From: Rafael J. Wysocki @ 2024-04-08 14:06 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: linux-kernel, linux-pm, rafael, dietmar.eggemann,
	linux-arm-kernel, sboyd, nm, linux-samsung-soc, daniel.lezcano,
	viresh.kumar, krzysztof.kozlowski, alim.akhtar, m.szyprowski,
	mhiramat
In-Reply-To: <20240403154907.1420245-1-lukasz.luba@arm.com>

On Wed, Apr 3, 2024 at 5:49 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi all,
>
> This is a follow-up patch aiming to add EM modification due to chip binning.
> The first RFC and the discussion can be found here [1].
>
> It uses Exynos chip driver code as a 1st user. The EM framework has been
> extended to handle this use case easily, when the voltage has been changed
> after setup. On my Odroid-xu4 in some OPPs I can observe ~20% power difference.
> According to that data in driver tables it could be up to ~29%.
>
> This chip binning is applicable to a lot of SoCs, so the EM framework should
> make it easy to update. It uses the existing OPP and DT information to
> re-calculate the new power values.
>
> It has dependency on Exynos SoC driver tree.
>
> Changes:
> v5:
> - adjusted aligning of the function arguments in patch 1/4 (Dietmar)
> - adjusted the in-code comment patch 4/4 (Dietmar)
> - added Reviewed-by to all patches (Dietmar)
> v4:
> - added asterisk in the comment section (test robot)
> - change the patch 2/4 header name and use 'Refactor'
> v3:
> - updated header description patch 2/4 (Dietmar)
> - removed 2 sentences from comment and adjusted in patch 3/4 (Dietmar)
> - patch 4/4 re-phrased code comment (Dietmar)
> - collected tags (Krzysztof, Viresh)
> v2:
> - removed 'ret' from error message which wasn't initialized (Christian)
> v1:
> - exported the OPP calculation function from the OPP/OF so it can be
>   used from EM fwk (Viresh)
> - refactored EM updating function to re-use common code
> - added new EM function which can be used by chip device drivers which
>   modify the voltage in OPPs
> RFC is at [1]
>
> Regards,
> Lukasz Luba
>
> [1] https://lore.kernel.org/lkml/20231220110339.1065505-1-lukasz.luba@arm.com/
>
> Lukasz Luba (4):
>   OPP: OF: Export dev_opp_pm_calc_power() for usage from EM
>   PM: EM: Refactor em_adjust_new_capacity()
>   PM: EM: Add em_dev_update_chip_binning()
>   soc: samsung: exynos-asv: Update Energy Model after adjusting voltage
>
>  drivers/opp/of.c                 |  17 +++--
>  drivers/soc/samsung/exynos-asv.c |  10 ++-
>  include/linux/energy_model.h     |   5 ++
>  include/linux/pm_opp.h           |   8 +++
>  kernel/power/energy_model.c      | 106 +++++++++++++++++++++++++------
>  5 files changed, 121 insertions(+), 25 deletions(-)
>
> --

All patches in the series applied as 6.10 material, thanks!

^ permalink raw reply

* Re: [PATCH v2] PM: s2idle: Make sure CPUs will wakeup directly on resume
From: Rafael J. Wysocki @ 2024-04-08 13:39 UTC (permalink / raw)
  To: Ulf Hansson, Anna-Maria Behnsen
  Cc: linux-kernel, Rafael J . Wysocki, Pavel Machek, Len Brown,
	linux-pm, Frederic Weisbecker, Thomas Gleixner, x86,
	Mario Limonciello, stable
In-Reply-To: <CAPDyKFo7KT4V8Nvn58N3mNfeW6ai=-5hampjN7N19kYaR7zdVA@mail.gmail.com>

On Mon, Apr 8, 2024 at 2:43 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Mon, 8 Apr 2024 at 09:02, Anna-Maria Behnsen
> <anna-maria@linutronix.de> wrote:
> >
> > s2idle works like a regular suspend with freezing processes and freezing
> > devices. All CPUs except the control CPU go into idle. Once this is
> > completed the control CPU kicks all other CPUs out of idle, so that they
> > reenter the idle loop and then enter s2idle state. The control CPU then
> > issues an swait() on the suspend state and therefore enters the idle loop
> > as well.
> >
> > Due to being kicked out of idle, the other CPUs leave their NOHZ states,
> > which means the tick is active and the corresponding hrtimer is programmed
> > to the next jiffie.
> >
> > On entering s2idle the CPUs shut down their local clockevent device to
> > prevent wakeups. The last CPU which enters s2idle shuts down its local
> > clockevent and freezes timekeeping.
> >
> > On resume, one of the CPUs receives the wakeup interrupt, unfreezes
> > timekeeping and its local clockevent and starts the resume process. At that
> > point all other CPUs are still in s2idle with their clockevents switched
> > off. They only resume when they are kicked by another CPU or after resuming
> > devices and then receiving a device interrupt.
> >
> > That means there is no guarantee that all CPUs will wakeup directly on
> > resume. As a consequence there is no guarantee that timers which are queued
> > on those CPUs and should expire directly after resume, are handled. Also
> > timer list timers which are remotely queued to one of those CPUs after
> > resume will not result in a reprogramming IPI as the tick is
> > active. Queueing a hrtimer will also not result in a reprogramming IPI
> > because the first hrtimer event is already in the past.
> >
> > The recent introduction of the timer pull model (7ee988770326 ("timers:
> > Implement the hierarchical pull model")) amplifies this problem, if the
> > current migrator is one of the non woken up CPUs. When a non pinned timer
> > list timer is queued and the queuing CPU goes idle, it relies on the still
> > suspended migrator CPU to expire the timer which will happen by chance.
> >
> > The problem exists since commit 8d89835b0467 ("PM: suspend: Do not pause
> > cpuidle in the suspend-to-idle path"). There the cpuidle_pause() call which
> > in turn invoked a wakeup for all idle CPUs was moved to a later point in
> > the resume process. This might not be reached or reached very late because
> > it waits on a timer of a still suspended CPU.
> >
> > Address this by kicking all CPUs out of idle after the control CPU returns
> > from swait() so that they resume their timers and restore consistent system
> > state.
> >
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218641
> > Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
> > Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> > Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> > Cc: stable@kernel.org
>
> Thanks for the detailed commit message! Please add:
>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Applied as 6.9-rc material, many thanks to everyone involved!

^ permalink raw reply

* Re: [RFC PATCH 1/1] clk: sunxi-ng: h6-r: add GPU power domain
From: Ulf Hansson @ 2024-04-08 12:55 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Jernej Skrabec, Chen-Yu Tsai, Samuel Holland, Michael Turquette,
	Stephen Boyd, linux-clk, linux-pm, linux-arm-kernel, linux-sunxi
In-Reply-To: <20240225160616.15001-2-andre.przywara@arm.com>

On Sun, 25 Feb 2024 at 17:08, Andre Przywara <andre.przywara@arm.com> wrote:
>
> The Allwinner H616 features register 0x7010254 in the PRCM MMIO frame,
> where bit 0 needs to be cleared to enable operation of the Mali GPU.
> With this bit set (the reset default), any access to the Mali registers
> hangs the bus and thus the whole system. The BSP code clears this bit
> in U-Boot and their kernel never touches it again.

Is the bit representing a reset or power-rail? If it's a reset, it's
probably better to model it like that.

>
> Register a power-domain device to control this bit. Since we claim this
> MMIO region in the H6 R-CCU driver, add the code here, so that we don't
> need to artificially split the MMIO range in the DT.
> Since there seems to be at least one other register with similar behaviour
> nearby (0x7010260), make the power domain take one cell, even though we
> only support domain #0 for now.

Seems like we need some updated DT bindings too to cover this?

>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c | 84 ++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
> index 02b28cfc5525e..363fb7a71e9f5 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
> @@ -4,9 +4,11 @@
>   */
>
>  #include <linux/clk-provider.h>
> +#include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>
>  #include "ccu_common.h"
>  #include "ccu_reset.h"
> @@ -217,6 +219,86 @@ static const struct sunxi_ccu_desc sun50i_h616_r_ccu_desc = {
>         .num_resets     = ARRAY_SIZE(sun50i_h616_r_ccu_resets),
>  };
>
> +#define        PD_H616_GPU_REG                 0x254
> +
> +struct sun50i_h616_ppu_pd {
> +       struct generic_pm_domain        genpd;
> +       void __iomem                    *base;
> +};
> +
> +#define to_sun50i_h616_ppu_pd(_genpd) \
> +       container_of(_genpd, struct sun50i_h616_ppu_pd, genpd)
> +
> +static bool sun50i_h616_ppu_power_status(const struct sun50i_h616_ppu_pd *pd)
> +{
> +       return !readl(pd->base + PD_H616_GPU_REG);
> +}
> +
> +static int sun50i_h616_ppu_pd_set_power(const struct sun50i_h616_ppu_pd *pd,
> +                                       bool power_on)
> +{
> +       if (power_on)
> +               writel(0, pd->base + PD_H616_GPU_REG);
> +       else
> +               writel(1, pd->base + PD_H616_GPU_REG);
> +
> +       return 0;
> +}
> +
> +static int sun50i_h616_ppu_pd_power_on(struct generic_pm_domain *genpd)
> +{
> +       const struct sun50i_h616_ppu_pd *pd = to_sun50i_h616_ppu_pd(genpd);
> +
> +       return sun50i_h616_ppu_pd_set_power(pd, true);
> +}
> +
> +static int sun50i_h616_ppu_pd_power_off(struct generic_pm_domain *genpd)
> +{
> +       const struct sun50i_h616_ppu_pd *pd = to_sun50i_h616_ppu_pd(genpd);
> +
> +       return sun50i_h616_ppu_pd_set_power(pd, false);
> +}
> +
> +static int sun50i_h616_register_ppu(struct platform_device *pdev,
> +                                   void __iomem *base)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct genpd_onecell_data *ppu;
> +       struct sun50i_h616_ppu_pd *pd;
> +       int ret;
> +
> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +       if (!pd)
> +               return -ENOMEM;
> +
> +       ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
> +       if (!ppu)
> +               return -ENOMEM;
> +
> +       ppu->domains = devm_kzalloc(dev, sizeof(*ppu->domains), GFP_KERNEL);
> +       if (!ppu->domains)
> +               return -ENOMEM;
> +
> +       ppu->num_domains = 1;
> +       pd->genpd.name          = "GPU";
> +       pd->genpd.power_off     = sun50i_h616_ppu_pd_power_off;
> +       pd->genpd.power_on      = sun50i_h616_ppu_pd_power_on;
> +       pd->base                = base;
> +
> +       ret = pm_genpd_init(&pd->genpd, NULL, !sun50i_h616_ppu_power_status(pd));
> +       if (ret) {
> +               dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ppu->domains[0] = &pd->genpd;
> +       ret = of_genpd_add_provider_onecell(dev->of_node, ppu);
> +       if (ret)
> +               dev_warn(dev, "Failed to add provider: %d\n", ret);
> +
> +       return 0;
> +}
> +
>  static int sun50i_h6_r_ccu_probe(struct platform_device *pdev)
>  {
>         const struct sunxi_ccu_desc *desc;
> @@ -230,6 +312,8 @@ static int sun50i_h6_r_ccu_probe(struct platform_device *pdev)
>         if (IS_ERR(reg))
>                 return PTR_ERR(reg);
>
> +       sun50i_h616_register_ppu(pdev, reg);
> +
>         return devm_sunxi_ccu_probe(&pdev->dev, reg, desc);
>  }
>

In general (for maintenance reasons) it's a good idea to put genpd
providers under drivers/pmdomain/*. It looks like that should work
this case too, right?

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH v2] PM: s2idle: Make sure CPUs will wakeup directly on resume
From: Ulf Hansson @ 2024-04-08 12:42 UTC (permalink / raw)
  To: Anna-Maria Behnsen
  Cc: linux-kernel, Rafael J . Wysocki, Pavel Machek, Len Brown,
	linux-pm, Frederic Weisbecker, Thomas Gleixner, x86,
	Mario Limonciello, stable
In-Reply-To: <87r0fg5ocg.fsf@somnus>

On Mon, 8 Apr 2024 at 09:02, Anna-Maria Behnsen
<anna-maria@linutronix.de> wrote:
>
> s2idle works like a regular suspend with freezing processes and freezing
> devices. All CPUs except the control CPU go into idle. Once this is
> completed the control CPU kicks all other CPUs out of idle, so that they
> reenter the idle loop and then enter s2idle state. The control CPU then
> issues an swait() on the suspend state and therefore enters the idle loop
> as well.
>
> Due to being kicked out of idle, the other CPUs leave their NOHZ states,
> which means the tick is active and the corresponding hrtimer is programmed
> to the next jiffie.
>
> On entering s2idle the CPUs shut down their local clockevent device to
> prevent wakeups. The last CPU which enters s2idle shuts down its local
> clockevent and freezes timekeeping.
>
> On resume, one of the CPUs receives the wakeup interrupt, unfreezes
> timekeeping and its local clockevent and starts the resume process. At that
> point all other CPUs are still in s2idle with their clockevents switched
> off. They only resume when they are kicked by another CPU or after resuming
> devices and then receiving a device interrupt.
>
> That means there is no guarantee that all CPUs will wakeup directly on
> resume. As a consequence there is no guarantee that timers which are queued
> on those CPUs and should expire directly after resume, are handled. Also
> timer list timers which are remotely queued to one of those CPUs after
> resume will not result in a reprogramming IPI as the tick is
> active. Queueing a hrtimer will also not result in a reprogramming IPI
> because the first hrtimer event is already in the past.
>
> The recent introduction of the timer pull model (7ee988770326 ("timers:
> Implement the hierarchical pull model")) amplifies this problem, if the
> current migrator is one of the non woken up CPUs. When a non pinned timer
> list timer is queued and the queuing CPU goes idle, it relies on the still
> suspended migrator CPU to expire the timer which will happen by chance.
>
> The problem exists since commit 8d89835b0467 ("PM: suspend: Do not pause
> cpuidle in the suspend-to-idle path"). There the cpuidle_pause() call which
> in turn invoked a wakeup for all idle CPUs was moved to a later point in
> the resume process. This might not be reached or reached very late because
> it waits on a timer of a still suspended CPU.
>
> Address this by kicking all CPUs out of idle after the control CPU returns
> from swait() so that they resume their timers and restore consistent system
> state.
>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218641
> Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Cc: stable@kernel.org

Thanks for the detailed commit message! Please add:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
> v2: Fix typos in commit message
> ---
>  kernel/power/suspend.c |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -106,6 +106,12 @@ static void s2idle_enter(void)
>         swait_event_exclusive(s2idle_wait_head,
>                     s2idle_state == S2IDLE_STATE_WAKE);
>
> +       /*
> +        * Kick all CPUs to ensure that they resume their timers and restore
> +        * consistent system state.
> +        */
> +       wake_up_all_idle_cpus();
> +
>         cpus_read_unlock();
>
>         raw_spin_lock_irq(&s2idle_lock);

^ permalink raw reply

* Re: [PATCH 3/3] hwmon: (coretemp) Use a model-specific bitmask to read registers
From: Guenter Roeck @ 2024-04-08 11:40 UTC (permalink / raw)
  To: Zhang, Rui
  Cc: ricardo.neri-calderon@linux.intel.com, Neri, Ricardo,
	linux-hwmon@vger.kernel.org, lukasz.luba@arm.com,
	daniel.lezcano@linaro.org, jdelvare@suse.com,
	srinivas.pandruvada@linux.intel.com, linux-kernel@vger.kernel.org,
	Wysocki, Rafael J, linux-pm@vger.kernel.org
In-Reply-To: <d0b5ae04b4d08e2003114c4d6b6d3a040f585995.camel@intel.com>

On Sun, Apr 07, 2024 at 08:39:51AM +0000, Zhang, Rui wrote:
> > I do not think it is appropriate to make a hardware monitoring driver
> > depend on the thermal subsystem.
> > 
> > NAK in the current form.
> > 
> Hi, Guenter,
> 
> Thanks for reviewing.
> 
> We've seen a couple of hwmon drivers depends on or imply THERMAL.
> That's why we think this is an applicable solution.

So far this was - unless someone sneaked something by - for drivers
which registered thermal zones, not for calling code which resides
inside thermal subsystem.

> Using the intel_tcc APIs can effectively reduce the future maintenance
> burden because we don't need to duplicate the model list in multiple
> places.
> 
> or do you have any suggestions?

The exported code should reside outside the thermal subsystem.

Also, as implemented, if INTEL_TCC=n, the returned temperature mask value
is 0x7f, and the offset mask is 0. So the alternative would be to just use
those values unconditionally since apparently that is sufficient.

Thanks,
Guenter

^ permalink raw reply

* Re: [PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper
From: Michael Ellerman @ 2024-04-08 11:11 UTC (permalink / raw)
  To: Lukas Wunner, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel
  Cc: linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi,
	Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm,
	Luis Chamberlain, linux-modules
In-Reply-To: <92ee0a0e83a5a3f3474845db6c8575297698933a.1712410202.git.lukas@wunner.de>

Lukas Wunner <lukas@wunner.de> writes:
> Deduplicate ->read() callbacks of bin_attributes which are backed by a
> simple buffer in memory:
>
> Use the newly introduced sysfs_bin_attr_simple_read() helper instead,
> either by referencing it directly or by declaring such bin_attributes
> with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO().
>
> Aside from a reduction of LoC, this shaves off a few bytes from vmlinux
> (304 bytes on an x86_64 allyesconfig).
>
> No functional change intended.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  arch/powerpc/platforms/powernv/opal.c              | 10 +--------
>  drivers/acpi/bgrt.c                                |  9 +-------
>  drivers/firmware/dmi_scan.c                        | 12 ++--------
>  drivers/firmware/efi/rci2-table.c                  | 10 +--------
>  drivers/gpu/drm/i915/gvt/firmware.c                | 26 +++++-----------------
>  .../intel/int340x_thermal/int3400_thermal.c        |  9 +-------
>  init/initramfs.c                                   | 10 +--------
>  kernel/module/sysfs.c                              | 13 +----------
>  8 files changed, 14 insertions(+), 85 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 45dd77e..5d0f35b 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void)
>  	return 0;
>  }
>  
> -static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
> -				struct bin_attribute *bin_attr, char *buf,
> -				loff_t off, size_t count)
> -{
> -	return memory_read_from_buffer(buf, count, &off, bin_attr->private,
> -				       bin_attr->size);
> -}
> -
>  static int opal_add_one_export(struct kobject *parent, const char *export_name,
>  			       struct device_node *np, const char *prop_name)
>  {
> @@ -826,7 +818,7 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name,
>  	sysfs_bin_attr_init(attr);
>  	attr->attr.name = name;
>  	attr->attr.mode = 0400;
> -	attr->read = export_attr_read;
> +	attr->read = sysfs_bin_attr_simple_read;
>  	attr->private = __va(vals[0]);
>  	attr->size = vals[1];

I gave it a quick boot and checked I could still read the attributes,
everything seems fine.

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

^ permalink raw reply

* Re: [PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper
From: Zhi Wang @ 2024-04-08 10:42 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
	Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare,
	Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev,
	Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules
In-Reply-To: <92ee0a0e83a5a3f3474845db6c8575297698933a.1712410202.git.lukas@wunner.de>

On Sat, 6 Apr 2024 15:52:02 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> Deduplicate ->read() callbacks of bin_attributes which are backed by a
> simple buffer in memory:
> 
> Use the newly introduced sysfs_bin_attr_simple_read() helper instead,
> either by referencing it directly or by declaring such bin_attributes
> with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO().
> 
> Aside from a reduction of LoC, this shaves off a few bytes from
> vmlinux (304 bytes on an x86_64 allyesconfig).
> 
> No functional change intended.
> 

As for GVT, looks good to me.

Acked-by: Zhi Wang <zhiwang@kernel.org>

> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  arch/powerpc/platforms/powernv/opal.c              | 10 +--------
>  drivers/acpi/bgrt.c                                |  9 +-------
>  drivers/firmware/dmi_scan.c                        | 12 ++--------
>  drivers/firmware/efi/rci2-table.c                  | 10 +--------
>  drivers/gpu/drm/i915/gvt/firmware.c                | 26
> +++++----------------- .../intel/int340x_thermal/int3400_thermal.c
>     |  9 +------- init/initramfs.c
> | 10 +-------- kernel/module/sysfs.c                              |
> 13 +---------- 8 files changed, 14 insertions(+), 85 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal.c
> b/arch/powerpc/platforms/powernv/opal.c index 45dd77e..5d0f35b 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void)
>  	return 0;
>  }
>  
> -static ssize_t export_attr_read(struct file *fp, struct kobject
> *kobj,
> -				struct bin_attribute *bin_attr, char
> *buf,
> -				loff_t off, size_t count)
> -{
> -	return memory_read_from_buffer(buf, count, &off,
> bin_attr->private,
> -				       bin_attr->size);
> -}
> -
>  static int opal_add_one_export(struct kobject *parent, const char
> *export_name, struct device_node *np, const char *prop_name)
>  {
> @@ -826,7 +818,7 @@ static int opal_add_one_export(struct kobject
> *parent, const char *export_name, sysfs_bin_attr_init(attr);
>  	attr->attr.name = name;
>  	attr->attr.mode = 0400;
> -	attr->read = export_attr_read;
> +	attr->read = sysfs_bin_attr_simple_read;
>  	attr->private = __va(vals[0]);
>  	attr->size = vals[1];
>  
> diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c
> index e4fb9e2..d1d9c92 100644
> --- a/drivers/acpi/bgrt.c
> +++ b/drivers/acpi/bgrt.c
> @@ -29,14 +29,7 @@
>  BGRT_SHOW(xoffset, image_offset_x);
>  BGRT_SHOW(yoffset, image_offset_y);
>  
> -static ssize_t image_read(struct file *file, struct kobject *kobj,
> -	       struct bin_attribute *attr, char *buf, loff_t off,
> size_t count) -{
> -	memcpy(buf, attr->private + off, count);
> -	return count;
> -}
> -
> -static BIN_ATTR_RO(image, 0);	/* size gets filled in later */
> +static BIN_ATTR_SIMPLE_RO(image);
>  
>  static struct attribute *bgrt_attributes[] = {
>  	&bgrt_attr_version.attr,
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 015c95a..3d0f773 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -746,16 +746,8 @@ static void __init dmi_scan_machine(void)
>  	pr_info("DMI not present or invalid.\n");
>  }
>  
> -static ssize_t raw_table_read(struct file *file, struct kobject
> *kobj,
> -			      struct bin_attribute *attr, char *buf,
> -			      loff_t pos, size_t count)
> -{
> -	memcpy(buf, attr->private + pos, count);
> -	return count;
> -}
> -
> -static BIN_ATTR(smbios_entry_point, S_IRUSR, raw_table_read, NULL,
> 0); -static BIN_ATTR(DMI, S_IRUSR, raw_table_read, NULL, 0);
> +static BIN_ATTR_SIMPLE_ADMIN_RO(smbios_entry_point);
> +static BIN_ATTR_SIMPLE_ADMIN_RO(DMI);
>  
>  static int __init dmi_init(void)
>  {
> diff --git a/drivers/firmware/efi/rci2-table.c
> b/drivers/firmware/efi/rci2-table.c index de1a9a1..4fd45d6 100644
> --- a/drivers/firmware/efi/rci2-table.c
> +++ b/drivers/firmware/efi/rci2-table.c
> @@ -40,15 +40,7 @@ struct rci2_table_global_hdr {
>  static u32 rci2_table_len;
>  unsigned long rci2_table_phys __ro_after_init =
> EFI_INVALID_TABLE_ADDR; 
> -static ssize_t raw_table_read(struct file *file, struct kobject
> *kobj,
> -			      struct bin_attribute *attr, char *buf,
> -			      loff_t pos, size_t count)
> -{
> -	memcpy(buf, attr->private + pos, count);
> -	return count;
> -}
> -
> -static BIN_ATTR(rci2, S_IRUSR, raw_table_read, NULL, 0);
> +static BIN_ATTR_SIMPLE_ADMIN_RO(rci2);
>  
>  static u16 checksum(void)
>  {
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
> b/drivers/gpu/drm/i915/gvt/firmware.c index 4dd52ac..5e66a26 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -50,21 +50,7 @@ struct gvt_firmware_header {
>  
>  #define dev_to_drm_minor(d) dev_get_drvdata((d))
>  
> -static ssize_t
> -gvt_firmware_read(struct file *filp, struct kobject *kobj,
> -	     struct bin_attribute *attr, char *buf,
> -	     loff_t offset, size_t count)
> -{
> -	memcpy(buf, attr->private + offset, count);
> -	return count;
> -}
> -
> -static struct bin_attribute firmware_attr = {
> -	.attr = {.name = "gvt_firmware", .mode = (S_IRUSR)},
> -	.read = gvt_firmware_read,
> -	.write = NULL,
> -	.mmap = NULL,
> -};
> +static BIN_ATTR_SIMPLE_ADMIN_RO(gvt_firmware);
>  
>  static int expose_firmware_sysfs(struct intel_gvt *gvt)
>  {
> @@ -107,10 +93,10 @@ static int expose_firmware_sysfs(struct
> intel_gvt *gvt) crc32_start = offsetof(struct gvt_firmware_header,
> version); h->crc32 = crc32_le(0, firmware + crc32_start, size -
> crc32_start); 
> -	firmware_attr.size = size;
> -	firmware_attr.private = firmware;
> +	bin_attr_gvt_firmware.size = size;
> +	bin_attr_gvt_firmware.private = firmware;
>  
> -	ret = device_create_bin_file(&pdev->dev, &firmware_attr);
> +	ret = device_create_bin_file(&pdev->dev,
> &bin_attr_gvt_firmware); if (ret) {
>  		vfree(firmware);
>  		return ret;
> @@ -122,8 +108,8 @@ static void clean_firmware_sysfs(struct intel_gvt
> *gvt) {
>  	struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev);
>  
> -	device_remove_bin_file(&pdev->dev, &firmware_attr);
> -	vfree(firmware_attr.private);
> +	device_remove_bin_file(&pdev->dev, &bin_attr_gvt_firmware);
> +	vfree(bin_attr_gvt_firmware.private);
>  }
>  
>  /**
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c index
> 427d370..6d4b51a 100644 ---
> a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c +++
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c @@ -73,14
> +73,7 @@ struct odvp_attr { struct device_attribute attr;
>  };
>  
> -static ssize_t data_vault_read(struct file *file, struct kobject
> *kobj,
> -	     struct bin_attribute *attr, char *buf, loff_t off,
> size_t count) -{
> -	memcpy(buf, attr->private + off, count);
> -	return count;
> -}
> -
> -static BIN_ATTR_RO(data_vault, 0);
> +static BIN_ATTR_SIMPLE_RO(data_vault);
>  
>  static struct bin_attribute *data_attributes[] = {
>  	&bin_attr_data_vault,
> diff --git a/init/initramfs.c b/init/initramfs.c
> index da79760..5193fae 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -575,15 +575,7 @@ static int __init initramfs_async_setup(char
> *str) #include <linux/initrd.h>
>  #include <linux/kexec.h>
>  
> -static ssize_t raw_read(struct file *file, struct kobject *kobj,
> -			struct bin_attribute *attr, char *buf,
> -			loff_t pos, size_t count)
> -{
> -	memcpy(buf, attr->private + pos, count);
> -	return count;
> -}
> -
> -static BIN_ATTR(initrd, 0440, raw_read, NULL, 0);
> +static BIN_ATTR(initrd, 0440, sysfs_bin_attr_simple_read, NULL, 0);
>  
>  void __init reserve_initrd_mem(void)
>  {
> diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
> index d964167..26efe13 100644
> --- a/kernel/module/sysfs.c
> +++ b/kernel/module/sysfs.c
> @@ -146,17 +146,6 @@ struct module_notes_attrs {
>  	struct bin_attribute attrs[] __counted_by(notes);
>  };
>  
> -static ssize_t module_notes_read(struct file *filp, struct kobject
> *kobj,
> -				 struct bin_attribute *bin_attr,
> -				 char *buf, loff_t pos, size_t count)
> -{
> -	/*
> -	 * The caller checked the pos and count against our size.
> -	 */
> -	memcpy(buf, bin_attr->private + pos, count);
> -	return count;
> -}
> -
>  static void free_notes_attrs(struct module_notes_attrs *notes_attrs,
>  			     unsigned int i)
>  {
> @@ -205,7 +194,7 @@ static void add_notes_attrs(struct module *mod,
> const struct load_info *info) nattr->attr.mode = 0444;
>  			nattr->size = info->sechdrs[i].sh_size;
>  			nattr->private = (void
> *)info->sechdrs[i].sh_addr;
> -			nattr->read = module_notes_read;
> +			nattr->read = sysfs_bin_attr_simple_read;
>  			++nattr;
>  		}
>  		++loaded;


^ permalink raw reply

* [PATCH v2] cppc_cpufreq: Fix possible null pointer dereference
From: Aleksandr Mishin @ 2024-04-08  9:35 UTC (permalink / raw)
  To: Ionela Voinescu
  Cc: Aleksandr Mishin, Rafael J. Wysocki, Viresh Kumar, linux-pm,
	linux-kernel, lvc-project
In-Reply-To: <20240405094005.18545-1-amishin@t-argos.ru>

cppc_cpufreq_get_rate() and hisi_cppc_cpufreq_get_rate() can be called from
different places with various parameters. So cpufreq_cpu_get() can return
null as 'policy' in some circumstances.
Fix this bug by adding null return check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: a28b2bfc099c ("cppc_cpufreq: replace per-cpu data array with a list")
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
---
v2: Fix mixed declarations

 drivers/cpufreq/cppc_cpufreq.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 64420d9cfd1e..15f1d41920a3 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -741,10 +741,15 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
 {
 	struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
 	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
-	struct cppc_cpudata *cpu_data = policy->driver_data;
+	struct cppc_cpudata *cpu_data;
 	u64 delivered_perf;
 	int ret;
 
+	if (!policy)
+		return -ENODEV;
+
+	cpu_data = policy->driver_data;
+
 	cpufreq_cpu_put(policy);
 
 	ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t0);
@@ -822,10 +827,15 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
 static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpu)
 {
 	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
-	struct cppc_cpudata *cpu_data = policy->driver_data;
+	struct cppc_cpudata *cpu_data;
 	u64 desired_perf;
 	int ret;
 
+	if (!policy)
+		return -ENODEV;
+
+	cpu_data = policy->driver_data;
+
 	cpufreq_cpu_put(policy);
 
 	ret = cppc_get_desired_perf(cpu, &desired_perf);
-- 
2.30.2


^ permalink raw reply related

* [Bug 218689] AMD_Pstate_EPP Ryzen 7000 issues. Freezing and static sound
From: bugzilla-daemon @ 2024-04-08  9:16 UTC (permalink / raw)
  To: linux-pm
In-Reply-To: <bug-218689-137361@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=218689

Artem S. Tashkinov (aros@gmx.com) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|aros@gmx.com                |

--- Comment #7 from Artem S. Tashkinov (aros@gmx.com) ---
> if there were* to be a hardware issue^^^

AFAIK you're the _only_ person using Linux who experiences such issues. So far
there have been _zero_ reports about complications related to the amd-pstate
driver.

It may not work at all but it doesn't and cannot lead to freezes or sounds.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks
From: Ard Biesheuvel @ 2024-04-08  8:42 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
	Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare,
	linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano,
	linux-pm, Luis Chamberlain, linux-modules
In-Reply-To: <cover.1712410202.git.lukas@wunner.de>

On Sat, 6 Apr 2024 at 15:52, Lukas Wunner <lukas@wunner.de> wrote:
>
> For my upcoming PCI device authentication v2 patches, I have the need
> to expose a simple buffer in virtual memory as a bin_attribute.
>
> It turns out we've duplicated the ->read() callback for such simple
> buffers a fair number of times across the tree.
>
> So instead of reinventing the wheel, I decided to introduce a common
> helper and eliminate all duplications I could find.
>
> I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read()
> name. ;)
>
> Lukas Wunner (2):
>   sysfs: Add sysfs_bin_attr_simple_read() helper
>   treewide: Use sysfs_bin_attr_simple_read() helper
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

>  arch/powerpc/platforms/powernv/opal.c              | 10 +-------
>  drivers/acpi/bgrt.c                                |  9 +-------
>  drivers/firmware/dmi_scan.c                        | 12 ++--------
>  drivers/firmware/efi/rci2-table.c                  | 10 +-------
>  drivers/gpu/drm/i915/gvt/firmware.c                | 26 +++++----------------
>  .../intel/int340x_thermal/int3400_thermal.c        |  9 +-------
>  fs/sysfs/file.c                                    | 27 ++++++++++++++++++++++
>  include/linux/sysfs.h                              | 15 ++++++++++++
>  init/initramfs.c                                   | 10 +-------
>  kernel/module/sysfs.c                              | 13 +----------
>  10 files changed, 56 insertions(+), 85 deletions(-)
>
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v2] PM: s2idle: Make sure CPUs will wakeup directly on resume
From: Peter Zijlstra @ 2024-04-08  8:40 UTC (permalink / raw)
  To: Anna-Maria Behnsen
  Cc: linux-kernel, Rafael J . Wysocki, Pavel Machek, Len Brown,
	Ulf Hansson, linux-pm, Frederic Weisbecker, Thomas Gleixner, x86,
	Mario Limonciello, stable
In-Reply-To: <87r0fg5ocg.fsf@somnus>

On Mon, Apr 08, 2024 at 09:02:23AM +0200, Anna-Maria Behnsen wrote:
> s2idle works like a regular suspend with freezing processes and freezing
> devices. All CPUs except the control CPU go into idle. Once this is
> completed the control CPU kicks all other CPUs out of idle, so that they
> reenter the idle loop and then enter s2idle state. The control CPU then
> issues an swait() on the suspend state and therefore enters the idle loop
> as well.
> 
> Due to being kicked out of idle, the other CPUs leave their NOHZ states,
> which means the tick is active and the corresponding hrtimer is programmed
> to the next jiffie.
> 
> On entering s2idle the CPUs shut down their local clockevent device to
> prevent wakeups. The last CPU which enters s2idle shuts down its local
> clockevent and freezes timekeeping.
> 
> On resume, one of the CPUs receives the wakeup interrupt, unfreezes
> timekeeping and its local clockevent and starts the resume process. At that
> point all other CPUs are still in s2idle with their clockevents switched
> off. They only resume when they are kicked by another CPU or after resuming
> devices and then receiving a device interrupt.
> 
> That means there is no guarantee that all CPUs will wakeup directly on
> resume. As a consequence there is no guarantee that timers which are queued
> on those CPUs and should expire directly after resume, are handled. Also
> timer list timers which are remotely queued to one of those CPUs after
> resume will not result in a reprogramming IPI as the tick is
> active. Queueing a hrtimer will also not result in a reprogramming IPI
> because the first hrtimer event is already in the past.
> 
> The recent introduction of the timer pull model (7ee988770326 ("timers:
> Implement the hierarchical pull model")) amplifies this problem, if the
> current migrator is one of the non woken up CPUs. When a non pinned timer
> list timer is queued and the queuing CPU goes idle, it relies on the still
> suspended migrator CPU to expire the timer which will happen by chance.
> 
> The problem exists since commit 8d89835b0467 ("PM: suspend: Do not pause
> cpuidle in the suspend-to-idle path"). There the cpuidle_pause() call which
> in turn invoked a wakeup for all idle CPUs was moved to a later point in
> the resume process. This might not be reached or reached very late because
> it waits on a timer of a still suspended CPU.
> 
> Address this by kicking all CPUs out of idle after the control CPU returns
> from swait() so that they resume their timers and restore consistent system
> state.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218641
> Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Cc: stable@kernel.org

Cute,

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  kernel/power/suspend.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -106,6 +106,12 @@ static void s2idle_enter(void)
>  	swait_event_exclusive(s2idle_wait_head,
>  		    s2idle_state == S2IDLE_STATE_WAKE);
>  
> +	/*
> +	 * Kick all CPUs to ensure that they resume their timers and restore
> +	 * consistent system state.
> +	 */
> +	wake_up_all_idle_cpus();
> +
>  	cpus_read_unlock();
>  
>  	raw_spin_lock_irq(&s2idle_lock);

^ permalink raw reply

* Re: [PATCH 2/2] thermal/drivers/mediatek/lvts_thermal: Improve some memory allocation
From: Dan Carpenter @ 2024-04-08  8:09 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Matthias Brugger, AngeloGioacchino Del Regno, linux-kernel,
	kernel-janitors, linux-pm, linux-arm-kernel, linux-mediatek
In-Reply-To: <8cb69f245311a348164b0b5ca3dbc59386746035.1712520052.git.christophe.jaillet@wanadoo.fr>

On Sun, Apr 07, 2024 at 10:01:49PM +0200, Christophe JAILLET wrote:
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 3003dc350766..b133f731c5ba 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -204,7 +204,7 @@ static const struct debugfs_reg32 lvts_regs[] = {
>  
>  static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
>  {
> -	struct debugfs_regset32 *regset;
> +	struct debugfs_regset32 *regsets;
>  	struct lvts_ctrl *lvts_ctrl;
>  	struct dentry *dentry;
>  	char name[64];
> @@ -214,8 +214,14 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
>  	if (IS_ERR(lvts_td->dom_dentry))
>  		return 0;
>  
> +	regsets = devm_kcalloc(dev, lvts_td->num_lvts_ctrl,
> +			       sizeof(*regsets), GFP_KERNEL);
> +	if (!regsets)
> +		return 0;

I understand that this preserved the behavior from the original code,
but the original code was wrong.  This should return -ENOMEM.

> +
>  	for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {
>  
> +		struct debugfs_regset32 *regset = &regsets[i];
>  		lvts_ctrl = &lvts_td->lvts_ctrl[i];

The blank line should come after the declaration.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH v2 1/6] media: platform: cros-ec: provide ID table for avoiding fallback match
From: Hans Verkuil @ 2024-04-08  7:36 UTC (permalink / raw)
  To: Tzung-Bi Shih, bleung, groeck, linus.walleij, brgl, mchehab, sre
  Cc: chrome-platform, pmalani, linux-gpio, linux-media, linux-pm, krzk,
	Krzysztof Kozlowski
In-Reply-To: <20240401030052.2887845-2-tzungbi@kernel.org>

On 01/04/2024 05:00, Tzung-Bi Shih wrote:
> Instead of using fallback driver name match, provide ID table[1] for the
> primary match.
> 
> [1]: https://elixir.bootlin.com/linux/v6.8/source/drivers/base/platform.c#L1353
> 
> Reviewed-by: Benson Leung <bleung@chromium.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Looks OK, I'll queue this patch up for v6.10.

Regards,

	Hans

> ---
> Changes from v1:
> - No code changes.
> - Add R-b tags.
> 
>  drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> index 48ed2993d2f0..8fbbb4091455 100644
> --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> @@ -8,6 +8,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
>  #include <linux/dmi.h>
>  #include <linux/pci.h>
> @@ -573,6 +574,12 @@ static void cros_ec_cec_remove(struct platform_device *pdev)
>  	}
>  }
>  
> +static const struct platform_device_id cros_ec_cec_id[] = {
> +	{ DRV_NAME, 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(platform, cros_ec_cec_id);
> +
>  static struct platform_driver cros_ec_cec_driver = {
>  	.probe = cros_ec_cec_probe,
>  	.remove_new = cros_ec_cec_remove,
> @@ -580,6 +587,7 @@ static struct platform_driver cros_ec_cec_driver = {
>  		.name = DRV_NAME,
>  		.pm = &cros_ec_cec_pm_ops,
>  	},
> +	.id_table = cros_ec_cec_id,
>  };
>  
>  module_platform_driver(cros_ec_cec_driver);
> @@ -587,4 +595,3 @@ module_platform_driver(cros_ec_cec_driver);
>  MODULE_DESCRIPTION("CEC driver for ChromeOS ECs");
>  MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>  MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:" DRV_NAME);


^ permalink raw reply

* [PATCH v2] PM: s2idle: Make sure CPUs will wakeup directly on resume
From: Anna-Maria Behnsen @ 2024-04-08  7:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rafael J . Wysocki, Pavel Machek, Len Brown, Ulf Hansson,
	linux-pm, Frederic Weisbecker, Thomas Gleixner, x86,
	Mario Limonciello, stable
In-Reply-To: <20240405083410.4896-1-anna-maria@linutronix.de>

s2idle works like a regular suspend with freezing processes and freezing
devices. All CPUs except the control CPU go into idle. Once this is
completed the control CPU kicks all other CPUs out of idle, so that they
reenter the idle loop and then enter s2idle state. The control CPU then
issues an swait() on the suspend state and therefore enters the idle loop
as well.

Due to being kicked out of idle, the other CPUs leave their NOHZ states,
which means the tick is active and the corresponding hrtimer is programmed
to the next jiffie.

On entering s2idle the CPUs shut down their local clockevent device to
prevent wakeups. The last CPU which enters s2idle shuts down its local
clockevent and freezes timekeeping.

On resume, one of the CPUs receives the wakeup interrupt, unfreezes
timekeeping and its local clockevent and starts the resume process. At that
point all other CPUs are still in s2idle with their clockevents switched
off. They only resume when they are kicked by another CPU or after resuming
devices and then receiving a device interrupt.

That means there is no guarantee that all CPUs will wakeup directly on
resume. As a consequence there is no guarantee that timers which are queued
on those CPUs and should expire directly after resume, are handled. Also
timer list timers which are remotely queued to one of those CPUs after
resume will not result in a reprogramming IPI as the tick is
active. Queueing a hrtimer will also not result in a reprogramming IPI
because the first hrtimer event is already in the past.

The recent introduction of the timer pull model (7ee988770326 ("timers:
Implement the hierarchical pull model")) amplifies this problem, if the
current migrator is one of the non woken up CPUs. When a non pinned timer
list timer is queued and the queuing CPU goes idle, it relies on the still
suspended migrator CPU to expire the timer which will happen by chance.

The problem exists since commit 8d89835b0467 ("PM: suspend: Do not pause
cpuidle in the suspend-to-idle path"). There the cpuidle_pause() call which
in turn invoked a wakeup for all idle CPUs was moved to a later point in
the resume process. This might not be reached or reached very late because
it waits on a timer of a still suspended CPU.

Address this by kicking all CPUs out of idle after the control CPU returns
from swait() so that they resume their timers and restore consistent system
state.

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218641
Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Cc: stable@kernel.org
---
v2: Fix typos in commit message
---
 kernel/power/suspend.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -106,6 +106,12 @@ static void s2idle_enter(void)
 	swait_event_exclusive(s2idle_wait_head,
 		    s2idle_state == S2IDLE_STATE_WAKE);
 
+	/*
+	 * Kick all CPUs to ensure that they resume their timers and restore
+	 * consistent system state.
+	 */
+	wake_up_all_idle_cpus();
+
 	cpus_read_unlock();
 
 	raw_spin_lock_irq(&s2idle_lock);

^ permalink raw reply

* [PATCH 1/2] dt-bindings: power: supply: ltc3350-charger: Add bindings
From: Mike Looijmans @ 2024-04-08  6:49 UTC (permalink / raw)
  To: linux-pm
  Cc: Mike Looijmans, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
	Sebastian Reichel, devicetree, linux-kernel
In-Reply-To: <1b153bce-a66a-45ee-a5c6-963ea6fb1c82.949ef384-8293-46b8-903f-40a477c056ae.3bc97c8f-843d-4e46-84f9-469b7ba2489d@emailsignatures365.codetwo.com>

The LTC3350 is a backup power controller that can charge and monitor
a series stack of one to four supercapacitors.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---

 .../bindings/power/supply/ltc3350.yaml        | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/ltc3350.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/ltc3350.yaml b/Documentation/devicetree/bindings/power/supply/ltc3350.yaml
new file mode 100644
index 000000000000..607a62fd25ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/ltc3350.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2024 Topic Embedded Products
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/ltc3350.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linear Technology (Analog Devices) LTC3350 Supercap Charger
+
+maintainers:
+  - Mike Looijmans <mike.looijmans@topic.nl>
+
+description: |
+  The LTC3350 is a  High Current Supercapacitor Backup Controller and System
+  Monitor.
+  Specifications about the charger can be found at:
+    https://www.analog.com/en/products/ltc3350.html
+
+properties:
+  compatible:
+    enum:
+      - lltc,ltc3350
+
+  reg:
+    maxItems: 1
+    description: I2C address of the charger.
+
+  lltc,rsnsc-micro-ohms:
+    description: Capacitor charger sense resistor in microohm.
+    minimum: 1000
+
+  lltc,rsnsi-micro-ohms:
+    description: Input current sense resistor in microohm.
+    minimum: 1000
+
+required:
+  - compatible
+  - reg
+  - lltc,rsnsc-micro-ohms
+  - lltc,rsnsi-micro-ohms
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      charger: battery-charger@9 {
+              compatible = "lltc,ltc3350";
+              reg = <0x9>;
+              lltc,rsnsc-micro-ohms = <10000>;
+              lltc,rsnsi-micro-ohms = <10000>;
+      };
+    };
-- 
2.34.1


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

^ permalink raw reply related

* [PATCH 2/2] power: supply: ltc3350-charger: Add driver
From: Mike Looijmans @ 2024-04-08  6:49 UTC (permalink / raw)
  To: linux-pm; +Cc: Mike Looijmans, Sebastian Reichel, linux-kernel
In-Reply-To: <20240408064905.20298-1-mike.looijmans@topic.nl>

The LTC3350 is a backup power controller that can charge and monitor
a series stack of one to four supercapacitors.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

---

 drivers/power/supply/Kconfig           |  10 +
 drivers/power/supply/Makefile          |   1 +
 drivers/power/supply/ltc3350-charger.c | 720 +++++++++++++++++++++++++
 3 files changed, 731 insertions(+)
 create mode 100644 drivers/power/supply/ltc3350-charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 3e31375491d5..7cb1a66e522d 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -514,6 +514,16 @@ config CHARGER_LT3651
 	  Say Y to include support for the Analog Devices (Linear Technology)
 	  LT3651 battery charger which reports its status via GPIO lines.
 
+config CHARGER_LTC3350
+	tristate "LTC3350 Supercapacitor Backup Controller and System Monitor"
+	depends on I2C
+	select REGMAP_I2C
+	select I2C_SMBUS
+	help
+	  Say Y to include support for the Analog Devices (Linear Technology)
+	  LTC3350 Supercapacitor Backup Controller and System Monitor connected
+	  to I2C.
+
 config CHARGER_LTC4162L
 	tristate "LTC4162-L charger"
 	depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 58b567278034..a8d618e4ac91 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_CHARGER_LP8788)	+= lp8788-charger.o
 obj-$(CONFIG_CHARGER_GPIO)	+= gpio-charger.o
 obj-$(CONFIG_CHARGER_MANAGER)	+= charger-manager.o
 obj-$(CONFIG_CHARGER_LT3651)	+= lt3651-charger.o
+obj-$(CONFIG_CHARGER_LTC3350)	+= ltc3350-charger.o
 obj-$(CONFIG_CHARGER_LTC4162L)	+= ltc4162-l-charger.o
 obj-$(CONFIG_CHARGER_MAX14577)	+= max14577_charger.o
 obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)	+= max14656_charger_detector.o
diff --git a/drivers/power/supply/ltc3350-charger.c b/drivers/power/supply/ltc3350-charger.c
new file mode 100644
index 000000000000..b3a23447ebf4
--- /dev/null
+++ b/drivers/power/supply/ltc3350-charger.c
@@ -0,0 +1,720 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for Analog Devices (Linear Technology) LTC3350
+ * High Current Supercapacitor Backup Controller and System Monitor
+ * Copyright (C) 2024, Topic Embedded Products
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/of_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/power_supply.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+/* Registers (names based on what datasheet uses) */
+#define LTC3350_REG_CLR_ALARMS		0x00
+#define LTC3350_REG_MSK_ALARMS		0x01
+#define LTC3350_REG_MSK_MON_STATUS	0x02
+#define LTC3350_REG_CAP_ESR_PER		0x04
+#define LTC3350_REG_VCAPFB_DAC		0x05
+#define LTC3350_REG_VSHUNT		0x06
+#define LTC3350_REG_CAP_UV_LVL		0x07
+#define LTC3350_REG_CAP_OV_LVL		0x08
+#define LTC3350_REG_GPI_UV_LVL		0x09
+#define LTC3350_REG_GPI_OV_LVL		0x0A
+#define LTC3350_REG_VIN_UV_LVL		0x0B
+#define LTC3350_REG_VIN_OV_LVL		0x0C
+#define LTC3350_REG_VCAP_UV_LVL		0x0D
+#define LTC3350_REG_VCAP_OV_LVL		0x0E
+#define LTC3350_REG_VOUT_UV_LVL		0x0F
+#define LTC3350_REG_VOUT_OV_LVL		0x10
+#define LTC3350_REG_IIN_OC_LVL		0x11
+#define LTC3350_REG_ICHG_UC_LVL		0x12
+#define LTC3350_REG_DTEMP_COLD_LVL	0x13
+#define LTC3350_REG_DTEMP_HOT_LVL	0x14
+#define LTC3350_REG_ESR_HI_LVL		0x15
+#define LTC3350_REG_CAP_LO_LVL		0x16
+#define LTC3350_REG_CTL_REG		0x17
+#define LTC3350_REG_NUM_CAPS		0x1A
+#define LTC3350_REG_CHRG_STATUS		0x1B
+#define LTC3350_REG_MON_STATUS		0x1C
+#define LTC3350_REG_ALARM_REG		0x1D
+#define LTC3350_REG_MEAS_CAP		0x1E
+#define LTC3350_REG_MEAS_ESR		0x1F
+#define LTC3350_REG_MEAS_VCAP1		0x20
+#define LTC3350_REG_MEAS_VCAP2		0x21
+#define LTC3350_REG_MEAS_VCAP3		0x22
+#define LTC3350_REG_MEAS_VCAP4		0x23
+#define LTC3350_REG_MEAS_GPI		0x24
+#define LTC3350_REG_MEAS_VIN		0x25
+#define LTC3350_REG_MEAS_VCAP		0x26
+#define LTC3350_REG_MEAS_VOUT		0x27
+#define LTC3350_REG_MEAS_IIN		0x28
+#define LTC3350_REG_MEAS_ICHG		0x29
+#define LTC3350_REG_MEAS_DTEMP		0x2A
+
+/* LTC3350_REG_CLR_ALARMS, LTC3350_REG_MASK_ALARMS, LTC3350_REG_ALARM_REG */
+#define LTC3350_MSK_CAP_UV	BIT(0)	/* capacitor undervoltage alarm */
+#define LTC3350_MSK_CAP_OV	BIT(1)	/* capacitor overvoltage alarm */
+#define LTC3350_MSK_GPI_UV	BIT(2)	/* GPI undervoltage alarm */
+#define LTC3350_MSK_GPI_OV	BIT(3)	/* GPI overvoltage alarm */
+#define LTC3350_MSK_VIN_UV	BIT(4)	/* VIN undervoltage alarm */
+#define LTC3350_MSK_VIN_OV	BIT(5)	/* VIN overvoltage alarm */
+#define LTC3350_MSK_VCAP_UV	BIT(6)	/* VCAP undervoltage alarm */
+#define LTC3350_MSK_VCAP_OV	BIT(7)	/* VCAP overvoltage alarm */
+#define LTC3350_MSK_VOUT_UV	BIT(8)	/* VOUT undervoltage alarm */
+#define LTC3350_MSK_VOUT_OV	BIT(9)	/* VOUT overvoltage alarm */
+#define LTC3350_MSK_IIN_OC	BIT(10)	/* input overcurrent alarm */
+#define LTC3350_MSK_ICHG_UC	BIT(11)	/* charge undercurrent alarm */
+#define LTC3350_MSK_DTEMP_COLD	BIT(12)	/* die temperature cold alarm */
+#define LTC3350_MSK_DTEMP_HOT	BIT(13)	/* die temperature hot alarm */
+#define LTC3350_MSK_ESR_HI	BIT(14)	/* ESR high alarm */
+#define LTC3350_MSK_CAP_LO	BIT(15)	/* capacitance low alarm */
+
+/* LTC3350_REG_MSK_MON_STATUS masks */
+#define LTC3350_MSK_MON_CAPESR_ACTIVE		BIT(0)
+#define LTC3350_MSK_MON_CAPESR_SCHEDULED	BIT(1)
+#define LTC3350_MSK_MON_CAPESR_PENDING		BIT(2)
+#define LTC3350_MSK_MON_CAP_DONE		BIT(3)
+#define LTC3350_MSK_MON_ESR_DONE		BIT(4)
+#define LTC3350_MSK_MON_CAP_FAILED		BIT(5)
+#define LTC3350_MSK_MON_ESR_FAILED		BIT(6)
+#define LTC3350_MSK_MON_POWER_FAILED		BIT(8)
+#define LTC3350_MSK_MON_POWER_RETURNED		BIT(9)
+
+/* LTC3350_REG_CTL_REG */
+/* Begin a capacitance and ESR measurement when possible */
+#define LTC3350_CTL_STRT_CAPESR		BIT(0)
+/* A one in this bit location enables the input buffer on the GPI pin */
+#define LTC3350_CTL_GPI_BUFFER_EN	BIT(1)
+/* Stops an active capacitance/ESR measurement */
+#define LTC3350_CTL_STOP_CAPESR		BIT(2)
+/* Increases capacitor measurement resolution by 100x for smaller capacitors */
+#define LTC3350_CTL_CAP_SCALE		BIT(3)
+
+/* LTC3350_REG_CHRG_STATUS */
+#define LTC3350_CHRG_STEPDOWN	BIT(0)	/* Synchronous controller in step-down mode (charging) */
+#define LTC3350_CHRG_STEPUP	BIT(1)	/* Synchronous controller in step-up mode (backup) */
+#define LTC3350_CHRG_CV		BIT(2)	/* The charger is in constant voltage mode */
+#define LTC3350_CHRG_UVLO	BIT(3)	/* The charger is in undervoltage lockout */
+#define LTC3350_CHRG_INPUT_ILIM	BIT(4)	/* The charger is in input current limit */
+#define LTC3350_CHRG_CAPPG	BIT(5)	/* The capacitor voltage is above power good threshold */
+#define LTC3350_CHRG_SHNT	BIT(6)	/* The capacitor manager is shunting */
+#define LTC3350_CHRG_BAL	BIT(7)	/* The capacitor manager is balancing */
+#define LTC3350_CHRG_DIS	BIT(8)	/* Charger disabled for capacitance measurement */
+#define LTC3350_CHRG_CI		BIT(9)	/* The charger is in constant current mode */
+#define LTC3350_CHRG_PFO	BIT(11)	/* Input voltage is below PFI threshold */
+
+/* LTC3350_REG_MON_STATUS */
+#define LTC3350_MON_CAPESR_ACTIVE	BIT(0)	/* Capacitance/ESR measurement in progress */
+#define LTC3350_MON_CAPESR_SCHEDULED	BIT(1)	/* Waiting programmed time */
+#define LTC3350_MON_CAPESR_PENDING	BIT(2)	/* Waiting for satisfactory conditions */
+#define LTC3350_MON_CAP_DONE		BIT(3)	/* Capacitance measurement has completed */
+#define LTC3350_MON_ESR_DONE		BIT(4)	/* ESR Measurement has completed */
+#define LTC3350_MON_CAP_FAILED		BIT(5)	/* Last capacitance measurement failed */
+#define LTC3350_MON_ESR_FAILED		BIT(6)	/* Last ESR measurement failed */
+#define LTC3350_MON_POWER_FAILED	BIT(8)	/* Unable to charge */
+#define LTC3350_MON_POWER_RETURNED	BIT(9)	/* Able to charge */
+
+
+struct ltc3350_info {
+	struct i2c_client	*client;
+	struct regmap		*regmap;
+	struct power_supply	*charger;
+	u32 rsnsc;	/* Series resistor that sets charge current, microOhm */
+	u32 rsnsi;	/* Series resistor to measure input current, microOhm */
+};
+
+/*
+ * About LTC3350 "alarm" functions: Setting a bit in the LTC3350_REG_MSK_ALARMS
+ * register enables the alarm. The alarm will trigger an SMBALERT only once.
+ * To reset the alarm, write a "1" bit to LTC3350_REG_CLR_ALARMS. Then the alarm
+ * will trigger another SMBALERT when conditions are met (may be immediately).
+ * After writing to one of the corresponding level registers, enable the alarm,
+ * so that a UEVENT triggers when the alarm goes off.
+ */
+static void ltc3350_enable_alarm(struct ltc3350_info *info, unsigned int reg)
+{
+	unsigned int mask;
+
+	/* Register locations correspond to alarm mask bits */
+	mask = BIT(reg - LTC3350_REG_CAP_UV_LVL);
+	/* Clear the alarm bit so it may trigger again */
+	regmap_write(info->regmap, LTC3350_REG_CLR_ALARMS, mask);
+	/* Enable the alarm */
+	regmap_update_bits(info->regmap, LTC3350_REG_MSK_ALARMS, mask, mask);
+}
+
+/* Convert enum value to POWER_SUPPLY_STATUS value */
+static int ltc3350_state_decode(unsigned int value)
+{
+	if (value & LTC3350_CHRG_STEPUP)
+		return POWER_SUPPLY_STATUS_DISCHARGING; /* running on backup */
+
+	if (value & LTC3350_CHRG_PFO)
+		return POWER_SUPPLY_STATUS_NOT_CHARGING;
+
+	if (value & LTC3350_CHRG_STEPDOWN) {
+		/* The chip remains in CV mode indefinitely, hence "full" */
+		if (value & LTC3350_CHRG_CV)
+			return POWER_SUPPLY_STATUS_FULL;
+
+		return POWER_SUPPLY_STATUS_CHARGING;
+	}
+
+	/* Not in step down? Must be full then (never seen this) */
+	return POWER_SUPPLY_STATUS_FULL;
+};
+
+static int ltc3350_get_status(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_CHRG_STATUS, &regval);
+	if (ret)
+		return ret;
+
+	val->intval = ltc3350_state_decode(regval);
+
+	return 0;
+}
+
+static int ltc3350_charge_status_decode(unsigned int value)
+{
+	if (!(value & LTC3350_CHRG_STEPDOWN))
+		return POWER_SUPPLY_CHARGE_TYPE_NONE;
+
+	/* constant voltage is "topping off" */
+	if (value & LTC3350_CHRG_CV)
+		return POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+
+	/* input limiter */
+	if (value & LTC3350_CHRG_INPUT_ILIM)
+		return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE;
+
+	/* constant current is "fast" */
+	if (value & LTC3350_CHRG_CI)
+		return POWER_SUPPLY_CHARGE_TYPE_FAST;
+
+	return POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+}
+
+static int ltc3350_get_charge_type(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_CHRG_STATUS, &regval);
+	if (ret)
+		return ret;
+
+	val->intval = ltc3350_charge_status_decode(regval);
+
+	return 0;
+}
+
+static int ltc3350_get_online(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_MON_STATUS, &regval);
+	if (ret)
+		return ret;
+
+	/* indicates if input voltage is sufficient to charge */
+	val->intval = !!(regval & LTC3350_MON_POWER_RETURNED);
+
+	return 0;
+}
+
+static int ltc3350_get_input_voltage(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_MEAS_VIN, &regval);
+	if (ret)
+		return ret;
+
+	/* 2.21mV/LSB */
+	val->intval =  regval * 2210;
+
+	return 0;
+}
+
+static int ltc3350_get_input_current(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_MEAS_IIN, &regval);
+	if (ret)
+		return ret;
+
+	/* 1.983µV/RSNSI amperes per LSB */
+	ret = regval * 19830;
+	ret /= info->rsnsi;
+	ret *= 100;
+
+	val->intval = ret;
+
+	return 0;
+}
+
+static int ltc3350_get_icharge(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_MEAS_ICHG, &regval);
+	if (ret)
+		return ret;
+
+	/* 1.983µV/RSNSC amperes per LSB */
+	ret = regval * 19830;
+	ret /= info->rsnsc;
+	ret *= 100;
+
+	val->intval = ret;
+
+	return 0;
+}
+
+static int ltc3350_get_icharge_max(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	/* I_CHG(MAX) = 32mV / RSNSC (Ampere) */
+	val->intval = 3200000000U / (info->rsnsc / 10);
+
+	return 0;
+}
+
+static int ltc3350_get_iin_max(struct ltc3350_info *info, union power_supply_propval *val)
+{
+	/* I_IN(MAX) = 32mV / RSNSI (Ampere) */
+	val->intval = 3200000000U / (info->rsnsi / 10);
+
+	return 0;
+}
+
+
+static int ltc3350_get_die_temp(struct ltc3350_info *info, unsigned int reg,
+				union power_supply_propval *val)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, reg, &regval);
+	if (ret)
+		return ret;
+
+	/* 0.028°C per LSB – 251.4°C */
+	ret = 280 * regval;
+	ret /= 100; /* Centidegrees scale */
+	ret -= 25140;
+	val->intval = ret;
+
+	return 0;
+}
+
+static int ltc3350_set_die_temp(struct ltc3350_info *info, unsigned int reg, int val)
+{
+	unsigned int regval;
+	int ret;
+
+	/* 0.028°C per LSB – 251.4°C */
+	regval = val + 25140;
+	regval *= 100;
+	regval /= 280;
+
+	ret = regmap_write(info->regmap, reg, regval);
+	if (ret)
+		return ret;
+
+	ltc3350_enable_alarm(info, reg);
+	return 0;
+}
+
+/* Custom properties */
+
+static ssize_t ltc3350_attr_show(struct device *dev,
+				 struct device_attribute *attr, char *buf,
+				 unsigned int reg, unsigned int scale)
+{
+	struct power_supply *psy = to_power_supply(dev);
+	struct ltc3350_info *info = power_supply_get_drvdata(psy);
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, reg, &regval);
+	if (ret)
+		return ret;
+
+	regval *= scale; /* Scale is in 10 μV units */
+	regval /= 10;
+
+	return sprintf(buf, "%u\n", regval);
+}
+
+static ssize_t ltc3350_attr_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count,
+				  unsigned int reg, unsigned int scale)
+{
+	struct power_supply *psy = to_power_supply(dev);
+	struct ltc3350_info *info = power_supply_get_drvdata(psy);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	val *= 10;
+	val = DIV_ROUND_CLOSEST(val, scale); /* Scale is in 10 μV units */
+
+	ret = regmap_write(info->regmap, reg, val);
+	if (ret)
+		return ret;
+
+	/* When writing to one of the LVL registers, update the alarm mask */
+	if (reg >= LTC3350_REG_CAP_UV_LVL && reg <= LTC3350_REG_CAP_LO_LVL)
+		ltc3350_enable_alarm(info, reg);
+
+	return count;
+}
+
+#define LTC3350_DEVICE_ATTR_RO(_name, _reg, _scale)				\
+static ssize_t _name##_show(struct device *dev, struct device_attribute *attr,	\
+			    char *buf)						\
+{										\
+	return ltc3350_attr_show(dev, attr, buf, _reg, _scale);			\
+}										\
+static DEVICE_ATTR_RO(_name)
+
+#define LTC3350_DEVICE_ATTR_RW(_name, _reg, _scale)				\
+static ssize_t _name##_show(struct device *dev, struct device_attribute *attr,	\
+			    char *buf)						\
+{										\
+	return ltc3350_attr_show(dev, attr, buf, _reg, _scale);			\
+}										\
+static ssize_t _name##_store(struct device *dev, struct device_attribute *attr, \
+			     const char *buf, size_t count)			\
+{										\
+	return ltc3350_attr_store(dev, attr, buf, count, _reg, _scale);		\
+}										\
+static DEVICE_ATTR_RW(_name)
+
+/* Shunt voltage, 183.5μV per LSB */
+LTC3350_DEVICE_ATTR_RW(vshunt, LTC3350_REG_VSHUNT, 1835);
+
+/* Single capacitor voltages, 183.5μV per LSB */
+LTC3350_DEVICE_ATTR_RO(vcap1, LTC3350_REG_MEAS_VCAP1, 1835);
+LTC3350_DEVICE_ATTR_RO(vcap2, LTC3350_REG_MEAS_VCAP2, 1835);
+LTC3350_DEVICE_ATTR_RO(vcap3, LTC3350_REG_MEAS_VCAP3, 1835);
+LTC3350_DEVICE_ATTR_RO(vcap4, LTC3350_REG_MEAS_VCAP4, 1835);
+LTC3350_DEVICE_ATTR_RW(cap_uv, LTC3350_REG_CAP_UV_LVL, 1835);
+LTC3350_DEVICE_ATTR_RW(cap_ov, LTC3350_REG_CAP_OV_LVL, 1835);
+
+/* General purpose input, 183.5μV per LSB */
+LTC3350_DEVICE_ATTR_RO(gpi, LTC3350_REG_MEAS_GPI, 1835);
+LTC3350_DEVICE_ATTR_RW(gpi_uv, LTC3350_REG_GPI_UV_LVL, 1835);
+LTC3350_DEVICE_ATTR_RW(gpi_ov, LTC3350_REG_GPI_OV_LVL, 1835);
+
+/* Input voltage, 2.21mV per LSB */
+LTC3350_DEVICE_ATTR_RO(vin, LTC3350_REG_MEAS_VIN, 22100);
+LTC3350_DEVICE_ATTR_RW(vin_uv, LTC3350_REG_VIN_UV_LVL, 22100);
+LTC3350_DEVICE_ATTR_RW(vin_ov, LTC3350_REG_VIN_OV_LVL, 22100);
+
+/* Capacitor stack voltage, 1.476 mV per LSB */
+LTC3350_DEVICE_ATTR_RO(vcap, LTC3350_REG_MEAS_VCAP, 14760);
+LTC3350_DEVICE_ATTR_RW(vcap_uv, LTC3350_REG_VCAP_UV_LVL, 14760);
+LTC3350_DEVICE_ATTR_RW(vcap_ov, LTC3350_REG_VCAP_OV_LVL, 14760);
+
+/* Output, 2.21mV per LSB */
+LTC3350_DEVICE_ATTR_RO(vout, LTC3350_REG_MEAS_VOUT, 22100);
+LTC3350_DEVICE_ATTR_RW(vout_uv, LTC3350_REG_VOUT_UV_LVL, 22100);
+LTC3350_DEVICE_ATTR_RW(vout_ov, LTC3350_REG_VOUT_OV_LVL, 22100);
+
+static ssize_t num_caps_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct power_supply *psy = to_power_supply(dev);
+	struct ltc3350_info *info = power_supply_get_drvdata(psy);
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_NUM_CAPS, &regval);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%u\n", regval + 1);
+}
+static DEVICE_ATTR_RO(num_caps);
+
+static ssize_t charge_status_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct power_supply *psy = to_power_supply(dev);
+	struct ltc3350_info *info = power_supply_get_drvdata(psy);
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(info->regmap, LTC3350_REG_CHRG_STATUS, &regval);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "0x%x\n", regval);
+}
+static DEVICE_ATTR_RO(charge_status);
+
+static struct attribute *ltc3350_sysfs_entries[] = {
+	&dev_attr_vshunt.attr,
+	&dev_attr_vcap1.attr,
+	&dev_attr_vcap2.attr,
+	&dev_attr_vcap3.attr,
+	&dev_attr_vcap4.attr,
+	&dev_attr_cap_uv.attr,
+	&dev_attr_cap_ov.attr,
+	&dev_attr_gpi.attr,
+	&dev_attr_gpi_uv.attr,
+	&dev_attr_gpi_ov.attr,
+	&dev_attr_vin_uv.attr,
+	&dev_attr_vin_ov.attr,
+	&dev_attr_vcap.attr,
+	&dev_attr_vcap_uv.attr,
+	&dev_attr_vcap_ov.attr,
+	&dev_attr_vin.attr,
+	&dev_attr_vin_uv.attr,
+	&dev_attr_vin_ov.attr,
+	&dev_attr_vout.attr,
+	&dev_attr_vout_uv.attr,
+	&dev_attr_vout_ov.attr,
+	&dev_attr_num_caps.attr,
+	&dev_attr_charge_status.attr,
+	NULL,
+};
+
+static const struct attribute_group ltc3350_attr_group = {
+	.name	= NULL,	/* put in device directory */
+	.attrs	= ltc3350_sysfs_entries,
+};
+
+static const struct attribute_group *ltc3350_attr_groups[] = {
+	&ltc3350_attr_group,
+	NULL,
+};
+
+static int ltc3350_get_property(struct power_supply *psy, enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct ltc3350_info *info = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		return ltc3350_get_status(info, val);
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		return ltc3350_get_charge_type(info, val);
+	case POWER_SUPPLY_PROP_ONLINE:
+		return ltc3350_get_online(info, val);
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		return ltc3350_get_input_voltage(info, val);
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		return ltc3350_get_input_current(info, val);
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		return ltc3350_get_icharge(info, val);
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		return ltc3350_get_icharge_max(info, val);
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		return ltc3350_get_iin_max(info, val);
+	case POWER_SUPPLY_PROP_TEMP:
+		return ltc3350_get_die_temp(info, LTC3350_REG_MEAS_DTEMP, val);
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+		return ltc3350_get_die_temp(info, LTC3350_REG_DTEMP_COLD_LVL, val);
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+		return ltc3350_get_die_temp(info, LTC3350_REG_DTEMP_HOT_LVL, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ltc3350_set_property(struct power_supply *psy, enum power_supply_property psp,
+				const union power_supply_propval *val)
+{
+	struct ltc3350_info *info = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+		return ltc3350_set_die_temp(info, LTC3350_REG_DTEMP_COLD_LVL, val->intval);
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+		return ltc3350_set_die_temp(info, LTC3350_REG_DTEMP_HOT_LVL, val->intval);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ltc3350_property_is_writeable(struct power_supply *psy, enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+/* Charger power supply property routines */
+static enum power_supply_property ltc3350_properties[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
+	POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
+};
+
+static const struct power_supply_desc ltc3350_desc = {
+	.name		= "ltc3350",
+	.type		= POWER_SUPPLY_TYPE_MAINS,
+	.properties	= ltc3350_properties,
+	.num_properties	= ARRAY_SIZE(ltc3350_properties),
+	.get_property	= ltc3350_get_property,
+	.set_property	= ltc3350_set_property,
+	.property_is_writeable = ltc3350_property_is_writeable,
+};
+
+static bool ltc3350_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	/* all registers up to this one are writeable */
+	return reg < LTC3350_REG_NUM_CAPS;
+}
+
+static bool ltc3350_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	/* read-only status registers and self-clearing register */
+	return reg > LTC3350_REG_NUM_CAPS || reg == LTC3350_REG_CLR_ALARMS;
+}
+
+static const struct regmap_config ltc3350_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 16,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.writeable_reg	= ltc3350_is_writeable_reg,
+	.volatile_reg	= ltc3350_is_volatile_reg,
+	.max_register	= LTC3350_REG_MEAS_DTEMP,
+	.cache_type	= REGCACHE_MAPLE,
+};
+
+static int ltc3350_probe(struct i2c_client *client)
+{
+	struct i2c_adapter *adapter = client->adapter;
+	struct device *dev = &client->dev;
+	struct ltc3350_info *info;
+	struct power_supply_config ltc3350_config = {};
+	int ret;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
+		dev_err(dev, "No support for SMBUS_WORD_DATA\n");
+		return -ENODEV;
+	}
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->client = client;
+	i2c_set_clientdata(client, info);
+
+	info->regmap = devm_regmap_init_i2c(client, &ltc3350_regmap_config);
+	if (IS_ERR(info->regmap)) {
+		dev_err(dev, "Failed to initialize register map\n");
+		return PTR_ERR(info->regmap);
+	}
+
+	ret = device_property_read_u32(dev, "lltc,rsnsc-micro-ohms",
+				       &info->rsnsc);
+	if (ret) {
+		dev_err(dev, "Missing lltc,rsnsc-micro-ohms property\n");
+		return ret;
+	}
+	if (!info->rsnsc)
+		return -EINVAL;
+
+	ret = device_property_read_u32(dev, "lltc,rsnsi-micro-ohms",
+				       &info->rsnsi);
+	if (ret) {
+		dev_err(dev, "Missing lltc,rsnsi-micro-ohms property\n");
+		return ret;
+	}
+	if (!info->rsnsi)
+		return -EINVAL;
+
+	/* Clear and disable all interrupt sources*/
+	ret = regmap_write(info->regmap, LTC3350_REG_CLR_ALARMS, 0xFFFF);
+	if (ret) {
+		dev_err(dev, "Failed to write configuration\n");
+		return ret;
+	}
+	regmap_write(info->regmap, LTC3350_REG_MSK_ALARMS, 0);
+	regmap_write(info->regmap, LTC3350_REG_MSK_MON_STATUS, 0);
+
+	ltc3350_config.of_node = dev->of_node;
+	ltc3350_config.drv_data = info;
+	ltc3350_config.attr_grp = ltc3350_attr_groups;
+
+	info->charger = devm_power_supply_register(dev, &ltc3350_desc,
+						   &ltc3350_config);
+	if (IS_ERR(info->charger)) {
+		dev_err(dev, "Failed to register charger\n");
+		return PTR_ERR(info->charger);
+	}
+
+	/* Enable interrupts on status changes */
+	regmap_write(info->regmap, LTC3350_REG_MSK_MON_STATUS,
+		     LTC3350_MON_POWER_FAILED | LTC3350_MON_POWER_RETURNED);
+
+	return 0;
+}
+
+static void ltc3350_alert(struct i2c_client *client, enum i2c_alert_protocol type,
+			  unsigned int flag)
+{
+	struct ltc3350_info *info = i2c_get_clientdata(client);
+
+	if (type != I2C_PROTOCOL_SMBUS_ALERT)
+		return;
+
+	power_supply_changed(info->charger);
+}
+
+static const struct i2c_device_id ltc3350_i2c_id_table[] = {
+	{ "ltc3350", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, ltc3350_i2c_id_table);
+
+static const struct of_device_id ltc3350_of_match[] = {
+	{ .compatible = "lltc,ltc3350", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ltc3350_of_match);
+
+static struct i2c_driver ltc3350_driver = {
+	.probe		= ltc3350_probe,
+	.alert		= ltc3350_alert,
+	.id_table	= ltc3350_i2c_id_table,
+	.driver = {
+		.name		= "ltc3350-charger",
+		.of_match_table	= of_match_ptr(ltc3350_of_match),
+	},
+};
+module_i2c_driver(ltc3350_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Mike Looijmans <mike.looijmans@topic.nl>");
+MODULE_DESCRIPTION("LTC3350 charger driver");
-- 
2.34.1


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

^ permalink raw reply related

* Re: [PATCH] power: supply: cros_usbpd: Don't show messages for no charging ports found
From: Tzung-Bi Shih @ 2024-04-08  6:29 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Benson Leung, Guenter Roeck, Sebastian Reichel,
	open list:CHROMEOS EC SUBDRIVERS,
	open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS, open list,
	Dustin L . Howett, Mario Limonciello
In-Reply-To: <20240406191734.137797-1-superm1@gmail.com>

On Sat, Apr 06, 2024 at 02:17:34PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> Framework 13 and 16 don't use cros_usbpd but do use cros_ec. The following
> sequence of messages is totally unnecessary.

In the case, the EC firmware shouldn't report the feature.  See [1].

[1]: https://elixir.bootlin.com/linux/v6.8/source/drivers/mfd/cros_ec_dev.c#L106

^ permalink raw reply

* [PATCH] powercap: intel_rapl: Add support for ArrowLake-H platform
From: Zhang Rui @ 2024-04-08  4:05 UTC (permalink / raw)
  To: rafael.j.wysocki; +Cc: linux-kernel, linux-pm

Add support for ArrowLake-H platform.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/powercap/intel_rapl_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index a28d54fd5222..c02851c73751 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -1263,6 +1263,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
 	X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,	&rapl_defaults_spr_server),
 	X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X,	&rapl_defaults_spr_server),
 	X86_MATCH_INTEL_FAM6_MODEL(LUNARLAKE_M,		&rapl_defaults_core),
+	X86_MATCH_INTEL_FAM6_MODEL(ARROWLAKE_H,		&rapl_defaults_core),
 	X86_MATCH_INTEL_FAM6_MODEL(ARROWLAKE,		&rapl_defaults_core),
 	X86_MATCH_INTEL_FAM6_MODEL(LAKEFIELD,		&rapl_defaults_core),
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH V2 3/3] powercap: intel_rapl_tpmi: Enable PMU support
From: Zhang Rui @ 2024-04-08  3:51 UTC (permalink / raw)
  To: rafael.j.wysocki; +Cc: linux-kernel, linux-pm, srinivas.pandruvada
In-Reply-To: <20240408035141.248644-1-rui.zhang@intel.com>

Enable RAPL PMU support for TPMI RAPL driver.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/powercap/intel_rapl_tpmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/powercap/intel_rapl_tpmi.c b/drivers/powercap/intel_rapl_tpmi.c
index f6b7f085977c..947544e4d229 100644
--- a/drivers/powercap/intel_rapl_tpmi.c
+++ b/drivers/powercap/intel_rapl_tpmi.c
@@ -302,6 +302,8 @@ static int intel_rapl_tpmi_probe(struct auxiliary_device *auxdev,
 		goto err;
 	}
 
+	rapl_package_add_pmu(trp->rp);
+
 	auxiliary_set_drvdata(auxdev, trp);
 
 	return 0;
@@ -314,6 +316,7 @@ static void intel_rapl_tpmi_remove(struct auxiliary_device *auxdev)
 {
 	struct tpmi_rapl_package *trp = auxiliary_get_drvdata(auxdev);
 
+	rapl_package_remove_pmu(trp->rp);
 	rapl_remove_package(trp->rp);
 	trp_release(trp);
 }
-- 
2.34.1


^ permalink raw reply related

* [PATCH V2 2/3] powercap: intel_rapl: Introduce APIs for PMU support
From: Zhang Rui @ 2024-04-08  3:51 UTC (permalink / raw)
  To: rafael.j.wysocki; +Cc: linux-kernel, linux-pm, srinivas.pandruvada
In-Reply-To: <20240408035141.248644-1-rui.zhang@intel.com>

Introduce two new APIs rapl_package_add_pmu()/rapl_package_remove_pmu().

RAPL driver can invoke these APIs to expose its supported energy
counters via perf PMU. The new RAPL PMU is fully compatible with current
MSR RAPL PMU, including using the same PMU name and events
name/id/unit/scale, etc.

For example, use below command
 perf stat -e power/energy-pkg/ -e power/energy-ram/ -e power/energy-cores/ FOO
to get the energy consumption for events in the "perf list" output.

This does not introduce any conflict because TPMI RAPL is the only user
of these APIs currently, and it never co-exists with MSR RAPL.

Note that, TPMI RAPL is probed dynamically, and the events supported by
each TPMI RAPL device can be different. Thus, for a new RAPL Package
device that wants PMU support,
1. if PMU has not been registered yet, register the PMU with events
   supported by current RAPL Package device
2. if PMU has been registered and no new events support is needed, do
   nothing
2. or else, unregister current PMU and re-register the PMU with events
   supported by all probed RAPL Package devices that need PMU support.
   For example, on a dual-package system using TPMI RAPL, it is possible
   that Package 1 behaves as TPMI domain root and supports Psys domain.
   In this case, register PMU without Psys event when probing Package 0,
   and re-register the PMU with Psys event when probing Package 1.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/powercap/intel_rapl_common.c | 537 +++++++++++++++++++++++++++
 include/linux/intel_rapl.h           |  23 ++
 2 files changed, 560 insertions(+)

diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index 1f4a7aa12d77..f51012f023b0 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -15,6 +15,8 @@
 #include <linux/list.h>
 #include <linux/log2.h>
 #include <linux/module.h>
+#include <linux/nospec.h>
+#include <linux/perf_event.h>
 #include <linux/platform_device.h>
 #include <linux/powercap.h>
 #include <linux/processor.h>
@@ -1506,6 +1508,541 @@ static int rapl_detect_domains(struct rapl_package *rp)
 	return 0;
 }
 
+#ifdef CONFIG_PERF_EVENTS
+
+/* Support for RAPL PMU */
+
+struct rapl_pmu {
+	struct pmu pmu;
+	u64 timer_ms;
+	cpumask_t cpu_mask;
+	unsigned long domain_map;	/* Events supported by all registered RAPL packages */
+	bool registered;
+};
+
+static struct rapl_pmu rapl_pmu;
+
+/* PMU helpers */
+static int get_pmu_cpu(struct rapl_package *rp)
+{
+	int cpu;
+
+	if (!rp->has_pmu)
+		return nr_cpu_ids;
+
+	if (rp->lead_cpu >= 0)
+		return rp->lead_cpu;
+
+	/* TPMI RAPL uses any CPU in the package for PMU */
+	for_each_online_cpu(cpu)
+		if (topology_physical_package_id(cpu) == rp->id)
+			return cpu;
+
+	return nr_cpu_ids;
+}
+
+static bool is_rp_pmu_cpu(struct rapl_package *rp, int cpu)
+{
+	if (!rp->has_pmu)
+		return false;
+
+	if (rp->lead_cpu >= 0)
+		return cpu == rp->lead_cpu;
+
+	/* TPMI RAPL uses any CPU in the package for PMU */
+	return topology_physical_package_id(cpu) == rp->id;
+}
+
+static struct rapl_package_pmu_data *event_to_pmu_data(struct perf_event *event)
+{
+	struct rapl_package *rp = event->pmu_private;
+
+	return &rp->pmu_data;
+}
+
+/* PMU event callbacks */
+
+/* Return 0 for unsupported events or failed read */
+static u64 event_read_counter(struct perf_event *event)
+{
+	struct rapl_package *rp = event->pmu_private;
+	u64 val;
+	int ret;
+
+	if (event->hw.idx < 0)
+		return 0;
+
+	ret = rapl_read_data_raw(&rp->domains[event->hw.idx], ENERGY_COUNTER, false, &val);
+	return ret ? 0 : val;
+}
+
+static void __rapl_pmu_event_start(struct perf_event *event)
+{
+	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
+
+	if (WARN_ON_ONCE(!(event->hw.state & PERF_HES_STOPPED)))
+		return;
+
+	event->hw.state = 0;
+
+	list_add_tail(&event->active_entry, &data->active_list);
+
+	local64_set(&event->hw.prev_count, event_read_counter(event));
+	if (++data->n_active == 1)
+		hrtimer_start(&data->hrtimer, data->timer_interval,
+			      HRTIMER_MODE_REL_PINNED);
+}
+
+static void rapl_pmu_event_start(struct perf_event *event, int mode)
+{
+	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&data->lock, flags);
+	__rapl_pmu_event_start(event);
+	raw_spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static u64 rapl_event_update(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
+	u64 prev_raw_count, new_raw_count;
+	s64 delta, sdelta;
+
+again:
+	prev_raw_count = local64_read(&hwc->prev_count);
+	new_raw_count = event_read_counter(event);
+
+	if (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
+			new_raw_count) != prev_raw_count)
+		goto again;
+
+	/*
+	 * Now we have the new raw value and have updated the prev
+	 * timestamp already. We can now calculate the elapsed delta
+	 * (event-)time and add that to the generic event.
+	 */
+	delta = new_raw_count - prev_raw_count;
+
+	/*
+	 * Scale delta to smallest unit (2^-32)
+	 * users must then scale back: count * 1/(1e9*2^32) to get Joules
+	 * or use ldexp(count, -32).
+	 * Watts = Joules/Time delta
+	 */
+	sdelta = delta * data->scale[event->hw.flags];
+
+	local64_add(sdelta, &event->count);
+
+	return new_raw_count;
+}
+
+static void rapl_pmu_event_stop(struct perf_event *event, int mode)
+{
+	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
+	struct hw_perf_event *hwc = &event->hw;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&data->lock, flags);
+
+	/* Mark event as deactivated and stopped */
+	if (!(hwc->state & PERF_HES_STOPPED)) {
+		WARN_ON_ONCE(data->n_active <= 0);
+		if (--data->n_active == 0)
+			hrtimer_cancel(&data->hrtimer);
+
+		list_del(&event->active_entry);
+
+		WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+		hwc->state |= PERF_HES_STOPPED;
+	}
+
+	/* Check if update of sw counter is necessary */
+	if ((mode & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
+		/*
+		 * Drain the remaining delta count out of a event
+		 * that we are disabling:
+		 */
+		rapl_event_update(event);
+		hwc->state |= PERF_HES_UPTODATE;
+	}
+
+	raw_spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static int rapl_pmu_event_add(struct perf_event *event, int mode)
+{
+	struct rapl_package_pmu_data *data = event_to_pmu_data(event);
+	struct hw_perf_event *hwc = &event->hw;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&data->lock, flags);
+
+	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
+
+	if (mode & PERF_EF_START)
+		__rapl_pmu_event_start(event);
+
+	raw_spin_unlock_irqrestore(&data->lock, flags);
+
+	return 0;
+}
+
+static void rapl_pmu_event_del(struct perf_event *event, int flags)
+{
+	rapl_pmu_event_stop(event, PERF_EF_UPDATE);
+}
+
+/*
+ * RAPL energy status counters
+ */
+enum perf_rapl_events {
+	PERF_RAPL_PP0 = 0,	/* all cores */
+	PERF_RAPL_PKG,		/* entire package */
+	PERF_RAPL_RAM,		/* DRAM */
+	PERF_RAPL_PP1,		/* gpu */
+	PERF_RAPL_PSYS,		/* psys */
+};
+#define RAPL_EVENT_MASK GENMASK(7, 0)
+
+static int event_to_domain(int event)
+{
+	switch (event) {
+	case PERF_RAPL_PP0:
+		return RAPL_DOMAIN_PP0;
+	case PERF_RAPL_PKG:
+		return RAPL_DOMAIN_PACKAGE;
+	case PERF_RAPL_RAM:
+		return RAPL_DOMAIN_DRAM;
+	case PERF_RAPL_PP1:
+		return RAPL_DOMAIN_PP1;
+	case PERF_RAPL_PSYS:
+		return RAPL_DOMAIN_PLATFORM;
+	default:
+		return RAPL_DOMAIN_MAX;
+	}
+}
+
+static int rapl_pmu_event_init(struct perf_event *event)
+{
+	struct rapl_package *pos, *rp = NULL;
+	u64 cfg = event->attr.config & RAPL_EVENT_MASK;
+	int domain, idx;
+
+	/* Only look at RAPL events */
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* Check only supported bits are set */
+	if (event->attr.config & ~RAPL_EVENT_MASK)
+		return -EINVAL;
+
+	if (event->cpu < 0)
+		return -EINVAL;
+
+	/* Find out which Package the event belongs to */
+	list_for_each_entry(pos, &rapl_packages, plist) {
+		if (is_rp_pmu_cpu(pos, event->cpu)) {
+			rp = pos;
+			break;
+		}
+	}
+	if (!rp)
+		return -ENODEV;
+
+	/* Find out which RAPL Domain the event belongs to */
+	domain = event_to_domain(cfg - 1);
+	if (domain >= RAPL_DOMAIN_MAX)
+		return -EINVAL;
+
+	event->event_caps |= PERF_EV_CAP_READ_ACTIVE_PKG;
+	event->pmu_private = rp;	/* Which package */
+	event->hw.flags = domain;	/* Which domain */
+
+	event->hw.idx = -1;
+	/* Find out the index in rp->domains[] to get domain pointer */
+	for (idx = 0; idx < rp->nr_domains; idx++) {
+		if (rp->domains[idx].id == domain) {
+			event->hw.idx = idx;
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static void rapl_pmu_event_read(struct perf_event *event)
+{
+	rapl_event_update(event);
+}
+
+static enum hrtimer_restart rapl_hrtimer_handle(struct hrtimer *hrtimer)
+{
+	struct rapl_package_pmu_data *data =
+		container_of(hrtimer, struct rapl_package_pmu_data, hrtimer);
+	struct perf_event *event;
+	unsigned long flags;
+
+	if (!data->n_active)
+		return HRTIMER_NORESTART;
+
+	raw_spin_lock_irqsave(&data->lock, flags);
+
+	list_for_each_entry(event, &data->active_list, active_entry)
+		rapl_event_update(event);
+
+	raw_spin_unlock_irqrestore(&data->lock, flags);
+
+	hrtimer_forward_now(hrtimer, data->timer_interval);
+
+	return HRTIMER_RESTART;
+}
+
+/* PMU sysfs attributes */
+
+/*
+ * There are no default events, but we need to create "events" group (with
+ * empty attrs) before updating it with detected events.
+ */
+static struct attribute *attrs_empty[] = {
+	NULL,
+};
+
+static struct attribute_group pmu_events_group = {
+	.name = "events",
+	.attrs = attrs_empty,
+};
+
+static ssize_t cpumask_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct rapl_package *rp;
+	int cpu;
+
+	cpus_read_lock();
+	cpumask_clear(&rapl_pmu.cpu_mask);
+
+	/* Choose a cpu for each RAPL Package */
+	list_for_each_entry(rp, &rapl_packages, plist) {
+		cpu = get_pmu_cpu(rp);
+		if (cpu < nr_cpu_ids)
+			cpumask_set_cpu(cpu, &rapl_pmu.cpu_mask);
+	}
+	cpus_read_unlock();
+
+	return cpumap_print_to_pagebuf(true, buf, &rapl_pmu.cpu_mask);
+}
+
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *pmu_cpumask_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL
+};
+
+static struct attribute_group pmu_cpumask_group = {
+	.attrs = pmu_cpumask_attrs,
+};
+
+PMU_FORMAT_ATTR(event, "config:0-7");
+static struct attribute *pmu_format_attr[] = {
+	&format_attr_event.attr,
+	NULL
+};
+
+static struct attribute_group pmu_format_group = {
+	.name = "format",
+	.attrs = pmu_format_attr,
+};
+
+static const struct attribute_group *pmu_attr_groups[] = {
+	&pmu_events_group,
+	&pmu_cpumask_group,
+	&pmu_format_group,
+	NULL
+};
+
+#define RAPL_EVENT_ATTR_STR(_name, v, str)					\
+static struct perf_pmu_events_attr event_attr_##v = {				\
+	.attr		= __ATTR(_name, 0444, perf_event_sysfs_show, NULL),	\
+	.event_str	= str,							\
+}
+
+RAPL_EVENT_ATTR_STR(energy-cores,	rapl_cores,	"event=0x01");
+RAPL_EVENT_ATTR_STR(energy-pkg,		rapl_pkg,	"event=0x02");
+RAPL_EVENT_ATTR_STR(energy-ram,		rapl_ram,	"event=0x03");
+RAPL_EVENT_ATTR_STR(energy-gpu,		rapl_gpu,	"event=0x04");
+RAPL_EVENT_ATTR_STR(energy-psys,	rapl_psys,	"event=0x05");
+
+RAPL_EVENT_ATTR_STR(energy-cores.unit,	rapl_unit_cores,	"Joules");
+RAPL_EVENT_ATTR_STR(energy-pkg.unit,	rapl_unit_pkg,		"Joules");
+RAPL_EVENT_ATTR_STR(energy-ram.unit,	rapl_unit_ram,		"Joules");
+RAPL_EVENT_ATTR_STR(energy-gpu.unit,	rapl_unit_gpu,		"Joules");
+RAPL_EVENT_ATTR_STR(energy-psys.unit,	rapl_unit_psys,		"Joules");
+
+RAPL_EVENT_ATTR_STR(energy-cores.scale,	rapl_scale_cores,	"2.3283064365386962890625e-10");
+RAPL_EVENT_ATTR_STR(energy-pkg.scale,	rapl_scale_pkg,		"2.3283064365386962890625e-10");
+RAPL_EVENT_ATTR_STR(energy-ram.scale,	rapl_scale_ram,		"2.3283064365386962890625e-10");
+RAPL_EVENT_ATTR_STR(energy-gpu.scale,	rapl_scale_gpu,		"2.3283064365386962890625e-10");
+RAPL_EVENT_ATTR_STR(energy-psys.scale,	rapl_scale_psys,	"2.3283064365386962890625e-10");
+
+#define RAPL_EVENT_GROUP(_name, domain)			\
+static struct attribute *pmu_attr_##_name[] = {		\
+	&event_attr_rapl_##_name.attr.attr,		\
+	&event_attr_rapl_unit_##_name.attr.attr,	\
+	&event_attr_rapl_scale_##_name.attr.attr,	\
+	NULL						\
+};							\
+static umode_t is_visible_##_name(struct kobject *kobj, struct attribute *attr, int event)	\
+{											\
+	return rapl_pmu.domain_map & BIT(domain) ? attr->mode : 0;	\
+}							\
+static struct attribute_group pmu_group_##_name = {	\
+	.name  = "events",				\
+	.attrs = pmu_attr_##_name,			\
+	.is_visible = is_visible_##_name,		\
+}
+
+RAPL_EVENT_GROUP(cores,	RAPL_DOMAIN_PP0);
+RAPL_EVENT_GROUP(pkg,	RAPL_DOMAIN_PACKAGE);
+RAPL_EVENT_GROUP(ram,	RAPL_DOMAIN_DRAM);
+RAPL_EVENT_GROUP(gpu,	RAPL_DOMAIN_PP1);
+RAPL_EVENT_GROUP(psys,	RAPL_DOMAIN_PLATFORM);
+
+static const struct attribute_group *pmu_attr_update[] = {
+	&pmu_group_cores,
+	&pmu_group_pkg,
+	&pmu_group_ram,
+	&pmu_group_gpu,
+	&pmu_group_psys,
+	NULL
+};
+
+static int rapl_pmu_update(struct rapl_package *rp)
+{
+	int ret;
+
+	/* Return if PMU already covers all events supported by current RAPL Package */
+	if (rapl_pmu.registered && !(rp->domain_map & (~rapl_pmu.domain_map)))
+		return 0;
+
+	/* Unregister previous registered PMU */
+	if (rapl_pmu.registered) {
+		perf_pmu_unregister(&rapl_pmu.pmu);
+		memset(&rapl_pmu.pmu, 0, sizeof(struct pmu));
+	}
+
+	rapl_pmu.domain_map |= rp->domain_map;
+
+	memset(&rapl_pmu.pmu, 0, sizeof(struct pmu));
+	rapl_pmu.pmu.attr_groups = pmu_attr_groups;
+	rapl_pmu.pmu.attr_update = pmu_attr_update;
+	rapl_pmu.pmu.task_ctx_nr = perf_invalid_context;
+	rapl_pmu.pmu.event_init = rapl_pmu_event_init;
+	rapl_pmu.pmu.add = rapl_pmu_event_add;
+	rapl_pmu.pmu.del = rapl_pmu_event_del;
+	rapl_pmu.pmu.start = rapl_pmu_event_start;
+	rapl_pmu.pmu.stop = rapl_pmu_event_stop;
+	rapl_pmu.pmu.read = rapl_pmu_event_read;
+	rapl_pmu.pmu.module = THIS_MODULE;
+	rapl_pmu.pmu.capabilities = PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT;
+	ret = perf_pmu_register(&rapl_pmu.pmu, "power", -1);
+	if (ret)
+		pr_warn("Failed to register PMU\n");
+
+	rapl_pmu.registered = !ret;
+
+	return ret;
+}
+
+int rapl_package_add_pmu(struct rapl_package *rp)
+{
+	struct rapl_package_pmu_data *data = &rp->pmu_data;
+	int idx;
+	int ret;
+
+	if (rp->has_pmu)
+		return -EEXIST;
+
+	guard(cpus_read_lock)();
+
+	for (idx = 0; idx < rp->nr_domains; idx++) {
+		struct rapl_domain *rd = &rp->domains[idx];
+		int domain = rd->id;
+		u64 val;
+
+		if (!test_bit(domain, &rp->domain_map))
+			continue;
+
+		/*
+		 * The RAPL PMU granularity is 2^-32 Joules
+		 * data->scale[]: times of 2^-32 Joules for each ENERGY COUNTER increase
+		 */
+		val = rd->energy_unit * (1ULL << 32);
+		do_div(val, ENERGY_UNIT_SCALE * 1000000);
+		data->scale[domain] = val;
+
+		if (!rapl_pmu.timer_ms) {
+			struct rapl_primitive_info *rpi = get_rpi(rp, ENERGY_COUNTER);
+
+			/*
+			 * Calculate the timer rate:
+			 * Use reference of 200W for scaling the timeout to avoid counter
+			 * overflows.
+			 *
+			 * max_count = rpi->mask >> rpi->shift + 1
+			 * max_energy_pj = max_count * rd->energy_unit
+			 * max_time_sec = (max_energy_pj / 1000000000) / 200w
+			 *
+			 * rapl_pmu.timer_ms = max_time_sec * 1000 / 2
+			 */
+			val = (rpi->mask >> rpi->shift) + 1;
+			val *= rd->energy_unit;
+			do_div(val, 1000000 * 200 * 2);
+			rapl_pmu.timer_ms = val;
+
+			pr_info("%llu ms ovfl timer\n", rapl_pmu.timer_ms);
+		}
+
+		pr_info("Domain %s: hw unit %lld * 2^-32 Joules\n", rd->name, data->scale[domain]);
+	}
+
+	/* Initialize per package PMU data */
+	raw_spin_lock_init(&data->lock);
+	INIT_LIST_HEAD(&data->active_list);
+	data->timer_interval = ms_to_ktime(rapl_pmu.timer_ms);
+	hrtimer_init(&data->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	data->hrtimer.function = rapl_hrtimer_handle;
+
+	ret = rapl_pmu_update(rp);
+	if (!ret)
+		rp->has_pmu = true;
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rapl_package_add_pmu);
+
+void rapl_package_remove_pmu(struct rapl_package *rp)
+{
+	struct rapl_package *pos;
+
+	if (!rp->has_pmu)
+		return;
+
+	guard(cpus_read_lock)();
+
+	list_for_each_entry(pos, &rapl_packages, plist) {
+		/* PMU is still needed */
+		if (pos->has_pmu)
+			return;
+	}
+
+	perf_pmu_unregister(&rapl_pmu.pmu);
+	memset(&rapl_pmu, 0, sizeof(struct rapl_pmu));
+}
+EXPORT_SYMBOL_GPL(rapl_package_remove_pmu);
+#endif
+
 /* called from CPU hotplug notifier, hotplug lock held */
 void rapl_remove_package_cpuslocked(struct rapl_package *rp)
 {
diff --git a/include/linux/intel_rapl.h b/include/linux/intel_rapl.h
index f3196f82fd8a..38e43dbb3dac 100644
--- a/include/linux/intel_rapl.h
+++ b/include/linux/intel_rapl.h
@@ -158,6 +158,17 @@ struct rapl_if_priv {
 	void *rpi;
 };
 
+#ifdef CONFIG_PERF_EVENTS
+struct rapl_package_pmu_data {
+	u64 scale[RAPL_DOMAIN_MAX];
+	raw_spinlock_t lock;
+	int n_active;
+	struct list_head active_list;
+	ktime_t timer_interval;
+	struct hrtimer hrtimer;
+};
+#endif
+
 /* maximum rapl package domain name: package-%d-die-%d */
 #define PACKAGE_DOMAIN_NAME_LENGTH 30
 
@@ -176,6 +187,10 @@ struct rapl_package {
 	struct cpumask cpumask;
 	char name[PACKAGE_DOMAIN_NAME_LENGTH];
 	struct rapl_if_priv *priv;
+#ifdef CONFIG_PERF_EVENTS
+	bool has_pmu;
+	struct rapl_package_pmu_data pmu_data;
+#endif
 };
 
 struct rapl_package *rapl_find_package_domain_cpuslocked(int id, struct rapl_if_priv *priv,
@@ -188,4 +203,12 @@ struct rapl_package *rapl_find_package_domain(int id, struct rapl_if_priv *priv,
 struct rapl_package *rapl_add_package(int id, struct rapl_if_priv *priv, bool id_is_cpu);
 void rapl_remove_package(struct rapl_package *rp);
 
+#ifdef CONFIG_PERF_EVENTS
+int rapl_package_add_pmu(struct rapl_package *rp);
+void rapl_package_remove_pmu(struct rapl_package *rp);
+#else
+static inline int rapl_package_add_pmu(struct rapl_package *rp) { return 0; }
+static inline void rapl_package_remove_pmu(struct rapl_package *rp) { }
+#endif
+
 #endif /* __INTEL_RAPL_H__ */
-- 
2.34.1


^ permalink raw reply related

* [PATCH V2 1/3] powercap: intel_rapl: Sort header files
From: Zhang Rui @ 2024-04-08  3:51 UTC (permalink / raw)
  To: rafael.j.wysocki; +Cc: linux-kernel, linux-pm, srinivas.pandruvada
In-Reply-To: <20240408035141.248644-1-rui.zhang@intel.com>

Sort header files alphabetically.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/powercap/intel_rapl_common.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
index a28d54fd5222..1f4a7aa12d77 100644
--- a/drivers/powercap/intel_rapl_common.c
+++ b/drivers/powercap/intel_rapl_common.c
@@ -5,27 +5,27 @@
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/bitmap.h>
 #include <linux/cleanup.h>
+#include <linux/cpu.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/intel_rapl.h>
 #include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/list.h>
-#include <linux/types.h>
-#include <linux/device.h>
-#include <linux/slab.h>
 #include <linux/log2.h>
-#include <linux/bitmap.h>
-#include <linux/delay.h>
-#include <linux/sysfs.h>
-#include <linux/cpu.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/powercap.h>
-#include <linux/suspend.h>
-#include <linux/intel_rapl.h>
 #include <linux/processor.h>
-#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
 
-#include <asm/iosf_mbi.h>
 #include <asm/cpu_device_id.h>
 #include <asm/intel-family.h>
+#include <asm/iosf_mbi.h>
 
 /* bitmasks for RAPL MSRs, used by primitive access functions */
 #define ENERGY_STATUS_MASK      0xffffffff
-- 
2.34.1


^ permalink raw reply related

* [PATCH V2 0/3] powercap: Introduce TPMI RAPL PMU support
From: Zhang Rui @ 2024-04-08  3:51 UTC (permalink / raw)
  To: rafael.j.wysocki; +Cc: linux-kernel, linux-pm, srinivas.pandruvada

RAPL energy counter MSRs are exposed via perf PMU. But this is done by
separate code which is not part of RAPL framework, and it cannot be
reused by other RAPL Interface drivers like TPMI RAPL.

Introduce two new APIs for PMU support in RAPL framework. This allows
TPMI RAPL PMU support and also makes it possible for future cleanups of
MSR RAPL PMU code.

Changes since V1:
- remove the MSR RAPL PMU conversion because it is a separate work that
  can be done later.
- instead of using a flag to indicate the need of PMU support, introduce
  two APIs for the RAPL Interface driver to invoke explicitly.
- minor code/comments/changelog improvements.

thanks,
rui

----------------------------------------------------------------
Zhang Rui (3):
      powercap: intel_rapl: Sort header files
      powercap: intel_rapl: Introduce APIs for PMU support
      powercap: intel_rapl_tpmi: Enable PMU support

 drivers/powercap/intel_rapl_common.c | 561 ++++++++++++++++++++++++++++++++++-
 drivers/powercap/intel_rapl_tpmi.c   |   3 +
 include/linux/intel_rapl.h           |  23 ++
 3 files changed, 575 insertions(+), 12 deletions(-)

^ permalink raw reply

* Re: [RFC PATCH 1/1] clk: sunxi-ng: h6-r: add GPU power domain
From: Stephen Boyd @ 2024-04-08  3:33 UTC (permalink / raw)
  To: Andre Przywara, Chen-Yu Tsai, Jernej Skrabec, Michael Turquette,
	Samuel Holland, Ulf Hansson
  Cc: linux-clk, linux-pm, linux-arm-kernel, linux-sunxi
In-Reply-To: <20240225160616.15001-2-andre.przywara@arm.com>

Quoting Andre Przywara (2024-02-25 08:06:16)
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
> index 02b28cfc5525e..363fb7a71e9f5 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
> @@ -217,6 +219,86 @@ static const struct sunxi_ccu_desc sun50i_h616_r_ccu_desc = {
[...]
> +static int sun50i_h616_register_ppu(struct platform_device *pdev,
> +                                   void __iomem *base)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct genpd_onecell_data *ppu;
> +       struct sun50i_h616_ppu_pd *pd;
> +       int ret;
> +
> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +       if (!pd)
> +               return -ENOMEM;
> +
> +       ppu = devm_kzalloc(dev, sizeof(*ppu), GFP_KERNEL);
> +       if (!ppu)
> +               return -ENOMEM;
> +
> +       ppu->domains = devm_kzalloc(dev, sizeof(*ppu->domains), GFP_KERNEL);
> +       if (!ppu->domains)
> +               return -ENOMEM;
> +
> +       ppu->num_domains = 1;
> +       pd->genpd.name          = "GPU";
> +       pd->genpd.power_off     = sun50i_h616_ppu_pd_power_off;
> +       pd->genpd.power_on      = sun50i_h616_ppu_pd_power_on;
> +       pd->base                = base;
> +
> +       ret = pm_genpd_init(&pd->genpd, NULL, !sun50i_h616_ppu_power_status(pd));
> +       if (ret) {
> +               dev_warn(dev, "Failed to add GPU power domain: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ppu->domains[0] = &pd->genpd;
> +       ret = of_genpd_add_provider_onecell(dev->of_node, ppu);

Is this provider removed somewhere when probe fails or the driver is
removed? It looks like the rest of the driver uses devm during probe.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox