Re: [PATCH 0/3] Thermal reset support in PMC

[Thierry Reding]

On Wed, Aug 13, 2014 at 01:41:52PM +0300, Mikko Perttunen wrote:
> 
> 
> On 13/08/14 13:36, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Wed, Aug 13, 2014 at 12:52:22PM +0300, Mikko Perttunen wrote:
> >>On 13/08/14 11:57, Thierry Reding wrote:
> >>>>Old Signed by an unknown key
> >>>
> >>>On Wed, Aug 13, 2014 at 11:42:53AM +0300, Mikko Perttunen wrote:
> >>>>
> >>>>
> >>>>On 13/08/14 11:12, Mikko Perttunen wrote:
> >>>>>On 13/08/14 11:07, Thierry Reding wrote:
> >>>>>>>Old Signed by an unknown key
> >>>>>>
> >>>>>>On Tue, Aug 05, 2014 at 11:12:57AM +0300, Mikko Perttunen wrote:
> >>>>>>>Hi,
> >>>>>>>
> >>>>>>>this series adds support for hardware-triggered thermal reset to the PMC
> >>>>>>>driver. Namely, it adds device tree properties for specifying the I2C
> >>>>>>>command to be sent when thermtrip is triggered. It is to be noted
> >>>>>>>that thermtrip won't be ever triggered without a soctherm driver to
> >>>>>>>calibrate the sensors, but I'll follow up with that patch.
> >>>>>>>
> >>>>>>>pmc.c required some juggling around to make the match data usable in
> >>>>>>>probe, since I didn't want to put the code into the initcall either, since
> >>>>>>>the soctherm driver won't be initialized by that point anyway.
> >>>>>>>
> >>>>>>>Series tested on Jetson-TK1. Should work on Tegra30 and Tegra114 too.
> >>>>>>
> >>>>>>Can you describe the procedure used to test this? We currently have a
> >>>>>>bunch of features in Tegra that some people have tested at some point
> >>>>>>during development but the test procedures never got documented. That
> >>>>>>means whenever we want to test something we need to go and reinvent a
> >>>>>>bunch of tests after the fact.
> >>>>>>
> >>>>>>So what I'd like to start doing is collect tests (preferably in some
> >>>>>>scripted way) so that they can be kept in a repository that people can
> >>>>>>easily clone and run on devices.
> >>>>>>
> >>>>>>Could you provide something like that for thermtrip?
> >>>>>
> >>>>>Sure. I'll see if I can make a just a test script or if a local patch is
> >>>>>needed to test. Btw, I also have a pretty nice test script for EMC
> >>>>>ready, and I agree that such a repository would be very nice.
> >>>>
> >>>>Here is a test program. It it works, the device with immediately shut down.
> >>>>
> >>>>https://gist.github.com/cyndis/66126c9c176b5f94a76f
> >>>
> >>>Is there a way to set the trip temperature without going through
> >>>/dev/mem? I'd expect the device to have a sysfs interface of some
> >>>sort.
> >>
> >>The thermtrip "device" isn't currently exposed in any way. If it were
> >>exposed, I suppose it would be exposed as thermal zone devices, each with
> >>one trip point. Even then, the thermal framework doesn't really support this
> >>properly; none of the trip point types really apply to this kind of trip
> >>point, and x86 systems don't expose their trips either.
> >
> >Okay. The reason why I asked is because I'm not sure yet that having C
> >programs in the test suite would be good, so having something that's
> >easily scriptable would be preferred.
> 
> I agree. Looks like Python's stdlib has mmap support, so maybe we could use
> that.

Yes, possibly. It still means that somebody needs to have some kind of
advanced scripting available to test this. While having a testsuite is
useful, it would still be nice to occasionally be able to test things
with just simple shell commands.

> >>Anyway, since debugging is pretty much the only use case for modifying
> >>the trip temperature, I thought adding the tz_devices would be a bit
> >>overkill.
> >
> >An alternative could be a file in debugfs.
> 
> True, though there's the "don't expose dangerous stuff in debugfs" argument
> against that. I don't really have problem with it, though.
> Although in this case I think the script approach would be good enough.

I've said this before and I don't think "dangerous stuff in debugfs" is
a valid argument. You need superuser privileges to access debugfs, and
if you have such privileges you can do pretty much what you want (run
the test program you posted for example).

Thierry

Attachment: pgp9r3S7yEzQG.pgp
Description: PGP signature