| 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: <CAK7LNASjwCCcRFk-3pXgBXBB4Nbx8Z-SY842=TZytqgqw6tfqg@mail.gmail.com>
Date: Fri, 21 Jul 2017 22:21:04 +0900
From: Masahiro Yamada <yamada.masahiro@...ionext.com>
To: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Cc: rui.zhang@...el.com, Eduardo Valentin <edubezval@...il.com>,
linux-pm@...r.kernel.org,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Masami Hiramatsu <masami.hiramatsu@...aro.org>,
Jassi Brar <jaswinder.singh@...aro.org>
Subject: Re: [PATCH v5 2/2] thermal: uniphier: add UniPhier thermal driver
2017-07-21 20:21 GMT+09:00 Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>:
> +static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev)
> +{
> + struct regmap *map = tdev->regmap;
> + u32 val;
> + int ret;
> +
> + /* stop PVT */
> + regmap_write_bits(map, tdev->data->block_base + PVTCTLEN,
> + PVTCTLEN_EN, 0);
> +
> + /*
> + * set default value if missing calibrated value
> + *
> + * Since SoC has a calibrated value that was set in advance,
> + * TMODCOEF shows non-zero and PVT refers the value internally.
> + *
> + * However, some boards don't have the calibrated value.
> + * In that case, TMODCOEF shows zero and the driver has to set
> + * default value manually.
> + */
> + ret = regmap_read(map, tdev->data->map_base + TMODCOEF, &val);
> + if (ret)
> + return ret;
> + if (!val)
> + regmap_write(map, tdev->data->tmod_setup_addr,
> + TMODSETUP0_EN | TMODSETUP0_VAL(tdev->tmod_calib0) |
> + TMODSETUP1_EN | TMODSETUP1_VAL(tdev->tmod_calib1));
This code is strange.
What if TMODCOEF has no calibrated value and
"socionext,tmod-calibration" is not set either?
->tmod_setupaddr will be set to zero
and the sensor would not work, right?
> + /* get tmod-calibration values */
> + calib = of_get_property(dev->of_node, "socionext,tmod-calibration",
> + NULL);
> + if (calib) {
> + tdev->tmod_calib0 = of_read_number(calib, 1);
> + tdev->tmod_calib1 = of_read_number(calib + 1, 1);
> + }
>From your DT change (https://patchwork.kernel.org/patch/9826391/),
this property seems a pair of u32 values, like follows:
socionext,tmod-calibration = <0x0f22 0x68ee>;
Why do you need to use of_read_number() to retrieve each u32 value?
See the comment and return value (u64):
------------------>8-----------------
/* Helper to read a big number; size is in cells (not bytes) */
static inline u64 of_read_number(const __be32 *cell, int size)
------------------8<-----------------
Also, you are not checking the length of the property.
of_read_number() may over-run the property.
of_property_read_u32_array() will be a better choice.
I'd propose the code like follows:
diff --git a/drivers/thermal/uniphier_thermal.c
b/drivers/thermal/uniphier_thermal.c
index 1a77c5bf6930..eea8a3053584 100644
--- a/drivers/thermal/uniphier_thermal.c
+++ b/drivers/thermal/uniphier_thermal.c
@@ -94,16 +94,16 @@ struct uniphier_tm_soc_data {
struct uniphier_tm_dev {
struct regmap *regmap;
bool alert_en[ALERT_CH_NUM];
- u32 tmod_calib0;
- u32 tmod_calib1;
struct thermal_zone_device *tz_dev;
const struct uniphier_tm_soc_data *data;
};
-static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev)
+static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev,
+ struct device *dev)
{
struct regmap *map = tdev->regmap;
u32 val;
+ u32 tmod_val[2];
int ret;
/* stop PVT */
@@ -123,10 +123,22 @@ static int uniphier_tm_initialize_sensor(struct
uniphier_tm_dev *tdev)
ret = regmap_read(map, tdev->data->map_base + TMODCOEF, &val);
if (ret)
return ret;
- if (!val)
+ if (!val) {
+ /*
+ * No preset value found in the register.
+ * Look for the fall-back values in DT.
+ */
+ ret = of_property_read_u32_array(dev->of_node,
+ "socionext,tmod-calibration",
+ tmod_val,
ARRAY_SIZE(tmod_val));
+ if (ret) {
+ dev_err(dev, "neither register nor DT has
calibrated values.\n");
+ return ret;
+ }
regmap_write(map, tdev->data->tmod_setup_addr,
- TMODSETUP0_EN | TMODSETUP0_VAL(tdev->tmod_calib0) |
- TMODSETUP1_EN | TMODSETUP1_VAL(tdev->tmod_calib1));
+ TMODSETUP0_EN | TMODSETUP0_VAL(tmod_val[0]) |
+ TMODSETUP1_EN | TMODSETUP1_VAL(tmod_val[1]));
+ }
/* select temperature mode */
regmap_write_bits(map, tdev->data->block_base + PVTCTLMODE,
@@ -254,7 +266,6 @@ static int uniphier_tm_probe(struct platform_device *pdev)
struct device_node *parent;
struct uniphier_tm_dev *tdev;
const struct thermal_trip *trips;
- const __be32 *calib;
int i, ret, irq, ntrips, crit_temp = INT_MAX;
tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL);
@@ -269,14 +280,6 @@ static int uniphier_tm_probe(struct platform_device *pdev)
if (irq < 0)
return irq;
- /* get tmod-calibration values */
- calib = of_get_property(dev->of_node, "socionext,tmod-calibration",
- NULL);
- if (calib) {
- tdev->tmod_calib0 = of_read_number(calib, 1);
- tdev->tmod_calib1 = of_read_number(calib + 1, 1);
- }
-
/* get regmap from syscon node */
parent = of_get_parent(dev->of_node); /* parent should be syscon node */
regmap = syscon_node_to_regmap(parent);
@@ -288,7 +291,7 @@ static int uniphier_tm_probe(struct platform_device *pdev)
}
tdev->regmap = regmap;
- ret = uniphier_tm_initialize_sensor(tdev);
+ ret = uniphier_tm_initialize_sensor(tdev, dev);
if (ret) {
dev_err(dev, "failed to initialize sensor\n");
return ret;
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists