Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp2163240yba; Fri, 17 May 2019 11:36:15 -0700 (PDT) X-Google-Smtp-Source: APXvYqxxIjkK34W0Dy/501wtNwU3lr9pKzH5Z7S7DEtos5wBXHyEB7XagbSzCweqtmss1Vv/HG+p X-Received: by 2002:a17:902:728d:: with SMTP id d13mr15953148pll.337.1558118175478; Fri, 17 May 2019 11:36:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1558118175; cv=none; d=google.com; s=arc-20160816; b=ercc5WR6Ho5Wy51BpZzdwiDp3HhFRukS8xnu6jzCNhsGaQ1e64on5BULF6ne4aP8ty IeSMHwVlyMMnQOp1gGuARLaLLPrz+qZVse7VW/jSbdTYCYFs//b+MRagws4XcVImQMlZ OClBRbja6f7+Iwtmknx1wHyd/zDSnKPu8lEFX2fHWF+7rJKqX2z6ipG5GXYsDVHytbd/ t2EhCig/xumklzf74lu2CuCpwjPArhO9FfmkqrQRyB4t1QsxQRsZ/wJfm0k1QpbRmHY4 4GCEFy9vClp+bCfKebGat0xz6vwqpHxtIOfP4czFlYE19tW/uwExxUJXYTgKQMI/abE8 Rj8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=8ICI9CdfR+Sv6JXgOHP/5oOIpKavCtZL50883X2efRw=; b=h4/tZ+/QzGowy3JcYaiQHT4xBSEi1tNlGolsHifMQxNqrSwv45JVVhCwkYONCALTJz rzSpbXqdYouGpnvbsAbbxbhhG9ZMUUey644OMKvT7/kEHbNwmtMe5U5CV9SUlG2z1ZBu pJFwdL2TeqdmEgN/vTHmOlg2EULBum+W+KynVGGIGzuprWy1dgmJoVJ9FcexnRnuEMxh Rv3yNwYE1ZQYBmVLjPop1irjgVsgMM/wtJ/z/AhdQsvxaUy8KLQ/BP9amtIfmeY8l/kG 9T72Qp6H7ztot3B+aM7zSwgKSWpMxKQQ3QboGx4/meW/MLVr4kWaH3R33ndz0XIRaSX1 21qQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=f4wV8lPN; 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=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q135si8350880pgq.429.2019.05.17.11.36.00; Fri, 17 May 2019 11:36:15 -0700 (PDT) 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=@gmail.com header.s=20161025 header.b=f4wV8lPN; 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=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728505AbfEQR1w (ORCPT + 99 others); Fri, 17 May 2019 13:27:52 -0400 Received: from mail-it1-f193.google.com ([209.85.166.193]:34241 "EHLO mail-it1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728467AbfEQR1w (ORCPT ); Fri, 17 May 2019 13:27:52 -0400 Received: by mail-it1-f193.google.com with SMTP id p18so11470262itm.1; Fri, 17 May 2019 10:27:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=8ICI9CdfR+Sv6JXgOHP/5oOIpKavCtZL50883X2efRw=; b=f4wV8lPNseh7Fo79RC+jiQD8g9H/sL1IxsS3OB4PaJnsQsdm4AR617fEQuI9BFkj6c f5aDiGoJg91SJxZIBKQ1Y0wsogLz9A8cNw/rYVB5DC6jFHe0bHj1v5oAMKjc1Sminwp6 Rr60ashoJRiDEFqwJU+zoFQzmhRClCnhuSlMLPzlfI3Jbb/TUGt8HTKHvjCSji4/upYd rcbmS3KKywLEEOgP2FT5o0lhatDAKBHN4Sse2rBjSEw6e9WQq6x4vX2TiQoL2kh7adl+ efVJKln3csKNlFGlAd1lcLSo+zzsAZZIsiuREN+1p9L/NeDOfG1piVexmrO7PrBkzVXU MNNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=8ICI9CdfR+Sv6JXgOHP/5oOIpKavCtZL50883X2efRw=; b=ueih0TehAj4x1xLeIemlreq3Oc/beglmEiG8PUHEiNX+17rF9/oeQdxjgYjpZiYW8Q ti01XAgwCJsBlo+o1Nu1p9u9kv40x5BRA/QF04S3AEjVRTZe/cpig245yov2s4QIUilg vHSJluoIOHO5rDu3Lm5559Fj6KAdx5yco3alsC6DCs54UfE8HgxuGQB7ReVYpeYDCMDz RJl93/7BHD2GkI66sBI/3o/WJQgQVHg0mAdOjPWe7bpeSRbMaXR9DYs1S6D8m2cKBOQ+ k5HImIE0zFRyDPUZYeFwafWeFUei/3o2jBwBjK+Wh3JqQIqCYdCBZ1efI7xs1f5w8r8P ++Pw== X-Gm-Message-State: APjAAAXDjHbJ2q/fxdsXFjG7ZCu/Nq5YRmYws3pfRFl819W6cmiN9ORu vWeW/pfvNQUAJpyMbVdhlqUfu7fEhilN2qZ7DTg= X-Received: by 2002:a02:a794:: with SMTP id e20mr36184091jaj.12.1558114070786; Fri, 17 May 2019 10:27:50 -0700 (PDT) MIME-Version: 1.0 References: <20190512082614.9045-1-tiny.windzz@gmail.com> <20190512082614.9045-3-tiny.windzz@gmail.com> <20190512133930.t5txssl7mou2gljt@flea> <20190517073634.izdmba3yqvxviyg3@flea> In-Reply-To: <20190517073634.izdmba3yqvxviyg3@flea> From: Frank Lee Date: Sat, 18 May 2019 01:27:39 +0800 Message-ID: Subject: Re: [PATCH 2/3] thermal: sun50i: add thermal driver for h6 To: Maxime Ripard Cc: rui.zhang@intel.com, Eduardo Valentin , Daniel Lezcano , robh+dt@kernel.org, Mark Rutland , Chen-Yu Tsai , catalin.marinas@arm.com, will.deacon@arm.com, David Miller , Mauro Carvalho Chehab , Greg Kroah-Hartman , Jonathan.Cameron@huawei.com, Nicolas Ferre , paulmck@linux.ibm.com, Andy Gross , olof@lixom.net, bjorn.andersson@linaro.org, Jagan Teki , marc.w.gonzalez@free.fr, stefan.wahren@i2se.com, enric.balletbo@collabora.com, Linux PM , devicetree@vger.kernel.org, Linux ARM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 17, 2019 at 3:36 PM Maxime Ripard w= rote: > > On Fri, May 17, 2019 at 01:51:56AM +0800, Frank Lee wrote: > > > > +struct sun50i_thermal_chip { > > > > + int sensor_num; > > > > + int offset; > > > > + int scale; > > > > + int ft_deviation; > > > > + int temp_calib_base; > > > > + int temp_data_base; > > > > + int (*enable)(struct tsens_device *tmdev); > > > > + int (*disable)(struct tsens_device *tmdev); > > > > +}; > > > > > > I'm not super fond of having a lot of quirks that are not needed. If > > > we ever need those quirks when adding support for a new SoC, then > > > yeah, we should totally have some, but only when and if it's needed. > > > > > > Otherwise, the driver is more complicated for no particular reason. > > > > This is unavoidable because of the difference in soc. > > I know, but this isn't my point. > > My point is that at this time of the driver development, we don't know > what is going to be needed to support all of those SoCs. > > Some of the parameters you added might not be needed, some parameters > might be missing, we don't know. So let's keep it simple for now. > > > > > +static int tsens_probe(struct platform_device *pdev) > > > > +{ > > > > + struct tsens_device *tmdev; > > > > + struct device *dev =3D &pdev->dev; > > > > + int ret; > > > > + > > > > + tmdev =3D devm_kzalloc(dev, sizeof(*tmdev), GFP_KERNEL); > > > > + if (!tmdev) > > > > + return -ENOMEM; > > > > + > > > > + tmdev->dev =3D dev; > > > > + tmdev->chip =3D of_device_get_match_data(&pdev->dev); > > > > + if (!tmdev->chip) > > > > + return -EINVAL; > > > > + > > > > + ret =3D tsens_init(tmdev); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + ret =3D tsens_register(tmdev); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + ret =3D tmdev->chip->enable(tmdev); > > > > + if (ret) > > > > + return ret; > > > > > > > > + platform_set_drvdata(pdev, tmdev); > > > > > > Your registration should be the very last thing you do. Otherwise, yo= u > > > have a small window where the get_temp callback can be called, but th= e > > > driver will not be functional yet. > > > > No. Anyway, ths data qcquisition is ms level. > > That's kind of irrelevant. There's nothing preventing get_temp to be > called right away. As Ond=C5=99ej said, Registration after enabling will lead to call tz update on non-registered t= z from an interrupt handler. > > > > > + ret =3D tsens_calibrate(tmdev); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + /* > > > > + * clkin =3D 24MHz > > > > + * T acquire =3D clkin / (SUN50I_THS_CTRL0_T_ACQ + 1) > > > > + * =3D 20us > > > > + */ > > > > + regmap_write(tmdev->regmap, SUN50I_THS_CTRL0, > > > > + SUN50I_THS_CTRL0_T_ACQ(479)); > > > > + /* average over 4 samples */ > > > > + regmap_write(tmdev->regmap, SUN50I_H6_THS_MFC, > > > > + SUN50I_THS_FILTER_EN | > > > > + SUN50I_THS_FILTER_TYPE(1)); > > > > + /* period =3D (SUN50I_H6_THS_PC_TEMP_PERIOD + 1) * 4096 / clk= in; ~10ms */ > > > > + regmap_write(tmdev->regmap, SUN50I_H6_THS_PC, > > > > + SUN50I_H6_THS_PC_TEMP_PERIOD(58)); > > > > + /* enable sensor */ > > > > + val =3D GENMASK(tmdev->chip->sensor_num - 1, 0); > > > > + regmap_write(tmdev->regmap, SUN50I_H6_THS_ENABLE, val); > > > > + > > > > + return 0; > > > > + > > > > +assert_reset: > > > > + reset_control_assert(tmdev->reset); > > > > + > > > > + return ret; > > > > > > Can't we do that with runtime_pm? > > > > Saving energy doesn't make much sense compared to system security. > > I'm not sure what you mean by security. Protect system hardware from damage. Thx, Yangtao > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com