linux-kernel - Re: [PATCH v4 1/1] ARM: mvebu: dts: Add dts file for DLink DNS-327L

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 20 May 2015 11:05:12 +0200
From:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To:	Andrew Andrianov <andrew@...mnt.org>,
	Jason Cooper <jason@...edaemon.net>,
	Andrew Lunn <andrew@...n.ch>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	linux-arm-kernel@...ts.infradead.org
CC:	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 1/1] ARM: mvebu: dts: Add dts file for DLink DNS-327L

On 19.05.2015 23:10, Andrew Andrianov wrote:
> DNS-327L is a 2-bay NAS with the following specs:
[...]
> Signed-off-by: Andrew Andrianov <andrew@...mnt.org>
> ---
>   arch/arm/boot/dts/Makefile                     |   1 +
>   arch/arm/boot/dts/armada-370-dlink-dns327l.dts | 356 +++++++++++++++++++++++++
>   2 files changed, 357 insertions(+)
>   create mode 100644 arch/arm/boot/dts/armada-370-dlink-dns327l.dts
[...]
> diff --git a/arch/arm/boot/dts/armada-370-dlink-dns327l.dts b/arch/arm/boot/dts/armada-370-dlink-dns327l.dts
> new file mode 100644
> index 0000000..0f4555c4
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-370-dlink-dns327l.dts
> @@ -0,0 +1,356 @@
[...]
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-0 = <&xhci_pwr_pin
> +			&sata_l_pwr_pin
> +			&sata_r_pwr_pin>;
> +
> +		pinctrl-names = "default";

Andrew,

I doubt pinctrl will not work as you expected. For regulators, unlike
gpio-keys/leds above, this node just describes the "bus" of regulators.

> +
> +		usb_power: regulator@1 {
> +			compatible = "regulator-fixed";
> +			reg = <1>;
> +			regulator-name = "USB3.0 Port Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-boot-on;
> +			regulator-always-on;
> +			gpio = <&gpio0 13 GPIO_ACTIVE_HIGH>;

Instead you'll have to put the corresponding pinctrl-0/names properties
in each of the regulator nodes, i.e.

			pinctrl-0 = <&xhci_pwr_pin>
			pinctrl-names = "default";

here and similar for the other regulator nodes.

> +		};
> +
> +		sata_r_power: regulator@2 {
> +			compatible = "regulator-fixed";
> +			reg = <2>;
> +			regulator-name = "SATA-R Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			startup-delay-us = <2000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio1 22 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		sata_l_power: regulator@3 {
> +			compatible = "regulator-fixed";
> +			reg = <3>;
> +			regulator-name = "SATA-L Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			startup-delay-us = <4000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio1 24 GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +};
> +
> +&pinctrl {
> +	sata_l_white_pin: sata-l-white-pin {
> +		marvell,pins = "mpp57";
> +		marvell,function = "sata0";
> +	};
> +
> +	sata_r_white_pin: sata-r-white-pin {
> +		marvell,pins = "mpp55";
> +		marvell,function = "sata1";
> +	};
> +
> +	sata_r_amber_pin: sata-r-amber-pin {
> +		marvell,pins = "mpp52";
> +		marvell,function = "gpio";
> +	};
> +

Remove extra empty line.

> +
> +	sata_l_amber_pin: sata-l-amber-pin {
> +		marvell,pins = "mpp53";
> +		marvell,function = "gpio";
> +	};
> +
> +	backup_led_pin: backup-led-pin {
> +		marvell,pins = "mpp61";
> +		marvell,function = "gpo";
> +	};
> +
> +	xhci_pwr_pin: xhci-pwr-pin {
> +		marvell,pins = "mpp13";
> +		marvell,function = "gpio";
> +	};
> +
> +	sata_r_pwr_pin: sata-r-pwr-pin {
> +		marvell,pins = "mpp54";
> +		marvell,function = "gpio";
> +	};
> +
> +	sata_l_pwr_pin: sata-l-pwr-pin {
> +		marvell,pins = "mpp56";
> +		marvell,function = "gpio";
> +	};
> +
> +	uart1_pins: uart1-pins {
> +		marvell,pins = "mpp60", "mpp61";
> +		marvell,function = "uart1";
> +	};
> +
> +	power_button_pin: power-button-pin {
> +		marvell,pins = "mpp65";
> +		marvell,function = "gpio";
> +	};
> +
> +	backup_button_pin: backup-button-pin {
> +		marvell,pins = "mpp63";
> +		marvell,function = "gpio";
> +	};
> +
> +	reset_button_pin: reset-button-pin {
> +		marvell,pins = "mpp64";
> +		marvell,function = "gpio";
> +	};
> +};
> +
> +/* Serial console */
> +&uart0 {
> +	status = "okay";
> +};
> +
> +/* Connected to Weltrend MCU */
> +&uart1 {
> +	pinctrl-0 = <&uart1_pins>;

missing pinctrl-names = "default";

> +	status = "okay";
> +};
> +
> +&mdio {
> +	phy0: ethernet-phy@0 { /* Marvell 88E1318 */
> +		reg = <0>;
> +		marvell,reg-init = <0x0 0x16 0x0 0x2>,
> +		                   <0x0 0x19 0x0 0x77>,
> +		                   <0x0 0x18 0x0 0x5747>;

nit: if you are at it, you could prepend the mdio reg data with 0, i.e.
<0x0 0x16 0x0 0x0002>.

If you address the comments and stick with this version, i.e. do not add
any new stuff now - you can add my

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>

Thanks!

> +	};
> +};
> +
> +&eth1 {
> +	phy = <&phy0>;
> +	phy-mode = "rgmii-id";
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	compatible = "marvell,mv64xxx-i2c";
> +	clock-frequency = <100000>;
> +	status = "okay";
> +};
>

--
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/