[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1cda400d-7ca1-9dcc-1d33-427dfd4ec92b@linaro.org>
Date: Tue, 11 Oct 2022 15:53:34 -0400
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: nick.hawkins@....com, verdun@....com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, linux@...linux.org.uk,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v1 3/5] ARM: dts: hpe: Add PLREG/SPI Support
On 11/10/2022 14:55, nick.hawkins@....com wrote:
> From: Nick Hawkins <nick.hawkins@....com>
>
> Adding support for the Programmable Logic Register driver in the HPE GXP
> SoC. Additionally adding support for the SPI driver that has already
> been committed to linux (See: drivers/spi/spi-gxp.c).
>
> Signed-off-by: Nick Hawkins <nick.hawkins@....com>
> ---
> arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 275 +++++++++++++++++++++++
> arch/arm/boot/dts/hpe-gxp.dtsi | 28 ++-
> 2 files changed, 302 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> index 3a7382ce40ef..c97b052c4868 100644
> --- a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> @@ -23,4 +23,279 @@
> device_type = "memory";
> reg = <0x40000000 0x20000000>;
> };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + led-power {
> + gpios = <&plreg 6 0>;
> + default_state = "off";
Use generic properties for color and function. Same applies to other nodes.
> + };
> +
> + led-heartbeat {
> + gpios = <&plreg 7 0>;
> + default_state = "off";
> + };
> +
> + led-identify {
> + gpios = <&plreg 56 0>;
> + default-state = "off";
> + };
> +
> + led-health_red {
> + gpios = <&plreg 57 0>;
> + default_state = "off";
> + };
> +
> + led-health_amber {
> + gpios = <&plreg 58 0>;
> + default-state = "off";
> + };
> + };
> +};
> +
> +&spifi {
> + status = "okay";
Blank line
> + flash@0 {
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + u-boot@0 {
> + label = "u-boot";
> + reg = <0x0 0x60000>;
> + };
> +
> + u-boot-env@...00 {
> + label = "u-boot-env";
> + reg = <0x60000 0x20000>;
> + };
> +
> + kernel@...00 {
> + label = "kernel";
> + reg = <0x80000 0x4c0000>;
> + };
> +
> + rofs@...000 {
> + label = "rofs";
> + reg = <0x540000 0x1740000>;
> + };
> +
> + rwfs@...0000 {
> + label = "rwfs";
> + reg = <0x1c80000 0x250000>;
> + };
> +
> + section@...0000{
> + label = "section";
> + reg = <0x1ed0000 0x130000>;
> + };
> + };
> + };
Blank line
> + flash@1 {
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + host-prime@0 {
> + label = "host-prime";
> + reg = <0x0 0x02000000>;
> + };
> +
> + host-second@...0000 {
> + label = "host-second";
> + reg = <0x02000000 0x02000000>;
> + };
> + };
> + };
> +};
> +
> +&plreg {
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-line-names =
> + "", "", "", "", "",
Messed indentation.
> + "", "POWER", "HEARTBEAT", "FAN1_INST", "FAN2_INST",
> + "FAN3_INST", "FAN4_INST", "FAN5_INST", "FAN6_INST", "FAN7_INST",
> + "FAN8_INST", "FAN9_INST", "FAN10_INST", "FAN11_INST", "FAN12_INST",
> + "FAN13_INST", "FAN14_INST", "FAN15_INST", "FAN16_INST", "FAN1_FAIL",
> + "FAN2_FAIL", "FAN3_FAIL", "FAN4_FAIL", "FAN5_FAIL", "FAN6_FAIL",
> + "FAN7_FAIL", "FAN8_FAIL", "FAN9_FAIL", "FAN10_FAIL", "FAN11_FAIL",
> + "FAN12_FAIL", "FAN13_FAIL", "FAN14_FAIL", "FAN15_FAIL", "FAN16_FAIL",
> + "", "", "", "", "",
> + "", "", "", "", "",
> + "", "", "", "", "",
> + "", "IDENTIFY", "HEALTH_RED", "HEALTH_AMBER", "POWER_BUTTON",
> + "", "SIO_POWER_GOOD", "NMI_BUTTON", "RESET_BUTTON", "SIO_S5",
> + "SIO_ONCONTROL", "", "", "", "",
> + "", "", "", "", "",
> + "", "", "", "", "",
> + "", "", "", "", "",
> + "", "", "", "", "",
> + "", "", "", "", "",
> + "", "", "", "", "";
> + fan1 {
fan-1? Difficult to suggest as you did not explain this in the binding.
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x01>;
> + };
> + fan2 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x02>;
> + };
> + fan3 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x04>;
> + };
> + fan4 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x08>;
> + };
> + fan5 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x10>;
> + };
> + fan6 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x40>;
> + };
> + fan7 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x40>;
> + };
> + fan8 {
> + inst = <0x27>;
> + fail = <0x29>;
> + id = <0x2B>;
> + bit = <0x80>;
> + };
> + fan9 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x01>;
> + };
> + fan10 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x02>;
> + };
> + fan11 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x04>;
> + };
> + fan12 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x08>;
> + };
> + fan13 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x10>;
> + };
> + fan14 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x20>;
> + };
> + fan15 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x40>;
> + };
> + fan16 {
> + inst = <0x28>;
> + fail = <0x2A>;
> + id = <0x2C>;
> + bit = <0x80>;
> + };
> + healthled {
> + red = <0x0D 0x20>;
> + amber = <0x0D 0x30>;
> + green = <0x0D 0x10>;
> + };
> + iopled1 {
> + on = <0x04 0x01>;
> + };
> + iopled2 {
> + on = <0x04 0x02>;
> + };
> + iopled3 {
> + on = <0x04 0x04>;
> + };
> + iopled4 {
> + on = <0x04 0x08>;
> + };
> + iopled5 {
> + on = <0x04 0x10>;
> + };
> + iopled6 {
> + on = <0x04 0x20>;
> + };
> + iopled7 {
> + on = <0x04 0x40>;
> + };
> + iopled8 {
> + on = <0x04 0x80>;
> + };
> + identifyled {
> + on = <0x05 0xC0>;
> + off = <0x05 0x40>;
> + };
> + acm {
> + forceoff = <0x0A 0x01>;
> + removed = <0x0A 0x02>;
> + unlatchreq = <0x0A 0x04>;
> + };
> + serverid {
> + lower = <0x01 0xFF>;
> + upper = <0x02 0xFF>;
> + };
> + sideband {
> + disabled = <0x40 0x03>;
> + embedded = <0x40 0x02>;
> + adaptive = <0x40 0x01>;
> + };
> + grp5intflag {
> + ack = <0xB0 0xFF>;
> + pwrbtn = <0xB0 0x01>;
> + uidpress = <0xB0 0x02>;
> + slpint = <0xB0 0x04>;
> + };
> + grp5intmask {
> + pwrbtn = <0xB1 0x01>;
> + slpint = <0xB1 0x40>;
> + };
> + grpintsmasks {
> + grp5 = <0x88 0x10>;
> + };
> + grpintsflags {
> + grp5 = <0x8C 0x10>;
> + };
> + pwrbtn {
> + latch = <0x0F 0xFF 0x04>;
> + };
> };
> diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
> index cf735b3c4f35..96003667bebe 100644
> --- a/arch/arm/boot/dts/hpe-gxp.dtsi
> +++ b/arch/arm/boot/dts/hpe-gxp.dtsi
> @@ -56,9 +56,28 @@
> compatible = "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> - ranges = <0x0 0xc0000000 0x30000000>;
> + ranges = <0x0 0xc0000000 0x40000000>;
> dma-ranges;
>
> + spifi: spi@200 {
> + compatible = "hpe,gxp-spifi";
> + reg = <0x200 0x80>, <0xc000 0x100>, <0x38000000 0x8000000>;
> + interrupts = <20>;
> + interrupt-parent = <&vic0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
Blank line.
> + flash@0 {
> + reg = <0>;
> + compatible = "jedec,spi-nor";
> + };
> +
> + flash@1 {
> + reg = <1>;
> + compatible = "jedec,spi-nor";
> + };
> + };
> +
> vic0: interrupt-controller@...0000 {
> compatible = "arm,pl192-vic";
> reg = <0xeff0000 0x1000>;
> @@ -122,6 +141,13 @@
> interrupts = <6>;
> interrupt-parent = <&vic0>;
> };
> +
> + plreg: plreg@...00000 {
Use same node name as in bindings example...
> + compatible = "hpe,gxp-plreg", "simple-mfd", "syscon";
> + reg = <0xd1000000 0xff>;
> + interrupts = <26>;
> + interrupt-parent = <&vic0>;
> + };
> };
> };
> };
Best regards,
Krzysztof
Powered by blists - more mailing lists