[U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
Peng Ma
peng.ma at nxp.com
Tue Apr 16 09:54:10 UTC 2019
>-----Original Message-----
>From: Michal Simek <michal.simek at xilinx.com>
>Sent: 2019年4月16日 17:31
>To: Peng Ma <peng.ma at nxp.com>; Michal Simek <michal.simek at xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam at nxp.com>; York Sun <york.sun at nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha at nxp.com>
>Cc: Andy Tang <andy.tang at nxp.com>; Yinbo Zhu <yinbo.zhu at nxp.com>;
>u-boot at 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.simek at xilinx.com>
>>> Sent: 2019年4月16日 16:04
>>> To: Peng Ma <peng.ma at nxp.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam at nxp.com>; York Sun
>>> <york.sun at nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha at nxp.com>
>>> Cc: Andy Tang <andy.tang at nxp.com>; Yinbo Zhu <yinbo.zhu at nxp.com>;
>>> michal.simek at xilinx.com; u-boot at 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.ma at 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
>
More information about the U-Boot
mailing list