From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F430C38A02 for ; Fri, 28 Oct 2022 16:17:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230376AbiJ1QRO (ORCPT ); Fri, 28 Oct 2022 12:17:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59274 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230367AbiJ1QQi (ORCPT ); Fri, 28 Oct 2022 12:16:38 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 9943F10B8; Fri, 28 Oct 2022 09:15:44 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A53FA1FB; Fri, 28 Oct 2022 09:15:50 -0700 (PDT) Received: from e120937-lin (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 706363F534; Fri, 28 Oct 2022 09:15:43 -0700 (PDT) Date: Fri, 28 Oct 2022 17:15:41 +0100 From: Cristian Marussi To: Guenter Roeck Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, sudeep.holla@arm.com, Daniel Lezcano , linux-hwmon@vger.kernel.org Subject: Re: [PATCH 7/8] hwmon: (scmi) Register explicitly with Thermal Framework Message-ID: References: <20221028140833.280091-1-cristian.marussi@arm.com> <20221028140833.280091-7-cristian.marussi@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org On Fri, Oct 28, 2022 at 08:58:58AM -0700, Guenter Roeck wrote: > On 10/28/22 08:35, Cristian Marussi wrote: > [ ... ] > > > > + /* > > > > + * Try to register a temperature sensor with the Thermal Framework: > > > > + * skip sensors not defined as part of any thermal zone (-ENODEV) but > > > > + * report any other errors related to misconfigured zones/sensors. > > > > + */ > > > > + tzd = devm_thermal_of_zone_register(dev, th_sensor->info->id, th_sensor, > > > > + &scmi_hwmon_thermal_ops); > > > > + if (IS_ERR(tzd)) { > > > > + devm_kfree(dev, th_sensor); > > > > + > > > > + if (PTR_ERR(tzd) != -ENODEV) > > > > + return PTR_ERR(tzd); > > > > + > > > > + dev_info(dev, "Sensor '%s' not attached to any thermal zone.\n", > > > > + sensor->name); > > > > > > There were complaints about this message as it is noisy. If you send > > > another version, please drop it unless attaching each sensor to a thermal > > > zone is strongly expected. If you don't send another version, I'll drop it > > > while applying. > > > > > > > Ok fine for me. I am waiting to have some feedback from Sudeep too, but > > I do not have plan for another version as of now. > > > > As a side note, though, I understand the 'noisiness' argument, but, > > sincerely this same message in the original HWMON code was the only > > reason why I spotted that something was wrong with the SCMI/HWMON > > interactions and discovered the indexes/ids mismatch...if not for > > that it would have gone un-noticed that a perfectly configured > > ThermalZone/Sensor was not working properly... > > (un-noticed at least until something would have been burnt to fire > > in my house .. joking :P) > > > > Good point. > > Did you ever check the returned error code ? Maybe we could use it to > distinguish "it is not attached to a thermal zone because it is not > associated with one" from "attaching to a thermal zone failed because > its configuration is bad/incomplete". > Yes, it is what I do already indeed, in this regards I mimicked what the hwmon-thermal bridge was doing. In scmi_thermal_sensor_register() this message is printed out only if Thermal registration returned -ENODEV and no err is reported (which means teh specified sensor was not found attached to any TZ), while in the caller of scmi_thermal_sensor_register() for any error returned but -ENOMEM I print: "Thermal zone misconfigured for %s. err=%d\n", since any error reported by Thermal other than ENODEV and ENOMEM means the DT parsing unveiled some configuration anomaly. Thanks, Cristian