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

[Yuantian Tang]

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 <Yuantian.Tang at freescale.com>
> 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 <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>
> > ---
> > 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