| Message ID | 20171121200225.23316-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Eduardo Valentin |
| Headers | show |
On Tue, 2017-11-21 at 21:02 +0100, Uwe Kleine-König wrote: > The values passed to imx_init_calib() and imx_init_temp_grade() are > read from specific OCOTP values. Use their names (in lower case) as > parameter name instead of "val" to make the code easier to understand. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/thermal/imx_thermal.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > index e7d4ffc3de7f..21b8c4c4da3c 100644 > --- a/drivers/thermal/imx_thermal.c > +++ b/drivers/thermal/imx_thermal.c > @@ -347,13 +347,13 @@ static struct thermal_zone_device_ops imx_tz_ops = { > .set_trip_temp = imx_set_trip_temp, > }; > > -static int imx_init_calib(struct platform_device *pdev, u32 val) > +static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1) > { > struct imx_thermal_data *data = platform_get_drvdata(pdev); > int t1, n1; > u64 temp64; > > - if (val == 0 || val == ~0) { > + if (ocotp_ana1 == 0 || ocotp_ana1 == ~0) { > dev_err(&pdev->dev, "invalid sensor calibration data\n"); > return -EINVAL; > } > @@ -364,7 +364,7 @@ static int imx_init_calib(struct platform_device *pdev, u32 val) > * Use universal formula now and only need sensor value @ 25C > * slope = 0.4297157 - (0.0015976 * 25C fuse) > */ > - n1 = val >> 20; > + n1 = ocotp_ana1 >> 20; > t1 = 25; /* t1 always 25C */ > > /* > @@ -392,12 +392,12 @@ static int imx_init_calib(struct platform_device *pdev, u32 val) > return 0; > } > > -static void imx_init_temp_grade(struct platform_device *pdev, u32 val) > +static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0) On imx7 (thermal not currently supported in upstream) the temperature grade is in OCOTP_TESTER3. So using a more generic parameter name here would be preferable or this would have to be changed again later. Unfortunately I can't find a good reference for the imx7 fusemap. But the IMX7D Reference Manual mentions a speed grade in 0x440 and something else for 0x480. -- Regards, Leonard
On Mon, Nov 27, 2017 at 08:39:31PM +0200, Leonard Crestez wrote: > On Tue, 2017-11-21 at 21:02 +0100, Uwe Kleine-König wrote: > > The values passed to imx_init_calib() and imx_init_temp_grade() are > > read from specific OCOTP values. Use their names (in lower case) as > > parameter name instead of "val" to make the code easier to understand. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > drivers/thermal/imx_thermal.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > > index e7d4ffc3de7f..21b8c4c4da3c 100644 > > --- a/drivers/thermal/imx_thermal.c > > +++ b/drivers/thermal/imx_thermal.c > > @@ -347,13 +347,13 @@ static struct thermal_zone_device_ops imx_tz_ops = { > > .set_trip_temp = imx_set_trip_temp, > > }; > > > > -static int imx_init_calib(struct platform_device *pdev, u32 val) > > +static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1) > > { > > struct imx_thermal_data *data = platform_get_drvdata(pdev); > > int t1, n1; > > u64 temp64; > > > > - if (val == 0 || val == ~0) { > > + if (ocotp_ana1 == 0 || ocotp_ana1 == ~0) { > > dev_err(&pdev->dev, "invalid sensor calibration data\n"); > > return -EINVAL; > > } > > @@ -364,7 +364,7 @@ static int imx_init_calib(struct platform_device *pdev, u32 val) > > * Use universal formula now and only need sensor value @ 25C > > * slope = 0.4297157 - (0.0015976 * 25C fuse) > > */ > > - n1 = val >> 20; > > + n1 = ocotp_ana1 >> 20; > > t1 = 25; /* t1 always 25C */ > > > > /* > > @@ -392,12 +392,12 @@ static int imx_init_calib(struct platform_device *pdev, u32 val) > > return 0; > > } > > > > -static void imx_init_temp_grade(struct platform_device *pdev, u32 val) > > +static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0) > > On imx7 (thermal not currently supported in upstream) the temperature > grade is in OCOTP_TESTER3. So using a more generic parameter name here > would be preferable or this would have to be changed again later. Then I suggest to change the semantic and make this: imx_init_temp_grade(struct platform_device *pdev, u32 grade) and let the caller do the bit shift and masking. Does the same problem exist for ocotp_ana1? Best regards Uwe
On Mon, 2017-11-27 at 20:50 +0100, Uwe Kleine-König wrote: > On Mon, Nov 27, 2017 at 08:39:31PM +0200, Leonard Crestez wrote: > > On Tue, 2017-11-21 at 21:02 +0100, Uwe Kleine-König wrote: > > > > > > The values passed to imx_init_calib() and imx_init_temp_grade() are > > > read from specific OCOTP values. Use their names (in lower case) as > > > parameter name instead of "val" to make the code easier to understand. > > > > > > -static void imx_init_temp_grade(struct platform_device *pdev, u32 val) > > > +static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0) > > On imx7 (thermal not currently supported in upstream) the temperature > > grade is in OCOTP_TESTER3. So using a more generic parameter name here > > would be preferable or this would have to be changed again later. > Then I suggest to change the semantic and make this: > > imx_init_temp_grade(struct platform_device *pdev, u32 grade) > > and let the caller do the bit shift and masking. That would work. It just so happens that the grade is stored in the same bits. It might be better to deal with this when adding imx7 thermal support instead. > Does the same problem exist for ocotp_ana1? No, not that I know of.
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c index e7d4ffc3de7f..21b8c4c4da3c 100644 --- a/drivers/thermal/imx_thermal.c +++ b/drivers/thermal/imx_thermal.c @@ -347,13 +347,13 @@ static struct thermal_zone_device_ops imx_tz_ops = { .set_trip_temp = imx_set_trip_temp, }; -static int imx_init_calib(struct platform_device *pdev, u32 val) +static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1) { struct imx_thermal_data *data = platform_get_drvdata(pdev); int t1, n1; u64 temp64; - if (val == 0 || val == ~0) { + if (ocotp_ana1 == 0 || ocotp_ana1 == ~0) { dev_err(&pdev->dev, "invalid sensor calibration data\n"); return -EINVAL; } @@ -364,7 +364,7 @@ static int imx_init_calib(struct platform_device *pdev, u32 val) * Use universal formula now and only need sensor value @ 25C * slope = 0.4297157 - (0.0015976 * 25C fuse) */ - n1 = val >> 20; + n1 = ocotp_ana1 >> 20; t1 = 25; /* t1 always 25C */ /* @@ -392,12 +392,12 @@ static int imx_init_calib(struct platform_device *pdev, u32 val) return 0; } -static void imx_init_temp_grade(struct platform_device *pdev, u32 val) +static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0) { struct imx_thermal_data *data = platform_get_drvdata(pdev); /* The maximum die temp is specified by the Temperature Grade */ - switch ((val >> 6) & 0x3) { + switch ((ocotp_mem0 >> 6) & 0x3) { case 0: /* Commercial (0 to 95C) */ data->temp_grade = "Commercial"; data->temp_max = 95000;
The values passed to imx_init_calib() and imx_init_temp_grade() are read from specific OCOTP values. Use their names (in lower case) as parameter name instead of "val" to make the code easier to understand. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/thermal/imx_thermal.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)