[PATCH] peci: aspeed: Clear clock_divider value before setting it

Iwona Winiarska posted 1 patch 2 weeks ago
drivers/peci/controller/peci-aspeed.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] peci: aspeed: Clear clock_divider value before setting it
Posted by Iwona Winiarska 2 weeks ago
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
Re: [PATCH] peci: aspeed: Clear clock_divider value before setting it
Posted by Andrew Jeffery 2 weeks ago
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
Re: [PATCH] peci: aspeed: Clear clock_divider value before setting it
Posted by Winiarska, Iwona 1 week, 6 days ago
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
> 
Re: [PATCH] peci: aspeed: Clear clock_divider value before setting it
Posted by Andrew Jeffery 1 week, 6 days ago
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