The Sensor Processing Engine(SPE) can trigger a software wake-up of
the device. To support this wake-up for the SPE, set SR_CAPTURE_EN
bit in WAKE_AOWAKE_CNTRL register associated with the wake-up for
the SPE. This SR capturing logic is expected to be enabled for wakes
with short pulse signalling requirements.
Signed-off-by: Viswanath L <[email protected]>
Signed-off-by: Petlozu Pravareshwar <[email protected]>
---
v1->v2:
* Rebase the change on latest code.
---
drivers/soc/tegra/pmc.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index cf4cfbf9f7c5..2a2342eff622 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -3,7 +3,7 @@
* drivers/soc/tegra/pmc.c
*
* Copyright (c) 2010 Google, Inc
- * Copyright (c) 2018-2022, NVIDIA CORPORATION. All rights reserved.
+ * Copyright (c) 2018-2023, NVIDIA CORPORATION. All rights reserved.
*
* Author:
* Colin Cross <[email protected]>
@@ -177,6 +177,7 @@
/* Tegra186 and later */
#define WAKE_AOWAKE_CNTRL(x) (0x000 + ((x) << 2))
#define WAKE_AOWAKE_CNTRL_LEVEL (1 << 3)
+#define WAKE_AOWAKE_CNTRL_SR_CAPTURE_EN (1 << 1)
#define WAKE_AOWAKE_MASK_W(x) (0x180 + ((x) << 2))
#define WAKE_AOWAKE_MASK_R(x) (0x300 + ((x) << 2))
#define WAKE_AOWAKE_STATUS_W(x) (0x30c + ((x) << 2))
@@ -191,6 +192,8 @@
#define WAKE_AOWAKE_CTRL 0x4f4
#define WAKE_AOWAKE_CTRL_INTR_POLARITY BIT(0)
+#define SW_WAKE_ID 83 /* wake83 */
+
/* for secure PMC */
#define TEGRA_SMC_PMC 0xc2fffe00
#define TEGRA_SMC_PMC_READ 0xaa
@@ -355,6 +358,7 @@ struct tegra_pmc_soc {
void (*setup_irq_polarity)(struct tegra_pmc *pmc,
struct device_node *np,
bool invert);
+ void (*set_wake_filters)(struct tegra_pmc *pmc);
int (*irq_set_wake)(struct irq_data *data, unsigned int on);
int (*irq_set_type)(struct irq_data *data, unsigned int type);
int (*powergate_set)(struct tegra_pmc *pmc, unsigned int id,
@@ -2416,6 +2420,17 @@ static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
return 0;
}
+static void tegra186_pmc_set_wake_filters(struct tegra_pmc *pmc)
+{
+ u32 value;
+
+ /* SW Wake (wake83) needs SR_CAPTURE filter to be enabled */
+ value = readl(pmc->wake + WAKE_AOWAKE_CNTRL(SW_WAKE_ID));
+ value |= WAKE_AOWAKE_CNTRL_SR_CAPTURE_EN;
+ writel(value, pmc->wake + WAKE_AOWAKE_CNTRL(SW_WAKE_ID));
+ dev_dbg(pmc->dev, "WAKE_AOWAKE_CNTRL_83 = 0x%x\n", value);
+}
+
static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
{
struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
@@ -3042,6 +3057,10 @@ static int tegra_pmc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pmc);
tegra_pm_init_suspend();
+ /* Some wakes require specific filter configuration */
+ if (pmc->soc->set_wake_filters)
+ pmc->soc->set_wake_filters(pmc);
+
return 0;
cleanup_powergates:
@@ -3938,6 +3957,7 @@ static const struct tegra_pmc_soc tegra186_pmc_soc = {
.regs = &tegra186_pmc_regs,
.init = tegra186_pmc_init,
.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
+ .set_wake_filters = tegra186_pmc_set_wake_filters,
.irq_set_wake = tegra186_pmc_irq_set_wake,
.irq_set_type = tegra186_pmc_irq_set_type,
.reset_sources = tegra186_reset_sources,
@@ -4122,6 +4142,7 @@ static const struct tegra_pmc_soc tegra194_pmc_soc = {
.regs = &tegra194_pmc_regs,
.init = tegra186_pmc_init,
.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
+ .set_wake_filters = tegra186_pmc_set_wake_filters,
.irq_set_wake = tegra186_pmc_irq_set_wake,
.irq_set_type = tegra186_pmc_irq_set_type,
.reset_sources = tegra194_reset_sources,
@@ -4247,6 +4268,7 @@ static const struct tegra_pmc_soc tegra234_pmc_soc = {
.regs = &tegra234_pmc_regs,
.init = tegra186_pmc_init,
.setup_irq_polarity = tegra186_pmc_setup_irq_polarity,
+ .set_wake_filters = tegra186_pmc_set_wake_filters,
.irq_set_wake = tegra186_pmc_irq_set_wake,
.irq_set_type = tegra186_pmc_irq_set_type,
.reset_sources = tegra234_reset_sources,
--
2.17.1
On Thu, Mar 30, 2023 at 05:06:21PM +0000, Petlozu Pravareshwar wrote:
> The Sensor Processing Engine(SPE) can trigger a software wake-up of
> the device. To support this wake-up for the SPE, set SR_CAPTURE_EN
> bit in WAKE_AOWAKE_CNTRL register associated with the wake-up for
> the SPE. This SR capturing logic is expected to be enabled for wakes
> with short pulse signalling requirements.
>
> Signed-off-by: Viswanath L <[email protected]>
> Signed-off-by: Petlozu Pravareshwar <[email protected]>
> ---
> v1->v2:
> * Rebase the change on latest code.
> ---
> drivers/soc/tegra/pmc.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
Applied, thanks.
Thierry
On 3/30/23 20:06, Petlozu Pravareshwar wrote:
> The Sensor Processing Engine(SPE) can trigger a software wake-up of
> the device. To support this wake-up for the SPE, set SR_CAPTURE_EN
> bit in WAKE_AOWAKE_CNTRL register associated with the wake-up for
> the SPE. This SR capturing logic is expected to be enabled for wakes
> with short pulse signalling requirements.
>
> Signed-off-by: Viswanath L <[email protected]>
> Signed-off-by: Petlozu Pravareshwar <[email protected]>
> ---
> v1->v2:
> * Rebase the change on latest code.
> ---
> drivers/soc/tegra/pmc.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index cf4cfbf9f7c5..2a2342eff622 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -3,7 +3,7 @@
> * drivers/soc/tegra/pmc.c
> *
> * Copyright (c) 2010 Google, Inc
> - * Copyright (c) 2018-2022, NVIDIA CORPORATION. All rights reserved.
> + * Copyright (c) 2018-2023, NVIDIA CORPORATION. All rights reserved.
> *
> * Author:
> * Colin Cross <[email protected]>
> @@ -177,6 +177,7 @@
> /* Tegra186 and later */
> #define WAKE_AOWAKE_CNTRL(x) (0x000 + ((x) << 2))
> #define WAKE_AOWAKE_CNTRL_LEVEL (1 << 3)
> +#define WAKE_AOWAKE_CNTRL_SR_CAPTURE_EN (1 << 1)
> #define WAKE_AOWAKE_MASK_W(x) (0x180 + ((x) << 2))
> #define WAKE_AOWAKE_MASK_R(x) (0x300 + ((x) << 2))
> #define WAKE_AOWAKE_STATUS_W(x) (0x30c + ((x) << 2))
> @@ -191,6 +192,8 @@
> #define WAKE_AOWAKE_CTRL 0x4f4
> #define WAKE_AOWAKE_CTRL_INTR_POLARITY BIT(0)
>
> +#define SW_WAKE_ID 83 /* wake83 */
> +
> /* for secure PMC */
> #define TEGRA_SMC_PMC 0xc2fffe00
> #define TEGRA_SMC_PMC_READ 0xaa
> @@ -355,6 +358,7 @@ struct tegra_pmc_soc {
> void (*setup_irq_polarity)(struct tegra_pmc *pmc,
> struct device_node *np,
> bool invert);
> + void (*set_wake_filters)(struct tegra_pmc *pmc);
> int (*irq_set_wake)(struct irq_data *data, unsigned int on);
> int (*irq_set_type)(struct irq_data *data, unsigned int type);
> int (*powergate_set)(struct tegra_pmc *pmc, unsigned int id,
> @@ -2416,6 +2420,17 @@ static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> return 0;
> }
>
> +static void tegra186_pmc_set_wake_filters(struct tegra_pmc *pmc)
> +{
> + u32 value;
> +
> + /* SW Wake (wake83) needs SR_CAPTURE filter to be enabled */
> + value = readl(pmc->wake + WAKE_AOWAKE_CNTRL(SW_WAKE_ID));
> + value |= WAKE_AOWAKE_CNTRL_SR_CAPTURE_EN;
> + writel(value, pmc->wake + WAKE_AOWAKE_CNTRL(SW_WAKE_ID));
> + dev_dbg(pmc->dev, "WAKE_AOWAKE_CNTRL_83 = 0x%x\n", value);
> +}
To me this needs to be moved to the SPE driver, which should get the PMC
regmap handle and enable wake only when needed, similarly how it's done
by USB Tegra drivers that also need to configure PMC. Otherwise this
looks like a hack/workaround.
--
Best regards,
Dmitry
On Mon, Apr 03, 2023 at 07:06:12PM +0300, Dmitry Osipenko wrote:
> On 3/30/23 20:06, Petlozu Pravareshwar wrote:
> > The Sensor Processing Engine(SPE) can trigger a software wake-up of
> > the device. To support this wake-up for the SPE, set SR_CAPTURE_EN
> > bit in WAKE_AOWAKE_CNTRL register associated with the wake-up for
> > the SPE. This SR capturing logic is expected to be enabled for wakes
> > with short pulse signalling requirements.
> >
> > Signed-off-by: Viswanath L <[email protected]>
> > Signed-off-by: Petlozu Pravareshwar <[email protected]>
> > ---
> > v1->v2:
> > * Rebase the change on latest code.
> > ---
> > drivers/soc/tegra/pmc.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> > index cf4cfbf9f7c5..2a2342eff622 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -3,7 +3,7 @@
> > * drivers/soc/tegra/pmc.c
> > *
> > * Copyright (c) 2010 Google, Inc
> > - * Copyright (c) 2018-2022, NVIDIA CORPORATION. All rights reserved.
> > + * Copyright (c) 2018-2023, NVIDIA CORPORATION. All rights reserved.
> > *
> > * Author:
> > * Colin Cross <[email protected]>
> > @@ -177,6 +177,7 @@
> > /* Tegra186 and later */
> > #define WAKE_AOWAKE_CNTRL(x) (0x000 + ((x) << 2))
> > #define WAKE_AOWAKE_CNTRL_LEVEL (1 << 3)
> > +#define WAKE_AOWAKE_CNTRL_SR_CAPTURE_EN (1 << 1)
> > #define WAKE_AOWAKE_MASK_W(x) (0x180 + ((x) << 2))
> > #define WAKE_AOWAKE_MASK_R(x) (0x300 + ((x) << 2))
> > #define WAKE_AOWAKE_STATUS_W(x) (0x30c + ((x) << 2))
> > @@ -191,6 +192,8 @@
> > #define WAKE_AOWAKE_CTRL 0x4f4
> > #define WAKE_AOWAKE_CTRL_INTR_POLARITY BIT(0)
> >
> > +#define SW_WAKE_ID 83 /* wake83 */
> > +
> > /* for secure PMC */
> > #define TEGRA_SMC_PMC 0xc2fffe00
> > #define TEGRA_SMC_PMC_READ 0xaa
> > @@ -355,6 +358,7 @@ struct tegra_pmc_soc {
> > void (*setup_irq_polarity)(struct tegra_pmc *pmc,
> > struct device_node *np,
> > bool invert);
> > + void (*set_wake_filters)(struct tegra_pmc *pmc);
> > int (*irq_set_wake)(struct irq_data *data, unsigned int on);
> > int (*irq_set_type)(struct irq_data *data, unsigned int type);
> > int (*powergate_set)(struct tegra_pmc *pmc, unsigned int id,
> > @@ -2416,6 +2420,17 @@ static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> > return 0;
> > }
> >
> > +static void tegra186_pmc_set_wake_filters(struct tegra_pmc *pmc)
> > +{
> > + u32 value;
> > +
> > + /* SW Wake (wake83) needs SR_CAPTURE filter to be enabled */
> > + value = readl(pmc->wake + WAKE_AOWAKE_CNTRL(SW_WAKE_ID));
> > + value |= WAKE_AOWAKE_CNTRL_SR_CAPTURE_EN;
> > + writel(value, pmc->wake + WAKE_AOWAKE_CNTRL(SW_WAKE_ID));
> > + dev_dbg(pmc->dev, "WAKE_AOWAKE_CNTRL_83 = 0x%x\n", value);
> > +}
>
> To me this needs to be moved to the SPE driver, which should get the PMC
> regmap handle and enable wake only when needed, similarly how it's done
> by USB Tegra drivers that also need to configure PMC. Otherwise this
> looks like a hack/workaround.
Wake is still only enabled when needed via the irq_set_wake() callback.
And this is slightly different in that it pretty has no side-effects. It
isn't a configuration option that needs to be switched every now and
then but rather just a bit in a register that happens to have the wrong
default value (or an inconvenient default value).
For USB there are actual side-effects and some of the settings only make
sense if a given USB port is actually used, etc. So doing the regmap
dance is actually worth it.
For SPE it just doesn't seem worth it to have all of these inter-
dependencies just for the sake of statically setting one bit once during
boot.
Thierry