From: Bjorn Andersson <bjorn.andersson@linaro.org> To: Damien Riegel <damien.riegel@savoirfairelinux.com>, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Andy Gross <andy.gross@linaro.org>, David Brown <david.brown@linaro.org>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, kernel@savoirfairelinux.com Subject: Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5 Date: Fri, 10 Nov 2017 23:56:12 -0800 [thread overview] Message-ID: <20171111075612.GA1236@tuxbook> (raw) In-Reply-To: <20171109171456.6t3nhig6t4bnkc77@workotop.localdomain> On Thu 09 Nov 09:14 PST 2017, Damien Riegel wrote: > Hi Bjorn, > > > On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote: > > On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote: > > > > I think it's better to use the word "nodes" (add nodes...) > > Will reword that. > > > > > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com> > > > --- > > > arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++ > > > arch/arm64/boot/dts/qcom/msm8916.dtsi | 45 +++++++++++++++++++ > > > 2 files changed, 117 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > index c67ad8ed8b60..1cec5b30ed6e 100644 > > > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > @@ -270,6 +270,30 @@ > > > }; > > > }; > > > > > > + i2c1_default: i2c1_default { > > > + pinmux { > > > + function = "blsp_i2c1"; > > > + pins = "gpio2", "gpio3"; > > > + }; > > > + pinconf { > > > + pins = "gpio2", "gpio3"; > > > + drive-strength = <16>; > > > + bias-disable; > > > + }; > > > > pinconf is typically board specific, so please leave these out from the > > base dtsi. > > I don't mind dropping that, but pinconf for i2c{2,4,6} is already in > msm8916-pins.dtsi, so we should either drop them all, or add pinconf for > i2c{1,3,5} for consistency. You're right, this needs to be consistent. So I would like us to update the existing nodes, > And if I read the pinctrl driver correctly, > I2C cannot be routed to other pins, the only properties that users may > want to modify are drive-strength and bias. Let me know which option you > prefer. > I discussed the matter with my colleagues and we concluded that we want to keep the muxing in the platform.dtsi and move the specification of the electrical properties (pinconf) to the board.dts(i). We did discuss having some "default values" in the platform file, but pushing these down to the board file gives an indication to others that these values are board specific. PS. If you're willing to provide a patch for the existing nodes I would be happy to review this as well! Regards, Bjorn
WARNING: multiple messages have this Message-ID (diff)
From: bjorn.andersson@linaro.org (Bjorn Andersson) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5 Date: Fri, 10 Nov 2017 23:56:12 -0800 [thread overview] Message-ID: <20171111075612.GA1236@tuxbook> (raw) In-Reply-To: <20171109171456.6t3nhig6t4bnkc77@workotop.localdomain> On Thu 09 Nov 09:14 PST 2017, Damien Riegel wrote: > Hi Bjorn, > > > On Thu, Nov 09, 2017 at 09:00:16AM -0800, Bjorn Andersson wrote: > > On Wed 01 Nov 10:53 PDT 2017, Damien Riegel wrote: > > > > I think it's better to use the word "nodes" (add nodes...) > > Will reword that. > > > > > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com> > > > --- > > > arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 72 ++++++++++++++++++++++++++++++ > > > arch/arm64/boot/dts/qcom/msm8916.dtsi | 45 +++++++++++++++++++ > > > 2 files changed, 117 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > index c67ad8ed8b60..1cec5b30ed6e 100644 > > > --- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi > > > @@ -270,6 +270,30 @@ > > > }; > > > }; > > > > > > + i2c1_default: i2c1_default { > > > + pinmux { > > > + function = "blsp_i2c1"; > > > + pins = "gpio2", "gpio3"; > > > + }; > > > + pinconf { > > > + pins = "gpio2", "gpio3"; > > > + drive-strength = <16>; > > > + bias-disable; > > > + }; > > > > pinconf is typically board specific, so please leave these out from the > > base dtsi. > > I don't mind dropping that, but pinconf for i2c{2,4,6} is already in > msm8916-pins.dtsi, so we should either drop them all, or add pinconf for > i2c{1,3,5} for consistency. You're right, this needs to be consistent. So I would like us to update the existing nodes, > And if I read the pinctrl driver correctly, > I2C cannot be routed to other pins, the only properties that users may > want to modify are drive-strength and bias. Let me know which option you > prefer. > I discussed the matter with my colleagues and we concluded that we want to keep the muxing in the platform.dtsi and move the specification of the electrical properties (pinconf) to the board.dts(i). We did discuss having some "default values" in the platform file, but pushing these down to the board file gives an indication to others that these values are board specific. PS. If you're willing to provide a patch for the existing nodes I would be happy to review this as well! Regards, Bjorn
next prev parent reply other threads:[~2017-11-11 7:56 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-01 17:53 [PATCH 0/4] arm64: dts: qcom: dts improvements Damien Riegel 2017-11-01 17:53 ` Damien Riegel 2017-11-01 17:53 ` [PATCH 1/4] arm64: dts: qcom: pm8916: fix wcd_codec indentation Damien Riegel 2017-11-01 17:53 ` Damien Riegel [not found] ` <20171101175335.22123-2-damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org> 2017-11-09 16:49 ` Bjorn Andersson 2017-11-09 16:49 ` Bjorn Andersson 2017-11-09 16:49 ` Bjorn Andersson 2017-11-01 17:53 ` [PATCH 2/4] arm64: dts: qcom: msm8916-pins: remove assignments to bias-disable Damien Riegel 2017-11-01 17:53 ` Damien Riegel 2017-11-09 16:50 ` Bjorn Andersson 2017-11-09 16:50 ` Bjorn Andersson 2017-11-01 17:53 ` [PATCH 3/4] arm64: dts: qcom: msm8916: normalize I2C bindings Damien Riegel 2017-11-01 17:53 ` Damien Riegel [not found] ` <20171101175335.22123-4-damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org> 2017-11-09 16:56 ` Bjorn Andersson 2017-11-09 16:56 ` Bjorn Andersson 2017-11-09 16:56 ` Bjorn Andersson 2017-11-01 17:53 ` [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5 Damien Riegel 2017-11-01 17:53 ` Damien Riegel [not found] ` <20171101175335.22123-5-damien.riegel-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/@public.gmane.org> 2017-11-09 17:00 ` Bjorn Andersson 2017-11-09 17:00 ` Bjorn Andersson 2017-11-09 17:00 ` Bjorn Andersson 2017-11-09 17:14 ` Damien Riegel 2017-11-09 17:14 ` Damien Riegel 2017-11-11 7:56 ` Bjorn Andersson [this message] 2017-11-11 7:56 ` Bjorn Andersson 2017-11-14 23:13 ` Damien Riegel 2017-11-14 23:13 ` Damien Riegel 2017-11-14 23:13 ` Damien Riegel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171111075612.GA1236@tuxbook \ --to=bjorn.andersson@linaro.org \ --cc=andy.gross@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=damien.riegel@savoirfairelinux.com \ --cc=david.brown@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=kernel@savoirfairelinux.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-soc@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=robh+dt@kernel.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.