linux-kernel - Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1, i2c3, i2c5

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Nov 2017 23:56:12 -0800
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Damien Riegel <damien.riegel@...oirfairelinux.com>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>, kernel@...oirfairelinux.com
Subject: Re: [PATCH 4/4] arm64: dts: qcom: msm8916: add bindings for i2c1,
 i2c3, i2c5

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@...oirfairelinux.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