| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5c1f1031-d5ad-a4fc-4b6e-195b0b0f37a7@gmail.com>
Date: Fri, 10 Jan 2020 16:37:51 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>,
Florian Fainelli <f.fainelli@...il.com>,
linux-arm-kernel@...ts.infradead.org
Cc: Markus Mayer <mmayer@...adcom.com>,
"maintainer:BROADCOM STB AVS TMON DRIVER"
<bcm-kernel-feedback-list@...adcom.com>,
Zhang Rui <rui.zhang@...el.com>,
Eduardo Valentin <edubezval@...il.com>,
Amit Kucheria <amit.kucheria@...durent.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
"open list:BROADCOM STB AVS TMON DRIVER" <linux-pm@...r.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/6] thermal: brcmstb_thermal: Prepare to support a
different process
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@...il.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
Powered by blists - more mailing lists