| 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
| ||
|
Date: Tue, 26 Aug 2014 05:42:09 -0400 (EDT)
From: kiran.padwal@...rtplayin.com
To: "Andreas Färber" <afaerber@...e.de>
Cc: robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
davidb@...eaurora.org, devicetree@...r.kernel.org,
linux@....linux.org.uk, linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ARM: apq8064: Add pinmux and i2c pinctrl nodes
On Wednesday, August 20, 2014 11:36am, "Andreas Färber" <afaerber@...e.de> said:
> Hi,
>
> Am 20.08.2014 14:02, schrieb Kiran Padwal:
>> This patch adds pinmux and i2c pinctrl DT node for IFC6410 board.
>> It also adds necessary DT support for i2c eeprom which is present on
>> IFC6410.
>>
>> Tested on IFC6410 board.
>>
>> Signed-off-by: Kiran Padwal <kiran.padwal@...rtplayin.com>
>> ---
>> Chages since v1:
>> - Renamed pinmux phandle "qcom_pinmux" to "tlmm_pinmux".
>> - Updated pinmux interrupt.
>>
>> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 29 ++++++++++++++
>> arch/arm/boot/dts/qcom-apq8064.dtsi | 59 ++++++++++++++++++++++++++++
>> 2 files changed, 88 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> index 7c2441d..d52ac3c 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> @@ -5,6 +5,15 @@
>> compatible = "qcom,apq8064-ifc6410", "qcom,apq8064";
>>
>> soc {
>> + pinmux@...000 {
>> + i2c1_pins: i2c1_pinmux {
>
> Is there a requirement to have "_pinmux" in the subnode of the "pinmux"
> node? Otherwise use just "i2c1"? I notice because the general convention
> seems to be dashes rather than underscores for node names.
Ok, I will change it to i2c1.
>
>> + mux {
>> + pins = "gpio20", "gpio21";
>> + function = "gsbi1";
>> + };
>> + };
>> + };
>> +
>> gsbi@...00000 {
>> status = "ok";
>> qcom,mode = <GSBI_PROT_I2C_UART>;
>> @@ -12,5 +21,25 @@
>> status = "ok";
>> };
>> };
>> +
>> + gsbi1: gsbi@...40000 {
>> + qcom,mode = <GSBI_PROT_I2C>;
>> + status = "ok";
>
> Usually the overridden status property goes first, as seen on the
> previous gsbi node.
>
> Further, its canonical value is "okay" (although in practice anything
> other than "disabled" should work), so suggest you adopt it for your new
> nodes.
>
> Also, you already provide a label "gsbi1" to this node in the .dtsi
> below, so you don't need to do it here again.
> Some architectures prefer in such a case that in the .dts the node is
> referenced as &gsbi1 {...}; below the root node in alphabetical order
> rather than duplicating the full /soc/foo@bar hierarchy.
Indeed, I'll change on v3.
>
>> +
>> + i2c1: i2c@...60000 {
>
> Suggest to move the i2c1 label to the .dtsi. Then optionally same could
> be done here as outlined for gsbi1.
Indeed, I'll change on v3.
>
>> + status = "ok";
>
> "okay".
Ok.
>
>> +
>> + clock-frequency = <200000>;
>> +
>> + pinctrl-0 = <&i2c1_pins>;
>> + pinctrl-names = "default";
>> +
>> + eeprom: eeprom@52 {
>> + compatible = "atmel,24c128";
>> + reg = <0x52>;
>> + pagesize = <32>;
>> + };
>> + };
>> + };
>> };
>> };
>> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi
>> b/arch/arm/boot/dts/qcom-apq8064.dtsi
>> index 92bf793..bb2ccde 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
>> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
>> @@ -70,6 +70,17 @@
>> ranges;
>> compatible = "simple-bus";
>>
>> + tlmm_pinmux: pinmux@...000 {
>> + compatible = "qcom,apq8064-pinctrl";
>> + reg = <0x800000 0x4000>;
>> +
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + interrupts = <0 16 0x4>;
>
> s/0x4/IRQ_TYPE_LEVEL_HIGH/?
Indeed, I'll change on v3.
>
>> + };
>> +
>> intc: interrupt-controller@...0000 {
>> compatible = "qcom,msm-qgic2";
>> interrupt-controller;
>> @@ -133,6 +144,54 @@
>> regulator;
>> };
>>
>> + gsbi1: gsbi@...40000 {
>> + compatible = "qcom,gsbi-v1.0.0";
>> + reg = <0x12440000 0x100>;
>> + clocks = <&gcc GSBI1_H_CLK>;
>> + clock-names = "iface";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> + status = "disabled";
>> +
>> + i2c@...60000 {
>> + compatible = "qcom,i2c-qup-v1.1.1";
>> + reg = <0x12460000 0x1000>;
>> + interrupts = <0 194 0>;
>
> The trailing 0 might be IRQ_TYPE_NONE?
Indeed, I'll change on v3.
>
>> +
>> + clocks = <&gcc GSBI1_QUP_CLK>, <&gcc GSBI1_H_CLK>;
>> + clock-names = "core", "iface";
>> + status = "disabled";
>
> I'd guess this is redundant since the parent is already disabled?
Indeed, I'll change on v3.
>
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + };
>> + };
>> +
>> + gsbi2: gsbi@...80000 {
>> + compatible = "qcom,gsbi-v1.0.0";
>> + reg = <0x12480000 0x100>;
>> + clocks = <&gcc GSBI2_H_CLK>;
>> + clock-names = "iface";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> + status = "disabled";
>> +
>> + i2c@...a0000 {
>> + compatible = "qcom,i2c-qup-v1.1.1";
>> + reg = <0x124a0000 0x1000>;
>> + interrupts = <0 196 0>;
>
> Again, IRQ_TYPE_NONE possibly?
Indeed, I'll change on v3.
Thanks.
>
> Cheers,
> Andreas
>
>> +
>> + clocks = <&gcc GSBI2_QUP_CLK>, <&gcc GSBI2_H_CLK>;
>> + clock-names = "core", "iface";
>> + status = "disabled";
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + };
>> + };
>> +
>> gsbi7: gsbi@...00000 {
>> status = "disabled";
>> compatible = "qcom,gsbi-v1.0.0";
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists