From patchwork Fri Jul 21 13:21:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Masahiro Yamada X-Patchwork-Id: 9856931 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id F0B4860388 for ; Fri, 21 Jul 2017 13:22:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D8D5128781 for ; Fri, 21 Jul 2017 13:22:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CDB2328786; Fri, 21 Jul 2017 13:22:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.4 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4189B28782 for ; Fri, 21 Jul 2017 13:22:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: References:In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=8G8YtwH522d1+3+dWKfMldQKwpMhmePdX7/iup7/im8=; b=u7p/iTD4gGPcBR LjnAPChgY67YSDANeKQJG1EyApBw9nCxPcnvPnWR4w+3S6JCdpHx4zqiG7aBAAfaxCmUxtIWkIBux Wredoag5/pyrv5v8b+bxhIVJJyMGVti3y8bpcRoX8zyeTZZ1Ckyvcy4IhL977+Q4vfiMpyZ0ISnLB 4+sDrT7N6t/NgQaKs3wuzLti7f2dCXvbSocJUSMkS7WmgIksR7ybYY/4kjEnIJURTVvfiSIA3UB3R m5O4IZchvrToZi043Tgxial7iuysqm2Py4uLc6+IFb4m3eT56FxWf5HEywAv4jJ2bssKFMXBkHJ1H HPxF8AhmxBSUcGVIjMBA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dYXsW-0002N0-VV; Fri, 21 Jul 2017 13:22:01 +0000 Received: from conssluserg-05.nifty.com ([210.131.2.90]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dYXsO-0002JI-NH for linux-arm-kernel@lists.infradead.org; Fri, 21 Jul 2017 13:21:59 +0000 Received: from mail-yw0-f171.google.com (mail-yw0-f171.google.com [209.85.161.171]) (authenticated) by conssluserg-05.nifty.com with ESMTP id v6LDL7dS028424 for ; Fri, 21 Jul 2017 22:21:07 +0900 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-05.nifty.com v6LDL7dS028424 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nifty.com; s=dec2015msa; t=1500643268; bh=srP4CLo8IXhVWf5Z+N0t0c3pgKrcOMToicSPkYkHzkc=; h=In-Reply-To:References:From:Date:Subject:To:Cc:From; b=ubXYTOc3+Gr5X1Wz9JF8O0TjfzeHzGLQrbpskvxjbQVEW/DDtQA8ku7JfWCtu+dMv IuDGY69Pp0CHMyEc5ShVupBFPPj9LYJfvoW7ZbQQnUKnjGXdz8Qt0WXxo+dCv1Ouva q464tTRf58rTzqpbCGn3VibaHKMnsOJXLTig2s//7DZXVEaSek5hT4NOFWSAb7eZee fjBTE+2l7VRAOC1Ce81FjJunfse1HT7GtqsMWsKin1PUxTfUyXtS0SmPazruky8PfX yaQ7PHYu/iDSeO1OhSV39myd94xeDYoprSLZHLr2CdSO/93mNjVrZgVbN2YfG7hJ4w jYvyOCW/fV7pQ== X-Nifty-SrcIP: [209.85.161.171] Received: by mail-yw0-f171.google.com with SMTP id a12so24277133ywh.3 for ; Fri, 21 Jul 2017 06:21:07 -0700 (PDT) X-Gm-Message-State: AIVw111qI1QiVYlLdeNSKwxvMCRA8VVEYh9OFkfs+t0uOv+uKbStUGvF 6fQGWKCi0pO9MEyOW+XQ2EMUEWEXkA== X-Received: by 10.129.189.72 with SMTP id n8mr6235550ywk.48.1500643266519; Fri, 21 Jul 2017 06:21:06 -0700 (PDT) MIME-Version: 1.0 Received: by 10.37.49.9 with HTTP; Fri, 21 Jul 2017 06:21:05 -0700 (PDT) In-Reply-To: <1500636105-11520-3-git-send-email-hayashi.kunihiko@socionext.com> References: <1500636105-11520-1-git-send-email-hayashi.kunihiko@socionext.com> <1500636105-11520-3-git-send-email-hayashi.kunihiko@socionext.com> From: Masahiro Yamada Date: Fri, 21 Jul 2017 22:21:05 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v5 2/2] thermal: uniphier: add UniPhier thermal driver To: Kunihiko Hayashi X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170721_062153_312986_9399DA1D X-CRM114-Status: GOOD ( 18.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Masami Hiramatsu , linux-pm@vger.kernel.org, Linux Kernel Mailing List , Eduardo Valentin , Jassi Brar , Rob Herring , rui.zhang@intel.com, linux-arm-kernel Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP 2017-07-21 20:21 GMT+09:00 Kunihiko Hayashi : > +static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev) > +{ > + struct regmap *map = tdev->regmap; > + u32 val; > + int ret; > + > + /* stop PVT */ > + regmap_write_bits(map, tdev->data->block_base + PVTCTLEN, > + PVTCTLEN_EN, 0); > + > + /* > + * set default value if missing calibrated value > + * > + * Since SoC has a calibrated value that was set in advance, > + * TMODCOEF shows non-zero and PVT refers the value internally. > + * > + * However, some boards don't have the calibrated value. > + * In that case, TMODCOEF shows zero and the driver has to set > + * default value manually. > + */ > + ret = regmap_read(map, tdev->data->map_base + TMODCOEF, &val); > + if (ret) > + return ret; > + if (!val) > + regmap_write(map, tdev->data->tmod_setup_addr, > + TMODSETUP0_EN | TMODSETUP0_VAL(tdev->tmod_calib0) | > + TMODSETUP1_EN | TMODSETUP1_VAL(tdev->tmod_calib1)); This code is strange. What if TMODCOEF has no calibrated value and "socionext,tmod-calibration" is not set either? ->tmod_setupaddr will be set to zero and the sensor would not work, right? > + /* get tmod-calibration values */ > + calib = of_get_property(dev->of_node, "socionext,tmod-calibration", > + NULL); > + if (calib) { > + tdev->tmod_calib0 = of_read_number(calib, 1); > + tdev->tmod_calib1 = of_read_number(calib + 1, 1); > + } From your DT change (https://patchwork.kernel.org/patch/9826391/), this property seems a pair of u32 values, like follows: socionext,tmod-calibration = <0x0f22 0x68ee>; Why do you need to use of_read_number() to retrieve each u32 value? See the comment and return value (u64): ------------------>8----------------- /* Helper to read a big number; size is in cells (not bytes) */ static inline u64 of_read_number(const __be32 *cell, int size) ------------------8<----------------- Also, you are not checking the length of the property. of_read_number() may over-run the property. of_property_read_u32_array() will be a better choice. I'd propose the code like follows: /* select temperature mode */ regmap_write_bits(map, tdev->data->block_base + PVTCTLMODE, @@ -254,7 +266,6 @@ static int uniphier_tm_probe(struct platform_device *pdev) struct device_node *parent; struct uniphier_tm_dev *tdev; const struct thermal_trip *trips; - const __be32 *calib; int i, ret, irq, ntrips, crit_temp = INT_MAX; tdev = devm_kzalloc(dev, sizeof(*tdev), GFP_KERNEL); @@ -269,14 +280,6 @@ static int uniphier_tm_probe(struct platform_device *pdev) if (irq < 0) return irq; - /* get tmod-calibration values */ - calib = of_get_property(dev->of_node, "socionext,tmod-calibration", - NULL); - if (calib) { - tdev->tmod_calib0 = of_read_number(calib, 1); - tdev->tmod_calib1 = of_read_number(calib + 1, 1); - } - /* get regmap from syscon node */ parent = of_get_parent(dev->of_node); /* parent should be syscon node */ regmap = syscon_node_to_regmap(parent); @@ -288,7 +291,7 @@ static int uniphier_tm_probe(struct platform_device *pdev) } tdev->regmap = regmap; - ret = uniphier_tm_initialize_sensor(tdev); + ret = uniphier_tm_initialize_sensor(tdev, dev); if (ret) { dev_err(dev, "failed to initialize sensor\n"); return ret; diff --git a/drivers/thermal/uniphier_thermal.c b/drivers/thermal/uniphier_thermal.c index 1a77c5bf6930..eea8a3053584 100644 --- a/drivers/thermal/uniphier_thermal.c +++ b/drivers/thermal/uniphier_thermal.c @@ -94,16 +94,16 @@ struct uniphier_tm_soc_data { struct uniphier_tm_dev { struct regmap *regmap; bool alert_en[ALERT_CH_NUM]; - u32 tmod_calib0; - u32 tmod_calib1; struct thermal_zone_device *tz_dev; const struct uniphier_tm_soc_data *data; }; -static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev) +static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev, + struct device *dev) { struct regmap *map = tdev->regmap; u32 val; + u32 tmod_val[2]; int ret; /* stop PVT */ @@ -123,10 +123,22 @@ static int uniphier_tm_initialize_sensor(struct uniphier_tm_dev *tdev) ret = regmap_read(map, tdev->data->map_base + TMODCOEF, &val); if (ret) return ret; - if (!val) + if (!val) { + /* + * No preset value found in the register. + * Look for the fall-back values in DT. + */ + ret = of_property_read_u32_array(dev->of_node, + "socionext,tmod-calibration", + tmod_val, ARRAY_SIZE(tmod_val)); + if (ret) { + dev_err(dev, "neither register nor DT has calibrated values.\n"); + return ret; + } regmap_write(map, tdev->data->tmod_setup_addr, - TMODSETUP0_EN | TMODSETUP0_VAL(tdev->tmod_calib0) | - TMODSETUP1_EN | TMODSETUP1_VAL(tdev->tmod_calib1)); + TMODSETUP0_EN | TMODSETUP0_VAL(tmod_val[0]) | + TMODSETUP1_EN | TMODSETUP1_VAL(tmod_val[1])); + }