Hi Sinan Akman,
> -----Original Message-----
> From: Sinan Akman [mailto:si...@writeme.com]
> Sent: Tuesday, December 01, 2015 2:29 PM
> To: Tang Yuantian-B29983 <yuantian.t...@freescale.com>; Sun York-R58495
> <york...@freescale.com>
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape
> ARMv8 board
>
>
> Hi Yuan
>
> On 01/12/15 01:16 AM, Yuantian Tang wrote:
> > Hi Sinan Akman,
> >
> > Please see my explanation inline.
> >
> >> -----Original Message-----
> >> From: Sinan Akman [mailto:si...@writeme.com]
> >> Sent: Tuesday, December 01, 2015 1:28 AM
> >> To: Tang Yuantian-B29983 <yuantian.t...@freescale.com>; Sun
> >> York-R58495 <york...@freescale.com>
> >> Cc: u-boot@lists.denx.de
> >> Subject: Re: [U-Boot] [PATCH v4] arm: Add sata support on Layerscape
> >> ARMv8 board
> >>
> >>
> >> Hi Yuan
> >>
> >> On 30/11/15 02:44 AM, yuantian.t...@freescale.com wrote:
> >>> From: Tang Yuantian <yuantian.t...@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.t...@freescale.com>
> >>> ---
> >>> 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(+)
> >>>
> >>> 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/config.h
> >>> b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> >>> index b5a2d28..be3acc3 100644
> >>> --- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> >>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> >>> @@ -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)
> >>
> >> 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.
>
> 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
> Regards
> Sinan Akman
>
> > It may lead to a little confusion, but I think it is acceptable.
> >
> > Regards,
> > Yuantian
> >
> >>> +
> >>> +#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
> >>> 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..bd7af00 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
> (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 ?
> >>
> >>> +
> >>> +#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
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
