Linux-Amlogic Archive on lore.kernel.org
 help / color / Atom feed
From: Neil Armstrong <narmstrong@baylibre.com>
To: Kevin Hilman <khilman@kernel.org>, linux-amlogic@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH v2 2/2] soc: amlogic: ee-pwrc: ensure driver state maches HW state
Date: Thu, 26 Sep 2019 11:05:52 +0200
Message-ID: <8936e777-8996-5c7b-ea9d-8e17c8d6c4b1@baylibre.com> (raw)
In-Reply-To: <20190925213528.21515-3-khilman@kernel.org>

On 25/09/2019 23:35, Kevin Hilman wrote:
> From: Kevin Hilman <khilman@baylibre.com>
> 
> During init, ensure that the driver on/off state as well as clock and
> reset state matches the hardware state.  Do this by always calling the
> drivers 'on' function, and then callling the 'off' function if the
> HW state was initially detected as off.
> 
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/soc/amlogic/meson-ee-pwrc.c | 29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/soc/amlogic/meson-ee-pwrc.c b/drivers/soc/amlogic/meson-ee-pwrc.c
> index dcce8e694a07..2e8eee0dc166 100644
> --- a/drivers/soc/amlogic/meson-ee-pwrc.c
> +++ b/drivers/soc/amlogic/meson-ee-pwrc.c
> @@ -323,6 +323,8 @@ static int meson_ee_pwrc_init_domain(struct platform_device *pdev,
>  				     struct meson_ee_pwrc *pwrc,
>  				     struct meson_ee_pwrc_domain *dom)
>  {
> +	bool is_off;
> +
>  	dom->pwrc = pwrc;
>  	dom->num_rstc = dom->desc.reset_names_count;
>  	dom->num_clks = dom->desc.clk_names_count;
> @@ -356,27 +358,12 @@ static int meson_ee_pwrc_init_domain(struct platform_device *pdev,
>  	dom->base.power_on = meson_ee_pwrc_on;
>  	dom->base.power_off = meson_ee_pwrc_off;
>  
> -	/*
> -         * TOFIX: This is a special case for the VPU power domain, which can
> -	 * be enabled previously by the bootloader. In this case the VPU
> -         * pipeline may be functional but no driver maybe never attach
> -         * to this power domain, and if the domain is disabled it could
> -         * cause system errors. This is why the pm_domain_always_on_gov
> -         * is used here.
> -         * For the same reason, the clocks should be enabled in case
> -         * we need to power the domain off, otherwise the internal clocks
> -         * prepare/enable counters won't be in sync.
> -         */
> -	if (dom->num_clks && dom->desc.is_off && !dom->desc.is_off(dom)) {
> -		int ret = clk_bulk_prepare_enable(dom->num_clks, dom->clks);
> -		if (ret)
> -			return ret;
> -
> -		pm_genpd_init(&dom->base, &pm_domain_always_on_gov, false);
> -	} else
> -		pm_genpd_init(&dom->base, NULL,
> -			      (dom->desc.is_off ?
> -			       dom->desc.is_off(dom) : true));
> +	/* Ensure that driver state matches HW state */
> +	is_off = dom->desc.is_off ? dom->desc.is_off(dom) : true;
> +	meson_ee_pwrc_on(&dom->base);
> +	if (is_off)
> +		meson_ee_pwrc_off(&dom->base);
> +	pm_genpd_init(&dom->base, NULL, is_off);
>  
>  	return 0;
>  }
> 

I don't see what you are trying to solve except simplifying the code.

And the case is more that "matching the clock state" here, the
pm_domain_always_on_gov was is a real case when booting from the Amlogic
U-boot.

The display power domain is complex and as been half solved by using
"simple-framebuffer" on gx and is missing on g12a/g12b/sm1.

For example, Debian installer runs without the modules, but will use
the EFIfb set by U-Boot, but in this precise case :
- the DRM driver isn't loaded
- we can't hook this power domain with EFIfb

When *not* in EFIfb, we use simple-framebuffer on GX, using this
power domain, but it hasn't been copied to G12A.

Personally I'll leave this code until we really tested and checked all
uses cases, not only on the sei510/sei610 using mainline u-boot.

Neil

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-25 21:35 [PATCH v2 0/2] soc: amlogic: ee-pwrc: cleanup init state Kevin Hilman
2019-09-25 21:35 ` [PATCH v2 1/2] soc: amlogic: ee-pwrc: rename get_power Kevin Hilman
2019-09-25 21:35 ` [PATCH v2 2/2] soc: amlogic: ee-pwrc: ensure driver state maches HW state Kevin Hilman
2019-09-26  9:05   ` Neil Armstrong [this message]
2019-09-26 19:08     ` Kevin Hilman
2019-09-27  6:37       ` Neil Armstrong
2019-09-27 16:02         ` Kevin Hilman
2019-09-30  8:22         ` Jerome Brunet
2019-09-30 15:32           ` Kevin Hilman

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=8936e777-8996-5c7b-ea9d-8e17c8d6c4b1@baylibre.com \
    --to=narmstrong@baylibre.com \
    --cc=khilman@kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    /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

Linux-Amlogic Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-amlogic/0 linux-amlogic/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-amlogic linux-amlogic/ https://lore.kernel.org/linux-amlogic \
		linux-amlogic@lists.infradead.org
	public-inbox-index linux-amlogic

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-amlogic


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git