| Message ID | 1363818997-23137-9-git-send-email-ezequiel.garcia@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 20, 2013 at 07:36:29PM -0300, Ezequiel Garcia wrote: > The temperature formula was taken from the Armada 510 datasheet. > This is wrong because the Kirkwood SoC thermal sensor has > different coefficients. > > Fix it using values from 88F682/88F683 Kirkwood datasheet. > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > drivers/thermal/mvebu_thermal.c | 7 +------ > 1 files changed, 1 insertions(+), 6 deletions(-) This is also a fix which should go at the beginning of the series. thx, Jason.
On Wed, Mar 20, 2013 at 07:36:29PM -0300, Ezequiel Garcia wrote: > The temperature formula was taken from the Armada 510 datasheet. > This is wrong because the Kirkwood SoC thermal sensor has > different coefficients. > > Fix it using values from 88F682/88F683 Kirkwood datasheet. Err, hang on. You've changed this from being a Kirkwood specific driver to being a "mvebu" driver, which I had understood was being done to cover all Kirkwood, Dove, etc SoCs. If that's the case, then this change is wrong because you break it for Dove. If the coefficients are different between Kirkwood and Dove, then it needs to be adapted at run time depending on whether it's running on Kirkwood or Dove - not just having the coefficients changed at compile time to suit a different set of platforms. If it's worth making a change, do it *properly*.
On Thu, Mar 21, 2013 at 07:20:46PM +0000, Russell King - ARM Linux wrote: > On Wed, Mar 20, 2013 at 07:36:29PM -0300, Ezequiel Garcia wrote: > > The temperature formula was taken from the Armada 510 datasheet. > > This is wrong because the Kirkwood SoC thermal sensor has > > different coefficients. > > > > Fix it using values from 88F682/88F683 Kirkwood datasheet. > > Err, hang on. You've changed this from being a Kirkwood specific > driver to being a "mvebu" driver, which I had understood was being > done to cover all Kirkwood, Dove, etc SoCs. > > If that's the case, then this change is wrong because you break it > for Dove. > > If the coefficients are different between Kirkwood and Dove, then > it needs to be adapted at run time depending on whether it's running > on Kirkwood or Dove - not just having the coefficients changed at > compile time to suit a different set of platforms. > > If it's worth making a change, do it *properly*. I'm not sure I'm following you. This patch fixes the Kirkwood formula on a driver that, at this point, only supports Kirkwood. Dove SoC thermal sensor is supported in dove-thermal, until it's merged in patch 13/16 (i.e. after this patch). AFAIK, I'm not breaking support for any platform in any point of the patchset. Perhaps I'm missing something?
diff --git a/drivers/thermal/mvebu_thermal.c b/drivers/thermal/mvebu_thermal.c index 5886a9c..ed4c9b0 100644 --- a/drivers/thermal/mvebu_thermal.c +++ b/drivers/thermal/mvebu_thermal.c @@ -48,14 +48,9 @@ static int mvebu_get_temp(struct thermal_zone_device *thermal, return -EIO; } - /* - * Calculate temperature. See Section 8.10.1 of the 88AP510, - * datasheet, which has the same sensor. - * Documentation/arm/Marvell/README - */ reg = (reg >> KIRKWOOD_THERMAL_TEMP_OFFSET) & KIRKWOOD_THERMAL_TEMP_MASK; - *temp = ((2281638UL - (7298*reg)) / 10); + *temp = ((2363302UL - (7339*reg)) / 10); return 0; }
The temperature formula was taken from the Armada 510 datasheet. This is wrong because the Kirkwood SoC thermal sensor has different coefficients. Fix it using values from 88F682/88F683 Kirkwood datasheet. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- drivers/thermal/mvebu_thermal.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-)