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 X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB448C432C0 for ; Thu, 28 Nov 2019 21:43:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A711E2084D for ; Thu, 28 Nov 2019 21:43:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="kZeh47RN" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726692AbfK1Vnq (ORCPT ); Thu, 28 Nov 2019 16:43:46 -0500 Received: from mail-pg1-f194.google.com ([209.85.215.194]:46455 "EHLO mail-pg1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726558AbfK1Vnq (ORCPT ); Thu, 28 Nov 2019 16:43:46 -0500 Received: by mail-pg1-f194.google.com with SMTP id k1so5046758pga.13 for ; Thu, 28 Nov 2019 13:43:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=K9FrLAOFp0g+hrdsabPVchjdViFjj8h0GNkeqZ4HQlM=; b=kZeh47RNC2BtHUa2wCS8rsZONatpvYYLR3K646pyo72NUYaJ/cDpbMeVmz5Yvj0lCo 1g50TyX6V1STK60yAw+cgJZzEYfha6eRSBmYJKBnzZxOAzADErDowc173BZOHaPbENxU 6XEK3SKBHCw4edPwidyFtxBU1GPm2Fc8/d6gF9GtscAYtRINmE3LoMEADij0to1zXv/u FDHww8EWCZu4DbibNnBJyYzbZkmBFEpKsZesGrvC9mPqVDMSsSLkonwZ+WIsJUl0grPT fpQubcqqSy7C7c9k6rCEm4zTvqF7lbdNMowTyWWA8U6O6FQ1eYS//74MurZ87a9CHWGH dCwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=K9FrLAOFp0g+hrdsabPVchjdViFjj8h0GNkeqZ4HQlM=; b=cHOmDq0plKbOHyYMKEBk4UizkhfyDJfEFFEQefsPqsdNpC5fd/kabXnSj0j/a7yhll wCKTN/z73nKqOEWjKFuIvWn5Zv9NJ61NisNOCdDMln4u0W9xaCsHtn1l96r+N5CJ/tQd qhFjnYAjPWBS9wIXM5f9kDaRXpVfP3rBPT2hBmNM0nhSwDLOgwh8HaobNdPmg5J/IAck BWFnDdQZ+wW3WnHw/OmbuBxBaOrF79Snh5Hl5c2cMOf5bIlZLeJ3Kh0Fghpqtkjd7M+7 h9NG9YSKzr7Jftg4fhR8asTIvZ/KpNneC4/kTdrVl7+U/2n0BC1WKoSG26Vk4otTLbj4 t9jg== X-Gm-Message-State: APjAAAVXpfuwCw8QDM1Hw2QYQman45srNX+RSxxZzMSnQJXCKxThDw2t CkdG79PmdZDmds4ZPqcyXlP+4A== X-Google-Smtp-Source: APXvYqxXK5VlxZHgjn3ehpw/1a/UvQMzGfyMHy3vxqme3i6QHac5tFm6KotaQWcLg7ThtP7KXY6L6w== X-Received: by 2002:aa7:90d0:: with SMTP id k16mr55284524pfk.131.1574977423508; Thu, 28 Nov 2019 13:43:43 -0800 (PST) Received: from yoga (104-188-17-28.lightspeed.sndgca.sbcglobal.net. [104.188.17.28]) by smtp.gmail.com with ESMTPSA id o23sm21753750pgj.90.2019.11.28.13.43.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Nov 2019 13:43:42 -0800 (PST) Date: Thu, 28 Nov 2019 13:43:39 -0800 From: Bjorn Andersson To: Amit Kucheria Cc: LKML , linux-arm-msm , Eduardo Valentin , Stephen Boyd , sivaa@codeaurora.org, Andy Gross , Daniel Lezcano , Linux PM list Subject: Re: [PATCH 1/3] drivers: thermal: tsens: Add critical interrupt support Message-ID: <20191128214339.GL82109@yoga> References: <4b949a4f401a7f9d403ed0f0c16c7feb083f3524.1573499020.git.amit.kucheria@linaro.org> <20191112193852.GC3140946@builder> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.2 (2019-09-21) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 28 Nov 10:46 PST 2019, Amit Kucheria wrote: > On Wed, Nov 13, 2019 at 1:08 AM Bjorn Andersson > wrote: > > > > On Mon 11 Nov 11:21 PST 2019, Amit Kucheria wrote: > > > > > TSENS IP v2.x adds critical threshold interrupt support for each sensor > > > in addition to the upper/lower threshold interrupt. Add support in the > > > driver. > > > > > > Signed-off-by: Amit Kucheria > > > --- > > > drivers/thermal/qcom/tsens-common.c | 129 ++++++++++++++++++++++++++-- > > > drivers/thermal/qcom/tsens-v2.c | 8 +- > > > drivers/thermal/qcom/tsens.c | 21 +++++ > > > drivers/thermal/qcom/tsens.h | 73 ++++++++++++++++ > > > 4 files changed, 220 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c > > > index 4359a4247ac3..2989cb952cdb 100644 > > > --- a/drivers/thermal/qcom/tsens-common.c > > > +++ b/drivers/thermal/qcom/tsens-common.c > > > @@ -23,6 +23,10 @@ > > > * @low_thresh: lower threshold temperature value > > > * @low_irq_mask: mask register for lower threshold irqs > > > * @low_irq_clear: clear register for lower threshold irqs > > > + * @crit_viol: critical threshold violated > > > > "violated" as in "temperature is above crit_thresh"? > > Yes. > > > > > > + * @crit_thresh: critical threshold temperature value > > > + * @crit_irq_mask: mask register for critical threshold irqs > > > + * @crit_irq_clear: clear register for critical threshold irqs > > > * > > [..] > > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > > > index 7d317660211e..784c4976c4f9 100644 > > > --- a/drivers/thermal/qcom/tsens.c > > > +++ b/drivers/thermal/qcom/tsens.c > > > @@ -121,6 +121,27 @@ static int tsens_register(struct tsens_priv *priv) > > > > > > enable_irq_wake(irq); > > > > > > + if (tsens_version(priv) > VER_1_X) { > > > + irq = platform_get_irq_byname(pdev, "critical"); > > > + if (irq < 0) { > > > > Treating this as a fatal error breaks backwards compatibility with > > current devicetree; and even within your patch series, tsens should fail > > to probe between this patch and the application of patch 3. > > Good catch. > > > Please flip this around and do: > > > > irq = platform_get_irq_byname(pdev, "critical"); > > if (irq >= 0 && tsens_version(priv) > VER_1_X) { > > request_irq()... > > } > > Won't this still break with current devicetree since irq < 0 until > patch 3? Or are you saying we shouldn't check for > platform_get_irq_byname() failure? > I'm trying to say that dtsi without "critical" defined should cause the driver to simply skip this segment, not fail to initialize. > I can see two ways out: > 1. We patch the dtsi before the code change. You're expected to maintain backwards compatibility with existing dtb files out there. The support for critical interrupt is an additional feature, so you should be able to do this by detecting if "critical" is defined (e.g. by checking the return value of platform_get_irq_byname()). > 2. We make critical interrupt failure non-fatal by just printing some > messages and still returning success. > Try to make it as specific as possible (without adding a bunch of code) and throw in a dev_info() if no "critical" is found. Regards, Bjorn > Regards, > Amit > > > > > + ret = irq; > > > + goto err_put_device; > > > + } > > > + > > > + ret = devm_request_threaded_irq(&pdev->dev, irq, > > > + NULL, tsens_critical_irq_thread, > > > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, > > > + dev_name(&pdev->dev), priv); > > > + if (ret) { > > > + dev_err(&pdev->dev, "%s: failed to get critical irq\n", __func__); > > > + goto err_put_device; > > > + } > > > + > > > + enable_irq_wake(irq); > > > + } > > > + > > > + return 0; > > > + > > > err_put_device: > > > put_device(&pdev->dev); > > > return ret; > > > > Regards, > > Bjorn