>-----Original Message----- >From: Michal Simek <michal.si...@xilinx.com> >Sent: 2019年4月16日 18:01 >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:54, Peng Ma wrote: >> >> >>> -----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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit >> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git >% >> >2Ftree%2FDocumentation%2Fdevicetree%2Fbindings%2Fata%2Fahci-fsl-qoriq. >> >txt%3Fh%3Dv5.1-rc5&data=02%7C01%7Cpeng.ma%40nxp.com%7C977d >7871292f >> >48332dbb08d6c2526b32%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C >0%7C6369 >> >10056581930082&sdata=KQryE2RvI8jTOAPKGlGnx2NWYdawFUaTURyzd8 >oets4%3 >> D&reserved=0) > >Interesting. Why did you create separate binding doc if you target the same >ceva sata IP core? Any reason not to merge it together? >Anyway I see that this was added in 2015. > [Peng Ma] Although they are both ceva sata, our sata driver is different from yours in kernel to Initializa Vendor-specific registers, you could see our driver at kernel driver/ata/ ahci_qoriq.c. >> I think your suggestion is better. But the first register cd address was got >> by >function dev_read_addr(), to keep consistency, I still prefer to use >dev_read_addr_index(). What do you think? > >We didn't have a need to have second address range that's why reg-names >property wasn't used. Do they have all ecc addresses? > [Peng Ma] Some of our socs need to fixed sata errata. So we should write ecc addr. I didn't add reg-names to keep consistency of Xilinx sata soc. >M
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot