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.
York
-------- Original message --------
From: Tang Yuantian-B29983
Date:08/18/2015 19:31 (GMT-08:00)
To: Sun York-R58495
Cc: u-boot@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
