[U-Boot] [PATCH v2] arm/ls1021a: Add sata support on qds and twr board

[York Sun]

The soc workarounds belongs to soc file, not board file, so you don't have to copy the code to every board. Our boards are not the only boards with this SoC.

York


-------- Original message --------
From: Tang Yuantian-B29983
Date:08/18/2015 20:34 (GMT-08:00)
To: Sun York-R58495
Cc: u-boot at lists.denx.de, Kushwaha Prabhakar-B32579 , Wang Huan-B18965
Subject: RE: [PATCH v2] arm/ls1021a: Add sata support on qds and twr board



From: Sun York-R58495
Sent: Wednesday, August 19, 2015 10:42 AM
To: Tang Yuantian-B29983; Sun York-R58495
Cc: u-boot at lists.denx.de; Kushwaha Prabhakar-B32579; Wang Huan-B18965
Subject: RE: [PATCH v2] arm/ls1021a: Add sata support on qds and twr board

Sorry for top posting, replying from my phone.

Please use macros instead of numbers. It's still better than putting magic numbers in the code. And please move it out of board files.

Yuantian:
Please move what out of board files, these magic numbers or sata init function?

Regards,
Yuantian

York

-------- Original message --------
From: Tang Yuantian-B29983
Date:08/18/2015 19:31 (GMT-08:00)
To: Sun York-R58495
Cc: u-boot at lists.denx.de<mailto:u-boot at lists.denx.de>, Kushwaha Prabhakar-B32579 , Wang Huan-B18965
Subject: RE: [PATCH v2] arm/ls1021a: Add sata support on qds and twr board

> > +#ifdef CONFIG_SYS_FSL_ERRATUM_A008407
> > +#define SATA_ECC_REG_ADDR  0x20220520
> > +   unsigned int __iomem *ecc_reg = (void *)SATA_ECC_REG_ADDR;
> #endif
> > +
> > +   out_le32(&ccsr_ahci->ppcfg, 0xa003fffe);
> > +   out_le32(&ccsr_ahci->pp2c, 0x28183411);
> > +   out_le32(&ccsr_ahci->pp3c, 0x0e081004);
> > +   out_le32(&ccsr_ahci->pp4c, 0x00480811);
> > +   out_le32(&ccsr_ahci->pp5c, 0x192c96a4);
> > +   out_le32(&ccsr_ahci->ptc, 0x08000025);
>
> What are these numbers?
>
I don't like magic number either, but I don't have a choice.
These numbers are used to overwrite the default value which are not working for sata. They are gotten from validation team.
Their meanings are documented in RM. Is it necessary to re-explain bit by bit again?
Same reasons for ls2085.

Regards,
Yuantian

> > +
> > +#ifdef CONFIG_SYS_FSL_ERRATUM_A008407
> > +   out_le32(ecc_reg, 0x00020000);
> > +#endif
>
> Same here.
>
> > +}
> > +
> > +#ifdef CONFIG_SCSI_AHCI_PLAT
> > +static int ls1021a_sata_start(void)
> > +{
> > +   struct ccsr_gur *gur = (struct ccsr_gur
> *)CONFIG_SYS_FSL_GUTS_ADDR;
> > +   u32 cfg;
> > +   int rc = -1;
> > +
> > +   cfg = in_be32(&gur->rcwsr[4]) & RCWSR4_SRDS1_PRTCL_MASK;
> > +   cfg >>= RCWSR4_SRDS1_PRTCL_SHIFT;
> > +
> > +   if (cfg != 0x30 && cfg != 0x70) {
> > +           printf("SATA disabled: serdes protocol doesn't support\n");
> > +           return rc;
> > +   }
> > +
> > +   rc = ahci_init((void __iomem *)AHCI_BASE_ADDR);
> > +   if (rc)
> > +           return rc;
> > +
> > +   scsi_scan(0);
> > +
> > +   return 0;
> > +}
> > +#endif
> > +
> >  #ifdef CONFIG_LS102XA_NS_ACCESS
> >  static struct csu_ns_dev ns_dev[] = {
> >      { CSU_CSLX_PCIE2_IO, CSU_ALL_RW },
> > @@ -327,6 +375,8 @@ int board_early_init_f(void)
> >              fsl_dp_disable_console();
> >  #endif
> >
> > +   ls1021a_sata_init();
>
> Is it OK to run this init regardless SerDes protocol?
>
> > +
> >      return 0;
> >  }
> >
> > @@ -388,6 +438,17 @@ void board_init_f(ulong dummy)  }  #endif
> >
> > +#ifdef CONFIG_BOARD_LATE_INIT
> > +int board_late_init(void)
> > +{
> > +#ifdef CONFIG_SCSI_AHCI_PLAT
> > +   ls1021a_sata_start();
> > +#endif
> > +
> > +   return 0;
> > +}
> > +#endif
> > +
> >  void config_etseccm_source(int etsec_gtx_125_mux)  {
> >      struct ccsr_scfg *scfg = (struct ccsr_scfg
> > *)CONFIG_SYS_FSL_SCFG_ADDR; diff --git
> > a/board/freescale/ls1021atwr/ls1021atwr.c
> > b/board/freescale/ls1021atwr/ls1021atwr.c
> > index b7458a9..6a964c3 100644
> > --- a/board/freescale/ls1021atwr/ls1021atwr.c
> > +++ b/board/freescale/ls1021atwr/ls1021atwr.c
> > @@ -22,6 +22,8 @@
> >  #include <tsec.h>
> >  #include <fsl_sec.h>
> >  #include <spl.h>
> > +#include <ahci.h>
> > +#include <scsi.h>
> >  #include "../common/sleep.h"
> >  #ifdef CONFIG_U_QE
> >  #include "../../../drivers/qe/qe.h"
> > @@ -173,6 +175,52 @@ struct cpld_data {
> >      u8 rev2;                /* Reserved */
> >  };
> >
> > +static void ls1021a_sata_init(void)
> > +{
> > +   struct ccsr_ahci __iomem *ccsr_ahci = (void *)AHCI_BASE_ADDR;
> > +
> > +#ifdef CONFIG_SYS_FSL_ERRATUM_A008407
>
> Looks like ERRATUM A008407 is a SoC erratum. The workaround shouldn't be
> in board file.
>
> York