All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: mturquette@baylibre.com, sboyd@codeaurora.org
Cc: linux-clk@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	James Hogan <james.hogan@imgtec.com>,
	Zhang Qing <zhangqing@rock-chips.com>
Subject: [PATCH v2] clk-divider: make sure read-only dividers do not write to their register
Date: Wed, 20 Jan 2016 20:51:06 +0100	[thread overview]
Message-ID: <3578615.iBrKplh45Q@phil> (raw)
In-Reply-To: <8710207.gKyLbM2bKY@phil>

Commit e6d5e7d90be9 ("clk-divider: Fix READ_ONLY when divider > 1") removed
the special ops struct for read-only clocks and instead opted to handle
them regularly.

On the rk3368 this results in breakage as aclkm now gets set a value.
While it is the same divider value, the A53 core still doesn't like it,
which can result in the cpu ending up in a hang.
The reason being that "ACLKENMasserts one clock cycle before the rising
edge of ACLKM" and the clock should only be touched when STANDBYWFIL2
is asserted.

So make sure read-only clocks don't touch the clock-register at all
even if only writing the same value, as even writing the same value
may not be safe in all cases.

Fixes: e6d5e7d90be9 ("clk-divider: Fix READ_ONLY when divider > 1")
Reported-by: Zhang Qing <zhangqing@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: James Hogan <james.hogan@imgtec.com>
---
changes in v2:
- emphasize that even setting the same divider may be unsafe

 drivers/clk/clk-divider.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index ded3ff4..6dfe261 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -391,6 +391,10 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long flags = 0;
 	u32 val;
 
+	/* don't do writes in read-only case, as it may be unsafe */
+	if (divider->flags & CLK_DIVIDER_READ_ONLY)
+		return 0;
+
 	value = divider_get_val(rate, parent_rate, divider->table,
 				divider->width, divider->flags);
 
-- 
2.6.4

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
To: mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
	sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>,
	linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Subject: [PATCH v2] clk-divider: make sure read-only dividers do not write to their register
Date: Wed, 20 Jan 2016 20:51:06 +0100	[thread overview]
Message-ID: <3578615.iBrKplh45Q@phil> (raw)
In-Reply-To: <8710207.gKyLbM2bKY@phil>

Commit e6d5e7d90be9 ("clk-divider: Fix READ_ONLY when divider > 1") removed
the special ops struct for read-only clocks and instead opted to handle
them regularly.

On the rk3368 this results in breakage as aclkm now gets set a value.
While it is the same divider value, the A53 core still doesn't like it,
which can result in the cpu ending up in a hang.
The reason being that "ACLKENMasserts one clock cycle before the rising
edge of ACLKM" and the clock should only be touched when STANDBYWFIL2
is asserted.

So make sure read-only clocks don't touch the clock-register at all
even if only writing the same value, as even writing the same value
may not be safe in all cases.

Fixes: e6d5e7d90be9 ("clk-divider: Fix READ_ONLY when divider > 1")
Reported-by: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Reviewed-by: James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
changes in v2:
- emphasize that even setting the same divider may be unsafe

 drivers/clk/clk-divider.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index ded3ff4..6dfe261 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -391,6 +391,10 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long flags = 0;
 	u32 val;
 
+	/* don't do writes in read-only case, as it may be unsafe */
+	if (divider->flags & CLK_DIVIDER_READ_ONLY)
+		return 0;
+
 	value = divider_get_val(rate, parent_rate, divider->table,
 				divider->width, divider->flags);
 
-- 
2.6.4

WARNING: multiple messages have this Message-ID (diff)
From: heiko@sntech.de (Heiko Stuebner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] clk-divider: make sure read-only dividers do not write to their register
Date: Wed, 20 Jan 2016 20:51:06 +0100	[thread overview]
Message-ID: <3578615.iBrKplh45Q@phil> (raw)
In-Reply-To: <8710207.gKyLbM2bKY@phil>

Commit e6d5e7d90be9 ("clk-divider: Fix READ_ONLY when divider > 1") removed
the special ops struct for read-only clocks and instead opted to handle
them regularly.

On the rk3368 this results in breakage as aclkm now gets set a value.
While it is the same divider value, the A53 core still doesn't like it,
which can result in the cpu ending up in a hang.
The reason being that "ACLKENMasserts one clock cycle before the rising
edge of ACLKM" and the clock should only be touched when STANDBYWFIL2
is asserted.

So make sure read-only clocks don't touch the clock-register at all
even if only writing the same value, as even writing the same value
may not be safe in all cases.

Fixes: e6d5e7d90be9 ("clk-divider: Fix READ_ONLY when divider > 1")
Reported-by: Zhang Qing <zhangqing@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: James Hogan <james.hogan@imgtec.com>
---
changes in v2:
- emphasize that even setting the same divider may be unsafe

 drivers/clk/clk-divider.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index ded3ff4..6dfe261 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -391,6 +391,10 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long flags = 0;
 	u32 val;
 
+	/* don't do writes in read-only case, as it may be unsafe */
+	if (divider->flags & CLK_DIVIDER_READ_ONLY)
+		return 0;
+
 	value = divider_get_val(rate, parent_rate, divider->table,
 				divider->width, divider->flags);
 
-- 
2.6.4

  parent reply	other threads:[~2016-01-20 19:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-20 14:42 [PATCH] clk-divider: make sure read-only dividers do not write to their register Heiko Stuebner
2016-01-20 14:42 ` Heiko Stuebner
2016-01-20 14:57 ` James Hogan
2016-01-20 14:57   ` James Hogan
2016-01-20 14:57   ` James Hogan
2016-01-20 19:51 ` Heiko Stuebner [this message]
2016-01-20 19:51   ` [PATCH v2] " Heiko Stuebner
2016-01-20 19:51   ` Heiko Stuebner
2016-01-20 22:16   ` Stephen Boyd
2016-01-20 22:16     ` Stephen Boyd
2016-01-20 22:29     ` Heiko Stuebner
2016-01-20 22:29       ` Heiko Stuebner

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=3578615.iBrKplh45Q@phil \
    --to=heiko@sntech.de \
    --cc=james.hogan@imgtec.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@codeaurora.org \
    --cc=zhangqing@rock-chips.com \
    /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.