All of lore.kernel.org
 help / color / mirror / Atom feed
From: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
To: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org,
	heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	"jinkun.hong"
	<jinkun.hong-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Subject: Re: [PATCH v17 3/4] soc: rockchip: power-domain: Add power domain driver
Date: Sun, 06 Sep 2015 11:04:58 +0800	[thread overview]
Message-ID: <55EBAD5A.9060107@rock-chips.com> (raw)
In-Reply-To: <7hbndkptfm.fsf-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Kevin,

Thanks for having a look into it.


在 2015年09月03日 02:28, Kevin Hilman 写道:
> Caesar Wang <wxt@rock-chips.com> writes:
>
>> This driver is found on RK3288 SoCs.
>>
>> In order to meet high performance and low power requirements, a power
>> management unit is designed or saving power when RK3288 in low power
>> mode.
>> The RK3288 PMU is dedicated for managing the power of the whole chip.
>>
>> PMU can work in the Low Power Mode by setting bit[0] of PMU_PWRMODE_CON
>> register. After setting the register, PMU would enter the Low Power mode.
>> In the low power mode, pmu will auto power on/off the specified power
>> domain, send idle req to specified power domain, shut down/up pll and
>> so on. All of above are configurable by setting corresponding registers.
>>
>> Signed-off-by: jinkun.hong <jinkun.hong@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> [...]
>
>> +static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < pd->num_clks; i++) {
>> +		clk_unprepare(pd->clks[i]);
>> +		clk_put(pd->clks[i]);
>> +	}
>
> You don't set pd->num_clks = 0 here, which means other places that
> iterate over the clocks might race with this and try to use clocks that
> have been unprepared/put.

Agree, we should set the "pd->num_cloks=0' in here.
> This might be over-paranoid, but in particular, this could race with
> rockchip_pd_power().
>
> Also not setting the pd->num_clks to zero would be a problem for a
> power-controller that is configured as a module which could be unloaded
> and reloaded (I know that doesn't really work now, but it will
> eventually, I hope.)

Yep.
>
> Maybe use the mutex here?  It should at least protect the zeroing of
> pm->num_clks.

Sound resonable.
Done.

---
Thanks,
Caesar
>
> Kevin
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


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

WARNING: multiple messages have this Message-ID (diff)
From: wxt@rock-chips.com (Caesar Wang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v17 3/4] soc: rockchip: power-domain: Add power domain driver
Date: Sun, 06 Sep 2015 11:04:58 +0800	[thread overview]
Message-ID: <55EBAD5A.9060107@rock-chips.com> (raw)
In-Reply-To: <7hbndkptfm.fsf@linaro.org>

Kevin,

Thanks for having a look into it.


? 2015?09?03? 02:28, Kevin Hilman ??:
> Caesar Wang <wxt@rock-chips.com> writes:
>
>> This driver is found on RK3288 SoCs.
>>
>> In order to meet high performance and low power requirements, a power
>> management unit is designed or saving power when RK3288 in low power
>> mode.
>> The RK3288 PMU is dedicated for managing the power of the whole chip.
>>
>> PMU can work in the Low Power Mode by setting bit[0] of PMU_PWRMODE_CON
>> register. After setting the register, PMU would enter the Low Power mode.
>> In the low power mode, pmu will auto power on/off the specified power
>> domain, send idle req to specified power domain, shut down/up pll and
>> so on. All of above are configurable by setting corresponding registers.
>>
>> Signed-off-by: jinkun.hong <jinkun.hong@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> [...]
>
>> +static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < pd->num_clks; i++) {
>> +		clk_unprepare(pd->clks[i]);
>> +		clk_put(pd->clks[i]);
>> +	}
>
> You don't set pd->num_clks = 0 here, which means other places that
> iterate over the clocks might race with this and try to use clocks that
> have been unprepared/put.

Agree, we should set the "pd->num_cloks=0' in here.
> This might be over-paranoid, but in particular, this could race with
> rockchip_pd_power().
>
> Also not setting the pd->num_clks to zero would be a problem for a
> power-controller that is configured as a module which could be unloaded
> and reloaded (I know that doesn't really work now, but it will
> eventually, I hope.)

Yep.
>
> Maybe use the mutex here?  It should at least protect the zeroing of
> pm->num_clks.

Sound resonable.
Done.

---
Thanks,
Caesar
>
> Kevin
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Caesar Wang <wxt@rock-chips.com>
To: Kevin Hilman <khilman@linaro.org>
Cc: Caesar Wang <wxt@rock-chips.com>,
	devicetree@vger.kernel.org, ulf.hansson@linaro.org,
	linux@arm.linux.org.uk, heiko@sntech.de, arnd@arndb.de,
	ijc+devicetree@hellion.org.uk,
	"jinkun.hong" <jinkun.hong@rock-chips.com>,
	linus.walleij@linaro.org, dmitry.torokhov@gmail.com,
	linux-kernel@vger.kernel.org, dianders@chromium.org,
	linux-rockchip@lists.infradead.org, robh+dt@kernel.org,
	galak@codeaurora.org, tomasz.figa@gmail.com,
	mturquette@baylibre.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v17 3/4] soc: rockchip: power-domain: Add power domain driver
Date: Sun, 06 Sep 2015 11:04:58 +0800	[thread overview]
Message-ID: <55EBAD5A.9060107@rock-chips.com> (raw)
In-Reply-To: <7hbndkptfm.fsf@linaro.org>

Kevin,

Thanks for having a look into it.


在 2015年09月03日 02:28, Kevin Hilman 写道:
> Caesar Wang <wxt@rock-chips.com> writes:
>
>> This driver is found on RK3288 SoCs.
>>
>> In order to meet high performance and low power requirements, a power
>> management unit is designed or saving power when RK3288 in low power
>> mode.
>> The RK3288 PMU is dedicated for managing the power of the whole chip.
>>
>> PMU can work in the Low Power Mode by setting bit[0] of PMU_PWRMODE_CON
>> register. After setting the register, PMU would enter the Low Power mode.
>> In the low power mode, pmu will auto power on/off the specified power
>> domain, send idle req to specified power domain, shut down/up pll and
>> so on. All of above are configurable by setting corresponding registers.
>>
>> Signed-off-by: jinkun.hong <jinkun.hong@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> [...]
>
>> +static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < pd->num_clks; i++) {
>> +		clk_unprepare(pd->clks[i]);
>> +		clk_put(pd->clks[i]);
>> +	}
>
> You don't set pd->num_clks = 0 here, which means other places that
> iterate over the clocks might race with this and try to use clocks that
> have been unprepared/put.

Agree, we should set the "pd->num_cloks=0' in here.
> This might be over-paranoid, but in particular, this could race with
> rockchip_pd_power().
>
> Also not setting the pd->num_clks to zero would be a problem for a
> power-controller that is configured as a module which could be unloaded
> and reloaded (I know that doesn't really work now, but it will
> eventually, I hope.)

Yep.
>
> Maybe use the mutex here?  It should at least protect the zeroing of
> pm->num_clks.

Sound resonable.
Done.

---
Thanks,
Caesar
>
> Kevin
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


  parent reply	other threads:[~2015-09-06  3:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-02  8:59 [PATCH v17 0/4] ARM: rk3288: Add PM Domain support Caesar Wang
2015-09-02  8:59 ` Caesar Wang
2015-09-02  8:59 ` Caesar Wang
2015-09-02  8:59 ` [PATCH v17 1/4] dt-bindings: add document of Rockchip power domains Caesar Wang
2015-09-02  8:59   ` Caesar Wang
     [not found]   ` <1441184380-13827-2-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2015-09-02 18:12     ` Kevin Hilman
2015-09-02 18:12       ` Kevin Hilman
2015-09-02 18:12       ` Kevin Hilman
2015-09-06 10:10       ` Caesar Wang
2015-09-06 10:10         ` Caesar Wang
2015-09-02  8:59 ` [PATCH v17 2/4] ARM: power-domain: rockchip: add all the domain type on RK3288 SoCs Caesar Wang
2015-09-02  8:59   ` Caesar Wang
     [not found] ` <1441184380-13827-1-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2015-09-02  8:59   ` [PATCH v17 3/4] soc: rockchip: power-domain: Add power domain driver Caesar Wang
2015-09-02  8:59     ` Caesar Wang
2015-09-02  8:59     ` Caesar Wang
2015-09-02 18:28     ` Kevin Hilman
2015-09-02 18:28       ` Kevin Hilman
     [not found]       ` <7hbndkptfm.fsf-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-09-06  3:04         ` Caesar Wang [this message]
2015-09-06  3:04           ` Caesar Wang
2015-09-06  3:04           ` Caesar Wang
2015-09-02  8:59 ` [PATCH v17 4/4] ARM: dts: add the support power-domain node on RK3288 SoCs Caesar Wang
2015-09-02  8:59   ` Caesar Wang

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=55EBAD5A.9060107@rock-chips.com \
    --to=wxt-tnx95d0mmh7dzftrwevzcw@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=jinkun.hong-TNX95d0MmH7DzftRWevZcw@public.gmane.org \
    --cc=khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org \
    --cc=linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.