arm: Add sata support on Layerscape ARMv8 board

PATCH Superseded Yuantian.Tang at freescale.com 3 participants 12 messages Nov 30, 2015 – Dec 2, 2015 Arm (32-bit)

lore

Share & tools

lore

Yuantian.Tang at freescale.com

author

2015-11-30 07:44 UTC #0 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

From: Tang Yuantian <[email protected]>

Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3 specification.
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.

Signed-off-by Tang Yuantian <[email protected]>

---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree

 arch/arm/cpu/armv8/fsl-layerscape/soc.c           | 43 +++++++++++++++++++++++
 arch/arm/include/asm/arch-fsl-layerscape/config.h | 18 ++++++++++
 arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31 ++++++++++++++++
 include/configs/ls1043aqds.h                      | 17 +++++++++
 4 files changed, 109 insertions(+)

arch/arm/cpu/armv8/fsl-layerscape/soc.c +43 -0

index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
 #include <common.h>
 #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
 #include <asm/arch/soc.h>
 #include <asm/io.h>
 #include <asm/global_data.h>

@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
 	erratum_a009203();
 }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
 #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void *)CONFIG_SYS_SATA;
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
 void fsl_lsch2_early_init_f(void)
 {
 	struct ccsr_cci400 *cci = (struct ccsr_cci400 *)CONFIG_SYS_CCI400_ADDR;

@@ -141,6 +180,10 @@ void fsl_lsch2_early_init_f(void)
 #ifdef CONFIG_BOARD_LATE_INIT
 int board_late_init(void)
 {
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
 	return 0;
 }
 #endif

arch/arm/include/asm/arch-fsl-layerscape/config.h +18 -0

index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
 #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR + 0x02200000)
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR + 0x02210000)
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE		(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
+						CONFIG_SYS_SCSI_MAX_LUN)
+
 /* Generic Interrupt Controller Definitions */
 #define GICD_BASE			0x06000000
 #define GICR_BASE			0x06100000

arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0

index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
 #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
 #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
 #ifdef CONFIG_FSL_LSCH3
 void fsl_lsch3_early_init_f(void);
 #elif defined(CONFIG_FSL_LSCH2)

include/configs/ls1043aqds.h +17 -0

index 4aeb238..bd7af00 100644
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
 #define CONFIG_SYS_FSL_PBL_RCW board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
 #endif
 
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA				(CONFIG_SYS_IMMR + 0x02200000)
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE		(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
+						CONFIG_SYS_SCSI_MAX_LUN)
+
 /*
  * IFC Definitions
  */

Signature · 1 line

2.1.0.27.g96db324

Sinan Akman

2015-11-30 17:27 UTC #1 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Yuan

Quoted 105 lines · Yuantian.Tang at freescale.com

From: Tang Yuantian <[email protected]>

Signed-off-by Tang Yuantian <[email protected]>

  arch/arm/cpu/armv8/fsl-layerscape/soc.c           | 43 +++++++++++++++++++++++
  arch/arm/include/asm/arch-fsl-layerscape/config.h | 18 ++++++++++
  arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31 ++++++++++++++++
  include/configs/ls1043aqds.h                      | 17 +++++++++
  4 files changed, 109 insertions(+)

arch/arm/cpu/armv8/fsl-layerscape/soc.c +43 -0

index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
  #include <common.h>
  #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
  #include <asm/arch/soc.h>
  #include <asm/io.h>
  #include <asm/global_data.h>

@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
  	erratum_a009203();
  }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
  #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void *)CONFIG_SYS_SATA;
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
  void fsl_lsch2_early_init_f(void)
  {
  	struct ccsr_cci400 *cci = (struct ccsr_cci400 *)CONFIG_SYS_CCI400_ADDR;

@@ -141,6 +180,10 @@ void fsl_lsch2_early_init_f(void)
  #ifdef CONFIG_BOARD_LATE_INIT
  int board_late_init(void)
  {
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
  	return 0;
  }
  #endif

arch/arm/include/asm/arch-fsl-layerscape/config.h +2 -0

index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
  #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
+#define CONFIG_LIBATA

arch/arm/include/asm/arch-fsl-layerscape/config.h line 68

Quoted 10 lines

arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0

+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR + 0x02200000)
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR + 0x02210000)

Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2
here and then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same macro value : (CONFIG_SYS_IMMR + 0x02200000)

Quoted 60 lines

arch/arm/include/asm/arch-fsl-layerscape/config.h +6 -0

+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE		(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
+						CONFIG_SYS_SCSI_MAX_LUN)
+
  /* Generic Interrupt Controller Definitions */
  #define GICD_BASE			0x06000000
  #define GICR_BASE			0x06100000

arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0

index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
  #ifdef CONFIG_FSL_LSCH3
  void fsl_lsch3_early_init_f(void);
  #elif defined(CONFIG_FSL_LSCH2)

include/configs/ls1043aqds.h +1 -0

index 4aeb238..bd7af00 100644
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
  #define CONFIG_SYS_FSL_PBL_RCW board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
  #endif
 
+/* SATA */

include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101

Quoted 10 lines

include/configs/ls1043aqds.h +10 -0

+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA				(CONFIG_SYS_IMMR + 0x02200000)

I think this is becoming ad-hoc. Should we not
have all these SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?

include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 110

Quoted 10 lines

include/configs/ls1043aqds.h +6 -0

+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE		(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
+						CONFIG_SYS_SCSI_MAX_LUN)
+
  /*
   * IFC Definitions
   */

Also, isn't there SATA support in ls1043ardb ?

Regards
Sinan Akman

Yuantian Tang

author

2015-12-01 06:16 UTC #2 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Sinan Akman,

Please see my explanation inline.

Quoted 132 lines

-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 19 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3 specification. This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
  arch/arm/cpu/armv8/fsl-layerscape/soc.c           | 43
+++++++++++++++++++++++
Quoted 2 lines
  arch/arm/include/asm/arch-fsl-layerscape/config.h | 18 ++++++++++
  arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 62 lines
  include/configs/ls1043aqds.h                      | 17 +++++++++
  4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +39 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
  #include <common.h>
  #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
  #include <asm/arch/soc.h>
  #include <asm/io.h>
  #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
  	erratum_a009203();
  }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
  #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void *)CONFIG_SYS_SATA;
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
  void fsl_lsch2_early_init_f(void)
  {
  	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
  #ifdef CONFIG_BOARD_LATE_INIT
  int board_late_init(void)
  {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
  	return 0;
  }
  #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
  #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)

Quoted 8 lines

arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here and
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same macro value : (CONFIG_SYS_IMMR + 0x02200000)

normally we put all those definitions in board specific head file.

but config.h is created for all layerscape ARMv8 board too which include ls2080a and ls1043a.
So I add those definitions here for ls2080a to avoid defining them in every ls2080a board head file. For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in board head file which is the normal way. It may lead to a little confusion, but I think it is acceptable.

Regards,
Yuantian

Quoted 98 lines

arch/arm/include/asm/arch-fsl-layerscape/config.h
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
  /* Generic Interrupt Controller Definitions */
  #define GICD_BASE			0x06000000
  #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
  #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
  #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
  #ifdef CONFIG_FSL_LSCH3
  void fsl_lsch3_early_init_f(void);
  #elif defined(CONFIG_FSL_LSCH2)
(quoted diff) +0 -0
b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
  #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void);
Quoted 13 lines
include/configs/ls1043aqds.h +11 -0
  #endif
 
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA				(CONFIG_SYS_IMMR
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these SYS_SATA
definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file.
Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 110
Quoted 5 lines
include/configs/ls1043aqds.h +1 -0
+
  /*
   * IFC Definitions
   */
Also, isn't there SATA support in ls1043ardb ?
Regards
Sinan Akman

Sinan Akman

2015-12-01 06:29 UTC #3 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Yuan

Quoted 144 lines · Yuantian Tang

Hi Sinan Akman,
Please see my explanation inline.
Quoted 132 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 19 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3 specification. This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
   arch/arm/cpu/armv8/fsl-layerscape/soc.c           | 43
+++++++++++++++++++++++
Quoted 2 lines
   arch/arm/include/asm/arch-fsl-layerscape/config.h | 18 ++++++++++
   arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 62 lines
   include/configs/ls1043aqds.h                      | 17 +++++++++
   4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +39 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
   #include <common.h>
   #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
   #include <asm/arch/soc.h>
   #include <asm/io.h>
   #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
   	erratum_a009203();
   }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
   #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void *)CONFIG_SYS_SATA;
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
   void fsl_lsch2_early_init_f(void)
   {
   	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
   #ifdef CONFIG_BOARD_LATE_INIT
   int board_late_init(void)
   {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
   	return 0;
   }
   #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
   #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 8 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here and
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same macro value : (CONFIG_SYS_IMMR + 0x02200000)

Quoted 5 lines

normally we put all those definitions in board specific head file.
but config.h is created for all layerscape ARMv8 board too which include ls2080a and ls1043a.
So I add those definitions here for ls2080a to avoid defining them in every ls2080a board head file. For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in board head file which is the normal way.

What is the real reason ls1043aqds is not using that config.h ? Regards
Sinan Akman

Quoted 107 lines

It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 93 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
   /* Generic Interrupt Controller Definitions */
   #define GICD_BASE			0x06000000
   #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
   #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
   #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
   #ifdef CONFIG_FSL_LSCH3
   void fsl_lsch3_early_init_f(void);
   #elif defined(CONFIG_FSL_LSCH2)
(quoted diff) +0 -0
b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
   #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void);
Quoted 13 lines
include/configs/ls1043aqds.h +11 -0
   #endif
 
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA				(CONFIG_SYS_IMMR
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these SYS_SATA
definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file.
Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 110
Quoted 5 lines
include/configs/ls1043aqds.h +1 -0
+
   /*
    * IFC Definitions
    */
Quoted 4 lines
Also, isn't there SATA support in ls1043ardb ?
Regards
Sinan Akman
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Yuantian Tang

author

2015-12-01 06:41 UTC #4 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Sinan Akman,

Quoted 162 lines

-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 2:29 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 21 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 17 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 4 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3
specification.
Quoted 115 lines
Quoted 115 lines
Quoted 15 lines
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
   arch/arm/cpu/armv8/fsl-layerscape/soc.c           | 43
+++++++++++++++++++++++
Quoted 2 lines
   arch/arm/include/asm/arch-fsl-layerscape/config.h | 18 ++++++++++
   arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 62 lines
   include/configs/ls1043aqds.h                      | 17 +++++++++
   4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +39 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
   #include <common.h>
   #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
   #include <asm/arch/soc.h>
   #include <asm/io.h>
   #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
   	erratum_a009203();
   }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
   #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void *)CONFIG_SYS_SATA;
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
   void fsl_lsch2_early_init_f(void)
   {
   	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
   #ifdef CONFIG_BOARD_LATE_INIT
   int board_late_init(void)
   {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
   	return 0;
   }
   #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
   #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 4 lines
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here
and
Quoted 2 lines
Quoted 2 lines
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same
macro
Quoted 4 lines
Quoted 2 lines
value : (CONFIG_SYS_IMMR + 0x02200000)
normally we put all those definitions in board specific head file.

Quoted 8 lines

Quoted 1 line
but config.h is created for all layerscape ARMv8 board too which include
ls2080a and ls1043a.
Quoted 1 line
So I add those definitions here for ls2080a to avoid defining them in every
ls2080a board head file.
Quoted 1 line
For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in
board head file which is the normal way.
What is the real reason ls1043aqds is not using that config.h ?

Config.h is used for both ls1043aqds and ls1043ardb. Since ls1043ardb doesn't support SATA, it's better to not use it.

Regards,
Yuantian

Quoted 111 lines

Regards
Sinan Akman
Quoted 68 lines
It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 63 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
   /* Generic Interrupt Controller Definitions */
   #define GICD_BASE			0x06000000
   #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
   #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
   #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
   #ifdef CONFIG_FSL_LSCH3
   void fsl_lsch3_early_init_f(void);
   #elif defined(CONFIG_FSL_LSCH2)
(quoted diff) +0 -0
b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
   #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
Quoted 2 lines
include/configs/ls1043aqds.h +0 -0
   #endif
 
Quoted 11 lines
Quoted 11 lines
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101
Quoted 11 lines
include/configs/ls1043aqds.h +11 -0
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA
(CONFIG_SYS_IMMR
Quoted 28 lines
Quoted 19 lines
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these
SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 110
Quoted 5 lines
include/configs/ls1043aqds.h +1 -0
+
   /*
    * IFC Definitions
    */
Quoted 4 lines
Also, isn't there SATA support in ls1043ardb ?
Regards
Sinan Akman
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Sinan Akman

2015-12-01 07:23 UTC #5 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Yuan

Quoted 165 lines · Yuantian Tang

Hi Sinan Akman,
Quoted 163 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 2:29 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 21 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 17 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 4 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3
specification.
Quoted 115 lines
Quoted 115 lines
Quoted 15 lines
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 43
+++++++++++++++++++++++
Quoted 2 lines
    arch/arm/include/asm/arch-fsl-layerscape/config.h | 18 ++++++++++
    arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 62 lines
    include/configs/ls1043aqds.h                      | 17 +++++++++
    4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +39 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
    #include <common.h>
    #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
    #include <asm/arch/soc.h>
    #include <asm/io.h>
    #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
    	erratum_a009203();
    }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
    #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void *)CONFIG_SYS_SATA;
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
    void fsl_lsch2_early_init_f(void)
    {
    	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
    #ifdef CONFIG_BOARD_LATE_INIT
    int board_late_init(void)
    {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
    	return 0;
    }
    #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
    #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 4 lines
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here
and
Quoted 2 lines
Quoted 2 lines
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same
macro
Quoted 5 lines
Quoted 2 lines
value : (CONFIG_SYS_IMMR + 0x02200000)
normally we put all those definitions in board specific head file.
but config.h is created for all layerscape ARMv8 board too which include

Quoted 8 lines

Quoted 7 lines
ls2080a and ls1043a.
Quoted 1 line
So I add those definitions here for ls2080a to avoid defining them in every
ls2080a board head file.
Quoted 1 line
For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in
board head file which is the normal way.
What is the real reason ls1043aqds is not using that config.h ?
Config.h is used for both ls1043aqds and ls1043ardb. Since ls1043ardb doesn't support SATA, it's better to not use it.

What I don't feel comfortable here is whether there is SATA support
or not can
be board specific but the SATA register addresses are not. They should be in SoC
or architecture specific include files. With your method if someone designs a new board
with ls1043 and wants to have SATA support this address has to be repeated in board
config file again. I suggest we keep a uniform approach. If config.h is not suitable for
ls1043 SoC then we should have another file defining SATA regs for ls1043. Each board can
then decide to initialize SATA based on a config value (e.g. CONFIG_FSL_SATA) for SATA
support which then can be included in the board specific config file in include/configs.
Take a look at T102xQDS.h vs T102xRDB.h as an example. Both could refer to CONFIG_SYS_MPC85xx_SATA1_ADDR but only T102xQDS is using SATA. So ls1043aqds vs ls1043rdb should be modeled similar to this. Does this make sense ?

Regards
Sinan Akman

Quoted 106 lines

Regards,
Yuantian
Quoted 103 lines
Regards
Sinan Akman
Quoted 68 lines
It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 63 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
    /* Generic Interrupt Controller Definitions */
    #define GICD_BASE			0x06000000
    #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
    #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
    #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
    #ifdef CONFIG_FSL_LSCH3
    void fsl_lsch3_early_init_f(void);
    #elif defined(CONFIG_FSL_LSCH2)
(quoted diff) +0 -0
b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
    #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
Quoted 2 lines
include/configs/ls1043aqds.h +0 -0
    #endif
 
Quoted 11 lines
Quoted 11 lines
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101
Quoted 11 lines
include/configs/ls1043aqds.h +11 -0
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA
(CONFIG_SYS_IMMR
Quoted 20 lines
Quoted 20 lines
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these
SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 6 lines
include/configs/ls1043aqds.h +1 -0
+
    /*
     * IFC Definitions
     */

Yuantian Tang

author

2015-12-01 08:36 UTC #6 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Sinan Akman,

Please see my explanation inline.

Quoted 194 lines

-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 3:24 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 56 lines · Yuantian Tang
Hi Sinan Akman,
Quoted 54 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 2:29 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 22 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 18 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape
ARMv8 board
Hi Yuan
Quoted 4 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3
specification.
Quoted 18 lines
Quoted 18 lines
Quoted 15 lines
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 43
+++++++++++++++++++++++
Quoted 1 line
    arch/arm/include/asm/arch-fsl-layerscape/config.h | 18
++++++++++
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 1 line
    arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 38 lines
    include/configs/ls1043aqds.h                      | 17 +++++++++
    4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +19 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
    #include <common.h>
    #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
    #include <asm/arch/soc.h>
    #include <asm/io.h>
    #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
    	erratum_a009203();
    }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c void fsl_lsch3_early_init_f(void)
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +7 -0
+}
+#endif
+
    #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void
*)CONFIG_SYS_SATA;
Quoted 66 lines
Quoted 66 lines
Quoted 49 lines
Quoted 49 lines
Quoted 16 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +13 -0
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
    void fsl_lsch2_early_init_f(void)
    {
    	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
    #ifdef CONFIG_BOARD_LATE_INIT
    int board_late_init(void)
    {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
    	return 0;
    }
    #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
    #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 4 lines
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here
and
Quoted 2 lines
Quoted 2 lines
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same
macro
Quoted 6 lines
Quoted 2 lines
value : (CONFIG_SYS_IMMR + 0x02200000)
normally we put all those definitions in board specific head file.
but config.h is created for all layerscape ARMv8 board too which include
ls2080a and ls1043a.
Quoted 2 lines
So I add those definitions here for ls2080a to avoid defining them in every
Quoted 7 lines
Quoted 6 lines
ls2080a board head file.
Quoted 2 lines
For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in
board head file which is the normal way.
What is the real reason ls1043aqds is not using that config.h ?
Config.h is used for both ls1043aqds and ls1043ardb. Since ls1043ardb
doesn't support SATA, it's better to not use it.

Quoted 16 lines

What I don't feel comfortable here is whether there is SATA support or not
can be board specific but the SATA register addresses are not. They should be in SoC or architecture specific include files. With your method if someone designs a new board with ls1043 and wants to have SATA support this address has to be repeated in board config file again. I suggest we keep a uniform approach. If config.h is not suitable for
ls1043 SoC then we should have another file defining SATA regs for ls1043. Each board can then decide to initialize SATA based on a config value (e.g. CONFIG_FSL_SATA) for SATA
support which then can be included in the board specific config file in include/configs.
Take a look at T102xQDS.h vs T102xRDB.h as an example. Both could refer to CONFIG_SYS_MPC85xx_SATA1_ADDR but only T102xQDS is using SATA. So ls1043aqds vs ls1043rdb should be modeled similar to this. Does this make sense ?

You may mix the SATA feature and SATA register.
Whether T102xRDB supports SATA is not decided by CONFIG_SYS_MPC85xx_SATA1_ADDR.

For ls1043a, the best place for CONFIG_SYS_SATA definition is at: immap_lsch2.h.
But I want to put all the SATA related definitions together. That's why I move it to config.h
For the rest of the micro definition which is to enable SATA feature, they should be placed in config.h or ls1043axxx.h. I think either file is acceptable.

If a new ls1043a board with SATA support, those micro should be defined like what we did on ls1043aqds.
It is not necessary to create a common file only for SATA since putting those SATA definition in board head file is a common way and no any drawback.

Regards,
Yuantian

Quoted 109 lines

Regards
Sinan Akman
Quoted 106 lines
Regards,
Yuantian
Quoted 103 lines
Regards
Sinan Akman
Quoted 68 lines
It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 63 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
    /* Generic Interrupt Controller Definitions */
    #define GICD_BASE			0x06000000
    #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
    #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
    #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
    #ifdef CONFIG_FSL_LSCH3
    void fsl_lsch3_early_init_f(void);
    #elif defined(CONFIG_FSL_LSCH2) diff --git
a/include/configs/ls1043aqds.h b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
    #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
Quoted 2 lines
include/configs/ls1043aqds.h +0 -0
    #endif
 
Quoted 11 lines
Quoted 11 lines
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101
Quoted 11 lines
include/configs/ls1043aqds.h +11 -0
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA
(CONFIG_SYS_IMMR
Quoted 20 lines
Quoted 20 lines
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these
SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 6 lines
include/configs/ls1043aqds.h +1 -0
+
    /*
     * IFC Definitions
     */

Sinan Akman

2015-12-01 14:46 UTC #7 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi Yuan

Quoted 214 lines · Yuantian Tang

Hi Sinan Akman,
Please see my explanation inline.
Quoted 194 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 3:24 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 56 lines · Yuantian Tang
Hi Sinan Akman,
Quoted 54 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 2:29 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 22 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 18 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape
ARMv8 board
Hi Yuan
Quoted 4 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3
specification.
Quoted 18 lines
Quoted 18 lines
Quoted 15 lines
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 43
+++++++++++++++++++++++
Quoted 1 line
     arch/arm/include/asm/arch-fsl-layerscape/config.h | 18
++++++++++
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 1 line
     arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 38 lines
     include/configs/ls1043aqds.h                      | 17 +++++++++
     4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +19 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
     #include <common.h>
     #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
     #include <asm/arch/soc.h>
     #include <asm/io.h>
     #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
     	erratum_a009203();
     }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c void fsl_lsch3_early_init_f(void) · line 149
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +7 -0
+}
+#endif
+
     #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void
*)CONFIG_SYS_SATA;
Quoted 66 lines
Quoted 66 lines
Quoted 49 lines
Quoted 49 lines
Quoted 16 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +13 -0
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
     void fsl_lsch2_early_init_f(void)
     {
     	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
     #ifdef CONFIG_BOARD_LATE_INIT
     int board_late_init(void)
     {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
     	return 0;
     }
     #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
     #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 4 lines
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here
and
Quoted 2 lines
Quoted 2 lines
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same
macro
Quoted 6 lines
Quoted 2 lines
value : (CONFIG_SYS_IMMR + 0x02200000)
normally we put all those definitions in board specific head file.
but config.h is created for all layerscape ARMv8 board too which include
ls2080a and ls1043a.
Quoted 2 lines
So I add those definitions here for ls2080a to avoid defining them in every
Quoted 7 lines
Quoted 6 lines
ls2080a board head file.
Quoted 2 lines
For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in
board head file which is the normal way.
What is the real reason ls1043aqds is not using that config.h ?
Config.h is used for both ls1043aqds and ls1043ardb. Since ls1043ardb
doesn't support SATA, it's better to not use it.
Quoted 16 lines
What I don't feel comfortable here is whether there is SATA support or not
can be board specific but the SATA register addresses are not. They should be in SoC or architecture specific include files. With your method if someone designs a new board with ls1043 and wants to have SATA support this address has to be repeated in board config file again. I suggest we keep a uniform approach. If config.h is not suitable for
ls1043 SoC then we should have another file defining SATA regs for ls1043. Each board can then decide to initialize SATA based on a config value (e.g. CONFIG_FSL_SATA) for SATA
support which then can be included in the board specific config file in include/configs.
Take a look at T102xQDS.h vs T102xRDB.h as an example. Both could refer to CONFIG_SYS_MPC85xx_SATA1_ADDR but only T102xQDS is using SATA. So ls1043aqds vs ls1043rdb should be modeled similar to this. Does this make sense ?

Quoted 1 line

You may mix the SATA feature and SATA register.

No I am mixing the two.

Quoted 1 line

Whether T102xRDB supports SATA is not decided by CONFIG_SYS_MPC85xx_SATA1_ADDR.

I am aware of this and therefore I referred to CONFIG_FSL_SATA
while explaining above,
please reread it if you prefer.

Quoted 3 lines

For ls1043a, the best place for CONFIG_SYS_SATA definition is at: immap_lsch2.h. But I want to put all the SATA related definitions together. That's why I move it to config.h

And I can see yet you didn't put SATA definition for ls1043 in that
file. If you did this I'd see no
problem. I agree all the SATA related definitions should be together and in this case it is not.

Quoted 1 line

For the rest of the micro definition which is to enable SATA feature, they should be placed in config.h or ls1043axxx.h. I think either file is acceptable.

I think the very deciding factor here is, config.h which is under
SoC or architecture specific
directory should contain SoC or architecture specific definitions. The SATA registers addresses
are SoC specific so they are expected to be there. On the other hand, e.g ls1043axxx.h or
any other board specific config files should have board specific config definitions. Otherwise
why would we have two separate files. So the fact that <boardname>.h should have particular
definitions for this board only and SATA register addresses are not board registers make your
approach not correct. It might be acceptable but not the cleanest thing to do.

Quoted 2 lines

If a new ls1043a board with SATA support, those micro should be defined like what we did on ls1043aqds.

As explained this is not what I see in the other boards I gave as an
example.

Quoted 1 line

It is not necessary to create a common file only for SATA since putting those SATA definition in board head file is a common way and no any drawback.

This is exactly my point that having CONFIG_SYS_IMMR + 0x02200000 in
every board
specific file is error prone and not the correct thing to do. You could have e.g

#define CONFIG_SYS_SATA CONFIG_SYS_SATA1

in a <board_name>.h file and use that CONFIG_SYS_SATA1 file from a common header file which is defined specific to SoC or architecture.

Yuan, I think I explained my points several times here clearly but
it seems that you already made up your mind and you don't see any issue having SATA register values unnecessarily repeated for each board that supports SATA.

I don't agree with this but I see no point of discussing any further.
You can go ahead and implement it the way you prefer.

Regards
Sinan Akman

Quoted 118 lines

Regards,
Yuantian
Quoted 109 lines
Regards
Sinan Akman
Quoted 106 lines
Regards,
Yuantian
Quoted 103 lines
Regards
Sinan Akman
Quoted 68 lines
It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 63 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 72
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
     /* Generic Interrupt Controller Definitions */
     #define GICD_BASE			0x06000000
     #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
     #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
     #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
     #ifdef CONFIG_FSL_LSCH3
     void fsl_lsch3_early_init_f(void);
     #elif defined(CONFIG_FSL_LSCH2) diff --git
a/include/configs/ls1043aqds.h b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
     #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
Quoted 2 lines
include/configs/ls1043aqds.h +0 -0
     #endif
 
Quoted 11 lines
Quoted 11 lines
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101
Quoted 11 lines
include/configs/ls1043aqds.h +11 -0
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA
(CONFIG_SYS_IMMR
Quoted 20 lines
Quoted 20 lines
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these
SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 6 lines
include/configs/ls1043aqds.h +1 -0
+
     /*
      * IFC Definitions
      */
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

York Sun

2015-12-01 16:44 UTC #8 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Sinan,

Thanks for your review.

Yuantian,

You are right about putting things together. We have some macros in wrong places, including config.h. If you can, please try to move them into proper files, such as immap_lsch3.h. Enabling SATA should be done at board level because it depends on the board to have physical connection. The features to enable file system and commands can stay in board file as well.

York

Quoted 396 lines · Sinan Akman

Hi Yuan
Quoted 214 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 194 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 3:24 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 56 lines · Yuantian Tang
Hi Sinan Akman,
Quoted 54 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 2:29 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 22 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 18 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape
ARMv8 board
Hi Yuan
Quoted 4 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3
specification.
Quoted 18 lines
Quoted 18 lines
Quoted 15 lines
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 43
+++++++++++++++++++++++
Quoted 1 line
     arch/arm/include/asm/arch-fsl-layerscape/config.h | 18
++++++++++
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 1 line
     arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 38 lines
     include/configs/ls1043aqds.h                      | 17 +++++++++
     4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +19 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
     #include <common.h>
     #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
     #include <asm/arch/soc.h>
     #include <asm/io.h>
     #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
     	erratum_a009203();
     }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c void fsl_lsch3_early_init_f(void) · line 149
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +7 -0
+}
+#endif
+
     #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void
*)CONFIG_SYS_SATA;
Quoted 66 lines
Quoted 66 lines
Quoted 49 lines
Quoted 49 lines
Quoted 16 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +13 -0
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
     void fsl_lsch2_early_init_f(void)
     {
     	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
     #ifdef CONFIG_BOARD_LATE_INIT
     int board_late_init(void)
     {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
     	return 0;
     }
     #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
     #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 4 lines
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here
and
Quoted 2 lines
Quoted 2 lines
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same
macro
Quoted 6 lines
Quoted 2 lines
value : (CONFIG_SYS_IMMR + 0x02200000)
normally we put all those definitions in board specific head file.
but config.h is created for all layerscape ARMv8 board too which include
ls2080a and ls1043a.
Quoted 2 lines
So I add those definitions here for ls2080a to avoid defining them in every
Quoted 7 lines
Quoted 6 lines
ls2080a board head file.
Quoted 2 lines
For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in
board head file which is the normal way.
What is the real reason ls1043aqds is not using that config.h ?
Config.h is used for both ls1043aqds and ls1043ardb. Since ls1043ardb
doesn't support SATA, it's better to not use it.
Quoted 16 lines
What I don't feel comfortable here is whether there is SATA support or not
can be board specific but the SATA register addresses are not. They should be in SoC or architecture specific include files. With your method if someone designs a new board with ls1043 and wants to have SATA support this address has to be repeated in board config file again. I suggest we keep a uniform approach. If config.h is not suitable for
ls1043 SoC then we should have another file defining SATA regs for ls1043. Each board can then decide to initialize SATA based on a config value (e.g. CONFIG_FSL_SATA) for SATA
support which then can be included in the board specific config file in include/configs.
Take a look at T102xQDS.h vs T102xRDB.h as an example. Both could refer to CONFIG_SYS_MPC85xx_SATA1_ADDR but only T102xQDS is using SATA. So ls1043aqds vs ls1043rdb should be modeled similar to this. Does this make sense ?
Quoted 1 line
You may mix the SATA feature and SATA register.
No I am mixing the two.
Quoted 1 line
Whether T102xRDB supports SATA is not decided by CONFIG_SYS_MPC85xx_SATA1_ADDR.
I am aware of this and therefore I referred to CONFIG_FSL_SATA
while explaining above,
please reread it if you prefer.
Quoted 3 lines
For ls1043a, the best place for CONFIG_SYS_SATA definition is at: immap_lsch2.h. But I want to put all the SATA related definitions together. That's why I move it to config.h
And I can see yet you didn't put SATA definition for ls1043 in that
file. If you did this I'd see no
problem. I agree all the SATA related definitions should be together and in this case it is not.
Quoted 1 line
For the rest of the micro definition which is to enable SATA feature, they should be placed in config.h or ls1043axxx.h. I think either file is acceptable.
I think the very deciding factor here is, config.h which is under
SoC or architecture specific
directory should contain SoC or architecture specific definitions. The SATA registers addresses
are SoC specific so they are expected to be there. On the other hand, e.g ls1043axxx.h or
any other board specific config files should have board specific config definitions. Otherwise
why would we have two separate files. So the fact that <boardname>.h should have particular
definitions for this board only and SATA register addresses are not board registers make your
approach not correct. It might be acceptable but not the cleanest thing to do.
Quoted 2 lines
If a new ls1043a board with SATA support, those micro should be defined like what we did on ls1043aqds.
As explained this is not what I see in the other boards I gave as an
example.
Quoted 1 line
It is not necessary to create a common file only for SATA since putting those SATA definition in board head file is a common way and no any drawback.
This is exactly my point that having CONFIG_SYS_IMMR + 0x02200000 in
every board
specific file is error prone and not the correct thing to do. You could have e.g
#define CONFIG_SYS_SATA CONFIG_SYS_SATA1
in a <board_name>.h file and use that CONFIG_SYS_SATA1 file from a common header file which is defined specific to SoC or architecture.
Yuan, I think I explained my points several times here clearly but
it seems that you already made up your mind and you don't see any issue having SATA register values unnecessarily repeated for each board that supports SATA.
I don't agree with this but I see no point of discussing any further.
You can go ahead and implement it the way you prefer.
Regards
Sinan Akman
Quoted 118 lines
Regards,
Yuantian
Quoted 109 lines
Regards
Sinan Akman
Quoted 106 lines
Regards,
Yuantian
Quoted 103 lines
Regards
Sinan Akman
Quoted 68 lines
It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 63 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 72
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
     /* Generic Interrupt Controller Definitions */
     #define GICD_BASE			0x06000000
     #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
     #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
     #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
     #ifdef CONFIG_FSL_LSCH3
     void fsl_lsch3_early_init_f(void);
     #elif defined(CONFIG_FSL_LSCH2) diff --git
a/include/configs/ls1043aqds.h b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
     #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
Quoted 2 lines
include/configs/ls1043aqds.h +0 -0
     #endif
 
Quoted 11 lines
Quoted 11 lines
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101
Quoted 11 lines
include/configs/ls1043aqds.h +11 -0
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA
(CONFIG_SYS_IMMR
Quoted 20 lines
Quoted 20 lines
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these
SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 6 lines
include/configs/ls1043aqds.h +1 -0
+
     /*
      * IFC Definitions
      */
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Sinan Akman

2015-12-01 16:54 UTC #9 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Hi York

Quoted 10 lines · York Sun

Sinan,
Thanks for your review.
Yuantian,
You are right about putting things together. We have some macros in wrong places, including config.h. If you can, please try to move them into proper files, such as immap_lsch3.h. Enabling SATA should be done at board level because it depends on the board to have physical connection.

Just to make this clear one final time, I am not against enabling
SATA at the board level but
my suggestion is to define the SATA register values (which is *not* board dependent) in the
soc specific header file, then at the board level if SATA is not enabled we don't use it, if it is
enabled we use it and use the same register values across all boards with this SoC. That's all.

Regards
Sinan Akman

Quoted 401 lines

The features to
enable file system and commands can stay in board file as well.
York
Quoted 266 lines · Sinan Akman
Hi Yuan
Quoted 214 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 194 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 3:24 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 56 lines · Yuantian Tang
Hi Sinan Akman,
Quoted 54 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 2:29 PM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Hi Yuan
Quoted 22 lines · Yuantian Tang
Hi Sinan Akman,
Please see my explanation inline.
Quoted 18 lines
-----Original Message-----
From: Sinan Akman [mailto:sinan at writeme.com]
Sent: Tuesday, December 01, 2015 1:28 AM
To: Tang Yuantian-B29983 <[email protected]>; Sun York-R58495 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape
ARMv8 board
Hi Yuan
Quoted 4 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Freescale ARM-based Layerscape contains a SATA controller which comply with the serial ATA 3.0 specification and the AHCI 1.3
specification.
Quoted 18 lines
Quoted 18 lines
Quoted 15 lines
This patch adds SATA feature on ls2080aqds, ls2080ardb and ls1043aqds boards.
Signed-off-by Tang Yuantian <[email protected]>
---
v4:
- rebase to lastest git tree
- add another ARMv8 platform which is ls1043aqds
v3:
- rename ls2085a to ls2080a
- rebase to the latest git tree
- replace the magic number with micro variable
v2:
- rebase to the latest git tree
Quoted 1 line
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 43
+++++++++++++++++++++++
Quoted 1 line
      arch/arm/include/asm/arch-fsl-layerscape/config.h | 18
++++++++++
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 40 lines
Quoted 1 line
      arch/arm/include/asm/arch-fsl-layerscape/soc.h    | 31
++++++++++++++++
Quoted 38 lines
      include/configs/ls1043aqds.h                      | 17 +++++++++
      4 files changed, 109 insertions(+)
arch/arm/cpu/armv8/fsl-layerscape/soc.c +19 -0
index 8896b70..574ffc4 100644
@@ -6,6 +6,8 @@
 
      #include <common.h>
      #include <fsl_ifc.h>
+#include <ahci.h>
+#include <scsi.h>
      #include <asm/arch/soc.h>
      #include <asm/io.h>
      #include <asm/global_data.h>
@@ -120,7 +122,44 @@ void fsl_lsch3_early_init_f(void)
      	erratum_a009203();
      }
 
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci;
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA2;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ccsr_ahci  = (void *)CONFIG_SYS_SATA1;
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA1);
+	scsi_scan(0);
+
+	return 0;
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c void fsl_lsch3_early_init_f(void) · line 149
Quoted 8 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +7 -0
+}
+#endif
+
      #elif defined(CONFIG_LS1043A)
+#ifdef CONFIG_SCSI_AHCI_PLAT
+int sata_init(void)
+{
+	struct ccsr_ahci __iomem *ccsr_ahci = (void
*)CONFIG_SYS_SATA;
Quoted 66 lines
Quoted 66 lines
Quoted 49 lines
Quoted 49 lines
Quoted 16 lines
arch/arm/cpu/armv8/fsl-layerscape/soc.c +13 -0
+
+	out_le32(&ccsr_ahci->ppcfg, AHCI_PORT_PHY_1_CFG);
+	out_le32(&ccsr_ahci->pp2c, AHCI_PORT_PHY_2_CFG);
+	out_le32(&ccsr_ahci->pp3c, AHCI_PORT_PHY_3_CFG);
+	out_le32(&ccsr_ahci->ptc, AHCI_PORT_TRANS_CFG);
+
+	ahci_init((void __iomem *)CONFIG_SYS_SATA);
+	scsi_scan(0);
+
+	return 0;
+}
+#endif
+
      void fsl_lsch2_early_init_f(void)
      {
      	struct ccsr_cci400 *cci = (struct ccsr_cci400
Quoted 1 line
*)CONFIG_SYS_CCI400_ADDR; @@ -141,6 +180,10 @@ void
fsl_lsch2_early_init_f(void)
Quoted 20 lines
      #ifdef CONFIG_BOARD_LATE_INIT
      int board_late_init(void)
      {
arch/arm/cpu/armv8/fsl-layerscape/soc.c +4 -0
+#ifdef CONFIG_SCSI_AHCI_PLAT
+	sata_init();
+#endif
+
      	return 0;
      }
      #endif
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
index b5a2d28..be3acc3 100644
@@ -54,6 +54,24 @@
 
      #define CONFIG_SYS_MEMAC_LITTLE_ENDIAN
 
+/* SATA */
arch/arm/include/asm/arch-fsl-layerscape/config.h line 67
Quoted 10 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +10 -0
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA1			(CONFIG_SYS_IMMR
+ 0x02200000)
Quoted 4 lines
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 68
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+#define CONFIG_SYS_SATA2			(CONFIG_SYS_IMMR
+ 0x02210000)
Why do we have CONFIG_SYS_SATA1 and CONFIG_SYS_SATA2 here
and
Quoted 2 lines
Quoted 2 lines
then CONFIG_SYS_SATA in another file (see later below)? CONFIG_SYS_SATA1 and CONFIG_SYS_SATA seem to have the same
macro
Quoted 6 lines
Quoted 2 lines
value : (CONFIG_SYS_IMMR + 0x02200000)
normally we put all those definitions in board specific head file.
but config.h is created for all layerscape ARMv8 board too which include
ls2080a and ls1043a.
Quoted 2 lines
So I add those definitions here for ls2080a to avoid defining them in every
Quoted 7 lines
Quoted 6 lines
ls2080a board head file.
Quoted 2 lines
For ls1043a, only ls1043aqds supports SATA, so, I put all those definitions in
board head file which is the normal way.
What is the real reason ls1043aqds is not using that config.h ?
Config.h is used for both ls1043aqds and ls1043ardb. Since ls1043ardb
doesn't support SATA, it's better to not use it.
Quoted 16 lines
What I don't feel comfortable here is whether there is SATA support or not
can be board specific but the SATA register addresses are not. They should be in SoC or architecture specific include files. With your method if someone designs a new board with ls1043 and wants to have SATA support this address has to be repeated in board config file again. I suggest we keep a uniform approach. If config.h is not suitable for
ls1043 SoC then we should have another file defining SATA regs for ls1043. Each board can then decide to initialize SATA based on a config value (e.g. CONFIG_FSL_SATA) for SATA
support which then can be included in the board specific config file in include/configs.
Take a look at T102xQDS.h vs T102xRDB.h as an example. Both could refer to CONFIG_SYS_MPC85xx_SATA1_ADDR but only T102xQDS is using SATA. So ls1043aqds vs ls1043rdb should be modeled similar to this. Does this make sense ?
Quoted 1 line
You may mix the SATA feature and SATA register.
No I am mixing the two.
Quoted 1 line
Whether T102xRDB supports SATA is not decided by CONFIG_SYS_MPC85xx_SATA1_ADDR.
I am aware of this and therefore I referred to CONFIG_FSL_SATA
while explaining above,
please reread it if you prefer.
Quoted 2 lines
For ls1043a, the best place for CONFIG_SYS_SATA definition is at: immap_lsch2.h. But I want to put all the SATA related definitions together. That's why I move it to config.h
And I can see yet you didn't put SATA definition for ls1043 in that
file. If you did this I'd see no
problem. I agree all the SATA related definitions should be together and in this case it is not.
Quoted 1 line
For the rest of the micro definition which is to enable SATA feature, they should be placed in config.h or ls1043axxx.h. I think either file is acceptable.
I think the very deciding factor here is, config.h which is under
SoC or architecture specific
directory should contain SoC or architecture specific definitions. The SATA registers addresses
are SoC specific so they are expected to be there. On the other hand, e.g ls1043axxx.h or
any other board specific config files should have board specific config definitions. Otherwise
why would we have two separate files. So the fact that <boardname>.h should have particular
definitions for this board only and SATA register addresses are not board registers make your
approach not correct. It might be acceptable but not the cleanest thing to do.
Quoted 1 line
If a new ls1043a board with SATA support, those micro should be defined like what we did on ls1043aqds.
As explained this is not what I see in the other boards I gave as an
example.
Quoted 1 line
It is not necessary to create a common file only for SATA since putting those SATA definition in board head file is a common way and no any drawback.
    This is exactly my point that having CONFIG_SYS_IMMR + 0x02200000 in
every board
specific file is error prone and not the correct thing to do. You could have e.g
#define CONFIG_SYS_SATA CONFIG_SYS_SATA1
in a <board_name>.h file and use that CONFIG_SYS_SATA1 file from a common header file which is defined specific to SoC or architecture.
Yuan, I think I explained my points several times here clearly but
it seems that you already made up your mind and you don't see any issue having SATA register values unnecessarily repeated for each board that supports SATA.
Quoted 123 lines
I don't agree with this but I see no point of discussing any further.
You can go ahead and implement it the way you prefer.
Regards
Sinan Akman
Quoted 117 lines
Regards,
Yuantian
Quoted 109 lines
Regards
Sinan Akman
Quoted 106 lines
Regards,
Yuantian
Quoted 103 lines
Regards
Sinan Akman
Quoted 68 lines
It may lead to a little confusion, but I think it is acceptable.
Regards,
Yuantian
Quoted 63 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h line 72
Quoted 4 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
arch/arm/include/asm/arch-fsl-layerscape/config.h line 73
Quoted 1 line
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 52 lines
arch/arm/include/asm/arch-fsl-layerscape/config.h +1 -0
+
      /* Generic Interrupt Controller Definitions */
      #define GICD_BASE			0x06000000
      #define GICR_BASE			0x06100000
arch/arm/include/asm/arch-fsl-layerscape/soc.h +31 -0
index 504c1f9..83186d6 100644
@@ -51,6 +51,37 @@ struct cpu_type {
      #define SVR_SOC_VER(svr)	(((svr) >> 8) & SVR_WO_E)
      #define IS_E_PROCESSOR(svr)	(!((svr >> 8) & 0x1))
 
+/* ahci port register default value */
+#define AHCI_PORT_PHY_1_CFG    0xa003fffe
+#define AHCI_PORT_PHY_2_CFG    0x28184d1f
+#define AHCI_PORT_PHY_3_CFG    0x0e081509
+#define AHCI_PORT_TRANS_CFG    0x08000025
+
+/* AHCI (sata) register map */
+struct ccsr_ahci {
+	u32 res1[0xa4/4];	/* 0x0 - 0xa4 */
+	u32 pcfg;	/* port config */
+	u32 ppcfg;	/* port phy1 config */
+	u32 pp2c;	/* port phy2 config */
+	u32 pp3c;	/* port phy3 config */
+	u32 pp4c;	/* port phy4 config */
+	u32 pp5c;	/* port phy5 config */
+	u32 axicc;	/* AXI cache control */
+	u32 paxic;	/* port AXI config */
+	u32 axipc;	/* AXI PROT control */
+	u32 ptc;	/* port Trans Config */
+	u32 pts;	/* port Trans Status */
+	u32 plc;	/* port link config */
+	u32 plc1;	/* port link config1 */
+	u32 plc2;	/* port link config2 */
+	u32 pls;	/* port link status */
+	u32 pls1;	/* port link status1 */
+	u32 pcmdc;	/* port CMD config */
+	u32 ppcs;	/* port phy control status */
+	u32 pberr;	/* port 0/1 BIST error */
+	u32 cmds;	/* port 0/1 CMD status error */
+};
+
      #ifdef CONFIG_FSL_LSCH3
      void fsl_lsch3_early_init_f(void);
      #elif defined(CONFIG_FSL_LSCH2) diff --git
a/include/configs/ls1043aqds.h b/include/configs/ls1043aqds.h index 4aeb238..bd7af00 100644
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 88
Quoted 1 line
include/configs/ls1043aqds.h +0 -0
@@ -88,6 +88,23 @@ unsigned long get_board_ddr_clk(void);
      #define CONFIG_SYS_FSL_PBL_RCW
board/freescale/ls1043aqds/ls1043aqds_rcw_sd_ifc.cfg
Quoted 2 lines
include/configs/ls1043aqds.h +0 -0
      #endif
 
Quoted 11 lines
Quoted 11 lines
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 101
Quoted 11 lines
include/configs/ls1043aqds.h +11 -0
+/* SATA */
+#define CONFIG_LIBATA
+#define CONFIG_SCSI_AHCI
+#define CONFIG_SCSI_AHCI_PLAT
+#define CONFIG_CMD_SCSI
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_DOS_PARTITION
+#define CONFIG_BOARD_LATE_INIT
+
+#define CONFIG_SYS_SATA
(CONFIG_SYS_IMMR
Quoted 20 lines
Quoted 20 lines
+ 0x02200000)
I think this is becoming ad-hoc. Should we not have all these
SYS_SATA definitions in the include files both for 2080a as well as ls1043 instead of throwing them in board specific config file. Is this something changes for each board ?
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 105
Quoted 4 lines
include/configs/ls1043aqds.h +4 -0
+
+#define CONFIG_SYS_SCSI_MAX_SCSI_ID		1
+#define CONFIG_SYS_SCSI_MAX_LUN			1
+#define CONFIG_SYS_SCSI_MAX_DEVICE
	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
include/configs/ls1043aqds.h unsigned long get_board_ddr_clk(void); · line 106
Quoted 1 line
include/configs/ls1043aqds.h +1 -0
+
CONFIG_SYS_SCSI_MAX_LUN)
Quoted 6 lines
include/configs/ls1043aqds.h +1 -0
+
      /*
       * IFC Definitions
       */
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

York Sun

2015-12-01 16:58 UTC #10 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

Quoted 15 lines · Sinan Akman

Hi York
Quoted 10 lines · York Sun
Sinan,
Thanks for your review.
Yuantian,
You are right about putting things together. We have some macros in wrong places, including config.h. If you can, please try to move them into proper files, such as immap_lsch3.h. Enabling SATA should be done at board level because it depends on the board to have physical connection.

Quoted 9 lines

Just to make this clear one final time, I am not against enabling
SATA at the board level but
my suggestion is to define the SATA register values (which is *not* board dependent) in the
soc specific header file, then at the board level if SATA is not enabled we don't use it, if it is
enabled we use it and use the same register values across all boards with this SoC. That's all.

Agreed.

York

Yuantian Tang

author

2015-12-02 02:42 UTC #11 lore raw

PATCH v4 1/1 arm: Add sata support on Layerscape ARMv8 board

OK, thanks.
I will rework this patch.

Regards,
Yuantian

Quoted 39 lines

-----Original Message-----
From: York Sun [mailto:yorksun at freescale.com]
Sent: Wednesday, December 02, 2015 12:59 AM
To: sinan at writeme.com; Tang Yuantian-B29983 <[email protected]>
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Quoted 14 lines · Sinan Akman
Hi York
Quoted 10 lines · York Sun
Sinan,
Thanks for your review.
Yuantian,
You are right about putting things together. We have some macros in wrong places, including config.h. If you can, please try to move them into proper files, such as immap_lsch3.h. Enabling SATA should be done at board level because it depends on the board to have physical
connection.
Quoted 8 lines
Just to make this clear one final time, I am not against enabling
SATA at the board level but my suggestion is to define the SATA register values (which is *not* board dependent) in the soc specific header file, then at the board level if SATA is not enabled we don't use it, if it is enabled we use it and use the same register values across all boards with this SoC. That's all.
Agreed.
York