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=-5.5 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_PASS,USER_AGENT_MUTT autolearn=ham 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 F0E3CC10F03 for ; Thu, 25 Apr 2019 04:16:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A4F48217FA for ; Thu, 25 Apr 2019 04:16:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="Kbt7LF9H" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726115AbfDYEQf (ORCPT ); Thu, 25 Apr 2019 00:16:35 -0400 Received: from mail-pl1-f194.google.com ([209.85.214.194]:42627 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725900AbfDYEQe (ORCPT ); Thu, 25 Apr 2019 00:16:34 -0400 Received: by mail-pl1-f194.google.com with SMTP id x15so6027319pln.9 for ; Wed, 24 Apr 2019 21:16:34 -0700 (PDT) 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=S2cmio0I/zzm+kU6sgRVXtWl3AJj1ErOF2wDo10O3hs=; b=Kbt7LF9HDh2lugf41w8TWhPAtL6ZAVstjKmpEcnCuffWGhXT7qae/UvHGLEVhntShR 2OvCNGsNh5u+Z6tTDXztC3DxhbFlDqwILbkr278K9hcctEXKQte4HRIj5uFMvK8nUo+w qwIieDten+daYwWpjv2AAN3thfgY27fjKYXnHZ6Kgxf137EBBAqqsC8X6ng232+xR4cv VG4c8t5mCHdcp3tsCx9Paq2zPYcpjRhNgjUjdUNE65HzcWiLN538FNtO/I6vV1BxMsPe NOkRDpg5jZTXz1/2IqR3zZWej50UkUW/OjBAn2mvv+gIwobg2jxkehgIxLqhGfw9JBZJ 7OqQ== 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=S2cmio0I/zzm+kU6sgRVXtWl3AJj1ErOF2wDo10O3hs=; b=GPElmxXDGx6wyDYucMS5ByHucOqfJu6nyslHocq0STnyjX2HIY08V/PqcOKifM73r7 qeozlrRaLiyd+9STWksClkTPqafYfIKXmYTdEospu0YAHr9ZElbm7nF+IRCptYCWh8I2 v/Quk0KyPFmOljs8RTYesa0ljyaSyRtIMv2iRJI9zjuz9Jxn9aWtABU57w1UP6/xe3Lc 6mTnwNZmilBb/aTR8GuYahMlOJ/oiAnBSP5GaFjD18stwkZYCrgovJ6mFmSI7Dfjh8TI Ixzf3vOskH7U9+yQ3NMA4HxqmQyHuWiTALmvBW91YFgoCgnFPPjBWDfbljY95t/tMA9s 7QwA== X-Gm-Message-State: APjAAAV/SFHDBgTFyclkXCh6VtrqLfmB/9IXyaDZjqHucsIFy9Nb+y8G GMtpTKM0hkBJPcAALzsCOuctYw== X-Google-Smtp-Source: APXvYqyqc/oxbfBAMdjcL6hDtJlPB60mP3tVxe/Cz3I5VfZC7h69Kd9h+YSr+pmqgoBhhPreO4gcaw== X-Received: by 2002:a17:902:e382:: with SMTP id ch2mr35981171plb.94.1556165793486; Wed, 24 Apr 2019 21:16:33 -0700 (PDT) Received: from tuxbook-pro (104-188-17-28.lightspeed.sndgca.sbcglobal.net. [104.188.17.28]) by smtp.gmail.com with ESMTPSA id i129sm33636524pfc.163.2019.04.24.21.16.32 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Apr 2019 21:16:32 -0700 (PDT) Date: Wed, 24 Apr 2019 21:16:30 -0700 From: Bjorn Andersson To: Marc Gonzalez Cc: gpio , MSM , Linus Walleij , Wolfram Sang , Jeffrey Hugo Subject: Re: Pinctrl DT for msm8998 Message-ID: <20190425041630.GA2867@tuxbook-pro> References: <7b8c15a8-8a17-e16e-8974-4a0c3a202daa@free.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7b8c15a8-8a17-e16e-8974-4a0c3a202daa@free.fr> User-Agent: Mutt/1.11.4 (2019-03-13) Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Wed 24 Apr 08:19 PDT 2019, Marc Gonzalez wrote: > Hello, > > linusw wrote: > > > Some drivers prefer to handle individual pins by name, such as msm. > > Some drivers, especially those with hardware tailored to handle pin > > muxing groupwise, prefer to handle them by function group name. In > > the case of msm, every group consists of one pin and these groups are > > all named "gpioNN". What the pin control subsystem does is assign > > functions to 1 or several groups of pins. In the msm case as there is > > just one pin in every group, this becomes a bit confusing, it is easy > > to mix up what is a pin and what is a group. The pin multiplexing is > > done on groups while the pin configuration is done on individual > > pins. As to why there is one-to-one pin to group name mapping in the > > msm driver you need to ask bamse, for the pin control subsystem this > > is just some string. I suppose there was a good reason to set it up > > this way on msm. I think the msm may be set up this way because the > > pins can be configured in so many ways that it is hard to come up > > with natural groups that map to physical use cases (these are often > > enumerable on other platforms). In many systems the driver authors > > are restricted to how groups can be activated with functions, see eg > > pinctrl-gemini.c, in many other cases the driver author tries to > > half-guess the groups based on use cases that makes sense. > > bamse wrote: > > > You don't have to specify pinconf/pinmux in different subnodes > > anymore, that changed a few years back. For the blsp you typically > > want subnodes for each pin though, because they have different > > configuration...but you don't need to split e.g gpio88 in a mux and > > conf. Or per your production example, you don't need any subnodes at > > all...just put the properties in i2c_5_{active,sleep} directly. And > > as each gpioXX is configurable we need to define how we want each one > > to be configured...and we need to list all the configureables in the > > driver. In the qcom platform you can mux i2c on gpio87 at the same > > time as running gpio88 as gpio. Not that it would make sense in this > > case, but there are other such cases where we need the control of > > each configurable item. > > > diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > index 6db70acd38ee..bc1a1a4081da 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi > @@ -75,4 +75,18 @@ > drive-strength = <2>; /* 2 mA */ > }; > }; > + > + i2c5_default: i2c5_default { > + pins = "gpio87", "gpio88"; > + function = "blsp_i2c5"; > + drive-strength = <2>; > + bias-disable; > + }; > + > + i2c5_sleep: i2c5_sleep { > + pins = "gpio87", "gpio88"; > + function = "blsp_i2c5"; > + drive-strength = <2>; > + bias-disable; > + }; > }; > > > Note that the two nodes are identical. Do I still need both then? > > > &blsp1_i2c5 { > status = "ok"; > clock-frequency = <100000>; > pinctrl-names = "default", "sleep"; > pinctrl-0 = <&i2c5_default>; > pinctrl-1 = <&i2c5_sleep>; > }; > > Couldn't I just change the above to: > > &blsp1_i2c5 { > status = "ok"; > clock-frequency = <100000>; > pinctrl-names = "default"; > pinctrl-0 = <&i2c5_default>; > }; > Per drivers/base/pinctrl.c as a device is about to be probed the driver framework will look for a pinctrl state named "init", then "default" and select this. Beyond that point the driver itself can choose to go to additional states. There exists standard helpers for "default", "sleep" and "idle" (pinctrl_pm_select_*_state() in drivers/pinctrl/core.c, but the driver could make up their own names. As we don't yet support hitting suspend or the lower sleep states most of our drivers doesn't deal with anything past getting the default set, so for now you'll be good. As Linus says, once the drivers starts to properly deal with going to "sleep" and then back to "default", the lack of sleep would mean that the pinctrl_pm_select_sleep_state() returns 0, without changing the current state and then on resume pinctrl_pm_select_default_state() would be a no-op and we would lack the proper state (unless we deal with this in the pinctrl driver itself perhaps). So for now you're fine with just having the default state and ignore the "sleep", but at some point we will actually reach this and your old DT might start malfunction. (Although having states there that was never tested is no guarantee that we won't have that issue anyways...) Regards, Bjorn