| Message ID | 1437471937-34218-1-git-send-email-yangbo.lu@freescale.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 21 July 2015 at 11:45, Yangbo Lu <yangbo.lu@freescale.com> wrote: > For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender > version value and sdhc spec version value. This will break down > the ADMA data transfer. So add workaround to get right value > VVN=0x13, SVN = 0x1. So T4240-R1.0-R2.0 is the version of the controller, right? If I understand correct you are checking what CPU/SoC you are running on, to figure out which controller version you are using, as that can't be fetched (trusted) from the registers of the esdhc controller itself!? Instead, you could deal with this directly in the DTS files. I assume you have some DTS file for each SoC/board variant, right? In principle, in your DTS file specific for the board/SoC that holds the T4240-R1.0-R2.0 version of the controller, should add a specific esdhc DT property to indicate this errata. This DT property will then be parsed by the esdhc driver. > > Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com> > --- > drivers/mmc/host/sdhci-esdhc.h | 5 +++++ > drivers/mmc/host/sdhci-of-esdhc.c | 18 ++++++++++++++++++ > 2 files changed, 23 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h > index 3497cfa..5174233 100644 > --- a/drivers/mmc/host/sdhci-esdhc.h > +++ b/drivers/mmc/host/sdhci-esdhc.h > @@ -47,4 +47,9 @@ > > #define ESDHC_HOST_CONTROL_RES 0x05 > > +unsigned int esdhc_errata; > + > +/* Incorrect host version in T4240 HOSTVER register */ > +#define ESDHC_ERRATA_T4240_ERROR_HOSTVER (1<<0) > + > #endif /* _DRIVERS_MMC_SDHCI_ESDHC_H */ > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c > index 1295a96..c0ad765 100644 > --- a/drivers/mmc/host/sdhci-of-esdhc.c > +++ b/drivers/mmc/host/sdhci-of-esdhc.c > @@ -59,6 +59,11 @@ static u16 esdhc_readw(struct sdhci_host *host, int reg) > ret = in_be32(host->ioaddr + base) & 0xffff; > else > ret = (in_be32(host->ioaddr + base) >> shift) & 0xffff; > + > + if ((reg == SDHCI_HOST_VERSION) && > + (esdhc_errata & ESDHC_ERRATA_T4240_ERROR_HOSTVER)) > + ret = (VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200; > + > return ret; > } > > @@ -352,6 +357,9 @@ static void esdhc_get_property(struct platform_device *pdev) > { > struct device_node *np = pdev->dev.of_node; > struct sdhci_host *host = platform_get_drvdata(pdev); > + struct device_node *cpus; > + const u32 *prop; > + u8 rev = 0xff; > > sdhci_get_of_property(pdev); > > @@ -359,6 +367,11 @@ static void esdhc_get_property(struct platform_device *pdev) > mmc_of_parse(host->mmc); > mmc_of_parse_voltage(np, &host->ocr_mask); > > + cpus = of_find_node_by_path("/cpus"); > + prop = of_get_property(cpus, "cpu-rev", NULL); > + if (prop) > + rev = *(u8 *)prop; > + > if (of_device_is_compatible(np, "fsl,p5040-esdhc") || > of_device_is_compatible(np, "fsl,p5020-esdhc") || > of_device_is_compatible(np, "fsl,p4080-esdhc") || > @@ -373,6 +386,11 @@ static void esdhc_get_property(struct platform_device *pdev) > */ > host->quirks2 |= SDHCI_QUIRK2_BROKEN_HOST_CONTROL; > } > + > + esdhc_errata = 0; > + > + if (of_device_is_compatible(np, "fsl,t4240-esdhc") && rev <= 0x20) > + esdhc_errata |= ESDHC_ERRATA_T4240_ERROR_HOSTVER; > } > > static int sdhci_esdhc_probe(struct platform_device *pdev) > -- > 2.1.0.27.g96db324 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-07-21 at 15:02 +0200, Ulf Hansson wrote: > On 21 July 2015 at 11:45, Yangbo Lu <yangbo.lu@freescale.com> wrote: > > For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender > > version value and sdhc spec version value. This will break down > > the ADMA data transfer. So add workaround to get right value > > VVN=0x13, SVN = 0x1. > > So T4240-R1.0-R2.0 is the version of the controller, right? > > If I understand correct you are checking what CPU/SoC you are running > on, to figure out which controller version you are using, as that > can't be fetched (trusted) from the registers of the esdhc controller > itself!? > > Instead, you could deal with this directly in the DTS files. I assume > you have some DTS file for each SoC/board variant, right? No, we do not have a separate DTS file for each revision of an SoC -- and if we did, we'd constantly have people using the wrong one. > In principle, in your DTS file specific for the board/SoC that holds > the T4240-R1.0-R2.0 version of the controller, should add a specific > esdhc DT property to indicate this errata. No, because (in addition to the above issue about chip revisions) the device tree is stable ABI and errata are often discovered after device trees are deployed. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25 July 2015 at 04:27, Scott Wood <scottwood@freescale.com> wrote: > On Tue, 2015-07-21 at 15:02 +0200, Ulf Hansson wrote: >> On 21 July 2015 at 11:45, Yangbo Lu <yangbo.lu@freescale.com> wrote: >> > For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender >> > version value and sdhc spec version value. This will break down >> > the ADMA data transfer. So add workaround to get right value >> > VVN=0x13, SVN = 0x1. >> >> So T4240-R1.0-R2.0 is the version of the controller, right? >> >> If I understand correct you are checking what CPU/SoC you are running >> on, to figure out which controller version you are using, as that >> can't be fetched (trusted) from the registers of the esdhc controller >> itself!? >> >> Instead, you could deal with this directly in the DTS files. I assume >> you have some DTS file for each SoC/board variant, right? > > No, we do not have a separate DTS file for each revision of an SoC -- and if > we did, we'd constantly have people using the wrong one. > >> In principle, in your DTS file specific for the board/SoC that holds >> the T4240-R1.0-R2.0 version of the controller, should add a specific >> esdhc DT property to indicate this errata. > > No, because (in addition to the above issue about chip revisions) the device > tree is stable ABI and errata are often discovered after device trees are > deployed. Fair enough. Then what is your suggestion for the solution here? As I understand it, you can't update the already deployed DTB. Shall we make Linux patch the DTB during boot instead, depending on the chip revision? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-07-27 at 09:58 +0200, Ulf Hansson wrote: > On 25 July 2015 at 04:27, Scott Wood <scottwood@freescale.com> wrote: > > On Tue, 2015-07-21 at 15:02 +0200, Ulf Hansson wrote: > > > On 21 July 2015 at 11:45, Yangbo Lu <yangbo.lu@freescale.com> wrote: > > > > For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender > > > > version value and sdhc spec version value. This will break down > > > > the ADMA data transfer. So add workaround to get right value > > > > VVN=0x13, SVN = 0x1. > > > > > > So T4240-R1.0-R2.0 is the version of the controller, right? > > > > > > If I understand correct you are checking what CPU/SoC you are running > > > on, to figure out which controller version you are using, as that > > > can't be fetched (trusted) from the registers of the esdhc controller > > > itself!? > > > > > > Instead, you could deal with this directly in the DTS files. I assume > > > you have some DTS file for each SoC/board variant, right? > > > > No, we do not have a separate DTS file for each revision of an SoC -- and > > if > > we did, we'd constantly have people using the wrong one. > > > > > In principle, in your DTS file specific for the board/SoC that holds > > > the T4240-R1.0-R2.0 version of the controller, should add a specific > > > esdhc DT property to indicate this errata. > > > > No, because (in addition to the above issue about chip revisions) the > > device > > tree is stable ABI and errata are often discovered after device trees are > > deployed. > > Fair enough. Then what is your suggestion for the solution here? As I said in my comment on patch 2/3, read SVR from the device-config/guts MMIO block, which works on both PPC and ARM. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27 July 2015 at 17:57, Scott Wood <scottwood@freescale.com> wrote: > On Mon, 2015-07-27 at 09:58 +0200, Ulf Hansson wrote: >> On 25 July 2015 at 04:27, Scott Wood <scottwood@freescale.com> wrote: >> > On Tue, 2015-07-21 at 15:02 +0200, Ulf Hansson wrote: >> > > On 21 July 2015 at 11:45, Yangbo Lu <yangbo.lu@freescale.com> wrote: >> > > > For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender >> > > > version value and sdhc spec version value. This will break down >> > > > the ADMA data transfer. So add workaround to get right value >> > > > VVN=0x13, SVN = 0x1. >> > > >> > > So T4240-R1.0-R2.0 is the version of the controller, right? >> > > >> > > If I understand correct you are checking what CPU/SoC you are running >> > > on, to figure out which controller version you are using, as that >> > > can't be fetched (trusted) from the registers of the esdhc controller >> > > itself!? >> > > >> > > Instead, you could deal with this directly in the DTS files. I assume >> > > you have some DTS file for each SoC/board variant, right? >> > >> > No, we do not have a separate DTS file for each revision of an SoC -- and >> > if >> > we did, we'd constantly have people using the wrong one. >> > >> > > In principle, in your DTS file specific for the board/SoC that holds >> > > the T4240-R1.0-R2.0 version of the controller, should add a specific >> > > esdhc DT property to indicate this errata. >> > >> > No, because (in addition to the above issue about chip revisions) the >> > device >> > tree is stable ABI and errata are often discovered after device trees are >> > deployed. >> >> Fair enough. Then what is your suggestion for the solution here? > > As I said in my comment on patch 2/3, read SVR from the device-config/guts > MMIO block, which works on both PPC and ARM. > That's okay, as long as don't have include sections of non generic header files in the driver. To be more clear, this is not okay to have: #include <mach/*> Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2015-08-26 at 02:49 -0500, Lu Yangbo-B47093 wrote: > > > > > > For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender > > > > > > version value and sdhc spec version value. This will break down > > > > > > the ADMA data transfer. So add workaround to get right value > > > > > > VVN=0x13, SVN = 0x1. > > > > > > > > > > So T4240-R1.0-R2.0 is the version of the controller, right? > > > > > > > > > > If I understand correct you are checking what CPU/SoC you are > > > > > running on, to figure out which controller version you are using, > > > > > as that can't be fetched (trusted) from the registers of the esdhc > > > > > controller itself!? > > > > > > > > > > Instead, you could deal with this directly in the DTS files. I > > > > > assume you have some DTS file for each SoC/board variant, right? > > > > > > > > No, we do not have a separate DTS file for each revision of an SoC > > > > -- and if we did, we'd constantly have people using the wrong one. > > > > > > > > > In principle, in your DTS file specific for the board/SoC that > > > > > holds the T4240-R1.0-R2.0 version of the controller, should add a > > > > > specific esdhc DT property to indicate this errata. > > > > > > > > No, because (in addition to the above issue about chip revisions) > > > > the device tree is stable ABI and errata are often discovered after > > > > device trees are deployed. > > > > > > Fair enough. Then what is your suggestion for the solution here? > > > > As I said in my comment on patch 2/3, read SVR from the device- > > config/guts MMIO block, which works on both PPC and ARM. > > > > -Scott > > Thanks, Scott. > I checked the device nodes of device-config/guts, finding all the platforms > has not a uniform compatible name. > Could I add a compatible name called "fsl, dcfg"("fsl, guts" if using guts) > for PowerPC P series, T series, B series boards, so that I could get this > device node by compatible without knowing platform? No. It's too vague. Linux should keep a list of all the nodes it can match against for this. Centralize it in one place rather than having each driver that cares about SVR do it. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h index 3497cfa..5174233 100644 --- a/drivers/mmc/host/sdhci-esdhc.h +++ b/drivers/mmc/host/sdhci-esdhc.h @@ -47,4 +47,9 @@ #define ESDHC_HOST_CONTROL_RES 0x05 +unsigned int esdhc_errata; + +/* Incorrect host version in T4240 HOSTVER register */ +#define ESDHC_ERRATA_T4240_ERROR_HOSTVER (1<<0) + #endif /* _DRIVERS_MMC_SDHCI_ESDHC_H */ diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 1295a96..c0ad765 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -59,6 +59,11 @@ static u16 esdhc_readw(struct sdhci_host *host, int reg) ret = in_be32(host->ioaddr + base) & 0xffff; else ret = (in_be32(host->ioaddr + base) >> shift) & 0xffff; + + if ((reg == SDHCI_HOST_VERSION) && + (esdhc_errata & ESDHC_ERRATA_T4240_ERROR_HOSTVER)) + ret = (VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200; + return ret; } @@ -352,6 +357,9 @@ static void esdhc_get_property(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct sdhci_host *host = platform_get_drvdata(pdev); + struct device_node *cpus; + const u32 *prop; + u8 rev = 0xff; sdhci_get_of_property(pdev); @@ -359,6 +367,11 @@ static void esdhc_get_property(struct platform_device *pdev) mmc_of_parse(host->mmc); mmc_of_parse_voltage(np, &host->ocr_mask); + cpus = of_find_node_by_path("/cpus"); + prop = of_get_property(cpus, "cpu-rev", NULL); + if (prop) + rev = *(u8 *)prop; + if (of_device_is_compatible(np, "fsl,p5040-esdhc") || of_device_is_compatible(np, "fsl,p5020-esdhc") || of_device_is_compatible(np, "fsl,p4080-esdhc") || @@ -373,6 +386,11 @@ static void esdhc_get_property(struct platform_device *pdev) */ host->quirks2 |= SDHCI_QUIRK2_BROKEN_HOST_CONTROL; } + + esdhc_errata = 0; + + if (of_device_is_compatible(np, "fsl,t4240-esdhc") && rev <= 0x20) + esdhc_errata |= ESDHC_ERRATA_T4240_ERROR_HOSTVER; } static int sdhci_esdhc_probe(struct platform_device *pdev)
For T4240-R1.0-R2.0, the HOSTVER register has incorrcet vender version value and sdhc spec version value. This will break down the ADMA data transfer. So add workaround to get right value VVN=0x13, SVN = 0x1. Signed-off-by: Yangbo Lu <yangbo.lu@freescale.com> --- drivers/mmc/host/sdhci-esdhc.h | 5 +++++ drivers/mmc/host/sdhci-of-esdhc.c | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+)