| Message ID | 20191211203143.2952-3-f.fainelli@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | brcmstb_thermal updates for new processes | expand |
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@gmail.com> > --- > 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? > > - 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) ? > + 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? > priv->dev = &pdev->dev; > platform_set_drvdata(pdev, priv); > >
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@gmail.com> >> --- >> 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!
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 */ - 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; + + 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)); priv->dev = &pdev->dev; platform_set_drvdata(pdev, priv);
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@gmail.com> --- drivers/thermal/broadcom/brcmstb_thermal.c | 54 ++++++++++++++++------ 1 file changed, 39 insertions(+), 15 deletions(-)