From mboxrd@z Thu Jan 1 00:00:00 1970 From: York Sun Date: Mon, 7 Dec 2015 20:29:02 -0800 Subject: [U-Boot] [PATCH v5] arm: Add sata support on Layerscape ARMv8 board In-Reply-To: References: <1449026847-35951-1-git-send-email-Yuantian.Tang@freescale.com> <56606D67.2040307@freescale.com> <5661CC68.6030602@freescale.com> <5665B36B.9060709@freescale.com> Message-ID: <56665C8E.2020002@nxp.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 12/07/2015 07:04 PM, Tang Yuantian-B29983 wrote: > Hi York, > >> -----Original Message----- >> From: York Sun [mailto:yorksun at freescale.com] >> Sent: Tuesday, December 08, 2015 12:27 AM >> To: Tang Yuantian-B29983 >> Cc: u-boot at lists.denx.de; sinan at 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 at freescale.com] >>>> Sent: Saturday, December 05, 2015 1:25 AM >>>> To: Tang Yuantian-B29983 >>>> Cc: u-boot at lists.denx.de; sinan at 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 at freescale.com] >>>>>> Sent: Friday, December 04, 2015 12:27 AM >>>>>> To: Tang Yuantian-B29983 >>>>>> Cc: u-boot at lists.denx.de; sinan at writeme.com >>>>>> Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 >>>>>> board >>>>>> >>>>>> >>>>>> >>>>>> On 12/01/2015 07:27 PM, Yuantian.Tang at freescale.com wrote: >>>>>>> From: Tang Yuantian >>>>>>> >>>>>>> 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 >>>>>>> --- >>>>>>> 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 >>>>>>> #include >>>>>>> +#include >>>>>>> +#include >>>>>>> #include >>>>>>> #include >>>>>>> #include >>>>>>> @@ -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