LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Sowjanya Komatineni <skomatineni@nvidia.com>
Cc: jonathanh@nvidia.com, jckuo@nvidia.com, talho@nvidia.com,
	josephl@nvidia.com, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V1 09/12] soc/tegra: pmc: add pmc wake support for tegra210
Date: Wed, 22 May 2019 15:01:55 +0200	[thread overview]
Message-ID: <20190522130155.GJ30938@ulmo> (raw)
In-Reply-To: <1558481483-22254-10-git-send-email-skomatineni@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 7031 bytes --]

On Tue, May 21, 2019 at 04:31:20PM -0700, Sowjanya Komatineni wrote:
> This patch implements PMC wakeup sequence for Tegra210 and defines
> common used wake events of RTC alarm and power key.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index f77ce4b827e3..5e68e1de1780 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -57,6 +57,7 @@
>  #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
>  #include <dt-bindings/gpio/tegra186-gpio.h>
>  #include <dt-bindings/gpio/tegra194-gpio.h>
> +#include <dt-bindings/gpio/tegra-gpio.h>
>  
>  #define PMC_CNTRL			0x0
>  #define  PMC_CNTRL_INTR_POLARITY	BIT(17) /* inverts INTR polarity */
> @@ -66,6 +67,12 @@
>  #define  PMC_CNTRL_SYSCLK_OE		BIT(11) /* system clock enable */
>  #define  PMC_CNTRL_SYSCLK_POLARITY	BIT(10) /* sys clk polarity */
>  #define  PMC_CNTRL_MAIN_RST		BIT(4)
> +#define  PMC_CNTRL_LATCH_WAKEUPS	BIT(5)
> +
> +#define PMC_WAKE_MASK			0x0c
> +#define PMC_WAKE_LEVEL			0x10
> +#define PMC_WAKE_STATUS			0x14
> +#define PMC_SW_WAKE_STATUS		0x18
>  
>  #define DPD_SAMPLE			0x020
>  #define  DPD_SAMPLE_ENABLE		BIT(0)
> @@ -96,6 +103,11 @@
>  
>  #define PMC_SCRATCH41			0x140
>  
> +#define PMC_WAKE2_MASK			0x160
> +#define PMC_WAKE2_LEVEL			0x164
> +#define PMC_WAKE2_STATUS		0x168
> +#define PMC_SW_WAKE2_STATUS		0x16c
> +
>  #define PMC_SENSOR_CTRL			0x1b0
>  #define  PMC_SENSOR_CTRL_SCRATCH_WRITE	BIT(2)
>  #define  PMC_SENSOR_CTRL_ENABLE_RST	BIT(1)
> @@ -1917,6 +1929,65 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>  	.alloc = tegra_pmc_irq_alloc,
>  };
>  
> +static inline void clear_pmc_sw_wake_status(void)
> +{
> +	tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
> +	if (tegra_get_chip_id() != TEGRA20)

Please don't use tegra_get_chip_id(). We already have pmc->soc, so it
should be easy enough to add a flag to identify Tegra generations that
have this register. Another alternative would be to fill in all the wake
events that the generation supports and then iterate over that to find
out what the maximum wake event ID is and use that to determine which
registers are valid. I'm assuming Tegra20 supports less than 32 wake
events, hence why it doesn't need PMC_SW_WAKE_STATUS.

> +		tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
> +}
> +
> +static inline void clear_pmc_wake_status(void)
> +{
> +	u32 reg;

u32 value for consistency with the rest of the driver.

> +
> +	reg = tegra_pmc_readl(pmc, PMC_WAKE_STATUS);
> +	if (reg)
> +		tegra_pmc_writel(pmc, reg, PMC_WAKE_STATUS);

Is there any harm in writing 0 to these registers? Not that it matters
much, but the code here looks a little odd.

> +	if (tegra_get_chip_id() != TEGRA20) {
> +		reg = tegra_pmc_readl(pmc, PMC_WAKE2_STATUS);
> +		if (reg)
> +			tegra_pmc_writel(pmc, reg, PMC_WAKE2_STATUS);
> +	}
> +}
> +
> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
> +{
> +	struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> +	unsigned int offset, bit;
> +	u32 pmc_wake_mask_reg;
> +	u32 value;
> +
> +	if (data->hwirq < 0)
> +		return 0;

Again, is this really necessary?

> +
> +	offset = data->hwirq / 32;
> +	bit = data->hwirq % 32;
> +
> +	clear_pmc_sw_wake_status();
> +
> +	/* enable PMC wake */
> +	value = tegra_pmc_readl(pmc, PMC_CNTRL);
> +	value |= PMC_CNTRL_LATCH_WAKEUPS;
> +	tegra_pmc_writel(pmc, value, PMC_CNTRL);
> +	usleep_range(110, 120);
> +
> +	value &= ~PMC_CNTRL_LATCH_WAKEUPS;
> +	tegra_pmc_writel(pmc, value, PMC_CNTRL);
> +	usleep_range(110, 120);
> +
> +	clear_pmc_wake_status();

Could you add a couple more comments describing what this does. Maybe
also inline the clear_pmc_{sw_,}wake_status() functions into this since
they are only used here.

> +
> +	pmc_wake_mask_reg = (offset) ? PMC_WAKE2_MASK : PMC_WAKE_MASK;

I think it'd be clearer to write this as:

	if (data->hwirq > 32)
		offset = PMC_WAKE2_MASK;
	else
		offset = PMC_WAKE_MASK;

> +	value = tegra_pmc_readl(pmc, pmc_wake_mask_reg);
> +	if (on)
> +		value |= 1 << bit;
> +	else
> +		value &= ~(1 << bit);
> +	tegra_pmc_writel(pmc, value, pmc_wake_mask_reg);
> +
> +	return 0;
> +}
> +
>  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);
> @@ -1948,6 +2019,46 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>  	return 0;
>  }
>  
> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> +	struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> +	unsigned int offset, bit;
> +	u32 pmc_wake_lvl_reg;
> +	u32 value;
> +
> +	if (data->hwirq < 0)
> +		return 0;
> +
> +	offset = data->hwirq / 32;
> +	bit = data->hwirq % 32;
> +
> +	pmc_wake_lvl_reg = (offset) ? PMC_WAKE2_LEVEL : PMC_WAKE_LEVEL;

Same comment as for the PMC_WAKE*_MASK registers.

Thierry

> +	value = tegra_pmc_readl(pmc, pmc_wake_lvl_reg);
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		value |= 1 << bit;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_FALLING:
> +	case IRQ_TYPE_LEVEL_LOW:
> +		value &= ~(1 << bit);
> +		break;
> +
> +	case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
> +		value ^= 1 << bit;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	tegra_pmc_writel(pmc, value, pmc_wake_lvl_reg);
> +
> +	return 0;
> +}
> +
>  static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>  {
>  	struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
> @@ -2535,6 +2646,11 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
>  	TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
>  };
>  
> +static const struct tegra_wake_event tegra210_wake_events[] = {
> +	TEGRA_WAKE_GPIO("power", 24, 0, 189), /*TEGRA_GPIO(X, 5)*/
> +	TEGRA_WAKE_IRQ("rtc", 16, 2),
> +};
> +
>  static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra210_powergates),
>  	.powergates = tegra210_powergates,
> @@ -2552,10 +2668,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.regs = &tegra20_pmc_regs,
>  	.init = tegra20_pmc_init,
>  	.setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
> +	.pmc_irq_set_wake = tegra210_pmc_irq_set_wake,
> +	.pmc_irq_set_type = tegra210_pmc_irq_set_type,
>  	.reset_sources = tegra210_reset_sources,
>  	.num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
>  	.reset_levels = NULL,
>  	.num_reset_levels = 0,
> +	.num_wake_events = ARRAY_SIZE(tegra210_wake_events),
> +	.wake_events = tegra210_wake_events,
>  };
>  
>  #define TEGRA186_IO_PAD_TABLE(_pad)					     \
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2019-05-22 13:02 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-21 23:31 [PATCH V1 00/12] LP0 entry and exit support for Tegra210 Sowjanya Komatineni
2019-05-21 23:31 ` [PATCH V1 01/12] irqchip: tegra: do not disable COP IRQ during suspend Sowjanya Komatineni
2019-05-22 12:12   ` Thierry Reding
2019-05-21 23:31 ` [PATCH V1 02/12] pinctrl: tegra: add suspend and resume support Sowjanya Komatineni
2019-05-22 12:37   ` Thierry Reding
2019-05-21 23:31 ` [PATCH V1 03/12] clk: tegra: save and restore PLLs state for system Sowjanya Komatineni
2019-05-22 13:31   ` Thierry Reding
2019-05-21 23:31 ` [PATCH V1 04/12] clk: tegra: add support for peripheral clock suspend and resume Sowjanya Komatineni
2019-05-21 23:31 ` [PATCH V1 05/12] clk: tegra: add support for OSC clock resume Sowjanya Komatineni
2019-05-21 23:31 ` [PATCH V1 06/12] clk: tegra: add suspend resume support for DFLL clock Sowjanya Komatineni
2019-05-21 23:31 ` [PATCH V1 07/12] clk: tegra: support for Tegra210 clocks suspend-resume Sowjanya Komatineni
2019-05-21 23:31 ` [PATCH V1 08/12] soc/tegra: pmc: allow support for more tegra wake models Sowjanya Komatineni
2019-05-22 12:49   ` Thierry Reding
2019-05-22 13:02   ` Thierry Reding
2019-05-21 23:31 ` [PATCH V1 09/12] soc/tegra: pmc: add pmc wake support for tegra210 Sowjanya Komatineni
2019-05-22 13:01   ` Thierry Reding [this message]
2019-05-21 23:31 ` [PATCH V1 10/12] gpio: tegra: implement wake event support for Tegra210 and prior GPIO Sowjanya Komatineni
2019-05-22 13:24   ` Thierry Reding
2019-05-25 20:39     ` Sowjanya Komatineni
2019-05-21 23:31 ` [PATCH V1 11/12] soc/tegra: pmc: configure tegra deep sleep control settings Sowjanya Komatineni
2019-05-22 13:28   ` Thierry Reding
2019-05-21 23:31 ` [PATCH V1 12/12] arm64: tegra: enable wake from deep sleep on RTC alarm Sowjanya Komatineni
2019-05-22 13:33 ` [PATCH V1 00/12] LP0 entry and exit support for Tegra210 Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190522130155.GJ30938@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=jckuo@nvidia.com \
    --cc=jonathanh@nvidia.com \
    --cc=josephl@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=skomatineni@nvidia.com \
    --cc=talho@nvidia.com \
    --subject='Re: [PATCH V1 09/12] soc/tegra: pmc: add pmc wake support for tegra210' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).