>-----Original Message----- >From: Michal Simek <michal.si...@xilinx.com> >Sent: 2019年4月16日 17:31 >To: Peng Ma <peng...@nxp.com>; Michal Simek <michal.si...@xilinx.com>; >albert.u.b...@aribaud.net; s...@chromium.org; Fabio Estevam ><fabio.este...@nxp.com>; York Sun <york....@nxp.com>; Prabhakar >Kushwaha <prabhakar.kushw...@nxp.com> >Cc: Andy Tang <andy.t...@nxp.com>; Yinbo Zhu <yinbo....@nxp.com>; >u-boot@lists.denx.de >Subject: Re: [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code > >WARNING: This email was created outside of NXP. DO NOT CLICK links or >attachments unless you recognize the sender and know the content is safe. > > > >On 16. 04. 19 11:22, Peng Ma wrote: >> >> >>> -----Original Message----- >>> From: Michal Simek <michal.si...@xilinx.com> >>> Sent: 2019年4月16日 16:04 >>> To: Peng Ma <peng...@nxp.com>; albert.u.b...@aribaud.net; >>> s...@chromium.org; Fabio Estevam <fabio.este...@nxp.com>; York Sun >>> <york....@nxp.com>; Prabhakar Kushwaha ><prabhakar.kushw...@nxp.com> >>> Cc: Andy Tang <andy.t...@nxp.com>; Yinbo Zhu <yinbo....@nxp.com>; >>> michal.si...@xilinx.com; u-boot@lists.denx.de >>> Subject: [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code >>> >>> WARNING: This email was created outside of NXP. DO NOT CLICK links or >>> attachments unless you recognize the sender and know the content is safe. >>> >>> >>> >>> On 16. 04. 19 9:28, Peng Ma wrote: >>>> Distinguish the ecc val by chassis version and move the ecc addr to dts. >>>> Add ls1028a soc support. >>>> >>>> Signed-off-by: Peng Ma <peng...@nxp.com> >>>> --- >>>> drivers/ata/sata_ceva.c | 43 >>> +++++++++++++++++++++++++------------------ >>>> 1 files changed, 25 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index >>>> 8887be9..d26f712 100644 >>>> --- a/drivers/ata/sata_ceva.c >>>> +++ b/drivers/ata/sata_ceva.c >>>> @@ -88,20 +88,16 @@ >>>> #define LS1021_CEVA_PHY4_CFG 0x064a080b #define >>> LS1021_CEVA_PHY5_CFG >>>> 0x2aa86470 >>>> >>>> -/* for ls1088a */ >>>> -#define LS1088_ECC_DIS_ADDR_CH2 0x100520 >>>> -#define LS1088_ECC_DIS_VAL_CH2 0x40000000 >>>> - >>>> -/* ecc addr-val pair */ >>>> -#define ECC_DIS_ADDR_CH2 0x20140520 >>>> +/* ecc val pair */ >>>> +#define ECC_DIS_VAL_CH1 0x00020000 >>>> #define ECC_DIS_VAL_CH2 0x80000000 >>>> -#define SATA_ECC_REG_ADDR 0x20220520 >>>> -#define SATA_ECC_DISABLE 0x00020000 >>>> +#define ECC_DIS_VAL_CH3 0x40000000 >>>> >>>> enum ceva_soc { >>>> CEVA_1V84, >>>> CEVA_LS1012A, >>>> CEVA_LS1021A, >>>> + CEVA_LS1028A, >>>> CEVA_LS1043A, >>>> CEVA_LS1046A, >>>> CEVA_LS1088A, >>>> @@ -110,12 +106,14 @@ enum ceva_soc { >>>> >>>> struct ceva_sata_priv { >>>> ulong base; >>>> + ulong ecc_base; >>>> enum ceva_soc soc; >>>> ulong flag; >>>> }; >>>> >>>> static int ceva_init_sata(struct ceva_sata_priv *priv) { >>>> + ulong ecc_addr = priv->ecc_base; >>>> ulong base = priv->base; >>>> ulong tmp; >>>> >>>> @@ -132,38 +130,38 @@ static int ceva_init_sata(struct >>>> ceva_sata_priv >>> *priv) >>>> break; >>>> >>>> case CEVA_LS1021A: >>>> - writel(SATA_ECC_DISABLE, SATA_ECC_REG_ADDR); >>>> + if (ecc_addr) >>>> + writel(ECC_DIS_VAL_CH1, ecc_addr); >>>> writel(CEVA_PHY1_CFG, base + AHCI_VEND_PPCFG); >>>> writel(LS1021_CEVA_PHY2_CFG, base + >>> AHCI_VEND_PP2C); >>>> writel(LS1021_CEVA_PHY3_CFG, base + >>> AHCI_VEND_PP3C); >>>> writel(LS1021_CEVA_PHY4_CFG, base + >>> AHCI_VEND_PP4C); >>>> writel(LS1021_CEVA_PHY5_CFG, base + >>> AHCI_VEND_PP5C); >>>> writel(CEVA_TRANS_CFG, base + AHCI_VEND_PTC); >>>> - if (priv->flag & FLAG_COHERENT) >>>> - writel(CEVA_AXICC_CFG, base + >>> LS1021_AHCI_VEND_AXICC); >>>> break; >>>> >>>> case CEVA_LS1012A: >>>> case CEVA_LS1043A: >>>> case CEVA_LS1046A: >>>> - writel(ECC_DIS_VAL_CH2, ECC_DIS_ADDR_CH2); >>>> - /* fallthrough */ >>>> case CEVA_LS2080A: >>>> + if (ecc_addr) >>>> + writel(ECC_DIS_VAL_CH2, ecc_addr); >>>> writel(CEVA_PHY1_CFG, base + AHCI_VEND_PPCFG); >>>> writel(CEVA_TRANS_CFG, base + AHCI_VEND_PTC); >>>> - if (priv->flag & FLAG_COHERENT) >>>> - writel(CEVA_AXICC_CFG, base + >>> AHCI_VEND_AXICC); >>>> break; >>>> >>>> + case CEVA_LS1028A: >>>> case CEVA_LS1088A: >>>> - writel(LS1088_ECC_DIS_VAL_CH2, >>> LS1088_ECC_DIS_ADDR_CH2); >>>> + if (ecc_addr) >>>> + writel(ECC_DIS_VAL_CH3, ecc_addr); >>>> writel(CEVA_PHY1_CFG, base + AHCI_VEND_PPCFG); >>>> writel(CEVA_TRANS_CFG, base + AHCI_VEND_PTC); >>>> - if (priv->flag & FLAG_COHERENT) >>>> - writel(CEVA_AXICC_CFG, base + >>> AHCI_VEND_AXICC); >>>> break; >>>> } >>>> >>>> + if (priv->flag & FLAG_COHERENT) >>>> + writel(CEVA_AXICC_CFG, base + AHCI_VEND_AXICC); >>>> + >>>> return 0; >>>> } >>>> >>>> @@ -187,6 +185,7 @@ static const struct udevice_id sata_ceva_ids[] = { >>>> { .compatible = "ceva,ahci-1v84", .data = CEVA_1V84 }, >>>> { .compatible = "fsl,ls1012a-ahci", .data = CEVA_LS1012A }, >>>> { .compatible = "fsl,ls1021a-ahci", .data = CEVA_LS1021A }, >>>> + { .compatible = "fsl,ls1028a-ahci", .data = CEVA_LS1028A }, >>>> { .compatible = "fsl,ls1043a-ahci", .data = CEVA_LS1043A }, >>>> { .compatible = "fsl,ls1046a-ahci", .data = CEVA_LS1046A }, >>>> { .compatible = "fsl,ls1088a-ahci", .data = CEVA_LS1088A }, @@ >>>> -205,8 +204,16 @@ static int sata_ceva_ofdata_to_platdata(struct >>>> udevice >>> *dev) >>>> if (priv->base == FDT_ADDR_T_NONE) >>>> return -EINVAL; >>>> >>>> + priv->ecc_base = dev_read_addr_index(dev, 1); >>> >>> It would be better to do it via reg-name instead of index. But that's >>> up to your binding doc. >>> >> [Peng Ma] OK, could I change arch/arm/dts/zynqmp.dtsi file sata node as >fallows? >> >> diff --git a/arch/arm/dts/zynqmp.dtsi b/arch/arm/dts/zynqmp.dtsi index >> dfb6ebc..9ad2d84 100644 >> --- a/arch/arm/dts/zynqmp.dtsi >> +++ b/arch/arm/dts/zynqmp.dtsi >> @@ -692,6 +692,7 @@ >> compatible = "ceva,ahci-1v84"; >> status = "disabled"; >> reg = <0x0 0xfd0c0000 0x0 0x2000>; >> + reg-names = "serdes"; >> interrupt-parent = <&gic>; >> interrupts = <0 133 4>; >> #stream-id-cells = <4>; > >Unfortunately it is not that simple. > >First of all you need to reflect this in dt binding doc in the kernel. >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern >el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftr >ee%2FDocumentation%2Fdevicetree%2Fbindings%2Fata%2Fahci-ceva.txt%3F >h%3Dv5.1-rc5&data=02%7C01%7Cpeng.ma%40nxp.com%7Cc588ddff1b1 >e471f645b08d6c24e54d7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C >0%7C636910039050031150&sdata=0Gsuo2uI3pot4VAhBr7SQUnCRYT7Xi >Ybybu6i1t%2Byzo%3D&reserved=0 > >Did you record your compatible strings anywhere? > [Peng Ma] Thanks your quick reply. You can find our binding document at: (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/ata/ahci-fsl-qoriq.txt?h=v5.1-rc5) I think your suggestion is better. But the first register address was got by function dev_read_addr(), to keep consistency, I still prefer to use dev_read_addr_index(). What do you think?
>Thanks, >Michal > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot