Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Tue, 28 Jun 2022 11:43:32 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc:     Daniel Lezcano <daniel.lezcano@linaro.org>,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        Amit Kucheria <amitk@kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Thierry Reding <thierry.reding@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Dmitry Osipenko <digetx@gmail.com>,
        "open list:TEGRA ARCHITECTURE SUPPORT" <linux-tegra@vger.kernel.org>,
        Hardware Monitoring <linux-hwmon@vger.kernel.org>,
        rafael@kernel.org
Subject: Re: [PATCH 2/3] thermal/drivers/tegra: Remove get_trend function
Message-ID: <20220628184332.GA3624671@roeck-us.net>
References: <20220616202537.303655-1-daniel.lezcano@linaro.org>
 <20220616202537.303655-2-daniel.lezcano@linaro.org>
 <7841a809-e180-70d2-df9b-b30b411647ce@linaro.org>
 <d186bb7d-cbe6-8ec4-82a1-8323b3901ac2@collabora.com>
 <20220628151030.GA3361452@roeck-us.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220628151030.GA3361452@roeck-us.net>
Precedence: bulk

On Tue, Jun 28, 2022 at 08:10:30AM -0700, Guenter Roeck wrote:
> On Tue, Jun 28, 2022 at 02:44:31PM +0300, Dmitry Osipenko wrote:
> > On 6/28/22 11:41, Daniel Lezcano wrote:
> > > 
> > > Thierry, Dmitry,
> > > 
> > > are fine with this patch?
> > 
> > Seems should be good. I couldn't test it using recent the linux-next
> > because of a lockup in LM90 driver. There were quite a lot of changes in
> > LM90 recently, adding Guenter.
> > 
> 
> Weird, I tested those changes to death with real hardware, and I don't
> see a code path where the mutex can be left in blocked state unless the
> underlying i2c driver locks up for some reason. What is the platform,
> and can you point me to the devicetree file ? Also, is there anything
> else lm90 or i2c related in the kernel log ?
> 

Follow-up question: I see that various Tegra systems use lm90 compatible
chips, and the interrupt line is in general wired up. Can you check if
you get lots of interrupts on that interrupt line ? Also, can you check
what happens if you read hwmon attributes directly ?

Thanks,
Guenter

> Thanks,
> Guenter
> 
> > INFO: task kworker/3:1:44 blocked for more than 61 seconds.
> >       Not tainted 5.19.0-rc4-next-20220627-00012-g08b697b94b8a #2
> > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > task:kworker/3:1     state:D stack:    0 pid:   44 ppid:     2
> > flags:0x00000000
> > Workqueue: events_freezable_power_ thermal_zone_device_check
> > Backtrace:
> >  __schedule from schedule+0x60/0xcc
> >  r10:c0fead70 r9:c2854c94 r8:df9a1dac r7:c2814b40 r6:00000002 r5:c1883020
> >  r4:c2814b40
> >  schedule from schedule_preempt_disabled+0x28/0x38
> >  r5:c1883020 r4:c2814b40
> >  schedule_preempt_disabled from __mutex_lock.constprop.0+0x1e0/0x9ac
> >  r5:c1883020 r4:c2854c90
> >  __mutex_lock.constprop.0 from __mutex_lock_slowpath+0x1c/0x20
> >  r10:00000000 r9:c1882ae0 r8:c2854c90 r7:c2854c40 r6:00000001 r5:00000001
> >  r4:c2854c90
> >  __mutex_lock_slowpath from mutex_lock+0x60/0x64
> >  mutex_lock from lm90_read+0x40/0x3d4
> >  r5:00000001 r4:c2854e08
> >  lm90_read from hwmon_thermal_get_temp+0x58/0x8c
> >  r9:c1882ae0 r8:c2814b40 r7:de6aee00 r6:c1db1660 r5:c0af7940 r4:df9a1eb8
> >  hwmon_thermal_get_temp from of_thermal_get_temp+0x38/0x44
> >  r5:df9a1eb8 r4:c1db1400
> >  of_thermal_get_temp from thermal_zone_get_temp+0x58/0x78
> >  thermal_zone_get_temp from thermal_zone_device_update.part.0+0x4c/0x450
> >  r7:de6aee00 r6:c1db1400 r5:00000000 r4:c1db1400
> >  thermal_zone_device_update.part.0 from thermal_zone_device_check+0x58/0x5c
> >  r10:00000000 r9:c1882ae0 r8:c2814b40 r7:de6aee00 r6:c1db1400 r5:c1db1660
> >  r4:00000001
> >  thermal_zone_device_check from process_one_work+0x21c/0x530
> >  r7:de6aee00 r6:de6ab600 r5:c2802c00 r4:c1db167c
> >  process_one_work from worker_thread+0x19c/0x5cc
> >  r10:00000008 r9:c2814b40 r8:c1703d40 r7:de6ab61c r6:c2802c18 r5:de6ab600
> >  r4:c2802c00
> >  worker_thread from kthread+0x100/0x120
> >  r10:00000000 r9:df895e80 r8:c285e3c0 r7:c2802c00 r6:c014cf84 r5:c285e300
> >  r4:c2814b40
> >  kthread from ret_from_fork+0x14/0x2c
> > Exception stack(0xdf9a1fb0 to 0xdf9a1ff8)
> > 
> > > On 16/06/2022 22:25, Daniel Lezcano wrote:
> > >> The get_trend function does already what the generic framework does.
> > >>
> > >> Remove it.
> > >>
> > >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > >> ---
> > >> ? drivers/thermal/tegra/soctherm.c | 32 --------------------------------
> > >> ? 1 file changed, 32 deletions(-)
> > >>
> > >> diff --git a/drivers/thermal/tegra/soctherm.c
> > >> b/drivers/thermal/tegra/soctherm.c
> > >> index 210325f92559..825eab526619 100644
> > >> --- a/drivers/thermal/tegra/soctherm.c
> > >> +++ b/drivers/thermal/tegra/soctherm.c
> > >> @@ -633,37 +633,6 @@ static int tegra_thermctl_set_trip_temp(void
> > >> *data, int trip, int temp)
> > >> ????? return 0;
> > >> ? }
> > >> ? -static int tegra_thermctl_get_trend(void *data, int trip,
> > >> -??????????????????? enum thermal_trend *trend)
> > >> -{
> > >> -??? struct tegra_thermctl_zone *zone = data;
> > >> -??? struct thermal_zone_device *tz = zone->tz;
> > >> -??? int trip_temp, temp, last_temp, ret;
> > >> -
> > >> -??? if (!tz)
> > >> -??????? return -EINVAL;
> > >> -
> > >> -??? ret = tz->ops->get_trip_temp(zone->tz, trip, &trip_temp);
> > >> -??? if (ret)
> > >> -??????? return ret;
> > >> -
> > >> -??? temp = READ_ONCE(tz->temperature);
> > >> -??? last_temp = READ_ONCE(tz->last_temperature);
> > >> -
> > >> -??? if (temp > trip_temp) {
> > >> -??????? if (temp >= last_temp)
> > >> -??????????? *trend = THERMAL_TREND_RAISING;
> > >> -??????? else
> > >> -??????????? *trend = THERMAL_TREND_STABLE;
> > >> -??? } else if (temp < trip_temp) {
> > >> -??????? *trend = THERMAL_TREND_DROPPING;
> > >> -??? } else {
> > >> -??????? *trend = THERMAL_TREND_STABLE;
> > >> -??? }
> > >> -
> > >> -??? return 0;
> > >> -}
> > >> -
> > >> ? static void thermal_irq_enable(struct tegra_thermctl_zone *zn)
> > >> ? {
> > >> ????? u32 r;
> > >> @@ -716,7 +685,6 @@ static int tegra_thermctl_set_trips(void *data,
> > >> int lo, int hi)
> > >> ? static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
> > >> ????? .get_temp = tegra_thermctl_get_temp,
> > >> ????? .set_trip_temp = tegra_thermctl_set_trip_temp,
> > >> -??? .get_trend = tegra_thermctl_get_trend,
> > >> ????? .set_trips = tegra_thermctl_set_trips,
> > >> ? };
> > >> ? 
> > > 
> > > 
> >