[U-Boot] [PATCH v4] arm: Add sata support on Layerscape ARMv8 board
Sinan Akman
sinan at writeme.com
Tue Dec 1 08:23:56 CET 2015
Hi Yuan
On 01/12/15 01:41 AM, Yuantian Tang wrote:
> Hi Sinan Akman,
>
>> -----Original Message-----
>> From: Sinan Akman [mailto:sinan at writeme.com]
>> Sent: Tuesday, December 01, 2015 2:29 PM
>> To: Tang Yuantian-B29983 <Yuantian.Tang at freescale.com>; Sun York-R58495
>> <yorksun at freescale.com>
>> Cc: u-boot at 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:sinan at writeme.com]
>>>> Sent: Tuesday, December 01, 2015 1:28 AM
>>>> To: Tang Yuantian-B29983 <Yuantian.Tang at freescale.com>; Sun
>>>> York-R58495 <yorksun at freescale.com>
>>>> Cc: u-boot at 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.Tang at freescale.com wrote:
>>>>> From: Tang Yuantian <Yuantian.Tang at 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 at 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.
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
> 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
>>>>> */
>>>>>
>>>>>
More information about the U-Boot
mailing list