* [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code
@ 2019-04-16 7:28 Peng Ma
2019-04-16 7:28 ` [U-Boot] [PATCH 2/2] ARM: dts: Freescale: Add ecc addr for sata node Peng Ma
2019-04-16 8:04 ` [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Michal Simek
0 siblings, 2 replies; 16+ messages in thread
From: Peng Ma @ 2019-04-16 7:28 UTC (permalink / raw)
To: u-boot
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@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);
+ if (priv->ecc_base == FDT_ADDR_T_NONE)
+ priv->ecc_base = 0;
+
priv->soc = dev_get_driver_data(dev);
+ debug("ccsr-sata-base %lx\t ecc-base %lx\n",
+ priv->base,
+ priv->ecc_base);
+
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 2/2] ARM: dts: Freescale: Add ecc addr for sata node
2019-04-16 7:28 [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Peng Ma
@ 2019-04-16 7:28 ` Peng Ma
2019-04-16 8:04 ` [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Michal Simek
1 sibling, 0 replies; 16+ messages in thread
From: Peng Ma @ 2019-04-16 7:28 UTC (permalink / raw)
To: u-boot
Move the ecc addr from driver to dts.
Signed-off-by: Peng Ma <peng.ma@nxp.com>
---
arch/arm/dts/fsl-ls1012a.dtsi | 3 ++-
arch/arm/dts/fsl-ls1043a.dtsi | 3 ++-
arch/arm/dts/fsl-ls1046a.dtsi | 3 ++-
arch/arm/dts/fsl-ls1088a.dtsi | 2 ++
arch/arm/dts/ls1021a.dtsi | 3 ++-
5 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1012a.dtsi b/arch/arm/dts/fsl-ls1012a.dtsi
index f22cbf4..30fe268 100644
--- a/arch/arm/dts/fsl-ls1012a.dtsi
+++ b/arch/arm/dts/fsl-ls1012a.dtsi
@@ -136,7 +136,8 @@
sata: sata at 3200000 {
compatible = "fsl,ls1012a-ahci";
- reg = <0x0 0x3200000 0x0 0x10000>;
+ reg = <0x0 0x3200000 0x0 0x10000 /* ccsr sata base */
+ 0x0 0x20140520 0x0 0x4>; /* ecc sata addr*/
interrupts = <0 69 4>;
clocks = <&clockgen 4 0>;
status = "disabled";
diff --git a/arch/arm/dts/fsl-ls1043a.dtsi b/arch/arm/dts/fsl-ls1043a.dtsi
index bb70992..3109765 100644
--- a/arch/arm/dts/fsl-ls1043a.dtsi
+++ b/arch/arm/dts/fsl-ls1043a.dtsi
@@ -290,7 +290,8 @@
sata: sata at 3200000 {
compatible = "fsl,ls1043a-ahci";
- reg = <0x0 0x3200000 0x0 0x10000>;
+ reg = <0x0 0x3200000 0x0 0x10000 /* ccsr sata base */
+ 0x0 0x20140520 0x0 0x4>; /* ecc sata addr*/
interrupts = <0 69 4>;
clocks = <&clockgen 4 0>;
status = "disabled";
diff --git a/arch/arm/dts/fsl-ls1046a.dtsi b/arch/arm/dts/fsl-ls1046a.dtsi
index 5ac10e0..e9c8243 100644
--- a/arch/arm/dts/fsl-ls1046a.dtsi
+++ b/arch/arm/dts/fsl-ls1046a.dtsi
@@ -294,7 +294,8 @@
sata: sata at 3200000 {
compatible = "fsl,ls1046a-ahci";
- reg = <0x0 0x3200000 0x0 0x10000>;
+ reg = <0x0 0x3200000 0x0 0x10000 /* ccsr sata base */
+ 0x0 0x20140520 0x0 0x4>; /* ecc sata addr*/
interrupts = <0 69 4>;
clocks = <&clockgen 4 1>;
status = "disabled";
diff --git a/arch/arm/dts/fsl-ls1088a.dtsi b/arch/arm/dts/fsl-ls1088a.dtsi
index 9455e03..6a1cd0d 100644
--- a/arch/arm/dts/fsl-ls1088a.dtsi
+++ b/arch/arm/dts/fsl-ls1088a.dtsi
@@ -154,6 +154,8 @@
sata: sata at 3200000 {
compatible = "fsl,ls1088a-ahci";
reg = <0x0 0x3200000 0x0 0x10000>;
+ reg = <0x0 0x3200000 0x0 0x10000 /* ccsr sata base */
+ 0x7 0x100520 0x0 0x4>; /* ecc sata addr*/
interrupts = <0 133 4>;
status = "disabled";
};
diff --git a/arch/arm/dts/ls1021a.dtsi b/arch/arm/dts/ls1021a.dtsi
index 7670a39..b3d1b07 100644
--- a/arch/arm/dts/ls1021a.dtsi
+++ b/arch/arm/dts/ls1021a.dtsi
@@ -406,7 +406,8 @@
sata: sata at 3200000 {
compatible = "fsl,ls1021a-ahci";
- reg = <0x3200000 0x10000>;
+ reg = <0x0 0x3200000 0x0 0x10000 /* ccsr sata base */
+ 0x0 0x20220520 0x0 0x4>; /* ecc sata addr*/
interrupts = <0 101 4>;
status = "disabled";
};
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 7:28 [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Peng Ma
2019-04-16 7:28 ` [U-Boot] [PATCH 2/2] ARM: dts: Freescale: Add ecc addr for sata node Peng Ma
@ 2019-04-16 8:04 ` Michal Simek
2019-04-16 9:22 ` [U-Boot] [EXT] " Peng Ma
1 sibling, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-16 8:04 UTC (permalink / raw)
To: u-boot
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@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.
Thanks,
Michal
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 8:04 ` [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Michal Simek
@ 2019-04-16 9:22 ` Peng Ma
2019-04-16 9:31 ` Michal Simek
0 siblings, 1 reply; 16+ messages in thread
From: Peng Ma @ 2019-04-16 9:22 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月16日 16:04
>To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
><york.sun@nxp.com>; Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@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>;
Best Regards,
Peng
>Thanks,
>Michal
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 9:22 ` [U-Boot] [EXT] " Peng Ma
@ 2019-04-16 9:31 ` Michal Simek
2019-04-16 9:54 ` Peng Ma
0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-16 9:31 UTC (permalink / raw)
To: u-boot
On 16. 04. 19 11:22, Peng Ma wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek <michal.simek@xilinx.com>
>> Sent: 2019年4月16日 16:04
>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>> <york.sun@nxp.com>; Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@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://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/ata/ahci-ceva.txt?h=v5.1-rc5
Did you record your compatible strings anywhere?
Thanks,
Michal
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 9:31 ` Michal Simek
@ 2019-04-16 9:54 ` Peng Ma
2019-04-16 10:00 ` Michal Simek
0 siblings, 1 reply; 16+ messages in thread
From: Peng Ma @ 2019-04-16 9:54 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月16日 17:31
>To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@xilinx.com>
>>> Sent: 2019年4月16日 16:04
>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>> <york.sun@nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>
>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 9:54 ` Peng Ma
@ 2019-04-16 10:00 ` Michal Simek
2019-04-16 10:29 ` Peng Ma
0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-16 10:00 UTC (permalink / raw)
To: u-boot
On 16. 04. 19 11:54, Peng Ma wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek <michal.simek@xilinx.com>
>> Sent: 2019年4月16日 17:31
>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>> albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
>> <fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>> Kushwaha <prabhakar.kushwaha@nxp.com>
>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@xilinx.com>
>>>> Sent: 2019年4月16日 16:04
>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>
>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@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)
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.
> 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?
M
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 10:00 ` Michal Simek
@ 2019-04-16 10:29 ` Peng Ma
2019-04-16 10:49 ` Michal Simek
0 siblings, 1 reply; 16+ messages in thread
From: Peng Ma @ 2019-04-16 10:29 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月16日 18:01
>To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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:54, Peng Ma wrote:
>>
>>
>>> -----Original Message-----
>>> From: Michal Simek <michal.simek@xilinx.com>
>>> Sent: 2019年4月16日 17:31
>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>> <york.sun@nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>
>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@xilinx.com>
>>>>> Sent: 2019年4月16日 16:04
>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>> <prabhakar.kushwaha@nxp.com>
>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 10:29 ` Peng Ma
@ 2019-04-16 10:49 ` Michal Simek
2019-04-17 2:50 ` Peng Ma
0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-16 10:49 UTC (permalink / raw)
To: u-boot
On 16. 04. 19 12:29, Peng Ma wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek <michal.simek@xilinx.com>
>> Sent: 2019年4月16日 18:01
>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>> albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
>> <fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>> Kushwaha <prabhakar.kushwaha@nxp.com>
>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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:54, Peng Ma wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>> Sent: 2019年4月16日 17:31
>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>
>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@xilinx.com>
>>>>>> Sent: 2019年4月16日 16:04
>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>> <prabhakar.kushwaha@nxp.com>
>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@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 have briefly looked at it. They should be merged together. It happened
to us too that we send driver out but didn't know who was the vendor in
past and then we found out.
>>> 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.
Ok. I think that in this case you should pass different .data to driver.
struct platform_data {
enum ceva_soc;
bool ecc_present;
}
It means have current enum followed by bool with "true" for specific IPs
which needs to have also ECC range.
Then you can better check that binding is correct.
And use _index there.
Thanks,
Michal
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-16 10:49 ` Michal Simek
@ 2019-04-17 2:50 ` Peng Ma
2019-04-17 5:58 ` Michal Simek
0 siblings, 1 reply; 16+ messages in thread
From: Peng Ma @ 2019-04-17 2:50 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月16日 18:49
>To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 12:29, Peng Ma wrote:
>>
>>
>>> -----Original Message-----
>>> From: Michal Simek <michal.simek@xilinx.com>
>>> Sent: 2019年4月16日 18:01
>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>> <york.sun@nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>
>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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:54, Peng Ma wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>> Sent: 2019年4月16日 17:31
>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>> <prabhakar.kushwaha@nxp.com>
>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@xilinx.com>
>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>Sun
>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
><yinbo.zhu@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@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%2Fg
>>>>> it
>>>>> .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%2Fg
>>>> it
>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.
>gi
>>>> t
>>> %
>>>>
>>>
>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 have briefly looked at it. They should be merged together. It happened to us
>too that we send driver out but didn't know who was the vendor in past and
>then we found out.
>
>>>> 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.
>
>Ok. I think that in this case you should pass different .data to driver.
>
>struct platform_data {
> enum ceva_soc;
> bool ecc_present;
>}
>
>It means have current enum followed by bool with "true" for specific IPs which
>needs to have also ECC range.
>Then you can better check that binding is correct.
>
>And use _index there.
[Peng Ma] As far as I am concerned, It is not necessary to do this, I would rather
lean on the former way as fallows:
diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
index d26f712..a1d452a 100644
--- a/drivers/ata/sata_ceva.c
+++ b/drivers/ata/sata_ceva.c
@@ -196,6 +196,10 @@ static const struct udevice_id sata_ceva_ids[] = {
static int sata_ceva_ofdata_to_platdata(struct udevice *dev)
{
struct ceva_sata_priv *priv = dev_get_priv(dev);
+ const void *blob = gd->fdt_blob;
+ int node = dev_of_offset(dev);
+ struct fdt_resource res_regs;
+ int ret;
if (dev_read_bool(dev, "dma-coherent"))
priv->flag |= FLAG_COHERENT;
@@ -204,9 +208,13 @@ 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);
- if (priv->ecc_base == FDT_ADDR_T_NONE)
+ ret = fdt_get_named_resource(blob, node, "reg", "reg-names",
+ "ecc-addr", &res_regs);
+ if (ret) {
+ debug("Error: can't get ecc addresses(ret = %d)!\n", ret);
priv->ecc_base = 0;
+ } else
+ priv->ecc_base = res_regs.start;
priv->soc = dev_get_driver_data(dev);
What do you think?
Best Regards,
Peng
>
>Thanks,
>Michal
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-17 2:50 ` Peng Ma
@ 2019-04-17 5:58 ` Michal Simek
2019-04-17 6:50 ` Peng Ma
0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-17 5:58 UTC (permalink / raw)
To: u-boot
On 17. 04. 19 4:50, Peng Ma wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek <michal.simek@xilinx.com>
>> Sent: 2019年4月16日 18:49
>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>> albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
>> <fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>> Kushwaha <prabhakar.kushwaha@nxp.com>
>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 12:29, Peng Ma wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>> Sent: 2019年4月16日 18:01
>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>
>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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:54, Peng Ma wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>> Sent: 2019年4月16日 17:31
>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>> <prabhakar.kushwaha@nxp.com>
>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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@xilinx.com>
>>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>> Sun
>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>> <yinbo.zhu@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@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%2Fg
>>>>>> it
>>>>>> .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%2Fg
>>>>> it
>>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.
>> gi
>>>>> t
>>>> %
>>>>>
>>>>
>> 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 have briefly looked at it. They should be merged together. It happened to us
>> too that we send driver out but didn't know who was the vendor in past and
>> then we found out.
>>
>>>>> 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.
>>
>> Ok. I think that in this case you should pass different .data to driver.
>>
>> struct platform_data {
>> enum ceva_soc;
>> bool ecc_present;
>> }
>>
>> It means have current enum followed by bool with "true" for specific IPs which
>> needs to have also ECC range.
>> Then you can better check that binding is correct.
>>
>> And use _index there.
> [Peng Ma] As far as I am concerned, It is not necessary to do this, I would rather
> lean on the former way as fallows:
> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
> index d26f712..a1d452a 100644
> --- a/drivers/ata/sata_ceva.c
> +++ b/drivers/ata/sata_ceva.c
> @@ -196,6 +196,10 @@ static const struct udevice_id sata_ceva_ids[] = {
> static int sata_ceva_ofdata_to_platdata(struct udevice *dev)
> {
> struct ceva_sata_priv *priv = dev_get_priv(dev);
> + const void *blob = gd->fdt_blob;
gd is suggesting that you use incorrect functions.
We are trying to get rid of gd from device drivers. Please take a look
at live tree functions if there is any alternative there.
> + int node = dev_of_offset(dev);
> + struct fdt_resource res_regs;
> + int ret;
>
> if (dev_read_bool(dev, "dma-coherent"))
> priv->flag |= FLAG_COHERENT;
> @@ -204,9 +208,13 @@ 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);
> - if (priv->ecc_base == FDT_ADDR_T_NONE)
> + ret = fdt_get_named_resource(blob, node, "reg", "reg-names",
> + "ecc-addr", &res_regs);
> + if (ret) {
> + debug("Error: can't get ecc addresses(ret = %d)!\n", ret);
> priv->ecc_base = 0;
> + } else
> + priv->ecc_base = res_regs.start;
>
> priv->soc = dev_get_driver_data(dev);
> What do you think?
One thing I would like to avoid is that we will get any error even in
debug for IPs which don't have this ecc space.
And also that we will get this errors for IPs which should have this ecc
space.
Thanks,
Michal
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-17 5:58 ` Michal Simek
@ 2019-04-17 6:50 ` Peng Ma
2019-04-17 6:57 ` Michal Simek
0 siblings, 1 reply; 16+ messages in thread
From: Peng Ma @ 2019-04-17 6:50 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月17日 13:58
>To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 4:50, Peng Ma wrote:
>>
>>
>>> -----Original Message-----
>>> From: Michal Simek <michal.simek@xilinx.com>
>>> Sent: 2019年4月16日 18:49
>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>> <york.sun@nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>
>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 12:29, Peng Ma wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>> Sent: 2019年4月16日 18:01
>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>> <prabhakar.kushwaha@nxp.com>
>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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:54, Peng Ma wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Sent: 2019年4月16日 17:31
>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>Sun
>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
><yinbo.zhu@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@xilinx.com>
>>>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>>> Sun
>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>> <yinbo.zhu@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@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%2
>>>>>>> Fg
>>>>>>> it
>>>>>>> .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%2
>>>>>> Fg
>>>>>> it
>>>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flin
>ux.
>>> gi
>>>>>> t
>>>>> %
>>>>>>
>>>>>
>>>
>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 have briefly looked at it. They should be merged together. It
>>> happened to us too that we send driver out but didn't know who was
>>> the vendor in past and then we found out.
>>>
>>>>>> 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.
>>>
>>> Ok. I think that in this case you should pass different .data to driver.
>>>
>>> struct platform_data {
>>> enum ceva_soc;
>>> bool ecc_present;
>>> }
>>>
>>> It means have current enum followed by bool with "true" for specific
>>> IPs which needs to have also ECC range.
>>> Then you can better check that binding is correct.
>>>
>>> And use _index there.
>> [Peng Ma] As far as I am concerned, It is not necessary to do this, I
>> would rather lean on the former way as fallows:
>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>> d26f712..a1d452a 100644
>> --- a/drivers/ata/sata_ceva.c
>> +++ b/drivers/ata/sata_ceva.c
>> @@ -196,6 +196,10 @@ static const struct udevice_id sata_ceva_ids[] =
>> { static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>> + const void *blob = gd->fdt_blob;
>
>gd is suggesting that you use incorrect functions.
>We are trying to get rid of gd from device drivers. Please take a look
>at live tree functions if there is any alternative there.
>
>
>> + int node = dev_of_offset(dev);
>> + struct fdt_resource res_regs;
>> + int ret;
>>
>> if (dev_read_bool(dev, "dma-coherent"))
>> priv->flag |= FLAG_COHERENT;
>> @@ -204,9 +208,13 @@ 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);
>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>> + ret = fdt_get_named_resource(blob, node, "reg", "reg-names",
>> + "ecc-addr", &res_regs);
>> + if (ret) {
>> + debug("Error: can't get ecc addresses(ret = %d)!\n", ret);
>> priv->ecc_base = 0;
>> + } else
>> + priv->ecc_base = res_regs.start;
>>
>> priv->soc = dev_get_driver_data(dev);
>> What do you think?
>
>One thing I would like to avoid is that we will get any error even in
>debug for IPs which don't have this ecc space.
>And also that we will get this errors for IPs which should have this ecc
>space.
>
[Peng Ma] OK，changed as fallows:
diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
index d26f712..bd98d85 100644
--- a/drivers/ata/sata_ceva.c
+++ b/drivers/ata/sata_ceva.c
@@ -8,6 +8,7 @@
#include <ahci.h>
#include <scsi.h>
#include <asm/io.h>
+#include <linux/ioport.h>
/* Vendor Specific Register Offsets */
#define AHCI_VEND_PCFG 0xA4
@@ -196,6 +197,8 @@ static const struct udevice_id sata_ceva_ids[] = {
static int sata_ceva_ofdata_to_platdata(struct udevice *dev)
{
struct ceva_sata_priv *priv = dev_get_priv(dev);
+ struct resource res_regs;
+ int ret;
if (dev_read_bool(dev, "dma-coherent"))
priv->flag |= FLAG_COHERENT;
@@ -204,9 +207,11 @@ 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);
- if (priv->ecc_base == FDT_ADDR_T_NONE)
+ ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
+ if (ret)
priv->ecc_base = 0;
+ else
+ priv->ecc_base = res_regs.start;
priv->soc = dev_get_driver_data(dev);
Best Regards,
Peng
>Thanks,
>Michal
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-17 6:50 ` Peng Ma
@ 2019-04-17 6:57 ` Michal Simek
2019-04-17 7:27 ` Peng Ma
0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-17 6:57 UTC (permalink / raw)
To: u-boot
On 17. 04. 19 8:50, Peng Ma wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek <michal.simek@xilinx.com>
>> Sent: 2019年4月17日 13:58
>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>> albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
>> <fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>> Kushwaha <prabhakar.kushwaha@nxp.com>
>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 4:50, Peng Ma wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>> Sent: 2019年4月16日 18:49
>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>
>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 12:29, Peng Ma wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>> Sent: 2019年4月16日 18:01
>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>> <prabhakar.kushwaha@nxp.com>
>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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:54, Peng Ma wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Sent: 2019年4月16日 17:31
>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>> Sun
>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>> <yinbo.zhu@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@xilinx.com>
>>>>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>>>> Sun
>>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>>> <yinbo.zhu@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@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%2
>>>>>>>> Fg
>>>>>>>> it
>>>>>>>> .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%2
>>>>>>> Fg
>>>>>>> it
>>>>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flin
>> ux.
>>>> gi
>>>>>>> t
>>>>>> %
>>>>>>>
>>>>>>
>>>>
>> 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 have briefly looked at it. They should be merged together. It
>>>> happened to us too that we send driver out but didn't know who was
>>>> the vendor in past and then we found out.
>>>>
>>>>>>> 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.
>>>>
>>>> Ok. I think that in this case you should pass different .data to driver.
>>>>
>>>> struct platform_data {
>>>> enum ceva_soc;
>>>> bool ecc_present;
>>>> }
>>>>
>>>> It means have current enum followed by bool with "true" for specific
>>>> IPs which needs to have also ECC range.
>>>> Then you can better check that binding is correct.
>>>>
>>>> And use _index there.
>>> [Peng Ma] As far as I am concerned, It is not necessary to do this, I
>>> would rather lean on the former way as fallows:
>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>>> d26f712..a1d452a 100644
>>> --- a/drivers/ata/sata_ceva.c
>>> +++ b/drivers/ata/sata_ceva.c
>>> @@ -196,6 +196,10 @@ static const struct udevice_id sata_ceva_ids[] =
>>> { static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>>> + const void *blob = gd->fdt_blob;
>>
>> gd is suggesting that you use incorrect functions.
>> We are trying to get rid of gd from device drivers. Please take a look
>> at live tree functions if there is any alternative there.
>>
>>
>>> + int node = dev_of_offset(dev);
>>> + struct fdt_resource res_regs;
>>> + int ret;
>>>
>>> if (dev_read_bool(dev, "dma-coherent"))
>>> priv->flag |= FLAG_COHERENT;
>>> @@ -204,9 +208,13 @@ 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);
>>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>>> + ret = fdt_get_named_resource(blob, node, "reg", "reg-names",
>>> + "ecc-addr", &res_regs);
>>> + if (ret) {
>>> + debug("Error: can't get ecc addresses(ret = %d)!\n", ret);
>>> priv->ecc_base = 0;
>>> + } else
>>> + priv->ecc_base = res_regs.start;
>>>
>>> priv->soc = dev_get_driver_data(dev);
>>> What do you think?
>>
>> One thing I would like to avoid is that we will get any error even in
>> debug for IPs which don't have this ecc space.
>> And also that we will get this errors for IPs which should have this ecc
>> space.
>>
> [Peng Ma] OK，changed as fallows:
>
> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
> index d26f712..bd98d85 100644
> --- a/drivers/ata/sata_ceva.c
> +++ b/drivers/ata/sata_ceva.c
> @@ -8,6 +8,7 @@
> #include <ahci.h>
> #include <scsi.h>
> #include <asm/io.h>
> +#include <linux/ioport.h>
>
> /* Vendor Specific Register Offsets */
> #define AHCI_VEND_PCFG 0xA4
> @@ -196,6 +197,8 @@ static const struct udevice_id sata_ceva_ids[] = {
> static int sata_ceva_ofdata_to_platdata(struct udevice *dev)
> {
> struct ceva_sata_priv *priv = dev_get_priv(dev);
> + struct resource res_regs;
> + int ret;
>
> if (dev_read_bool(dev, "dma-coherent"))
> priv->flag |= FLAG_COHERENT;
> @@ -204,9 +207,11 @@ 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);
> - if (priv->ecc_base == FDT_ADDR_T_NONE)
> + ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
> + if (ret)
> priv->ecc_base = 0;
> + else
> + priv->ecc_base = res_regs.start;
>
> priv->soc = dev_get_driver_data(dev);
This looks good but still missing information that in your case there
are some versions where this ecc-addr is required. It means if it is
required you should error it out when it is not present in DT.
Thanks,
Michal
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-17 6:57 ` Michal Simek
@ 2019-04-17 7:27 ` Peng Ma
2019-04-17 7:38 ` Michal Simek
0 siblings, 1 reply; 16+ messages in thread
From: Peng Ma @ 2019-04-17 7:27 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月17日 14:58
>To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 8:50, Peng Ma wrote:
>>
>>
>>> -----Original Message-----
>>> From: Michal Simek <michal.simek@xilinx.com>
>>> Sent: 2019年4月17日 13:58
>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>> <york.sun@nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>
>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 4:50, Peng Ma wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>> Sent: 2019年4月16日 18:49
>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>> <prabhakar.kushwaha@nxp.com>
>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 12:29, Peng Ma wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Sent: 2019年4月16日 18:01
>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>Sun
>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
><yinbo.zhu@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:54, Peng Ma wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>> Sent: 2019年4月16日 17:31
>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>>> Sun
>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>> <yinbo.zhu@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@xilinx.com>
>>>>>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>;
>York
>>>>> Sun
>>>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>>>> <yinbo.zhu@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@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
>>>>>>>>> %2
>>>>>>>>> Fg
>>>>>>>>> it
>>>>>>>>> .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
>>>>>>>> %2
>>>>>>>> Fg
>>>>>>>> it
>>>>>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2F
>lin
>>> ux.
>>>>> gi
>>>>>>>> t
>>>>>>> %
>>>>>>>>
>>>>>>>
>>>>>
>>>
>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 have briefly looked at it. They should be merged together. It
>>>>> happened to us too that we send driver out but didn't know who was
>>>>> the vendor in past and then we found out.
>>>>>
>>>>>>>> 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.
>>>>>
>>>>> Ok. I think that in this case you should pass different .data to driver.
>>>>>
>>>>> struct platform_data {
>>>>> enum ceva_soc;
>>>>> bool ecc_present;
>>>>> }
>>>>>
>>>>> It means have current enum followed by bool with "true" for
>>>>> specific IPs which needs to have also ECC range.
>>>>> Then you can better check that binding is correct.
>>>>>
>>>>> And use _index there.
>>>> [Peng Ma] As far as I am concerned, It is not necessary to do this,
>>>> I would rather lean on the former way as fallows:
>>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>>>> d26f712..a1d452a 100644
>>>> --- a/drivers/ata/sata_ceva.c
>>>> +++ b/drivers/ata/sata_ceva.c
>>>> @@ -196,6 +196,10 @@ static const struct udevice_id sata_ceva_ids[]
>>>> = { static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>>>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>>>> + const void *blob = gd->fdt_blob;
>>>
>>> gd is suggesting that you use incorrect functions.
>>> We are trying to get rid of gd from device drivers. Please take a
>>> look at live tree functions if there is any alternative there.
>>>
>>>
>>>> + int node = dev_of_offset(dev);
>>>> + struct fdt_resource res_regs;
>>>> + int ret;
>>>>
>>>> if (dev_read_bool(dev, "dma-coherent"))
>>>> priv->flag |= FLAG_COHERENT; @@ -204,9 +208,13
>@@
>>>> 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);
>>>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>>>> + ret = fdt_get_named_resource(blob, node, "reg", "reg-names",
>>>> + "ecc-addr", &res_regs);
>>>> + if (ret) {
>>>> + debug("Error: can't get ecc addresses(ret = %d)!\n",
>>>> + ret);
>>>> priv->ecc_base = 0;
>>>> + } else
>>>> + priv->ecc_base = res_regs.start;
>>>>
>>>> priv->soc = dev_get_driver_data(dev); What do you think?
>>>
>>> One thing I would like to avoid is that we will get any error even in
>>> debug for IPs which don't have this ecc space.
>>> And also that we will get this errors for IPs which should have this
>>> ecc space.
>>>
>> [Peng Ma] OK，changed as fallows:
>>
>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>> d26f712..bd98d85 100644
>> --- a/drivers/ata/sata_ceva.c
>> +++ b/drivers/ata/sata_ceva.c
>> @@ -8,6 +8,7 @@
>> #include <ahci.h>
>> #include <scsi.h>
>> #include <asm/io.h>
>> +#include <linux/ioport.h>
>>
>> /* Vendor Specific Register Offsets */ #define AHCI_VEND_PCFG 0xA4
>> @@ -196,6 +197,8 @@ static const struct udevice_id sata_ceva_ids[] = {
>> static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>> + struct resource res_regs;
>> + int ret;
>>
>> if (dev_read_bool(dev, "dma-coherent"))
>> priv->flag |= FLAG_COHERENT; @@ -204,9 +207,11
>@@
>> 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);
>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>> + ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
>> + if (ret)
>> priv->ecc_base = 0;
>> + else
>> + priv->ecc_base = res_regs.start;
>>
>> priv->soc = dev_get_driver_data(dev);
>
>This looks good but still missing information that in your case there are some
>versions where this ecc-addr is required. It means if it is required you should
>error it out when it is not present in DT.
>
[Peng Ma] Ok, It will return error when some socs need ecc address with the regs is not
Present in DT.
diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
index d26f712..6e28a38 100644
--- a/drivers/ata/sata_ceva.c
+++ b/drivers/ata/sata_ceva.c
@@ -8,6 +8,7 @@
#include <ahci.h>
#include <scsi.h>
#include <asm/io.h>
+#include <linux/ioport.h>
/* Vendor Specific Register Offsets */
#define AHCI_VEND_PCFG 0xA4
@@ -130,8 +131,9 @@ static int ceva_init_sata(struct ceva_sata_priv *priv)
break;
case CEVA_LS1021A:
- if (ecc_addr)
- writel(ECC_DIS_VAL_CH1, ecc_addr);
+ if (!ecc_addr)
+ return -EINVAL;
+ 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);
@@ -143,17 +145,19 @@ static int ceva_init_sata(struct ceva_sata_priv *priv)
case CEVA_LS1012A:
case CEVA_LS1043A:
case CEVA_LS1046A:
+ if (!ecc_addr)
+ return -EINVAL;
+ writel(ECC_DIS_VAL_CH2, ecc_addr);
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);
break;
case CEVA_LS1028A:
case CEVA_LS1088A:
- if (ecc_addr)
- writel(ECC_DIS_VAL_CH3, ecc_addr);
+ if (!ecc_addr)
+ return -EINVAL;
+ writel(ECC_DIS_VAL_CH3, ecc_addr);
writel(CEVA_PHY1_CFG, base + AHCI_VEND_PPCFG);
writel(CEVA_TRANS_CFG, base + AHCI_VEND_PTC);
break;
@@ -196,6 +200,8 @@ static const struct udevice_id sata_ceva_ids[] = {
static int sata_ceva_ofdata_to_platdata(struct udevice *dev)
{
struct ceva_sata_priv *priv = dev_get_priv(dev);
+ struct resource res_regs;
+ int ret;
if (dev_read_bool(dev, "dma-coherent"))
priv->flag |= FLAG_COHERENT;
@@ -204,9 +210,11 @@ 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);
- if (priv->ecc_base == FDT_ADDR_T_NONE)
+ ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
+ if (ret)
priv->ecc_base = 0;
+ else
+ priv->ecc_base = res_regs.start;
priv->soc = dev_get_driver_data(dev);
Best Regards,
peng
>Thanks,
>Michal
>
>
>
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-17 7:27 ` Peng Ma
@ 2019-04-17 7:38 ` Michal Simek
2019-04-17 7:40 ` Peng Ma
0 siblings, 1 reply; 16+ messages in thread
From: Michal Simek @ 2019-04-17 7:38 UTC (permalink / raw)
To: u-boot
On 17. 04. 19 9:27, Peng Ma wrote:
>
>
>> -----Original Message-----
>> From: Michal Simek <michal.simek@xilinx.com>
>> Sent: 2019年4月17日 14:58
>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>> albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
>> <fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>> Kushwaha <prabhakar.kushwaha@nxp.com>
>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 8:50, Peng Ma wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>> Sent: 2019年4月17日 13:58
>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>
>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 4:50, Peng Ma wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>> Sent: 2019年4月16日 18:49
>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>> <prabhakar.kushwaha@nxp.com>
>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 12:29, Peng Ma wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Sent: 2019年4月16日 18:01
>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>> Sun
>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>> <yinbo.zhu@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:54, Peng Ma wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>> Sent: 2019年4月16日 17:31
>>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>>>> Sun
>>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>>> <yinbo.zhu@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@xilinx.com>
>>>>>>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; albert.u.boot at aribaud.net;
>>>>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>;
>> York
>>>>>> Sun
>>>>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>>>>> <yinbo.zhu@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@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
>>>>>>>>>> %2
>>>>>>>>>> Fg
>>>>>>>>>> it
>>>>>>>>>> .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
>>>>>>>>> %2
>>>>>>>>> Fg
>>>>>>>>> it
>>>>>>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2F
>> lin
>>>> ux.
>>>>>> gi
>>>>>>>>> t
>>>>>>>> %
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>> 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 have briefly looked at it. They should be merged together. It
>>>>>> happened to us too that we send driver out but didn't know who was
>>>>>> the vendor in past and then we found out.
>>>>>>
>>>>>>>>> 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.
>>>>>>
>>>>>> Ok. I think that in this case you should pass different .data to driver.
>>>>>>
>>>>>> struct platform_data {
>>>>>> enum ceva_soc;
>>>>>> bool ecc_present;
>>>>>> }
>>>>>>
>>>>>> It means have current enum followed by bool with "true" for
>>>>>> specific IPs which needs to have also ECC range.
>>>>>> Then you can better check that binding is correct.
>>>>>>
>>>>>> And use _index there.
>>>>> [Peng Ma] As far as I am concerned, It is not necessary to do this,
>>>>> I would rather lean on the former way as fallows:
>>>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>>>>> d26f712..a1d452a 100644
>>>>> --- a/drivers/ata/sata_ceva.c
>>>>> +++ b/drivers/ata/sata_ceva.c
>>>>> @@ -196,6 +196,10 @@ static const struct udevice_id sata_ceva_ids[]
>>>>> = { static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>>>>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>>>>> + const void *blob = gd->fdt_blob;
>>>>
>>>> gd is suggesting that you use incorrect functions.
>>>> We are trying to get rid of gd from device drivers. Please take a
>>>> look at live tree functions if there is any alternative there.
>>>>
>>>>
>>>>> + int node = dev_of_offset(dev);
>>>>> + struct fdt_resource res_regs;
>>>>> + int ret;
>>>>>
>>>>> if (dev_read_bool(dev, "dma-coherent"))
>>>>> priv->flag |= FLAG_COHERENT; @@ -204,9 +208,13
>> @@
>>>>> 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);
>>>>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>>>>> + ret = fdt_get_named_resource(blob, node, "reg", "reg-names",
>>>>> + "ecc-addr", &res_regs);
>>>>> + if (ret) {
>>>>> + debug("Error: can't get ecc addresses(ret = %d)!\n",
>>>>> + ret);
>>>>> priv->ecc_base = 0;
>>>>> + } else
>>>>> + priv->ecc_base = res_regs.start;
>>>>>
>>>>> priv->soc = dev_get_driver_data(dev); What do you think?
>>>>
>>>> One thing I would like to avoid is that we will get any error even in
>>>> debug for IPs which don't have this ecc space.
>>>> And also that we will get this errors for IPs which should have this
>>>> ecc space.
>>>>
>>> [Peng Ma] OK，changed as fallows:
>>>
>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>>> d26f712..bd98d85 100644
>>> --- a/drivers/ata/sata_ceva.c
>>> +++ b/drivers/ata/sata_ceva.c
>>> @@ -8,6 +8,7 @@
>>> #include <ahci.h>
>>> #include <scsi.h>
>>> #include <asm/io.h>
>>> +#include <linux/ioport.h>
>>>
>>> /* Vendor Specific Register Offsets */ #define AHCI_VEND_PCFG 0xA4
>>> @@ -196,6 +197,8 @@ static const struct udevice_id sata_ceva_ids[] = {
>>> static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>>> + struct resource res_regs;
>>> + int ret;
>>>
>>> if (dev_read_bool(dev, "dma-coherent"))
>>> priv->flag |= FLAG_COHERENT; @@ -204,9 +207,11
>> @@
>>> 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);
>>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>>> + ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
>>> + if (ret)
>>> priv->ecc_base = 0;
>>> + else
>>> + priv->ecc_base = res_regs.start;
>>>
>>> priv->soc = dev_get_driver_data(dev);
>>
>> This looks good but still missing information that in your case there are some
>> versions where this ecc-addr is required. It means if it is required you should
>> error it out when it is not present in DT.
>>
> [Peng Ma] Ok, It will return error when some socs need ecc address with the regs is not
> Present in DT.
>
> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
> index d26f712..6e28a38 100644
> --- a/drivers/ata/sata_ceva.c
> +++ b/drivers/ata/sata_ceva.c
> @@ -8,6 +8,7 @@
> #include <ahci.h>
> #include <scsi.h>
> #include <asm/io.h>
> +#include <linux/ioport.h>
>
> /* Vendor Specific Register Offsets */
> #define AHCI_VEND_PCFG 0xA4
> @@ -130,8 +131,9 @@ static int ceva_init_sata(struct ceva_sata_priv *priv)
> break;
>
> case CEVA_LS1021A:
> - if (ecc_addr)
> - writel(ECC_DIS_VAL_CH1, ecc_addr);
> + if (!ecc_addr)
> + return -EINVAL;
> + 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);
> @@ -143,17 +145,19 @@ static int ceva_init_sata(struct ceva_sata_priv *priv)
> case CEVA_LS1012A:
> case CEVA_LS1043A:
> case CEVA_LS1046A:
> + if (!ecc_addr)
> + return -EINVAL;
> + writel(ECC_DIS_VAL_CH2, ecc_addr);
> 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);
> break;
>
> case CEVA_LS1028A:
> case CEVA_LS1088A:
> - if (ecc_addr)
> - writel(ECC_DIS_VAL_CH3, ecc_addr);
> + if (!ecc_addr)
> + return -EINVAL;
> + writel(ECC_DIS_VAL_CH3, ecc_addr);
> writel(CEVA_PHY1_CFG, base + AHCI_VEND_PPCFG);
> writel(CEVA_TRANS_CFG, base + AHCI_VEND_PTC);
> break;
> @@ -196,6 +200,8 @@ static const struct udevice_id sata_ceva_ids[] = {
> static int sata_ceva_ofdata_to_platdata(struct udevice *dev)
> {
> struct ceva_sata_priv *priv = dev_get_priv(dev);
> + struct resource res_regs;
> + int ret;
>
> if (dev_read_bool(dev, "dma-coherent"))
> priv->flag |= FLAG_COHERENT;
> @@ -204,9 +210,11 @@ 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);
> - if (priv->ecc_base == FDT_ADDR_T_NONE)
> + ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
> + if (ret)
> priv->ecc_base = 0;
> + else
> + priv->ecc_base = res_regs.start;
>
> priv->soc = dev_get_driver_data(dev);
>
ok. Looks good.
M
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [EXT] Re: [PATCH 1/2] scsi: ceva: Clean up the driver code
2019-04-17 7:38 ` Michal Simek
@ 2019-04-17 7:40 ` Peng Ma
0 siblings, 0 replies; 16+ messages in thread
From: Peng Ma @ 2019-04-17 7:40 UTC (permalink / raw)
To: u-boot
>-----Original Message-----
>From: Michal Simek <michal.simek@xilinx.com>
>Sent: 2019年4月17日 15:38
>To: Peng Ma <peng.ma@nxp.com>; Michal Simek <michal.simek@xilinx.com>;
>albert.u.boot at aribaud.net; sjg at chromium.org; Fabio Estevam
><fabio.estevam@nxp.com>; York Sun <york.sun@nxp.com>; Prabhakar
>Kushwaha <prabhakar.kushwaha@nxp.com>
>Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 9:27, Peng Ma wrote:
>>
>>
>>> -----Original Message-----
>>> From: Michal Simek <michal.simek@xilinx.com>
>>> Sent: 2019年4月17日 14:58
>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>> <york.sun@nxp.com>; Prabhakar Kushwaha
><prabhakar.kushwaha@nxp.com>
>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 8:50, Peng Ma wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>> Sent: 2019年4月17日 13:58
>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York Sun
>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>> <prabhakar.kushwaha@nxp.com>
>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu <yinbo.zhu@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 17. 04. 19 4:50, Peng Ma wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>> Sent: 2019年4月16日 18:49
>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>Sun
>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
><yinbo.zhu@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 12:29, Peng Ma wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>> Sent: 2019年4月16日 18:01
>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>; York
>>> Sun
>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>> <yinbo.zhu@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:54, Peng Ma wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>>> Sent: 2019年4月16日 17:31
>>>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>; Michal Simek
>>>>>>>>>>> <michal.simek@xilinx.com>; albert.u.boot at aribaud.net;
>>>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>;
>York
>>>>> Sun
>>>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>>>> <yinbo.zhu@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@xilinx.com>
>>>>>>>>>>>>> Sent: 2019年4月16日 16:04
>>>>>>>>>>>>> To: Peng Ma <peng.ma@nxp.com>;
>albert.u.boot at aribaud.net;
>>>>>>>>>>>>> sjg at chromium.org; Fabio Estevam <fabio.estevam@nxp.com>;
>>> York
>>>>>>> Sun
>>>>>>>>>>>>> <york.sun@nxp.com>; Prabhakar Kushwaha
>>>>>>>>>>> <prabhakar.kushwaha@nxp.com>
>>>>>>>>>>>>> Cc: Andy Tang <andy.tang@nxp.com>; Yinbo Zhu
>>>>>>> <yinbo.zhu@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@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
>>>>>>>>>>> %2
>>>>>>>>>>> Fg
>>>>>>>>>>> it
>>>>>>>>>>> .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
>>>>>>>>>> %2
>>>>>>>>>> Fg
>>>>>>>>>> it
>>>>>>>>>> .kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%
>2F
>>> lin
>>>>> ux.
>>>>>>> gi
>>>>>>>>>> t
>>>>>>>>> %
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>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 have briefly looked at it. They should be merged together. It
>>>>>>> happened to us too that we send driver out but didn't know who
>>>>>>> was the vendor in past and then we found out.
>>>>>>>
>>>>>>>>>> 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.
>>>>>>>
>>>>>>> Ok. I think that in this case you should pass different .data to driver.
>>>>>>>
>>>>>>> struct platform_data {
>>>>>>> enum ceva_soc;
>>>>>>> bool ecc_present;
>>>>>>> }
>>>>>>>
>>>>>>> It means have current enum followed by bool with "true" for
>>>>>>> specific IPs which needs to have also ECC range.
>>>>>>> Then you can better check that binding is correct.
>>>>>>>
>>>>>>> And use _index there.
>>>>>> [Peng Ma] As far as I am concerned, It is not necessary to do
>>>>>> this, I would rather lean on the former way as fallows:
>>>>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
>>>>>> index d26f712..a1d452a 100644
>>>>>> --- a/drivers/ata/sata_ceva.c
>>>>>> +++ b/drivers/ata/sata_ceva.c
>>>>>> @@ -196,6 +196,10 @@ static const struct udevice_id
>>>>>> sata_ceva_ids[] = { static int sata_ceva_ofdata_to_platdata(struct
>udevice *dev) {
>>>>>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>>>>>> + const void *blob = gd->fdt_blob;
>>>>>
>>>>> gd is suggesting that you use incorrect functions.
>>>>> We are trying to get rid of gd from device drivers. Please take a
>>>>> look at live tree functions if there is any alternative there.
>>>>>
>>>>>
>>>>>> + int node = dev_of_offset(dev);
>>>>>> + struct fdt_resource res_regs;
>>>>>> + int ret;
>>>>>>
>>>>>> if (dev_read_bool(dev, "dma-coherent"))
>>>>>> priv->flag |= FLAG_COHERENT; @@ -204,9
>+208,13
>>> @@
>>>>>> 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);
>>>>>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>>>>>> + ret = fdt_get_named_resource(blob, node, "reg",
>"reg-names",
>>>>>> + "ecc-addr", &res_regs);
>>>>>> + if (ret) {
>>>>>> + debug("Error: can't get ecc addresses(ret =
>>>>>> + %d)!\n", ret);
>>>>>> priv->ecc_base = 0;
>>>>>> + } else
>>>>>> + priv->ecc_base = res_regs.start;
>>>>>>
>>>>>> priv->soc = dev_get_driver_data(dev); What do you think?
>>>>>
>>>>> One thing I would like to avoid is that we will get any error even
>>>>> in debug for IPs which don't have this ecc space.
>>>>> And also that we will get this errors for IPs which should have
>>>>> this ecc space.
>>>>>
>>>> [Peng Ma] OK，changed as fallows:
>>>>
>>>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>>>> d26f712..bd98d85 100644
>>>> --- a/drivers/ata/sata_ceva.c
>>>> +++ b/drivers/ata/sata_ceva.c
>>>> @@ -8,6 +8,7 @@
>>>> #include <ahci.h>
>>>> #include <scsi.h>
>>>> #include <asm/io.h>
>>>> +#include <linux/ioport.h>
>>>>
>>>> /* Vendor Specific Register Offsets */ #define AHCI_VEND_PCFG
>>>> 0xA4 @@ -196,6 +197,8 @@ static const struct udevice_id
>>>> sata_ceva_ids[] = { static int sata_ceva_ofdata_to_platdata(struct udevice
>*dev) {
>>>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>>>> + struct resource res_regs;
>>>> + int ret;
>>>>
>>>> if (dev_read_bool(dev, "dma-coherent"))
>>>> priv->flag |= FLAG_COHERENT; @@ -204,9 +207,11
>>> @@
>>>> 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);
>>>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>>>> + ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
>>>> + if (ret)
>>>> priv->ecc_base = 0;
>>>> + else
>>>> + priv->ecc_base = res_regs.start;
>>>>
>>>> priv->soc = dev_get_driver_data(dev);
>>>
>>> This looks good but still missing information that in your case there
>>> are some versions where this ecc-addr is required. It means if it is
>>> required you should error it out when it is not present in DT.
>>>
>> [Peng Ma] Ok, It will return error when some socs need ecc address
>> with the regs is not Present in DT.
>>
>> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c index
>> d26f712..6e28a38 100644
>> --- a/drivers/ata/sata_ceva.c
>> +++ b/drivers/ata/sata_ceva.c
>> @@ -8,6 +8,7 @@
>> #include <ahci.h>
>> #include <scsi.h>
>> #include <asm/io.h>
>> +#include <linux/ioport.h>
>>
>> /* Vendor Specific Register Offsets */ #define AHCI_VEND_PCFG 0xA4
>> @@ -130,8 +131,9 @@ static int ceva_init_sata(struct ceva_sata_priv *priv)
>> break;
>>
>> case CEVA_LS1021A:
>> - if (ecc_addr)
>> - writel(ECC_DIS_VAL_CH1, ecc_addr);
>> + if (!ecc_addr)
>> + return -EINVAL;
>> + 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);
>> @@ -143,17 +145,19 @@ static int ceva_init_sata(struct ceva_sata_priv
>*priv)
>> case CEVA_LS1012A:
>> case CEVA_LS1043A:
>> case CEVA_LS1046A:
>> + if (!ecc_addr)
>> + return -EINVAL;
>> + writel(ECC_DIS_VAL_CH2, ecc_addr);
>> 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);
>> break;
>>
>> case CEVA_LS1028A:
>> case CEVA_LS1088A:
>> - if (ecc_addr)
>> - writel(ECC_DIS_VAL_CH3, ecc_addr);
>> + if (!ecc_addr)
>> + return -EINVAL;
>> + writel(ECC_DIS_VAL_CH3, ecc_addr);
>> writel(CEVA_PHY1_CFG, base + AHCI_VEND_PPCFG);
>> writel(CEVA_TRANS_CFG, base + AHCI_VEND_PTC);
>> break;
>> @@ -196,6 +200,8 @@ static const struct udevice_id sata_ceva_ids[] = {
>> static int sata_ceva_ofdata_to_platdata(struct udevice *dev) {
>> struct ceva_sata_priv *priv = dev_get_priv(dev);
>> + struct resource res_regs;
>> + int ret;
>>
>> if (dev_read_bool(dev, "dma-coherent"))
>> priv->flag |= FLAG_COHERENT; @@ -204,9 +210,11
>@@
>> 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);
>> - if (priv->ecc_base == FDT_ADDR_T_NONE)
>> + ret = dev_read_resource_byname(dev, "ecc-addr", &res_regs);
>> + if (ret)
>> priv->ecc_base = 0;
>> + else
>> + priv->ecc_base = res_regs.start;
>>
>> priv->soc = dev_get_driver_data(dev);
>>
>
>ok. Looks good.
>
[Peng Ma] Thanks very much for your patient guidance.
Best Regards,
Peng
>M
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-04-17 7:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 7:28 [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Peng Ma
2019-04-16 7:28 ` [U-Boot] [PATCH 2/2] ARM: dts: Freescale: Add ecc addr for sata node Peng Ma
2019-04-16 8:04 ` [U-Boot] [PATCH 1/2] scsi: ceva: Clean up the driver code Michal Simek
2019-04-16 9:22 ` [U-Boot] [EXT] " Peng Ma
2019-04-16 9:31 ` Michal Simek
2019-04-16 9:54 ` Peng Ma
2019-04-16 10:00 ` Michal Simek
2019-04-16 10:29 ` Peng Ma
2019-04-16 10:49 ` Michal Simek
2019-04-17 2:50 ` Peng Ma
2019-04-17 5:58 ` Michal Simek
2019-04-17 6:50 ` Peng Ma
2019-04-17 6:57 ` Michal Simek
2019-04-17 7:27 ` Peng Ma
2019-04-17 7:38 ` Michal Simek
2019-04-17 7:40 ` Peng Ma
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.