Hi Cédric,
> -----Original Message-----
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Friday, July 1, 2022 5:42 PM
> To: Chin-Ting Kuo <chin-ting_...@aspeedtech.com>; ChiaWei Wang
> <chiawei_w...@aspeedtech.com>; lu...@denx.de; sean...@gmail.com;
> Ryan Chen <ryan_c...@aspeedtech.com>; BMC-SW
> <bmc...@aspeedtech.com>; ja...@amarulasolutions.com; vigne...@ti.com;
> u-boot@lists.denx.de; p.ya...@ti.com; Joel Stanley <j...@jms.id.au>
> Subject: Re: [v4 06/12] arm: dts: aspeed: Update SPI flash node settings
>
> On 5/24/22 07:56, Chin-Ting Kuo wrote:
> > For both AST2500 and AST2600, there are three SPI controllers,
> > FMC(Firmware Memory Controller),
> > SPI1 and SPI2. The clock source is HCLK. Following is the basic
> > information for ASPEED SPI controller.
> >
> > AST2500:
> > - FMC:
> > CS number: 3
> > controller reg: 0x1e620000 - 0x1e62ffff
> > decoded address: 0x20000000 - 0x2fffffff
> >
> > - SPI1:
> > CS number: 2
> > controller reg: 0x1e630000 - 0x1e630fff
> > decoded address: 0x30000000 - 0x37ffffff
> >
> > - SPI2:
> > CS number: 2
> > controller reg: 0x1e631000 - 0x1e631fff
> > decoded address: 0x38000000 - 0x3fffffff
> >
> > AST2600:
> > - FMC:
> > CS number: 3
> > controller reg: 0x1e620000 - 0x1e62ffff
> > decoded address: 0x20000000 - 0x2fffffff
> >
> > - SPI1:
> > CS number: 2
> > controller reg: 0x1e630000 - 0x1e630fff
> > decoded address: 0x30000000 - 0x3fffffff
> >
> > - SPI2:
> > CS number: 3
> > controller reg: 0x1e631000 - 0x1e631fff
> > decoded address: 0x50000000 - 0x5fffffff
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_...@aspeedtech.com>
>
> I might be wrong for the comment I did on 'num-cs' in the patch adding the
> driver. Joel, what's your opinion ? Hard coded in the driver or a property in
> the
> DT ?
>
Do you mean " num_cs" or "max-cs" here?
If "num_cs", I think that it should be detected during runtime, as the original
implementation.
If "max-cs", I think that both hard coded or a property in the DT are okay.
Thanks.
Chin-Ting
> Thanks,
>
> C.
>
> > ---
> > arch/arm/dts/ast2500-evb.dts | 33
> +++++++++++++++++++++++++++++++++
> > arch/arm/dts/ast2500.dtsi | 23 ++++++++++++++++-------
> > arch/arm/dts/ast2600-evb.dts | 8 --------
> > arch/arm/dts/ast2600.dtsi | 34 +++++++++++++++++++---------------
> > 4 files changed, 68 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/arm/dts/ast2500-evb.dts
> > b/arch/arm/dts/ast2500-evb.dts index 4796ed445f..c6b7675902 100644
> > --- a/arch/arm/dts/ast2500-evb.dts
> > +++ b/arch/arm/dts/ast2500-evb.dts
> > @@ -73,3 +73,36 @@
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_sd2_default>;
> > };
> > +
> > +&fmc {
> > + status = "okay";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_fwspics1_default>;
> > +
> > + flash@0 {
> > + status = "okay";
> > + spi-max-frequency = <50000000>;
> > + spi-tx-bus-width = <2>;
> > + spi-rx-bus-width = <2>;
> > + };
> > +
> > + flash@1 {
> > + status = "okay";
> > + spi-max-frequency = <50000000>;
> > + spi-tx-bus-width = <2>;
> > + spi-rx-bus-width = <2>;
> > + };
> > +};
> > +
> > +&spi1 {
> > + status = "okay";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_spi1cs1_default>;
> > +
> > + flash@0 {
> > + status = "okay";
> > + spi-max-frequency = <50000000>;
> > + spi-tx-bus-width = <2>;
> > + spi-rx-bus-width = <2>;
> > + };
> > +};
> > diff --git a/arch/arm/dts/ast2500.dtsi b/arch/arm/dts/ast2500.dtsi
> > index ee66ef6704..d78a53aeb7 100644
> > --- a/arch/arm/dts/ast2500.dtsi
> > +++ b/arch/arm/dts/ast2500.dtsi
> > @@ -57,23 +57,26 @@
> > ranges;
> >
> > fmc: flash-controller@1e620000 {
> > - reg = < 0x1e620000 0xc4
> > - 0x20000000 0x10000000 >;
> > + reg = <0x1e620000 0xc4>, <0x20000000 0x10000000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2500-fmc";
> > + clocks = <&scu ASPEED_CLK_AHB>;
> > + num-cs = <3>;
> > status = "disabled";
> > - interrupts = <19>;
> > +
> > flash@0 {
> > reg = < 0 >;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@1 {
> > reg = < 1 >;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@2 {
> > reg = < 2 >;
> > compatible = "jedec,spi-nor";
> > @@ -82,17 +85,20 @@
> > };
> >
> > spi1: flash-controller@1e630000 {
> > - reg = < 0x1e630000 0xc4
> > - 0x30000000 0x08000000 >;
> > + reg = <0x1e630000 0xc4>, <0x30000000 0x08000000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2500-spi";
> > + clocks = <&scu ASPEED_CLK_AHB>;
> > + num-cs = <2>;
> > status = "disabled";
> > +
> > flash@0 {
> > reg = < 0 >;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@1 {
> > reg = < 1 >;
> > compatible = "jedec,spi-nor";
> > @@ -101,17 +107,20 @@
> > };
> >
> > spi2: flash-controller@1e631000 {
> > - reg = < 0x1e631000 0xc4
> > - 0x38000000 0x08000000 >;
> > + reg = <0x1e631000 0xc4>, <0x38000000 0x08000000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2500-spi";
> > + clocks = <&scu ASPEED_CLK_AHB>;
> > + num-cs = <2>;
> > status = "disabled";
> > +
> > flash@0 {
> > reg = < 0 >;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@1 {
> > reg = < 1 >;
> > compatible = "jedec,spi-nor";
> > diff --git a/arch/arm/dts/ast2600-evb.dts
> > b/arch/arm/dts/ast2600-evb.dts index 0d65054313..c1965e9093 100644
> > --- a/arch/arm/dts/ast2600-evb.dts
> > +++ b/arch/arm/dts/ast2600-evb.dts
> > @@ -72,12 +72,10 @@
> >
> > &fmc {
> > status = "okay";
> > -
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_fmcquad_default>;
> >
> > flash@0 {
> > - compatible = "spi-flash", "sst,w25q256";
> > status = "okay";
> > spi-max-frequency = <50000000>;
> > spi-tx-bus-width = <4>;
> > @@ -85,7 +83,6 @@
> > };
> >
> > flash@1 {
> > - compatible = "spi-flash", "sst,w25q256";
> > status = "okay";
> > spi-max-frequency = <50000000>;
> > spi-tx-bus-width = <4>;
> > @@ -93,7 +90,6 @@
> > };
> >
> > flash@2 {
> > - compatible = "spi-flash", "sst,w25q256";
> > status = "okay";
> > spi-max-frequency = <50000000>;
> > spi-tx-bus-width = <4>;
> > @@ -103,14 +99,12 @@
> >
> > &spi1 {
> > status = "okay";
> > -
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_spi1_default &pinctrl_spi1abr_default
> > &pinctrl_spi1cs1_default &pinctrl_spi1wp_default
> > &pinctrl_spi1wp_default &pinctrl_spi1quad_default>;
> >
> > flash@0 {
> > - compatible = "spi-flash", "sst,w25q256";
> > status = "okay";
> > spi-max-frequency = <50000000>;
> > spi-tx-bus-width = <4>;
> > @@ -120,13 +114,11 @@
> >
> > &spi2 {
> > status = "okay";
> > -
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_spi2_default &pinctrl_spi2cs1_default
> > &pinctrl_spi2cs2_default &pinctrl_spi2quad_default>;
> >
> > flash@0 {
> > - compatible = "spi-flash", "sst,w25q256";
> > status = "okay";
> > spi-max-frequency = <50000000>;
> > spi-tx-bus-width = <4>;
> > diff --git a/arch/arm/dts/ast2600.dtsi b/arch/arm/dts/ast2600.dtsi
> > index 64074309b7..dd36f55f20 100644
> > --- a/arch/arm/dts/ast2600.dtsi
> > +++ b/arch/arm/dts/ast2600.dtsi
> > @@ -129,74 +129,78 @@
> > };
> >
> > fmc: flash-controller@1e620000 {
> > - reg = < 0x1e620000 0xc4
> > - 0x20000000 0x10000000 >;
> > + reg = <0x1e620000 0xc4>, <0x20000000 0x10000000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2600-fmc";
> > status = "disabled";
> > - interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> > clocks = <&scu ASPEED_CLK_AHB>;
> > num-cs = <3>;
> > +
> > flash@0 {
> > - reg = < 0 >;
> > + reg = <0>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@1 {
> > - reg = < 1 >;
> > + reg = <1>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@2 {
> > - reg = < 2 >;
> > + reg = <2>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > };
> >
> > spi1: flash-controller@1e630000 {
> > - reg = < 0x1e630000 0xc4
> > - 0x30000000 0x08000000 >;
> > + reg = <0x1e630000 0xc4>, <0x30000000 0x10000000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2600-spi";
> > clocks = <&scu ASPEED_CLK_AHB>;
> > num-cs = <2>;
> > status = "disabled";
> > +
> > flash@0 {
> > - reg = < 0 >;
> > + reg = <0>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@1 {
> > - reg = < 1 >;
> > + reg = <1>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > };
> >
> > spi2: flash-controller@1e631000 {
> > - reg = < 0x1e631000 0xc4
> > - 0x50000000 0x08000000 >;
> > + reg = <0x1e631000 0xc4>, <0x50000000 0x10000000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2600-spi";
> > clocks = <&scu ASPEED_CLK_AHB>;
> > num-cs = <3>;
> > status = "disabled";
> > +
> > flash@0 {
> > - reg = < 0 >;
> > + reg = <0>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@1 {
> > - reg = < 1 >;
> > + reg = <1>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
> > +
> > flash@2 {
> > - reg = < 2 >;
> > + reg = <2>;
> > compatible = "jedec,spi-nor";
> > status = "disabled";
> > };
