On 1/9/20 1:17 PM, Daniel Lezcano wrote: > On 11/12/2019 21:31, Florian Fainelli wrote: >> The driver is currently assuming that it is operating with a 28nm >> process chip, which has a specific formula to convert temperature to a >> code and vice versa. Update the code to support providing two key >> values: offset and multiplier to derive the correct formulas. >> >> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx> >> --- >> drivers/thermal/broadcom/brcmstb_thermal.c | 54 ++++++++++++++++------ >> 1 file changed, 39 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c >> index 680f1a070606..68f89f7c7e7f 100644 >> --- a/drivers/thermal/broadcom/brcmstb_thermal.c >> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c >> @@ -102,18 +102,27 @@ static struct avs_tmon_trip avs_tmon_trips[] = { >> }, >> }; >> >> +struct brcmstb_thermal_params { >> + unsigned int offset; >> + unsigned int mult; >> +}; >> + >> struct brcmstb_thermal_priv { >> void __iomem *tmon_base; >> struct device *dev; >> struct thermal_zone_device *thermal; >> + /* Process specific thermal parameters used for calculations */ >> + struct brcmstb_thermal_params temp_params; >> }; >> >> /* Convert a HW code to a temperature reading (millidegree celsius) */ >> -static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz, >> +static inline int avs_tmon_code_to_temp(struct brcmstb_thermal_priv *priv, >> u32 code) >> { >> - return (AVS_TMON_TEMP_OFFSET - >> - (int)((code & AVS_TMON_TEMP_MAX) * AVS_TMON_TEMP_SLOPE)); >> + int offset = priv->temp_params.offset; >> + int mult = priv->temp_params.mult; >> + >> + return (offset - (int)((code & AVS_TMON_TEMP_MASK) * mult)); >> } >> >> /* >> @@ -122,21 +131,22 @@ static inline int avs_tmon_code_to_temp(struct thermal_zone_device *tz, >> * @temp: temperature to convert >> * @low: if true, round toward the low side >> */ >> -static inline u32 avs_tmon_temp_to_code(struct thermal_zone_device *tz, >> +static inline u32 avs_tmon_temp_to_code(struct brcmstb_thermal_priv *priv, >> int temp, bool low) >> { >> + int offset = priv->temp_params.offset; >> + int mult = priv->temp_params.mult; >> + >> if (temp < AVS_TMON_TEMP_MIN) >> - return AVS_TMON_TEMP_MAX; /* Maximum code value */ >> + return AVS_TMON_TEMP_MASK; /* Maximum code value */ > > Why this change? No reason to change, thanks. > >> >> - if (temp >= AVS_TMON_TEMP_OFFSET) >> + if (temp >= offset) >> return 0; /* Minimum code value */ >> >> if (low) >> - return (u32)(DIV_ROUND_UP(AVS_TMON_TEMP_OFFSET - temp, >> - AVS_TMON_TEMP_SLOPE)); >> + return (u32)(DIV_ROUND_UP(offset - temp, mult)); >> else >> - return (u32)((AVS_TMON_TEMP_OFFSET - temp) / >> - AVS_TMON_TEMP_SLOPE); >> + return (u32)((offset - temp) / mult); >> } >> >> static int brcmstb_get_temp(void *data, int *temp) >> @@ -154,7 +164,7 @@ static int brcmstb_get_temp(void *data, int *temp) >> >> val = (val & AVS_TMON_STATUS_data_msk) >> AVS_TMON_STATUS_data_shift; >> >> - t = avs_tmon_code_to_temp(priv->thermal, val); >> + t = avs_tmon_code_to_temp(priv, val); >> if (t < 0) >> *temp = 0; >> else >> @@ -188,7 +198,7 @@ static int avs_tmon_get_trip_temp(struct brcmstb_thermal_priv *priv, >> val &= trip->reg_msk; >> val >>= trip->reg_shift; >> >> - return avs_tmon_code_to_temp(priv->thermal, val); >> + return avs_tmon_code_to_temp(priv, val); >> } >> >> static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv, >> @@ -201,7 +211,7 @@ static void avs_tmon_set_trip_temp(struct brcmstb_thermal_priv *priv, >> dev_dbg(priv->dev, "set temp %d to %d\n", type, temp); >> >> /* round toward low temp for the low interrupt */ >> - val = avs_tmon_temp_to_code(priv->thermal, temp, >> + val = avs_tmon_temp_to_code(priv, temp, >> type == TMON_TRIP_TYPE_LOW); >> >> val <<= trip->reg_shift; >> @@ -218,7 +228,7 @@ static int avs_tmon_get_intr_temp(struct brcmstb_thermal_priv *priv) >> u32 val; >> >> val = __raw_readl(priv->tmon_base + AVS_TMON_TEMP_INT_CODE); >> - return avs_tmon_code_to_temp(priv->thermal, val); >> + return avs_tmon_code_to_temp(priv, val); >> } >> >> static irqreturn_t brcmstb_tmon_irq_thread(int irq, void *data) >> @@ -282,19 +292,32 @@ static const struct thermal_zone_of_device_ops of_ops = { >> .set_trips = brcmstb_set_trips, >> }; >> >> +static const struct brcmstb_thermal_params brcmstb_28nm_params = { >> + .offset = 410040, >> + .mult = 487, >> +}; >> + >> static const struct of_device_id brcmstb_thermal_id_table[] = { >> - { .compatible = "brcm,avs-tmon" }, >> + { .compatible = "brcm,avs-tmon", .data = &brcmstb_28nm_params }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, brcmstb_thermal_id_table); >> >> static int brcmstb_thermal_probe(struct platform_device *pdev) >> { >> + const struct brcmstb_thermal_params *params; >> + const struct of_device_id *of_id = NULL; >> struct thermal_zone_device *thermal; >> struct brcmstb_thermal_priv *priv; >> struct resource *res; >> int irq, ret; >> >> + of_id = of_match_node(brcmstb_thermal_id_table, pdev->dev.of_node); >> + if (!of_id || !of_id->data) >> + return -EINVAL; > > of_device_get_match_data(&pdev->dev) ? Yes. > >> + params = of_id->data; >> + >> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); >> if (!priv) >> return -ENOMEM; >> @@ -304,6 +327,7 @@ static int brcmstb_thermal_probe(struct platform_device *pdev) >> if (IS_ERR(priv->tmon_base)) >> return PTR_ERR(priv->tmon_base); >> >> + memcpy(&priv->temp_params, params, sizeof(priv->temp_params)); > > Do you really need a copy here? Why not convert to a pointer and assign it? I would rather not reference data that is possibly __initconst or __initdata, this is not the case here but I have been burned by this before, I will change it to a pointer. Thanks! -- Florian