On 8/30/23 20:32, Billy Tsai wrote: > +static int aspeed_tach_hwmon_write(struct device *dev, > + enum hwmon_sensor_types type, u32 attr, > + int channel, long val) > +{ > + struct aspeed_pwm_tach_data *priv = dev_get_drvdata(dev); > + u32 reg_val; > + > + switch (attr) { > + case hwmon_fan_div: > + if (!is_power_of_2(val) || (ilog2(val) % 2) || > + DIV_TO_REG(val) > 0xb) > + return -EINVAL; > + priv->tach_divisor = val; > + reg_val = readl(priv->base + TACH_ASPEED_CTRL(channel)); > + reg_val &= ~TACH_ASPEED_CLK_DIV_T_MASK; > + reg_val |= FIELD_GET(TACH_ASPEED_CLK_DIV_T_MASK, > + DIV_TO_REG(priv->tach_divisor)); Hi Billy, I notice the fanX_div is always shows 1 after I set 1024. I think FIELD_GET() needs to replaced with FIELD_PREP(). > + writel(reg_val, priv->base + TACH_ASPEED_CTRL(channel)); > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + return 0; > +} > +static void aspeed_present_fan_tach(struct aspeed_pwm_tach_data *priv, u32 tach_ch) > +{ > + u32 val; > + > + priv->tach_present[tach_ch] = true; > + priv->tach_divisor = DEFAULT_TACH_DIV; > + > + val = readl(priv->base + TACH_ASPEED_CTRL(tach_ch)); > + val &= ~(TACH_ASPEED_INVERS_LIMIT | TACH_ASPEED_DEBOUNCE_MASK | > + TACH_ASPEED_IO_EDGE_MASK | TACH_ASPEED_CLK_DIV_T_MASK | > + TACH_ASPEED_THRESHOLD_MASK); > + val |= (DEBOUNCE_3_CLK << TACH_ASPEED_DEBOUNCE_BIT) | F2F_EDGES | > + FIELD_GET(TACH_ASPEED_CLK_DIV_T_MASK, > + DIV_TO_REG(priv->tach_divisor)); And here as well. > + writel(val, priv->base + TACH_ASPEED_CTRL(tach_ch)); > + > + aspeed_tach_ch_enable(priv, tach_ch, true); > +} > + >