From: Florian Fainelli <f.fainelli@gmail.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>,
Florian Fainelli <f.fainelli@gmail.com>,
linux-arm-kernel@lists.infradead.org
Cc: Markus Mayer <mmayer@broadcom.com>,
"maintainer:BROADCOM STB AVS TMON DRIVER"
<bcm-kernel-feedback-list@broadcom.com>,
Zhang Rui <rui.zhang@intel.com>,
Eduardo Valentin <edubezval@gmail.com>,
Amit Kucheria <amit.kucheria@verdurent.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
"open list:BROADCOM STB AVS TMON DRIVER"
<linux-pm@vger.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/6] thermal: brcmstb_thermal: Prepare to support a different process
Date: Fri, 10 Jan 2020 16:37:51 -0800 [thread overview]
Message-ID: <5c1f1031-d5ad-a4fc-4b6e-195b0b0f37a7@gmail.com> (raw)
In-Reply-To: <a8e6cb1a-fa03-a443-eaa2-2a6385ac6a37@linaro.org>
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!
--
Florian
next prev parent reply other threads:[~2020-01-11 0:38 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-11 20:31 [PATCH v2 0/6] brcmstb_thermal updates for new processes Florian Fainelli
2019-12-11 20:31 ` [PATCH v2 1/6] thermal: brcmstb_thermal: Do not use DT coefficients Florian Fainelli
2019-12-11 20:31 ` [PATCH v2 2/6] thermal: brcmstb_thermal: Prepare to support a different process Florian Fainelli
2020-01-09 21:17 ` Daniel Lezcano
2020-01-11 0:37 ` Florian Fainelli [this message]
2019-12-11 20:31 ` [PATCH v2 3/6] dt-bindings: thermal: Define BCM7216 thermal sensor compatible Florian Fainelli
2019-12-11 20:31 ` [PATCH v2 4/6] thermal: brcmstb_thermal: Add 16nm process thermal parameters Florian Fainelli
2019-12-11 20:31 ` [PATCH v2 5/6] thermal: brcmstb_thermal: Restructure interrupt registration Florian Fainelli
2019-12-11 20:31 ` [PATCH v2 6/6] thermal: brcmstb_thermal: Register different ops per process Florian Fainelli
2019-12-26 15:54 ` [PATCH v2 0/6] brcmstb_thermal updates for new processes Florian Fainelli
2020-01-08 18:07 ` Florian Fainelli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5c1f1031-d5ad-a4fc-4b6e-195b0b0f37a7@gmail.com \
--to=f.fainelli@gmail.com \
--cc=amit.kucheria@verdurent.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=daniel.lezcano@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=edubezval@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mmayer@broadcom.com \
--cc=robh+dt@kernel.org \
--cc=rui.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).