RE: [v4 06/12] arm: dts: aspeed: Update SPI flash node settings

2022-07-03 Thread Chin-Ting Kuo
Hi Cédric,

> -Original Message-
> From: Cédric Le Goater 
> Sent: Friday, July 1, 2022 5:42 PM
> To: Chin-Ting Kuo ; ChiaWei Wang
> ; lu...@denx.de; sean...@gmail.com;
> Ryan Chen ; BMC-SW
> ; ja...@amarulasolutions.com; vigne...@ti.com;
> u-boot@lists.denx.de; p.ya...@ti.com; Joel Stanley 
> 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: 0x1e62 - 0x1e62
> >decoded address: 0x2000 - 0x2fff
> >
> >- SPI1:
> >CS number: 2
> >controller reg: 0x1e63 - 0x1e630fff
> >decoded address: 0x3000 - 0x37ff
> >
> >- SPI2:
> >CS number: 2
> >controller reg: 0x1e631000 - 0x1e631fff
> >decoded address: 0x3800 - 0x3fff
> >
> > AST2600:
> >- FMC:
> >CS number: 3
> >controller reg: 0x1e62 - 0x1e62
> >decoded address: 0x2000 - 0x2fff
> >
> >- SPI1:
> >CS number: 2
> >controller reg: 0x1e63 - 0x1e630fff
> >decoded address: 0x3000 - 0x3fff
> >
> >- SPI2:
> >CS number: 3
> >controller reg: 0x1e631000 - 0x1e631fff
> >decoded address: 0x5000 - 0x5fff
> >
> > Signed-off-by: Chin-Ting Kuo 
> 
> 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 = <_sd2_default>;
> >   };
> > +
> > + {
> > +   status = "okay";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_fwspics1_default>;
> > +
> > +   flash@0 {
> > +   status = "okay";
> > +   spi-max-frequency = <5000>;
> > +   spi-tx-bus-width = <2>;
> > +   spi-rx-bus-width = <2>;
> > +   };
> > +
> > +   flash@1 {
> > +   status = "okay";
> > +   spi-max-frequency = <5000>;
> > +   spi-tx-bus-width = <2>;
> > +   spi-rx-bus-width = <2>;
> > +   };
> > +};
> > +
> > + {
> > +   status = "okay";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_spi1cs1_default>;
> > +
> > +   flash@0 {
> > +   status = "okay";
> > +   spi-max-frequency = <5000>;
> > +   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@1e62 {
> > -   reg = < 0x1e62 0xc4
> > -   0x2000 0x1000 >;
> > +   reg = <0x1e62 0xc4>, <0x2000 0x1000>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "aspeed,ast2500-fmc";
> 

Re: [v4 06/12] arm: dts: aspeed: Update SPI flash node settings

2022-07-01 Thread Cédric Le Goater

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: 0x1e62 - 0x1e62
   decoded address: 0x2000 - 0x2fff

   - SPI1:
   CS number: 2
   controller reg: 0x1e63 - 0x1e630fff
   decoded address: 0x3000 - 0x37ff

   - SPI2:
   CS number: 2
   controller reg: 0x1e631000 - 0x1e631fff
   decoded address: 0x3800 - 0x3fff

AST2600:
   - FMC:
   CS number: 3
   controller reg: 0x1e62 - 0x1e62
   decoded address: 0x2000 - 0x2fff

   - SPI1:
   CS number: 2
   controller reg: 0x1e63 - 0x1e630fff
   decoded address: 0x3000 - 0x3fff

   - SPI2:
   CS number: 3
   controller reg: 0x1e631000 - 0x1e631fff
   decoded address: 0x5000 - 0x5fff

Signed-off-by: Chin-Ting Kuo 


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 ?

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 = <_sd2_default>;
  };
+
+ {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <_fwspics1_default>;
+
+   flash@0 {
+   status = "okay";
+   spi-max-frequency = <5000>;
+   spi-tx-bus-width = <2>;
+   spi-rx-bus-width = <2>;
+   };
+
+   flash@1 {
+   status = "okay";
+   spi-max-frequency = <5000>;
+   spi-tx-bus-width = <2>;
+   spi-rx-bus-width = <2>;
+   };
+};
+
+ {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <_spi1cs1_default>;
+
+   flash@0 {
+   status = "okay";
+   spi-max-frequency = <5000>;
+   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@1e62 {

-   reg = < 0x1e62 0xc4
-   0x2000 0x1000 >;
+   reg = <0x1e62 0xc4>, <0x2000 0x1000>;
#address-cells = <1>;
#size-cells = <0>;
compatible = "aspeed,ast2500-fmc";
+   clocks = < 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@1e63 {

-   reg = < 0x1e63 0xc4
-   0x3000 0x0800 >;
+   reg = <0x1e63 0xc4>, <0x3000 0x0800>;
#address-cells = <1>;
#size-cells = <0>;
compatible = "aspeed,ast2500-spi";
+   clocks = < 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
-