References: <20220106033130.37623-1-liang.yang@amlogic.com>
 <20220106091246.08ca66e0@xps13>
User-agent: mu4e 1.6.10; emacs 27.1
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Miquel Raynal <miquel.raynal@bootlin.com>,
        Liang Yang <liang.yang@amlogic.com>
Cc:     Vignesh Raghavendra <vigneshr@ti.com>,
        linux-mtd@lists.infradead.org, Richard Weinberger <richard@nod.at>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Kevin Hilman <khilman@baylibre.com>,
        Jianxin Pan <jianxin.pan@amlogic.com>,
        Victor Wan <victor.wan@amlogic.com>,
        XianWei Zhao <xianwei.zhao@amlogic.com>,
        Kelvin Zhang <kelvin.zhang@amlogic.com>,
        BiChao Zheng <bichao.zheng@amlogic.com>,
        YongHui Yu <yonghui.yu@amlogic.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: nand: meson: fix controller clock
Date:   Thu, 06 Jan 2022 09:43:56 +0100
In-reply-to: <20220106091246.08ca66e0@xps13>
Message-ID: <1jbl0pz1t4.fsf@starbuckisacylon.baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


On Thu 06 Jan 2022 at 09:12, Miquel Raynal <miquel.raynal@bootlin.com> wrot=
e:

> liang.yang@amlogic.com wrote on Thu, 6 Jan 2022 11:31:30 +0800:
>
>> Change-Id: I1425b491d8b95061e1ce358ef33143433fc94d24
>> ---
>>  .../bindings/mtd/amlogic,meson-nand.txt        | 18 +++---------------
>
> I forgot to mention, while you're at it, after fixing the bindings,
> could you please convert it to yaml?
>
>>  1 file changed, 3 insertions(+), 15 deletions(-)
>>=20
>> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.tx=
t b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> index 5794ab1147c1..37f16fe4fe66 100644
>> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.txt
>> @@ -14,11 +14,6 @@ Required properties:
>>  - clock-names: Should contain the following:
>>  	"core" - NFC module gate clock
>>  	"device" - device clock from eMMC sub clock controller

If I refer to the related driver change, this is not true anymore and
should be updated

>> -	"rx" - rx clock phase
>> -	"tx" - tx clock phase
>> -
>> -- amlogic,mmc-syscon	: Required for NAND clocks, it's shared with SD/eM=
MC
>> -				controller port C
>>=20=20
>>  Optional children nodes:
>>  Children nodes represent the available nand chips.
>> @@ -28,11 +23,6 @@ see Documentation/devicetree/bindings/mtd/nand-contro=
ller.yaml for generic bindi
>>=20=20
>>  Example demonstrate on AXG SoC:
>>=20=20
>> -	sd_emmc_c_clkc: mmc@7000 {
>> -		compatible =3D "amlogic,meson-axg-mmc-clkc", "syscon";
>> -		reg =3D <0x0 0x7000 0x0 0x800>;
>> -	};
>> -
>>  	nand-controller@7800 {
>>  		compatible =3D "amlogic,meson-axg-nfc";
>>  		reg =3D <0x0 0x7800 0x0 0x100>;
>> @@ -41,11 +31,9 @@ Example demonstrate on AXG SoC:
>>  		interrupts =3D <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>;
>>=20=20
>>  		clocks =3D <&clkc CLKID_SD_EMMC_C>,
>> -			<&sd_emmc_c_clkc CLKID_MMC_DIV>,
>> -			<&sd_emmc_c_clkc CLKID_MMC_PHASE_RX>,
>> -			<&sd_emmc_c_clkc CLKID_MMC_PHASE_TX>;
>> -		clock-names =3D "core", "device", "rx", "tx";
>> -		amlogic,mmc-syscon =3D <&sd_emmc_c_clkc>;
>> +			<&clkc CLKID_FCLK_DIV2>;
>> +		clock-names =3D "core", "device";

If you want to re-implement the mmc clock part directly in the nand
driver, you should provide both clock inputs, like the mmc driver (in
addition to the pclk) ... Even if you plan on using only FDIV2 in the
driver, this what the HW is.

Something like:

clock-names =3D "core", "clkin0", "clkin1";
clocks =3D <&clkc CLKID_SD_EMMC_C>,
         <&clkc CLKID_SD_EMMC_C_CLK0>,
	 <&clkc CLKID_FCLK_DIV2>;

>> +		sd_emmc_c_clkc =3D <0xffe07000>;

This is not how you provide memory regions

>>=20=20
>>  		pinctrl-names =3D "default";
>>  		pinctrl-0 =3D <&nand_pins>;
>
>
> Thanks,
> Miqu=C3=A8l