openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] peci: aspeed: Clear clock_divider value before setting it
@ 2024-04-17 13:48 Iwona Winiarska
  2024-04-17 23:41 ` Andrew Jeffery
  0 siblings, 1 reply; 4+ messages in thread
From: Iwona Winiarska @ 2024-04-17 13:48 UTC (permalink / raw)
  To: openbmc, linux-kernel, linux-aspeed, linux-arm-kernel
  Cc: Billy Tsai, Iwona Winiarska

PECI clock divider is programmed on 10:8 bits of PECI Control register.
Before setting a new value, clear bits read from hardware.

Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
---
 drivers/peci/controller/peci-aspeed.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/peci/controller/peci-aspeed.c b/drivers/peci/controller/peci-aspeed.c
index 7fdc25afcf2f..de7046e6b9c4 100644
--- a/drivers/peci/controller/peci-aspeed.c
+++ b/drivers/peci/controller/peci-aspeed.c
@@ -351,6 +351,7 @@ static int clk_aspeed_peci_set_rate(struct clk_hw *hw, unsigned long rate,
 	clk_aspeed_peci_find_div_values(this_rate, &msg_timing, &clk_div_exp);
 
 	val = readl(aspeed_peci->base + ASPEED_PECI_CTRL);
+	val &= ~ASPEED_PECI_CTRL_CLK_DIV_MASK;
 	val |= FIELD_PREP(ASPEED_PECI_CTRL_CLK_DIV_MASK, clk_div_exp);
 	writel(val, aspeed_peci->base + ASPEED_PECI_CTRL);
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] peci: aspeed: Clear clock_divider value before setting it
  2024-04-17 13:48 [PATCH] peci: aspeed: Clear clock_divider value before setting it Iwona Winiarska
@ 2024-04-17 23:41 ` Andrew Jeffery
  2024-04-18 13:41   ` Winiarska, Iwona
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Jeffery @ 2024-04-17 23:41 UTC (permalink / raw)
  To: Iwona Winiarska, openbmc, linux-kernel, linux-aspeed, linux-arm-kernel
  Cc: Billy Tsai

Hi Iwona,

On Wed, 2024-04-17 at 15:48 +0200, Iwona Winiarska wrote:
> PECI clock divider is programmed on 10:8 bits of PECI Control register.
> Before setting a new value, clear bits read from hardware.
> 
> Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>

I think it would be best to add a Fixes: tag and Cc: stable in
accordance with the stable tree rules. Are you happy to do so?

Andrew


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] peci: aspeed: Clear clock_divider value before setting it
  2024-04-17 23:41 ` Andrew Jeffery
@ 2024-04-18 13:41   ` Winiarska, Iwona
  2024-04-18 23:25     ` Andrew Jeffery
  0 siblings, 1 reply; 4+ messages in thread
From: Winiarska, Iwona @ 2024-04-18 13:41 UTC (permalink / raw)
  To: linux-aspeed, linux-arm-kernel, openbmc, linux-kernel, andrew; +Cc: billy_tsai

On Thu, 2024-04-18 at 09:11 +0930, Andrew Jeffery wrote:
> Hi Iwona,
> 
> On Wed, 2024-04-17 at 15:48 +0200, Iwona Winiarska wrote:
> > PECI clock divider is programmed on 10:8 bits of PECI Control register.
> > Before setting a new value, clear bits read from hardware.
> > 
> > Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
> 
> I think it would be best to add a Fixes: tag and Cc: stable in
> accordance with the stable tree rules. Are you happy to do so?

Hi!

Technically, the initial value of this register should be 0, but I've added the
clear just to be on the safe side and to not have to rely on that.
I don't think we're ever programming invalid values in the real-world scenarios,
and because of that - I don't think this is stable material.

Thanks
-Iwona

> 
> Andrew
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] peci: aspeed: Clear clock_divider value before setting it
  2024-04-18 13:41   ` Winiarska, Iwona
@ 2024-04-18 23:25     ` Andrew Jeffery
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Jeffery @ 2024-04-18 23:25 UTC (permalink / raw)
  To: Winiarska, Iwona, linux-aspeed, linux-arm-kernel, openbmc, linux-kernel
  Cc: billy_tsai

On Thu, 2024-04-18 at 13:41 +0000, Winiarska, Iwona wrote:
> On Thu, 2024-04-18 at 09:11 +0930, Andrew Jeffery wrote:
> > Hi Iwona,
> > 
> > On Wed, 2024-04-17 at 15:48 +0200, Iwona Winiarska wrote:
> > > PECI clock divider is programmed on 10:8 bits of PECI Control register.
> > > Before setting a new value, clear bits read from hardware.
> > > 
> > > Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
> > 
> > I think it would be best to add a Fixes: tag and Cc: stable in
> > accordance with the stable tree rules. Are you happy to do so?
> 
> Hi!
> 
> Technically, the initial value of this register should be 0, but I've added the
> clear just to be on the safe side and to not have to rely on that.

Yeah, it could cause havoc with an unbind/bind sequence if people are
messing with the clocks in between.

> I don't think we're ever programming invalid values in the real-world scenarios,
> and because of that - I don't think this is stable material.

Right, I don't expect people are doing the above in environments where
stability is a concern.

Thanks for elaborating.

Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-04-18 23:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 13:48 [PATCH] peci: aspeed: Clear clock_divider value before setting it Iwona Winiarska
2024-04-17 23:41 ` Andrew Jeffery
2024-04-18 13:41   ` Winiarska, Iwona
2024-04-18 23:25     ` Andrew Jeffery

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).