lkml.org 
[lkml]   [2020]   [Sep]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v1] soc: fsl: rcpm: Add ACPI support
Date
Hi Ard,

On Wednesday, September 16, 2020 2:11 PM, Ard Biesheuvel wrote:
> Subject: Re: [PATCH v1] soc: fsl: rcpm: Add ACPI support
>
> On 9/16/20 3:32 AM, Ran Wang wrote:
> > Hi Ard,
> >
> > On Tuesday, September 15, 2020 7:10 PM, Ard Biesheuvel wrote:
> >> Subject: Re: [PATCH v1] soc: fsl: rcpm: Add ACPI support
> >>
> >> On 9/15/20 1:06 PM, kuldip dwivedi wrote:
> >>> Add ACPI support in fsl RCPM driver. This is required to support
> >>> ACPI
> >>> S3 state. S3 is the ACPI sleep state that is known as "sleep" or
> >>> "suspend to RAM".
> >>> It essentially turns off most power of the system but keeps memory
> >>> powered.
> >>>
> >>> Signed-off-by: tanveer <tanveer.alam@puresoftware.com>
> >>> Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>
> >>
> >> Why does the OS need to program this device? Can't this be done by
> >> firmware?
> >
> > This device is use to tell HW which IP (such as USB, SDHC, SATA, etc)
> > should not be clock gated during system enter low power state (to
> > allow that IP work as a wakeup source). And user does this configuration in
> device tree.
>
> The point of ACPI is *not* to describe a DT topology using a table format that
> is not suited for it. The point of ACPI is to describe a machine that is more
> abstracted from the hardware than is typically possible with DT, where the
> abstractions are implemented by AML code that is provided by the firmware,
> but executed in the context of the OS.
>
> So the idea is *not* finding the shortest possible path to get your existing DT
> driver code running on a system that boots via ACPI.
> Instead, you should carefully think about the abstract ACPI machine that you
> will expose to the OS, and hide everything else in firmware.
>
> In this particular case, it seems like your USB, SDHC and SATA device objects
> may need power state dependent AML methods that program this block
> directly.

Actually the scenario is a little bit complicated for RCPM function: it need to query
kernel wakeup source framework (see for_each_wakeup_source(ws)) to fetch all
potential candidates then do the programming accordingly. If we implement
this logic in AML methods, I have no idea how to collect those information stored in
wakeup source data of kernel.

Regards,
Ran

>
>
> > So implement
> > this RCPM driver to do it in kernel rather than firmware.
> >
> > Regards,
> > Ran
> >
> >>> ---
> >>>
> >>> Notes:
> >>> 1. Add ACPI match table
> >>> 2. NXP team members are added for confirming HID changes
> >>> 3. There is only one node in ACPI so no need to check for
> >>> current device explicitly
> >>> 4. These changes are tested on LX2160A and LS1046A platforms
> >>>
> >>> drivers/soc/fsl/rcpm.c | 22 +++++++++++++++++++---
> >>> 1 file changed, 19 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index
> >>> a093dbe6d2cb..e75a436fb159 100644
> >>> --- a/drivers/soc/fsl/rcpm.c
> >>> +++ b/drivers/soc/fsl/rcpm.c
> >>> @@ -2,10 +2,12 @@
> >>> //
> >>> // rcpm.c - Freescale QorIQ RCPM driver
> >>> //
> >>> -// Copyright 2019 NXP
> >>> +// Copyright 2019-2020 NXP
> >>> +// Copyright 2020 Puresoftware Ltd.
> >>> //
> >>> // Author: Ran Wang <ran.wang_1@nxp.com>
> >>>
> >>> +#include <linux/acpi.h>
> >>> #include <linux/init.h>
> >>> #include <linux/module.h>
> >>> #include <linux/platform_device.h> @@ -57,8 +59,13 @@ static int
> >>> rcpm_pm_prepare(struct device *dev)
> >>> rcpm->wakeup_cells + 1);
> >>>
> >>> /* Wakeup source should refer to current rcpm device */
> >>> - if (ret || (np->phandle != value[0]))
> >>> - continue;
> >>> + if (is_acpi_node(dev->fwnode)) {
> >>> + if (ret)
> >>> + continue;
> >>> + } else {
> >>> + if (ret || (np->phandle != value[0]))
> >>> + continue;
> >>> + }
> >>>
> >>> /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
> >>> * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
> >>> @@ -139,10 +146,19 @@ static const struct of_device_id
> >>> rcpm_of_match[]
> >> = {
> >>> };
> >>> MODULE_DEVICE_TABLE(of, rcpm_of_match);
> >>>
> >>> +#ifdef CONFIG_ACPI
> >>> +static const struct acpi_device_id rcpm_acpi_match[] = {
> >>> + { "NXP0015", },
> >>> + { }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_match); #endif
> >>> +
> >>> static struct platform_driver rcpm_driver = {
> >>> .driver = {
> >>> .name = "rcpm",
> >>> .of_match_table = rcpm_of_match,
> >>> + .acpi_match_table = ACPI_PTR(rcpm_acpi_match),
> >>> .pm = &rcpm_pm_ops,
> >>> },
> >>> .probe = rcpm_probe,
> >>>
> >

\
 
 \ /
  Last update: 2020-09-16 08:41    [W:0.076 / U:3.792 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site