[U-Boot] [PATCH v5] arm: Add sata support on Layerscape ARMv8 board

From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com --- v5: - re-organize the code 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 ++++++++++++++++++++++ .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31 ++++++++++++++++ include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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 diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h index cd96604..91f3ce8 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h @@ -69,6 +69,10 @@ #define TZASC_REGION_ATTRIBUTES_0(x) ((TZASC1_BASE + (x * 0x10000)) + 0x110) #define TZASC_REGION_ID_ACCESS_0(x) ((TZASC1_BASE + (x * 0x10000)) + 0x114)
+/* SATA */ +#define AHCI_BASE_ADDR1 (CONFIG_SYS_IMMR + 0x02200000) +#define AHCI_BASE_ADDR2 (CONFIG_SYS_IMMR + 0x02210000) + /* PCIe */ #define CONFIG_SYS_PCIE1_ADDR (CONFIG_SYS_IMMR + 0x2400000) #define CONFIG_SYS_PCIE2_ADDR (CONFIG_SYS_IMMR + 0x2500000) diff --git a/arch/arm/include/asm/arch-fsl-layerscape/soc.h b/arch/arm/include/asm/arch-fsl-layerscape/soc.h index 504c1f9..83186d6 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h @@ -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..398f1c3 100644 --- a/include/configs/ls1043aqds.h +++ b/include/configs/ls1043aqds.h @@ -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 AHCI_BASE_ADDR + +#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 */ diff --git a/include/configs/ls2080aqds.h b/include/configs/ls2080aqds.h index b8bdf62..100c0ee 100644 --- a/include/configs/ls2080aqds.h +++ b/include/configs/ls2080aqds.h @@ -40,6 +40,24 @@ unsigned long get_board_ddr_clk(void); #endif #define CONFIG_FSL_DDR_BIST /* enable built-in memory test */
+/* 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 AHCI_BASE_ADDR1 +#define CONFIG_SYS_SATA2 AHCI_BASE_ADDR2 + +#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) + /* undefined CONFIG_FSL_DDR_SYNC_REFRESH for simulator */
#define CONFIG_SYS_NOR0_CSPR_EXT (0x0) diff --git a/include/configs/ls2080ardb.h b/include/configs/ls2080ardb.h index 65d4f64..75d98dc 100644 --- a/include/configs/ls2080ardb.h +++ b/include/configs/ls2080ardb.h @@ -42,6 +42,24 @@ unsigned long get_board_sys_clk(void); #endif #define CONFIG_FSL_DDR_BIST /* enable built-in memory test */
+/* 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 AHCI_BASE_ADDR1 +#define CONFIG_SYS_SATA2 AHCI_BASE_ADDR2 + +#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) + /* undefined CONFIG_FSL_DDR_SYNC_REFRESH for simulator */
#define CONFIG_SYS_NOR0_CSPR_EXT (0x0)

On 01/12/15 10:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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 ++++++++++++++++++++++ .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31 ++++++++++++++++ include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
Thanks for the update.
Reviewed-by: Sinan Akman sinan@writeme.com
Regards Sinan Akman

On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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 ++++++++++++++++++++++ .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31 ++++++++++++++++ include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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(); }
Yuantian,
Please help me understand below.
+#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
- 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);
You only call ahci_init() here but not above. Is SATA2 active?
- 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
York

Hi York,
Please see my explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Friday, December 04, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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
++++++++++++++++++++++
.../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31
++++++++++++++++
include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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(); }
Yuantian,
Please help me understand below.
+#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
Those settings of registers vary from soc to soc. If the default value will be used if the register is not updated explicitly. Speaking of this, I got new information from validation team about giving a new value to PTC register. So please hold this patch for a while, I will update it in next version.
- 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);
You only call ahci_init() here but not above. Is SATA2 active?
AHCI SATA driver only supports one SATA port. On ls2080a we have two ports, so we have to choice one. In this case I choice the first one which is SATA1.
Regards, Yuantian
- 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
York

On 12/03/2015 06:47 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see my explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Friday, December 04, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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
++++++++++++++++++++++
.../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31
++++++++++++++++
include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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(); }
Yuantian,
Please help me understand below.
+#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
Those settings of registers vary from soc to soc. If the default value will be used if the register is not updated explicitly.
If you put the macros for each SoC, you probably can use one function for all. You only want to keep them separated if they have not much in common.
Speaking of this, I got new information from validation team about giving a new value to PTC register. So please hold this patch for a while, I will update it in next version.
- 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);
You only call ahci_init() here but not above. Is SATA2 active?
AHCI SATA driver only supports one SATA port. On ls2080a we have two ports, so we have to choice one. In this case I choice the first one which is SATA1.
This should be put into comment, or README if you have one.
York

Hi York,
Please see explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Saturday, December 05, 2015 1:25 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/03/2015 06:47 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see my explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Friday, December 04, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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
++++++++++++++++++++++
.../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31
++++++++++++++++
include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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(); }
Yuantian,
Please help me understand below.
+#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
Those settings of registers vary from soc to soc. If the default value will be
used if the register is not updated explicitly.
If you put the macros for each SoC, you probably can use one function for all. You only want to keep them separated if they have not much in common.
I was trying to use one function for all, but I found separating them is better. Take ls1043a and ls2080a as an example, ls2080a has two controllers, while ls1043a has one. Ls2080a has two registers that need to be updated while ls1043a has four. A lot of #ifdef are needed if we unify them, not mention that in the future, changing one of the platforms' register will affect the other. Maybe I am not thinking it through. If you can give me more detail that viable, I can give a try.
Speaking of this, I got new information from validation team about giving a
new value to PTC register.
So please hold this patch for a while, I will update it in next version.
- 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);
You only call ahci_init() here but not above. Is SATA2 active?
AHCI SATA driver only supports one SATA port. On ls2080a we have two
ports, so we have to choice one. In this case I choice the first one which is SATA1.
This should be put into comment, or README if you have one.
This phenomenon is not LS platform specific, that's uboot's issue which needs another patch to fix. I think uboot know that and choice to not fix it because for uboot supporting two sata port is not that significant. It is just like that uboot doesn't support local sata and PCIe sata simutaniously.
If a comment is needed, it would be better to put it in our own README (in this case, ls2080a) document.
Regards, Yuantian
York

Hi Yuantian
On 06/12/15 10:09 PM, Yuantian Tang wrote:
Hi York,
Please see explanation inline. [...] I was trying to use one function for all, but I found separating them is better. Take ls1043a and ls2080a as an example, ls2080a has two controllers, while ls1043a has one. Ls2080a has two registers that need to be updated while ls1043a has four. A lot of #ifdef are needed if we unify them, not mention that in the future, changing one of the platforms' register will affect the other.
You might want to take into consideration that in the near future we will be moving this to dm. In that respect having all that in one file already will probably make things much easier. If you consider this, perhaps you will have a different view.
Maybe I am not thinking it through. If you can give me more detail that viable, I can give a try.
[...] ports, so we have to choice one. In this case I choice the first one which is SATA1.
This should be put into comment, or README if you have one.
This phenomenon is not LS platform specific, that's uboot's issue which needs another patch to fix. I think uboot know that and choice to not fix it because for uboot supporting two sata port is not that significant.
Again, with dm and reading all the hardware properties from device tree will also change this. If both device nodes are enabled we will have to support both as long as there is no hardware limitation. So I think there is no reason why having both SATA and PCIe would not be significant. It is just that the current implementation has this limitation and there is already some timeline for removing these limitations.
Regards Sinan Akman

Hi Sinan,
-----Original Message----- From: Sinan Akman [mailto:sinan@writeme.com] Sent: Monday, December 07, 2015 2:04 PM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com; Sun York-R58495 yorksun@freescale.com Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
Hi Yuantian
On 06/12/15 10:09 PM, Yuantian Tang wrote:
Hi York,
Please see explanation inline. [...] I was trying to use one function for all, but I found separating them is
better.
Take ls1043a and ls2080a as an example, ls2080a has two controllers, while
ls1043a has one.
Ls2080a has two registers that need to be updated while ls1043a has four. A lot of #ifdef are needed if we unify them, not mention that in the future,
changing one of the platforms' register will affect the other.
You might want to take into consideration that in the near future we will be
moving this to dm. In that respect having all that in one file already will probably make things much easier. If you consider this, perhaps you will have a different view.
They are in the same file but different functions.
Maybe I am not thinking it through. If you can give me more detail that
viable, I can give a try.
[...] ports, so we have to choice one. In this case I choice the first one which is SATA1.
This should be put into comment, or README if you have one.
This phenomenon is not LS platform specific, that's uboot's issue which
needs another patch to fix.
I think uboot know that and choice to not fix it because for uboot
supporting two sata port is not that significant.
Again, with dm and reading all the hardware properties from device tree will also change this. If both device nodes are enabled we will have to support both as long as there is no hardware limitation. So I think there is no reason why having both SATA and PCIe would not be significant. It is just that the current implementation has this limitation and there is already some timeline for removing these limitations.
I am not seeing what we are arguing here? Are we talking about if this limitation is important? Please point out what's wrong with this patch.
Regards, Yuantian
Regards Sinan Akman

Hi Yuantian
On 07/12/15 02:04 AM, Yuantian Tang wrote:
[...] They are in the same file but different functions.
OK
I think uboot know that and choice to not fix it because for uboot
supporting two sata port is not that significant. [...] Please point out what's wrong with this patch.
I only expressed my opinion on your view for u-boot's assumption on supporting two ports. Please don't take it personal.
Regards Sinan Akman

On 12/06/2015 07:09 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Saturday, December 05, 2015 1:25 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/03/2015 06:47 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see my explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Friday, December 04, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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
++++++++++++++++++++++
.../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31
++++++++++++++++
include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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(); }
Yuantian,
Please help me understand below.
+#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
Those settings of registers vary from soc to soc. If the default value will be
used if the register is not updated explicitly.
If you put the macros for each SoC, you probably can use one function for all. You only want to keep them separated if they have not much in common.
I was trying to use one function for all, but I found separating them is better. Take ls1043a and ls2080a as an example, ls2080a has two controllers, while ls1043a has one. Ls2080a has two registers that need to be updated while ls1043a has four. A lot of #ifdef are needed if we unify them, not mention that in the future, changing one of the platforms' register will affect the other. Maybe I am not thinking it through. If you can give me more detail that viable, I can give a try.
Yuantian,
I was thinking to set all registers, including those with default values. Then you can use one function for both. My understand is LS1043 and LS2080 has different default value. It will be easier to update the macros if you need different values, than changing the functions. If we have a new SoC in the same family, you don't have to add another function.
Try it to see if you still have to separate them.
York

Hi York,
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Tuesday, December 08, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/06/2015 07:09 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Saturday, December 05, 2015 1:25 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/03/2015 06:47 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see my explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Friday, December 04, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote:
From: Tang Yuantian Yuantian.Tang@freescale.com
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 Yuantian.Tang@freescale.com
v5:
- re-organize the code
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
++++++++++++++++++++++
.../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31
++++++++++++++++
include/configs/ls1043aqds.h | 17 +++++++++ include/configs/ls2080aqds.h | 18 +++++++++ include/configs/ls2080ardb.h | 18 +++++++++ 6 files changed, 131 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 8896b70..574ffc4 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -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(); }
Yuantian,
Please help me understand below.
+#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
Those settings of registers vary from soc to soc. If the default value will be
used if the register is not updated explicitly.
If you put the macros for each SoC, you probably can use one function for
all.
You only want to keep them separated if they have not much in common.
I was trying to use one function for all, but I found separating them is
better.
Take ls1043a and ls2080a as an example, ls2080a has two controllers, while
ls1043a has one.
Ls2080a has two registers that need to be updated while ls1043a has four. A lot of #ifdef are needed if we unify them, not mention that in the future,
changing one of the platforms' register will affect the other.
Maybe I am not thinking it through. If you can give me more detail that
viable, I can give a try.
Yuantian,
I was thinking to set all registers, including those with default values. Then you can use one function for both. My understand is LS1043 and LS2080 has different default value. It will be easier to update the macros if you need different values, than changing the functions. If we have a new SoC in the same family, you don't have to add another function.
Try it to see if you still have to separate them.
I didn't see any benefit this way. We have 20 registers to set for each soc in this way. In order to use one function, we have to define 20 micro for each soc too. If the soc's version is upgraded, we may have to redefine the value if the default value is changed. All what we do is just to make it in one function. Currently, we just need to set the register that requires to change which only have 4 and 2 registers respectively.
I thank your suggestion complicates things.
Regards, Yuantian
York

On 12/07/2015 07:04 PM, Tang Yuantian-B29983 wrote:
Hi York,
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Tuesday, December 08, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/06/2015 07:09 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Saturday, December 05, 2015 1:25 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/03/2015 06:47 PM, Tang Yuantian-B29983 wrote:
Hi York,
Please see my explanation inline.
-----Original Message----- From: York Sun [mailto:yorksun@freescale.com] Sent: Friday, December 04, 2015 12:27 AM To: Tang Yuantian-B29983 Yuantian.Tang@freescale.com Cc: u-boot@lists.denx.de; sinan@writeme.com Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 board
On 12/01/2015 07:27 PM, Yuantian.Tang@freescale.com wrote: > From: Tang Yuantian Yuantian.Tang@freescale.com > > 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 Yuantian.Tang@freescale.com > --- > v5: > - re-organize the code > 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 ++++++++++++++++++++++ > .../include/asm/arch-fsl-layerscape/immap_lsch3.h | 4 ++ > arch/arm/include/asm/arch-fsl-layerscape/soc.h | 31 ++++++++++++++++ > include/configs/ls1043aqds.h | 17 +++++++++ > include/configs/ls2080aqds.h | 18 +++++++++ > include/configs/ls2080ardb.h | 18 +++++++++ > 6 files changed, 131 insertions(+) > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c > b/arch/arm/cpu/armv8/fsl-layerscape/soc.c > index 8896b70..574ffc4 100644 > --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c > +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c > @@ -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(); > } >
Yuantian,
Please help me understand below.
> +#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);
You didn't set pp2c or pp3c. Is it because the default values are OK or something else?
Those settings of registers vary from soc to soc. If the default value will be
used if the register is not updated explicitly.
If you put the macros for each SoC, you probably can use one function for
all.
You only want to keep them separated if they have not much in common.
I was trying to use one function for all, but I found separating them is
better.
Take ls1043a and ls2080a as an example, ls2080a has two controllers, while
ls1043a has one.
Ls2080a has two registers that need to be updated while ls1043a has four. A lot of #ifdef are needed if we unify them, not mention that in the future,
changing one of the platforms' register will affect the other.
Maybe I am not thinking it through. If you can give me more detail that
viable, I can give a try.
Yuantian,
I was thinking to set all registers, including those with default values. Then you can use one function for both. My understand is LS1043 and LS2080 has different default value. It will be easier to update the macros if you need different values, than changing the functions. If we have a new SoC in the same family, you don't have to add another function.
Try it to see if you still have to separate them.
I didn't see any benefit this way. We have 20 registers to set for each soc in this way. In order to use one function, we have to define 20 micro for each soc too.
20 registers? I didn't see that coming. In that case, you can keep it your way.
York
participants (5)
-
Sinan Akman
-
York Sun
-
York Sun
-
Yuantian Tang
-
Yuantian.Tangļ¼ freescale.com