Received: by 2002:a25:86ce:0:0:0:0:0 with SMTP id y14csp1229736ybm; Tue, 21 May 2019 10:34:55 -0700 (PDT) X-Google-Smtp-Source: APXvYqzp9E5RX/Ljcx/bedN7x8btnylwWeOxScWHQ8dxg8CwJnkjSX1U9cZ3Zd9q/KB/HF2tm9WB X-Received: by 2002:a17:902:a508:: with SMTP id s8mr65890601plq.186.1558460095103; Tue, 21 May 2019 10:34:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1558460095; cv=none; d=google.com; s=arc-20160816; b=J+B/75yopfK1QC7yysMzHh9fQob72lBucXWC2B7142BGqmrLL/YQD7idfetKnyYq6u kOJ4hkI0Sm/lXYp+lnMdKNym4dg2AeUe4bZOjahAK7GfEuacPxIcugrO5FvCAQTbXGlZ 2RB4Dqf4C40BvhTfXOsXzip7cjKOnN7vPD0wo8JceO82K2BJc8hWAJFZjqNcAcbD/0Dt 8gHpWxriGqTDSvJfumm7o5SgfpCzksBJeJPoETHPW9ptqThOOGru8G4Z6/2lGQNHe5MX ZpSBdS6MRXfUemKpI7gixDCshERa93q//ujnoNgBlbPgaztnZzSRr/zAoaLpMPkEYy3G ZBwg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:from:date:dkim-signature; bh=hqJ9F0LX3sw/oRMB6TWKIt/BNKGcDcIyC1GO/Dc0tOo=; b=MEy22CwfO3HMdppXsSIi6LNq+wUz0uie9ihAt0R7RkJv6snp1VcFyktunKL3aqJfXE IXs3jjfFEKLmyHJ5BxPefxyKKL5OIK7yxXcOq/ICwxbS1MjyG09HDbvbOYQ8u9Xh1Jj+ EaG4QqdgkJVB1oJRbsO+I9cP1PLmZ4v/SzdC3x/s6FB0PJ4mUhr1wziAsa3FeuSh1d3Z 4u+FQOomRNS7sAEfO9kAQYTiPjw/Zo1h6IwbOTxi/s3Yq8+YXD8QvVoLpIJZOk7Gle7e MdopPk1Ku1MAePhJQyx6XGJXOqFOvAa2DeaqYhScTyDZV4kru2rMAm/6u3EMJ7SnzT4J TX3g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@megous.com header.s=mail header.b="dIy/gWb9"; 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=REJECT sp=REJECT dis=NONE) header.from=megous.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 19si21049774pgu.457.2019.05.21.10.34.39; Tue, 21 May 2019 10:34:55 -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=@megous.com header.s=mail header.b="dIy/gWb9"; 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=REJECT sp=REJECT dis=NONE) header.from=megous.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728500AbfEURdc (ORCPT + 99 others); Tue, 21 May 2019 13:33:32 -0400 Received: from vps.xff.cz ([195.181.215.36]:45898 "EHLO vps.xff.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726900AbfEURdc (ORCPT ); Tue, 21 May 2019 13:33:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=megous.com; s=mail; t=1558460009; bh=8XpWGxJiEQwnYFL7fQjANvL039ojO3exLChRMSO028s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dIy/gWb9Qz7kwegZvq4raKYo3HNXNjX27y7C9IzTA5mcebmVIWVMdCkUupCWGaQ81 OsUGgwvtEx5tMhX2kmvM0cNrc11Pw9UHzcX9MB1GXziih22AgLwnp68mqh2WvI5JsJ LgGTnKlBmlSUwO5izjC1MK0JURrtSDg9FUT8p8+A= Date: Tue, 21 May 2019 19:33:28 +0200 From: =?utf-8?Q?Ond=C5=99ej?= Jirman To: Maxime Ripard Cc: Frank Lee , Mark Rutland , Daniel Lezcano , catalin.marinas@arm.com, will.deacon@arm.com, bjorn.andersson@linaro.org, Mauro Carvalho Chehab , paulmck@linux.ibm.com, stefan.wahren@i2se.com, Linux PM , Chen-Yu Tsai , Jagan Teki , Andy Gross , rui.zhang@intel.com, devicetree@vger.kernel.org, marc.w.gonzalez@free.fr, Eduardo Valentin , enric.balletbo@collabora.com, robh+dt@kernel.org, Jonathan.Cameron@huawei.com, Linux ARM , Greg Kroah-Hartman , Linux Kernel Mailing List , olof@lixom.net, David Miller Subject: Re: [PATCH 2/3] thermal: sun50i: add thermal driver for h6 Message-ID: <20190521173328.bzwbyuy6mobzyvb5@core.my.home> Mail-Followup-To: Maxime Ripard , Frank Lee , Mark Rutland , Daniel Lezcano , catalin.marinas@arm.com, will.deacon@arm.com, bjorn.andersson@linaro.org, Mauro Carvalho Chehab , paulmck@linux.ibm.com, stefan.wahren@i2se.com, Linux PM , Chen-Yu Tsai , Jagan Teki , Andy Gross , rui.zhang@intel.com, devicetree@vger.kernel.org, marc.w.gonzalez@free.fr, Eduardo Valentin , enric.balletbo@collabora.com, robh+dt@kernel.org, Jonathan.Cameron@huawei.com, Linux ARM , Greg Kroah-Hartman , Linux Kernel Mailing List , olof@lixom.net, David Miller References: <20190512082614.9045-1-tiny.windzz@gmail.com> <20190512082614.9045-3-tiny.windzz@gmail.com> <20190512133930.t5txssl7mou2gljt@flea> <20190517073634.izdmba3yqvxviyg3@flea> <20190521080515.qlni2lnmcwh7itl7@flea> <20190521102721.5hgks6guzlhubj6d@core.my.home> <20190521142734.fvhdomqhuntpop52@flea> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190521142734.fvhdomqhuntpop52@flea> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 21, 2019 at 04:27:34PM +0200, Maxime Ripard wrote: > On Tue, May 21, 2019 at 12:27:21PM +0200, Ondřej Jirman wrote: > > On Tue, May 21, 2019 at 10:05:15AM +0200, Maxime Ripard wrote: > > > On Sat, May 18, 2019 at 01:27:39AM +0800, Frank Lee wrote: > > > > On Fri, May 17, 2019 at 3:36 PM Maxime Ripard wrote: > > > > > > > > > > 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 = &pdev->dev; > > > > > > > > + int ret; > > > > > > > > + > > > > > > > > + tmdev = devm_kzalloc(dev, sizeof(*tmdev), GFP_KERNEL); > > > > > > > > + if (!tmdev) > > > > > > > > + return -ENOMEM; > > > > > > > > + > > > > > > > > + tmdev->dev = dev; > > > > > > > > + tmdev->chip = of_device_get_match_data(&pdev->dev); > > > > > > > > + if (!tmdev->chip) > > > > > > > > + return -EINVAL; > > > > > > > > + > > > > > > > > + ret = tsens_init(tmdev); > > > > > > > > + if (ret) > > > > > > > > + return ret; > > > > > > > > + > > > > > > > > + ret = tsens_register(tmdev); > > > > > > > > + if (ret) > > > > > > > > + return ret; > > > > > > > > + > > > > > > > > + ret = tmdev->chip->enable(tmdev); > > > > > > > > + if (ret) > > > > > > > > + return ret; > > > > > > > > > > > > > > > > + platform_set_drvdata(pdev, tmdev); > > > > > > > > > > > > > > Your registration should be the very last thing you do. Otherwise, you > > > > > > > have a small window where the get_temp callback can be called, but the > > > > > > > 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řej said, > > > > > > > > Registration after enabling will lead to call tz update on non-registered tz > > > > from an interrupt handler. > > > > > > I'm probably missing something but you're not using the interrupts, so > > > how could an interrupt handler call it? > > > > > > Also, other drivers seem to be doing that just fine (mtk_thermal for > > > example), so surely there's a way? > > > > Last version is using the interrupts. > > > > Drivers do it in various ways. For example imx_thermal (and others like > > hisi_thermal) does it the suggested way. It enables interrupts after thermal > > zone registration, so that IRQ handler doesn't get invoked before the tzd is > > registered. > > Enabling the interrupts after the registration makes sense, yes, but > filling the device private pointer with the private structure, > enabling the clocks, setting up the controller and so on can be done > before. I agree. o. > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel