| Message ID | 20240417134849.5793-1-iwona.winiarska@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | peci: aspeed: Clear clock_divider value before setting it | expand |
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
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 >
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
> 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. Reviewed-by: Billy Tsai <billy_tsai@aspeedtech.com> Billy
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);
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(+)