Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp2077124imu; Wed, 28 Nov 2018 21:57:48 -0800 (PST) X-Google-Smtp-Source: AFSGD/WpyrBJQtap9mSZJMCA0IofPjtk0vXQ9u3iWivs3xiqWWRHGQv6VceQHDF4MqfYhSCQRinH X-Received: by 2002:a17:902:6909:: with SMTP id j9mr191408plk.196.1543471068908; Wed, 28 Nov 2018 21:57:48 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1543471068; cv=none; d=google.com; s=arc-20160816; b=m/LJXmIPcBzdKHb7gXo7mNnW7T66toDlUOcL08QnFL2YvchJ+WTyI4iLW1ZsohE1+u ixLX9qg/Wx1XvKj+/9WRiHopStv1h78ghxylMPYoHrPduRhcMTxDsPjvchdL5iDuou98 rX/dMO6YwJ8e7IGZ6kyt/R3aNSEMJEERWEHXzkOKpU6P9UPSSz7klxSK2j79oIKolq+y HSaR+9lHh4ZMlAssEyxVyI2ukk1aYsLs+CQ++TxL2w0w2hmesmCkLcpIYXNXDGjqtCOn nP59yLzP+Xo8yXGUOsrJrIdwn5JMhVAX7RwxPECOyrrI9khM4sehs9Ke4BOzlRmBIbzy MiAg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:dkim-signature:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=NrGxoq3Zscu4vNW7g2jr9T0rBHCD/ijwUD9IK5VnHgE=; b=jdxubXlzbYw6wnMonwc/sU1XjPzGhU8kpUNdVvXRh8+VEO85xI+MgDJ8O0ArlVz9ua QKZ4YtlN5HsdSNQfcDojTiOuHUZiyBU+/waiqYQjwNp5iFHwHGQy4USWiVdpPjLr04nK cZnOt79tiylI3aApmHApaDMhXf13kItWEx/1rH8PiR802OsH5vkX9Jj1HGE/xAzp/HrU 5CbJ3/Ho4KNeaM9dJVEghdXVwxqS3rf4mXyTxJwnz7zvOEQQD96L1qU5ohIQiYTsLg/C FflOrDopLqPiFSn0fXekjhdI6oroRohW1aEG1NIM1uHxi4r3gvaFha0xq0fpWoedaqM4 YasA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=Igh5O5xF; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o12si1021836pgn.145.2018.11.28.21.57.34; Wed, 28 Nov 2018 21:57:48 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=Igh5O5xF; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727864AbeK2Q7S (ORCPT + 99 others); Thu, 29 Nov 2018 11:59:18 -0500 Received: from hqemgate15.nvidia.com ([216.228.121.64]:2245 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727406AbeK2Q7S (ORCPT ); Thu, 29 Nov 2018 11:59:18 -0500 Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqemgate15.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Wed, 28 Nov 2018 21:54:24 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Wed, 28 Nov 2018 21:55:07 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Wed, 28 Nov 2018 21:55:07 -0800 Received: from [10.19.225.182] (172.20.13.39) by HQMAIL101.nvidia.com (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Thu, 29 Nov 2018 05:55:04 +0000 Subject: Re: [PATCH v3 3/3] thermal: tegra: parse sensor id before sensor register To: Thierry Reding CC: , , , , , References: <1543383877-20555-1-git-send-email-wni@nvidia.com> <1543383877-20555-4-git-send-email-wni@nvidia.com> <20181128102520.GC20723@ulmo> From: Wei Ni Message-ID: <9c373693-1d3c-cd02-4972-dde338651c0e@nvidia.com> Date: Thu, 29 Nov 2018 13:55:02 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181128102520.GC20723@ulmo> X-Originating-IP: [172.20.13.39] X-ClientProxiedBy: HQMAIL103.nvidia.com (172.20.187.11) To HQMAIL101.nvidia.com (172.20.187.10) Content-Type: text/plain; charset="windows-1252" Content-Language: en-US Content-Transfer-Encoding: 7bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1543470864; bh=NrGxoq3Zscu4vNW7g2jr9T0rBHCD/ijwUD9IK5VnHgE=; h=X-PGP-Universal:Subject:To:CC:References:From:Message-ID:Date: User-Agent:MIME-Version:In-Reply-To:X-Originating-IP: X-ClientProxiedBy:Content-Type:Content-Language: Content-Transfer-Encoding; b=Igh5O5xFlCGVin3qyePCrnCXSD748wlMH8mHaVqq7TJp9DkoKDJ2WkyiJlK5hzEkg tYihd6NH5UQni2VIQmW66zYFhjAZ7dvtEINBiKsV5guAtbjfV/nzCHunRkzS1loK5K fxXKWD+aZZduHynYYjhi39HKF773/3T7aUvMAj4c2yR9Nm6Lbvlxalttu57iInO/0C m1+XGxVF01mKLzZGqvYRfrSC1cuuOt4tx6z+zfcpv5zDbZILq8tKcoA9YtPTDxAWpW bLq6uNMq4DznauTuBupn4Fe7fvjyknOK0QrWSUN9UH+rPmlNa4V6YLgo1w7X45sIzQ ecjvV9R4kFjvg== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 28/11/2018 6:25 PM, Thierry Reding wrote: > On Wed, Nov 28, 2018 at 01:44:37PM +0800, Wei Ni wrote: >> Since different platforms may not support all 4 >> sensors, so the sensor registration may be failed. >> Add codes to parse dt to find sensor id which >> need to be registered. So that the registration >> can be successful on all platform. >> >> Signed-off-by: Wei Ni >> --- >> drivers/thermal/tegra/soctherm.c | 46 ++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 44 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c >> index 375cadbc24cd..79e4628224d7 100644 >> --- a/drivers/thermal/tegra/soctherm.c >> +++ b/drivers/thermal/tegra/soctherm.c >> @@ -1224,6 +1224,44 @@ static void soctherm_init(struct platform_device *pdev) >> tegra_soctherm_throttle(&pdev->dev); >> } >> >> +static bool tegra_soctherm_find_sensor_id(int sensor_id) >> +{ >> + int id; > > You might want to make this and the sensor_id parameter unsigned int to > match the signedness of the ID in the specifier arguments and the sensor > groups. Ok, will change it. > > Thierry > >> + bool ret = false; >> + struct of_phandle_args sensor_specs; >> + struct device_node *np, *sensor_np; >> + >> + np = of_find_node_by_name(NULL, "thermal-zones"); >> + if (!np) >> + return ret; >> + >> + sensor_np = of_get_next_child(np, NULL); >> + for_each_available_child_of_node(np, sensor_np) { > > Aren't we leaking np here? I think we need of_node_put() after > of_get_next_child() to make sure the reference to the "thermal-zones" > node is properly released. No, we will not leak np here. Because the for_each_available_child_of_node will call of_get_next_available_child(), which will call of_node_put(prev) to decrease refcount of the prev node. So we just need to of_node_put the last node after break from this for block. > >> + if (of_parse_phandle_with_args(sensor_np, "thermal-sensors", >> + "#thermal-sensor-cells", >> + 0, &sensor_specs)) >> + continue; >> + >> + if (sensor_specs.args_count != 1) { >> + WARN(sensor_specs.args_count > 1, >> + "%s: wrong cells in sensor specifier %d\n", >> + sensor_specs.np->name, sensor_specs.args_count); >> + continue; > > This is odd. You check for args_count != 1 but then WARN on args_count > > 1. Shouldn't both of these conditions be the same? Sorry, it's my mistake, will fix it. > >> + } else { > > Also, since the above has "continue;", we don't really need the else > block. Will fix it. > >> + id = sensor_specs.args[0]; >> + if (sensor_id == id) { > > It might not be worth to store the ID in a separate variable, we could > just do: > > if (sensor_specs.args[0] == sensor_id) > > Thierry Sure, will fix it. >> + ret = true; >> + break; >> + } >> + } >> + } >> + >> + of_node_put(np); We decrease refcount of the last np. >> + of_node_put(sensor_np); >> + >> + return ret; >> +} >> + >> static const struct of_device_id tegra_soctherm_of_match[] = { >> #ifdef CONFIG_ARCH_TEGRA_124_SOC >> { >> @@ -1365,13 +1403,15 @@ static int tegra_soctherm_probe(struct platform_device *pdev) >> zone->sg = soc->ttgs[i]; >> zone->ts = tegra; >> >> + if (!tegra_soctherm_find_sensor_id(soc->ttgs[i]->id)) >> + continue; >> z = devm_thermal_zone_of_sensor_register(&pdev->dev, > > I'd would prefer a blank line after the if block for readability. Sure, will update it. > >> soc->ttgs[i]->id, zone, >> &tegra_of_thermal_ops); >> if (IS_ERR(z)) { >> err = PTR_ERR(z); >> - dev_err(&pdev->dev, "failed to register sensor: %d\n", >> - err); >> + dev_err(&pdev->dev, "failed to register sensor %s: %d\n", >> + soc->ttgs[i]->name, err); >> goto disable_clocks; >> } >> >> @@ -1434,6 +1474,8 @@ static int __maybe_unused soctherm_resume(struct device *dev) >> struct thermal_zone_device *tz; >> >> tz = tegra->thermctl_tzs[soc->ttgs[i]->id]; >> + if (!tz) >> + continue; >> err = tegra_soctherm_set_hwtrips(dev, soc->ttgs[i], tz); > > Same here: > > if (!tz) > continue; > > err = ... > > Thierry Will update it. >