On 17. 04. 19 9:27, Peng Ma wrote: > > >> -----Original Message----- >> From: Michal Simek <[email protected]> >> Sent: 2019年4月17日 14:58 >> To: Peng Ma <[email protected]>; Michal Simek <[email protected]>; >> [email protected]; [email protected]; Fabio Estevam >> <[email protected]>; York Sun <[email protected]>; Prabhakar >> Kushwaha <[email protected]> >> Cc: Andy Tang <[email protected]>; Yinbo Zhu <[email protected]>; >> [email protected] >> 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 <[email protected]> >>>> Sent: 2019年4月17日 13:58 >>>> To: Peng Ma <[email protected]>; Michal Simek >>>> <[email protected]>; [email protected]; >>>> [email protected]; Fabio Estevam <[email protected]>; York Sun >>>> <[email protected]>; Prabhakar Kushwaha >> <[email protected]> >>>> Cc: Andy Tang <[email protected]>; Yinbo Zhu <[email protected]>; >>>> [email protected] >>>> 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 <[email protected]> >>>>>> Sent: 2019年4月16日 18:49 >>>>>> To: Peng Ma <[email protected]>; Michal Simek >>>>>> <[email protected]>; [email protected]; >>>>>> [email protected]; Fabio Estevam <[email protected]>; York Sun >>>>>> <[email protected]>; Prabhakar Kushwaha >>>> <[email protected]> >>>>>> Cc: Andy Tang <[email protected]>; Yinbo Zhu <[email protected]>; >>>>>> [email protected] >>>>>> 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 <[email protected]> >>>>>>>> Sent: 2019年4月16日 18:01 >>>>>>>> To: Peng Ma <[email protected]>; Michal Simek >>>>>>>> <[email protected]>; [email protected]; >>>>>>>> [email protected]; Fabio Estevam <[email protected]>; York >> Sun >>>>>>>> <[email protected]>; Prabhakar Kushwaha >>>>>> <[email protected]> >>>>>>>> Cc: Andy Tang <[email protected]>; Yinbo Zhu >> <[email protected]>; >>>>>>>> [email protected] >>>>>>>> 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 <[email protected]> >>>>>>>>>> Sent: 2019年4月16日 17:31 >>>>>>>>>> To: Peng Ma <[email protected]>; Michal Simek >>>>>>>>>> <[email protected]>; [email protected]; >>>>>>>>>> [email protected]; Fabio Estevam <[email protected]>; York >>>> Sun >>>>>>>>>> <[email protected]>; Prabhakar Kushwaha >>>>>>>> <[email protected]> >>>>>>>>>> Cc: Andy Tang <[email protected]>; Yinbo Zhu >>>> <[email protected]>; >>>>>>>>>> [email protected] >>>>>>>>>> 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 <[email protected]> >>>>>>>>>>>> Sent: 2019年4月16日 16:04 >>>>>>>>>>>> To: Peng Ma <[email protected]>; [email protected]; >>>>>>>>>>>> [email protected]; Fabio Estevam <[email protected]>; >> York >>>>>> Sun >>>>>>>>>>>> <[email protected]>; Prabhakar Kushwaha >>>>>>>>>> <[email protected]> >>>>>>>>>>>> Cc: Andy Tang <[email protected]>; Yinbo Zhu >>>>>> <[email protected]>; >>>>>>>>>>>> [email protected]; [email protected] >>>>>>>>>>>> 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 <[email protected]> >>>>>>>>>>>>> --- >>>>>>>>>>>>> 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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
