linux-kernel - Re: [PATCH 2/3] thermal: sun50i: add thermal driver for h6

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190521074122.syyctwvfsorl45dv@flea>
Date:   Tue, 21 May 2019 09:41:22 +0200
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Frank Lee <tiny.windzz@...il.com>
Cc:     Mark Rutland <mark.rutland@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        catalin.marinas@....com, will.deacon@....com,
        bjorn.andersson@...aro.org,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        paulmck@...ux.ibm.com, stefan.wahren@...e.com,
        Linux PM <linux-pm@...r.kernel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jagan Teki <jagan@...rulasolutions.com>,
        Andy Gross <andy.gross@...aro.org>, rui.zhang@...el.com,
        devicetree@...r.kernel.org, marc.w.gonzalez@...e.fr,
        Eduardo Valentin <edubezval@...il.com>,
        enric.balletbo@...labora.com, robh+dt@...nel.org,
        Jonathan.Cameron@...wei.com,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        olof@...om.net, David Miller <davem@...emloft.net>
Subject: Re: [PATCH 2/3] thermal: sun50i: add thermal driver for h6

Hi,

On Sat, May 18, 2019 at 01:19:54AM +0800, Frank Lee wrote:
> On Fri, May 17, 2019 at 3:32 PM Maxime Ripard <maxime.ripard@...tlin.com> wrote:
> >
> > On Fri, May 17, 2019 at 02:10:47AM +0800, Frank Lee wrote:
> > > > On Sun, May 12, 2019 at 11:41:28PM +0200, Ondřej Jirman wrote:
> > > > > > > +static int tsens_get_temp(void *data, int *temp)
> > > > > > > +{
> > > > > > > + struct tsensor *s = data;
> > > > > > > + struct tsens_device *tmdev = s->tmdev;
> > > > > > > + int val;
> > > > > > > +
> > > > > > > + regmap_read(tmdev->regmap, tmdev->chip->temp_data_base +
> > > > > > > +             0x4 * s->id, &val);
> > > > > > > +
> > > > > > > + if (unlikely(val == 0))
> > > > > > > +         return -EBUSY;
> > > > > >
> > > > > > I'm not sure why a val equals to 0 would be associated with EBUSY?
> > > > >
> > > > > Thermal zone driver can (will) call get_temp before we got the
> > > > > first interrupt and the thermal data. In that case val will be 0.
> > > > >
> > > > > Resulting in:
> > > > >
> > > > >  (val + offset) * scale = (-2794) * -67 = 187198
> > > > >
> > > > > 187°C and immediate shutdown during boot - based on cirtical
> > > > > temperature being reached.
> > > > >
> > > > > Busy here means, get_temp does not yet have data. Thermal zone
> > > > > driver just reports any error to dmesg output.
> > > >
> > > > Ah, that makes sense.
> > > >
> > > > I guess if we're switching to an interrupt-based driver, then we can
> > > > just use a waitqueue, or is get_temp supposed to be atomic?
> > >
> > > I think get_temp should not be bloacked.
> >
> > Why not?
>
> Maybe, I am wrong. I also want to know if we should do this.

I guess it really all depends on whether you can sleep or not in
get_temps. If you can, then you should wait for the value to be
converted and the THS raising an interrupt.

If you can't, then we should ask what the thermal frameworks expects
in such a case.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)