On 12/07/2015 07:04 PM, Tang Yuantian-B29983 wrote: > Hi York, > >> -----Original Message----- >> From: York Sun [mailto:york...@freescale.com] >> Sent: Tuesday, December 08, 2015 12:27 AM >> To: Tang Yuantian-B29983 <yuantian.t...@freescale.com> >> Cc: u-boot@lists.denx.de; si...@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:york...@freescale.com] >>>> Sent: Saturday, December 05, 2015 1:25 AM >>>> To: Tang Yuantian-B29983 <yuantian.t...@freescale.com> >>>> Cc: u-boot@lists.denx.de; si...@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:york...@freescale.com] >>>>>> Sent: Friday, December 04, 2015 12:27 AM >>>>>> To: Tang Yuantian-B29983 <yuantian.t...@freescale.com> >>>>>> Cc: u-boot@lists.denx.de; si...@writeme.com >>>>>> Subject: Re: [PATCH v5] arm: Add sata support on Layerscape ARMv8 >>>>>> board >>>>>> >>>>>> >>>>>> >>>>>> On 12/01/2015 07:27 PM, 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> >>>>>>> --- >>>>>>> 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
