Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 22 Aug 2022 12:00:56 -0500
From:   Rob Herring <robh@kernel.org>
To:     Pali =?iso-8859-1?Q?Roh=E1r?= <pali@kernel.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc/85xx: DTS: Add CPLD definitions for P1021RDB
 Combo Board CPL Design
Message-ID: <20220822170056.GA4135542-robh@kernel.org>
References: <20220819084433.26011-1-pali@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220819084433.26011-1-pali@kernel.org>
Precedence: bulk

On Fri, Aug 19, 2022 at 10:44:33AM +0200, Pali Roh?r wrote:
> P1021RDB Combo Board CPLD Design is used on following Freescale boards:
> P1021RDB-PC, P1020RDB-PD, P1020MBG-PC, P1020UTM-PC and P2020RDB-PCA.
> 
> Add CPLD definitions for all these boards for which already exist DTS file.
> 
> CPLD has bank size 128kB, it is connected via CS3 on LBC and mapped to
> memory range 0xFFA00000~0xFFA1FFFF.
> 
> As CPLD firmware is common on all these boards, use just one compatible
> string "fsl,p1021rdb-pc-cpld".
> 
> In some DTS files is CPLD already defined, but definition is either
> incomplete or wrong. So fix it.
> 
> All these boards have via CPLD connected max6370 watchdog at offset 0x2
> with GPIO 11, status led at offset 0x8 and reset controller at offset 0xd.
> Additionally P1020MBG-PC and P1020RDB-PD boards have FXO led at offset 0x9
> and FXS leds at offset 0xa.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> ---
>  arch/powerpc/boot/dts/fsl/p1020mbg-pc.dtsi    | 92 +++++++++++++++++++
>  arch/powerpc/boot/dts/fsl/p1020mbg-pc_32b.dts |  6 +-
>  arch/powerpc/boot/dts/fsl/p1020mbg-pc_36b.dts |  6 +-
>  arch/powerpc/boot/dts/fsl/p1020rdb-pd.dts     | 44 +++++++--
>  arch/powerpc/boot/dts/fsl/p1020utm-pc.dtsi    | 37 ++++++++
>  arch/powerpc/boot/dts/fsl/p1020utm-pc_32b.dts |  4 +-
>  arch/powerpc/boot/dts/fsl/p1020utm-pc_36b.dts |  4 +-
>  arch/powerpc/boot/dts/fsl/p1021rdb-pc.dtsi    | 37 ++++++++
>  arch/powerpc/boot/dts/fsl/p1021rdb-pc_32b.dts |  5 +-
>  arch/powerpc/boot/dts/fsl/p1021rdb-pc_36b.dts |  5 +-
>  arch/powerpc/boot/dts/fsl/p2020rdb-pc.dtsi    | 33 ++++++-
>  11 files changed, 251 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/p1020mbg-pc.dtsi b/arch/powerpc/boot/dts/fsl/p1020mbg-pc.dtsi
> index a24699cfea9c..c73996dcd809 100644
> --- a/arch/powerpc/boot/dts/fsl/p1020mbg-pc.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1020mbg-pc.dtsi
> @@ -83,6 +83,95 @@
>  		compatible = "vitesse-7385";
>  		reg = <0x2 0x0 0x20000>;
>  	};
> +
> +	cpld@3,0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "fsl,p1021rdb-pc-cpld", "simple-bus", "syscon";
> +		reg = <0x3 0x0 0x20000>;
> +		ranges = <0x0 0x3 0x0 0x20000>;
> +
> +		watchdog@2 {
> +			compatible = "maxim,max6370";
> +			reg = <0x2 0x1>;
> +			gpios = <&gpio 11 1>;
> +		};
> +
> +		led@8 {

The register-bit-led schema says this should be 'led@8,0'. Did you 
run 'dtbs_check'? 

But that's going to conflict with what simple-bus schema says. I don't 
know that 'simple-bus' is really appropriate here. The CPLD isn't really 
just a bus. 'simple-mfd' is what's more commonly used with 'syscon'.


> +			compatible = "register-bit-led";
> +			reg = <0x8 0x1>;
> +			offset = <0x8>;
> +			mask = <0x1>;
> +			active-low;
> +			default-state = "keep";
> +			label = "status";
> +			function = "status";
> +			color = <6>; /* LED_COLOR_ID_YELLOW */
> +		};