2016-03-09 10:18:15

by Yangbo Lu

[permalink] [raw]
Subject: [v6, 0/5] Fix eSDHC host version register bug

This patchset is used to fix a host version register bug in the T4240-R1.0-R2.0
eSDHC controller. To get the SoC version and revision, it's needed to add the
GUTS driver to access the global utilities registers.

So, the first three patches are to add the GUTS driver.
The following two patches are to enable GUTS driver support to get SVR in eSDHC
driver and fix host version for T4240.

Yangbo Lu (5):
ARM64: dts: ls2080a: add device configuration node
soc: fsl: add GUTS driver for QorIQ platforms
dt: move guts devicetree doc out of powerpc directory
powerpc/fsl: move mpc85xx.h to include/linux/fsl
mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

.../bindings/{powerpc => soc}/fsl/guts.txt | 3 +
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 6 ++
drivers/clk/clk-qoriq.c | 3 +-
drivers/i2c/busses/i2c-mpc.c | 2 +-
drivers/iommu/fsl_pamu.c | 3 +-
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-of-esdhc.c | 23 ++++
drivers/net/ethernet/freescale/gianfar.c | 2 +-
drivers/soc/Kconfig | 2 +-
drivers/soc/fsl/Kconfig | 8 ++
drivers/soc/fsl/Makefile | 1 +
drivers/soc/fsl/guts.c | 119 +++++++++++++++++++++
include/linux/fsl/guts.h | 100 ++++++++---------
.../asm/mpc85xx.h => include/linux/fsl/svr.h | 4 +-
14 files changed, 220 insertions(+), 57 deletions(-)
rename Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
create mode 100644 drivers/soc/fsl/Kconfig
create mode 100644 drivers/soc/fsl/guts.c
rename arch/powerpc/include/asm/mpc85xx.h => include/linux/fsl/svr.h (97%)

--
2.1.0.27.g96db324


2016-03-09 10:33:03

by Yangbo Lu

[permalink] [raw]
Subject: [v6, 2/5] soc: fsl: add GUTS driver for QorIQ platforms

The global utilities block controls power management, I/O device
enabling, power-onreset(POR) configuration monitoring, alternate
function selection for multiplexed signals,and clock control.

This patch adds GUTS driver to manage and access global utilities
block.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v2:
- None
Changes for v3:
- None
Changes for v4:
- Added this patch
Changes for v5:
- Modified copyright info
- Changed MODULE_LICENSE to GPL
- Changed EXPORT_SYMBOL_GPL to EXPORT_SYMBOL
- Made FSL_GUTS user-invisible
- Added a complete compatible list for GUTS
- Stored guts info in file-scope variable
- Added mfspr() getting SVR
- Redefined GUTS APIs
- Called fsl_guts_init rather than using platform driver
- Removed useless parentheses
- Removed useless 'extern' key words
Changes for v6:
- Made guts thread safe in fsl_guts_init
---
drivers/soc/Kconfig | 2 +-
drivers/soc/fsl/Kconfig | 8 ++++
drivers/soc/fsl/Makefile | 1 +
drivers/soc/fsl/guts.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fsl/guts.h | 100 ++++++++++++++++++++-------------------
5 files changed, 181 insertions(+), 49 deletions(-)
create mode 100644 drivers/soc/fsl/Kconfig
create mode 100644 drivers/soc/fsl/guts.c

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 8826020..c4b6043 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -2,7 +2,7 @@ menu "SOC (System On Chip) specific Drivers"

source "drivers/soc/bcm/Kconfig"
source "drivers/soc/brcmstb/Kconfig"
-source "drivers/soc/fsl/qe/Kconfig"
+source "drivers/soc/fsl/Kconfig"
source "drivers/soc/mediatek/Kconfig"
source "drivers/soc/qcom/Kconfig"
source "drivers/soc/rockchip/Kconfig"
diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
new file mode 100644
index 0000000..b313759
--- /dev/null
+++ b/drivers/soc/fsl/Kconfig
@@ -0,0 +1,8 @@
+#
+# Freescale SOC drivers
+#
+
+source "drivers/soc/fsl/qe/Kconfig"
+
+config FSL_GUTS
+ bool
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 203307f..02afb7f 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -4,3 +4,4 @@

obj-$(CONFIG_QUICC_ENGINE) += qe/
obj-$(CONFIG_CPM) += qe/
+obj-$(CONFIG_FSL_GUTS) += guts.o
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
new file mode 100644
index 0000000..fa155e6
--- /dev/null
+++ b/drivers/soc/fsl/guts.c
@@ -0,0 +1,119 @@
+/*
+ * Freescale QorIQ Platforms GUTS Driver
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/fsl/guts.h>
+
+struct guts {
+ struct ccsr_guts __iomem *regs;
+ bool little_endian;
+};
+
+static struct guts *guts;
+static DEFINE_MUTEX(guts_lock);
+
+u32 fsl_guts_get_svr(void)
+{
+ u32 svr = 0;
+
+ if (!guts || !guts->regs) {
+#ifdef CONFIG_PPC
+ svr = mfspr(SPRN_SVR);
+#endif
+ return svr;
+ }
+
+ if (guts->little_endian)
+ svr = ioread32(&guts->regs->svr);
+ else
+ svr = ioread32be(&guts->regs->svr);
+
+ return svr;
+}
+EXPORT_SYMBOL(fsl_guts_get_svr);
+
+/*
+ * Table for matching compatible strings, for device tree
+ * guts node, for Freescale QorIQ SOCs.
+ */
+static const struct of_device_id guts_of_match[] = {
+ /* For T4 & B4 Series SOCs */
+ { .compatible = "fsl,qoriq-device-config-1.0", },
+ /* For P Series SOCs */
+ { .compatible = "fsl,qoriq-device-config-2.0", },
+ { .compatible = "fsl,p1010-guts", },
+ { .compatible = "fsl,p1020-guts", },
+ { .compatible = "fsl,p1021-guts", },
+ { .compatible = "fsl,p1022-guts", },
+ { .compatible = "fsl,p1023-guts", },
+ { .compatible = "fsl,p2020-guts", },
+ /* For BSC Series SOCs */
+ { .compatible = "fsl,bsc9131-guts", },
+ { .compatible = "fsl,bsc9132-guts", },
+ /* For MPC85xx Series SOCs */
+ { .compatible = "fsl,mpc8536-guts", },
+ { .compatible = "fsl,mpc8544-guts", },
+ { .compatible = "fsl,mpc8548-guts", },
+ { .compatible = "fsl,mpc8568-guts", },
+ { .compatible = "fsl,mpc8569-guts", },
+ { .compatible = "fsl,mpc8572-guts", },
+ /* For Layerscape Series SOCs */
+ { .compatible = "fsl,ls1021a-dcfg", },
+ { .compatible = "fsl,ls1043a-dcfg", },
+ { .compatible = "fsl,ls2080a-dcfg", },
+ {}
+};
+
+int fsl_guts_init(void)
+{
+ struct device_node *np;
+ int ret;
+
+ mutex_lock(&guts_lock);
+ /* Initialize guts only once */
+ if (guts) {
+ ret = guts->regs ? 0 : -ENOMEM;
+ goto out;
+ }
+
+ np = of_find_matching_node(NULL, guts_of_match);
+ if (!np) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ guts = kzalloc(sizeof(*guts), GFP_KERNEL);
+ if (!guts) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ guts->little_endian = of_property_read_bool(np, "little-endian");
+
+ guts->regs = of_iomap(np, 0);
+ if (!guts->regs) {
+ ret = -ENOMEM;
+ kfree(guts);
+ goto out;
+ }
+
+ of_node_put(np);
+ ret = 0;
+out:
+ mutex_unlock(&guts_lock);
+ return ret;
+}
+EXPORT_SYMBOL(fsl_guts_init);
diff --git a/include/linux/fsl/guts.h b/include/linux/fsl/guts.h
index 84d971f..b4db430 100644
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -29,83 +29,87 @@
* #ifdefs.
*/
struct ccsr_guts {
- __be32 porpllsr; /* 0x.0000 - POR PLL Ratio Status Register */
- __be32 porbmsr; /* 0x.0004 - POR Boot Mode Status Register */
- __be32 porimpscr; /* 0x.0008 - POR I/O Impedance Status and Control Register */
- __be32 pordevsr; /* 0x.000c - POR I/O Device Status Register */
- __be32 pordbgmsr; /* 0x.0010 - POR Debug Mode Status Register */
- __be32 pordevsr2; /* 0x.0014 - POR device status register 2 */
+ u32 porpllsr; /* 0x.0000 - POR PLL Ratio Status Register */
+ u32 porbmsr; /* 0x.0004 - POR Boot Mode Status Register */
+ u32 porimpscr; /* 0x.0008 - POR I/O Impedance Status and Control Register */
+ u32 pordevsr; /* 0x.000c - POR I/O Device Status Register */
+ u32 pordbgmsr; /* 0x.0010 - POR Debug Mode Status Register */
+ u32 pordevsr2; /* 0x.0014 - POR device status register 2 */
u8 res018[0x20 - 0x18];
- __be32 porcir; /* 0x.0020 - POR Configuration Information Register */
+ u32 porcir; /* 0x.0020 - POR Configuration Information Register */
u8 res024[0x30 - 0x24];
- __be32 gpiocr; /* 0x.0030 - GPIO Control Register */
+ u32 gpiocr; /* 0x.0030 - GPIO Control Register */
u8 res034[0x40 - 0x34];
- __be32 gpoutdr; /* 0x.0040 - General-Purpose Output Data Register */
+ u32 gpoutdr; /* 0x.0040 - General-Purpose Output Data Register */
u8 res044[0x50 - 0x44];
- __be32 gpindr; /* 0x.0050 - General-Purpose Input Data Register */
+ u32 gpindr; /* 0x.0050 - General-Purpose Input Data Register */
u8 res054[0x60 - 0x54];
- __be32 pmuxcr; /* 0x.0060 - Alternate Function Signal Multiplex Control */
- __be32 pmuxcr2; /* 0x.0064 - Alternate function signal multiplex control 2 */
- __be32 dmuxcr; /* 0x.0068 - DMA Mux Control Register */
+ u32 pmuxcr; /* 0x.0060 - Alternate Function Signal Multiplex Control */
+ u32 pmuxcr2; /* 0x.0064 - Alternate function signal multiplex control 2 */
+ u32 dmuxcr; /* 0x.0068 - DMA Mux Control Register */
u8 res06c[0x70 - 0x6c];
- __be32 devdisr; /* 0x.0070 - Device Disable Control */
+ u32 devdisr; /* 0x.0070 - Device Disable Control */
#define CCSR_GUTS_DEVDISR_TB1 0x00001000
#define CCSR_GUTS_DEVDISR_TB0 0x00004000
- __be32 devdisr2; /* 0x.0074 - Device Disable Control 2 */
+ u32 devdisr2; /* 0x.0074 - Device Disable Control 2 */
u8 res078[0x7c - 0x78];
- __be32 pmjcr; /* 0x.007c - 4 Power Management Jog Control Register */
- __be32 powmgtcsr; /* 0x.0080 - Power Management Status and Control Register */
- __be32 pmrccr; /* 0x.0084 - Power Management Reset Counter Configuration Register */
- __be32 pmpdccr; /* 0x.0088 - Power Management Power Down Counter Configuration Register */
- __be32 pmcdr; /* 0x.008c - 4Power management clock disable register */
- __be32 mcpsumr; /* 0x.0090 - Machine Check Summary Register */
- __be32 rstrscr; /* 0x.0094 - Reset Request Status and Control Register */
- __be32 ectrstcr; /* 0x.0098 - Exception reset control register */
- __be32 autorstsr; /* 0x.009c - Automatic reset status register */
- __be32 pvr; /* 0x.00a0 - Processor Version Register */
- __be32 svr; /* 0x.00a4 - System Version Register */
+ u32 pmjcr; /* 0x.007c - 4 Power Management Jog Control Register */
+ u32 powmgtcsr; /* 0x.0080 - Power Management Status and Control Register */
+ u32 pmrccr; /* 0x.0084 - Power Management Reset Counter Configuration Register */
+ u32 pmpdccr; /* 0x.0088 - Power Management Power Down Counter Configuration Register */
+ u32 pmcdr; /* 0x.008c - 4Power management clock disable register */
+ u32 mcpsumr; /* 0x.0090 - Machine Check Summary Register */
+ u32 rstrscr; /* 0x.0094 - Reset Request Status and Control Register */
+ u32 ectrstcr; /* 0x.0098 - Exception reset control register */
+ u32 autorstsr; /* 0x.009c - Automatic reset status register */
+ u32 pvr; /* 0x.00a0 - Processor Version Register */
+ u32 svr; /* 0x.00a4 - System Version Register */
u8 res0a8[0xb0 - 0xa8];
- __be32 rstcr; /* 0x.00b0 - Reset Control Register */
+ u32 rstcr; /* 0x.00b0 - Reset Control Register */
u8 res0b4[0xc0 - 0xb4];
- __be32 iovselsr; /* 0x.00c0 - I/O voltage select status register
+ u32 iovselsr; /* 0x.00c0 - I/O voltage select status register
Called 'elbcvselcr' on 86xx SOCs */
u8 res0c4[0x100 - 0xc4];
- __be32 rcwsr[16]; /* 0x.0100 - Reset Control Word Status registers
+ u32 rcwsr[16]; /* 0x.0100 - Reset Control Word Status registers
There are 16 registers */
u8 res140[0x224 - 0x140];
- __be32 iodelay1; /* 0x.0224 - IO delay control register 1 */
- __be32 iodelay2; /* 0x.0228 - IO delay control register 2 */
+ u32 iodelay1; /* 0x.0224 - IO delay control register 1 */
+ u32 iodelay2; /* 0x.0228 - IO delay control register 2 */
u8 res22c[0x604 - 0x22c];
- __be32 pamubypenr; /* 0x.604 - PAMU bypass enable register */
+ u32 pamubypenr; /* 0x.604 - PAMU bypass enable register */
u8 res608[0x800 - 0x608];
- __be32 clkdvdr; /* 0x.0800 - Clock Divide Register */
+ u32 clkdvdr; /* 0x.0800 - Clock Divide Register */
u8 res804[0x900 - 0x804];
- __be32 ircr; /* 0x.0900 - Infrared Control Register */
+ u32 ircr; /* 0x.0900 - Infrared Control Register */
u8 res904[0x908 - 0x904];
- __be32 dmacr; /* 0x.0908 - DMA Control Register */
+ u32 dmacr; /* 0x.0908 - DMA Control Register */
u8 res90c[0x914 - 0x90c];
- __be32 elbccr; /* 0x.0914 - eLBC Control Register */
+ u32 elbccr; /* 0x.0914 - eLBC Control Register */
u8 res918[0xb20 - 0x918];
- __be32 ddr1clkdr; /* 0x.0b20 - DDR1 Clock Disable Register */
- __be32 ddr2clkdr; /* 0x.0b24 - DDR2 Clock Disable Register */
- __be32 ddrclkdr; /* 0x.0b28 - DDR Clock Disable Register */
+ u32 ddr1clkdr; /* 0x.0b20 - DDR1 Clock Disable Register */
+ u32 ddr2clkdr; /* 0x.0b24 - DDR2 Clock Disable Register */
+ u32 ddrclkdr; /* 0x.0b28 - DDR Clock Disable Register */
u8 resb2c[0xe00 - 0xb2c];
- __be32 clkocr; /* 0x.0e00 - Clock Out Select Register */
+ u32 clkocr; /* 0x.0e00 - Clock Out Select Register */
u8 rese04[0xe10 - 0xe04];
- __be32 ddrdllcr; /* 0x.0e10 - DDR DLL Control Register */
+ u32 ddrdllcr; /* 0x.0e10 - DDR DLL Control Register */
u8 rese14[0xe20 - 0xe14];
- __be32 lbcdllcr; /* 0x.0e20 - LBC DLL Control Register */
- __be32 cpfor; /* 0x.0e24 - L2 charge pump fuse override register */
+ u32 lbcdllcr; /* 0x.0e20 - LBC DLL Control Register */
+ u32 cpfor; /* 0x.0e24 - L2 charge pump fuse override register */
u8 rese28[0xf04 - 0xe28];
- __be32 srds1cr0; /* 0x.0f04 - SerDes1 Control Register 0 */
- __be32 srds1cr1; /* 0x.0f08 - SerDes1 Control Register 0 */
+ u32 srds1cr0; /* 0x.0f04 - SerDes1 Control Register 0 */
+ u32 srds1cr1; /* 0x.0f08 - SerDes1 Control Register 0 */
u8 resf0c[0xf2c - 0xf0c];
- __be32 itcr; /* 0x.0f2c - Internal transaction control register */
+ u32 itcr; /* 0x.0f2c - Internal transaction control register */
u8 resf30[0xf40 - 0xf30];
- __be32 srds2cr0; /* 0x.0f40 - SerDes2 Control Register 0 */
- __be32 srds2cr1; /* 0x.0f44 - SerDes2 Control Register 0 */
+ u32 srds2cr0; /* 0x.0f40 - SerDes2 Control Register 0 */
+ u32 srds2cr1; /* 0x.0f44 - SerDes2 Control Register 0 */
} __attribute__ ((packed));

+#ifdef CONFIG_FSL_GUTS
+u32 fsl_guts_get_svr(void);
+int fsl_guts_init(void);
+#endif

/* Alternate function signal multiplex control */
#define MPC85xx_PMUXCR_QE(x) (0x8000 >> (x))
--
2.1.0.27.g96db324

2016-03-09 10:34:09

by Yangbo Lu

[permalink] [raw]
Subject: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

The eSDHC of T4240-R1.0-R2.0 has incorrect vender version and spec version.
Acturally the right version numbers should be VVN=0x13 and SVN = 0x1.
This patch adds the GUTS driver support for eSDHC driver to get SVR(System
version register). And fix host version to avoid that incorrect version
numbers break down the ADMA data transfer.

Signed-off-by: Yangbo Lu <[email protected]>
Acked-by: Ulf Hansson <[email protected]>
---
Changes for v2:
- Got SVR through iomap instead of dts
Changes for v3:
- Managed GUTS through syscon instead of iomap in eSDHC driver
Changes for v4:
- Got SVR by GUTS driver instead of SYSCON
Changes for v5:
- Changed to get SVR through API fsl_guts_get_svr()
- Combined patch 4, patch 5 and patch 6 into one
Changes for v6:
- Added 'Acked-by: Ulf Hansson'
---
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-of-esdhc.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 04feea8..5743b05 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -142,6 +142,7 @@ config MMC_SDHCI_OF_ESDHC
depends on MMC_SDHCI_PLTFM
depends on PPC || ARCH_MXC || ARCH_LAYERSCAPE
select MMC_SDHCI_IO_ACCESSORS
+ select FSL_GUTS
help
This selects the Freescale eSDHC controller support.

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 3f34d35..68cc020 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -18,6 +18,8 @@
#include <linux/of.h>
#include <linux/delay.h>
#include <linux/module.h>
+#include <linux/fsl/svr.h>
+#include <linux/fsl/guts.h>
#include <linux/mmc/host.h>
#include "sdhci-pltfm.h"
#include "sdhci-esdhc.h"
@@ -28,6 +30,8 @@
struct sdhci_esdhc {
u8 vendor_ver;
u8 spec_ver;
+ u32 soc_ver;
+ u8 soc_rev;
};

/**
@@ -73,6 +77,8 @@ static u32 esdhc_readl_fixup(struct sdhci_host *host,
static u16 esdhc_readw_fixup(struct sdhci_host *host,
int spec_reg, u32 value)
{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host);
u16 ret;
int shift = (spec_reg & 0x2) * 8;

@@ -80,6 +86,13 @@ static u16 esdhc_readw_fixup(struct sdhci_host *host,
ret = value & 0xffff;
else
ret = (value >> shift) & 0xffff;
+
+ /* Workaround for T4240-R1.0-R2.0 eSDHC which has incorrect
+ * vendor version and spec version information.
+ */
+ if ((spec_reg == SDHCI_HOST_VERSION) &&
+ (esdhc->soc_ver == SVR_T4240) && (esdhc->soc_rev <= 0x20))
+ ret = (VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200;
return ret;
}

@@ -567,10 +580,20 @@ static void esdhc_init(struct platform_device *pdev, struct sdhci_host *host)
struct sdhci_pltfm_host *pltfm_host;
struct sdhci_esdhc *esdhc;
u16 host_ver;
+ u32 svr;

pltfm_host = sdhci_priv(host);
esdhc = sdhci_pltfm_priv(pltfm_host);

+ fsl_guts_init();
+ svr = fsl_guts_get_svr();
+ if (svr) {
+ esdhc->soc_ver = SVR_SOC_VER(svr);
+ esdhc->soc_rev = SVR_REV(svr);
+ } else {
+ dev_err(&pdev->dev, "Failed to get SVR value!\n");
+ }
+
host_ver = sdhci_readw(host, SDHCI_HOST_VERSION);
esdhc->vendor_ver = (host_ver & SDHCI_VENDOR_VER_MASK) >>
SDHCI_VENDOR_VER_SHIFT;
--
2.1.0.27.g96db324

2016-03-09 10:34:28

by Yangbo Lu

[permalink] [raw]
Subject: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

Move guts devicetree doc to Documentation/devicetree/bindings/soc/fsl/
since it's used by not only PowerPC but also ARM. And add a specification
for 'little-endian' property.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v2:
- None
Changes for v3:
- None
Changes for v4:
- Added this patch
Changes for v5:
- Modified the description for little-endian property
Changes for v6:
- None
---
Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt | 3 +++
1 file changed, 3 insertions(+)
rename Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt b/Documentation/devicetree/bindings/soc/fsl/guts.txt
similarity index 91%
rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
index b71b203..07adca9 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
@@ -25,6 +25,9 @@ Recommended properties:
- fsl,liodn-bits : Indicates the number of defined bits in the LIODN
registers, for those SOCs that have a PAMU device.

+ - little-endian : Indicates that the global utilities block is little
+ endian. The default is big endian.
+
Examples:
global-utilities@e0000 { /* global utilities block */
compatible = "fsl,mpc8548-guts";
--
2.1.0.27.g96db324

2016-03-09 10:50:56

by Yangbo Lu

[permalink] [raw]
Subject: [v6, 1/5] ARM64: dts: ls2080a: add device configuration node

Add the dts node for device configuration unit that provides
general purpose configuration and status for the device.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v2:
- None
Changes for v3:
- None
Changes for v4:
- None
Changes for v5:
- Added this patch
Changes for v6:
- None
---
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index 2b23d03..6ad4710 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -191,6 +191,12 @@
clocks = <&sysclk>;
};

+ dcfg: dcfg@1e00000 {
+ compatible = "fsl,ls2080a-dcfg", "syscon";
+ reg = <0x0 0x1e00000 0x0 0x10000>;
+ little-endian;
+ };
+
serial0: serial@21c0500 {
compatible = "fsl,ns16550", "ns16550a";
reg = <0x0 0x21c0500 0x0 0x100>;
--
2.1.0.27.g96db324

2016-03-09 10:52:09

by Yangbo Lu

[permalink] [raw]
Subject: [v6, 4/5] powerpc/fsl: move mpc85xx.h to include/linux/fsl

Move mpc85xx.h to include/linux/fsl and rename it to svr.h as
a common header file. It has been used for mpc85xx and it will
be used for ARM-based SoC as well.

Signed-off-by: Yangbo Lu <[email protected]>
---
Changes for v2:
- None
Changes for v3:
- None
Changes for v4:
- None
Changes for v5:
- Changed to Move mpc85xx.h to include/linux/fsl/
- Adjusted '#include <linux/fsl/svr.h>' position in file
Changes for v6:
- None
---
drivers/clk/clk-qoriq.c | 3 +--
drivers/i2c/busses/i2c-mpc.c | 2 +-
drivers/iommu/fsl_pamu.c | 3 +--
drivers/net/ethernet/freescale/gianfar.c | 2 +-
arch/powerpc/include/asm/mpc85xx.h => include/linux/fsl/svr.h | 4 ++--
5 files changed, 6 insertions(+), 8 deletions(-)
rename arch/powerpc/include/asm/mpc85xx.h => include/linux/fsl/svr.h (97%)

diff --git a/drivers/clk/clk-qoriq.c b/drivers/clk/clk-qoriq.c
index 7bc1c45..fc7f722 100644
--- a/drivers/clk/clk-qoriq.c
+++ b/drivers/clk/clk-qoriq.c
@@ -13,6 +13,7 @@
#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/fsl/guts.h>
+#include <linux/fsl/svr.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -1148,8 +1149,6 @@ bad_args:
}

#ifdef CONFIG_PPC
-#include <asm/mpc85xx.h>
-
static const u32 a4510_svrs[] __initconst = {
(SVR_P2040 << 8) | 0x10, /* P2040 1.0 */
(SVR_P2040 << 8) | 0x11, /* P2040 1.1 */
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 48ecffe..600704c 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -27,9 +27,9 @@
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
+#include <linux/fsl/svr.h>

#include <asm/mpc52xx.h>
-#include <asm/mpc85xx.h>
#include <sysdev/fsl_soc.h>

#define DRV_NAME "mpc-i2c"
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index a34355f..af8fb27 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -21,11 +21,10 @@
#include "fsl_pamu.h"

#include <linux/fsl/guts.h>
+#include <linux/fsl/svr.h>
#include <linux/interrupt.h>
#include <linux/genalloc.h>

-#include <asm/mpc85xx.h>
-
/* define indexes for each operation mapping scenario */
#define OMI_QMAN 0x00
#define OMI_FMAN 0x01
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 2aa7b40..675da4a 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -86,11 +86,11 @@
#include <linux/udp.h>
#include <linux/in.h>
#include <linux/net_tstamp.h>
+#include <linux/fsl/svr.h>

#include <asm/io.h>
#ifdef CONFIG_PPC
#include <asm/reg.h>
-#include <asm/mpc85xx.h>
#endif
#include <asm/irq.h>
#include <asm/uaccess.h>
diff --git a/arch/powerpc/include/asm/mpc85xx.h b/include/linux/fsl/svr.h
similarity index 97%
rename from arch/powerpc/include/asm/mpc85xx.h
rename to include/linux/fsl/svr.h
index 213f3a8..8d13836 100644
--- a/arch/powerpc/include/asm/mpc85xx.h
+++ b/include/linux/fsl/svr.h
@@ -9,8 +9,8 @@
* (at your option) any later version.
*/

-#ifndef __ASM_PPC_MPC85XX_H
-#define __ASM_PPC_MPC85XX_H
+#ifndef FSL_SVR_H
+#define FSL_SVR_H

#define SVR_REV(svr) ((svr) & 0xFF) /* SOC design resision */
#define SVR_MAJ(svr) (((svr) >> 4) & 0xF) /* Major revision field*/
--
2.1.0.27.g96db324

2016-03-12 15:53:06

by Wolfram Sang

[permalink] [raw]
Subject: Re: [v6, 4/5] powerpc/fsl: move mpc85xx.h to include/linux/fsl

On Wed, Mar 09, 2016 at 06:08:50PM +0800, Yangbo Lu wrote:
> Move mpc85xx.h to include/linux/fsl and rename it to svr.h as
> a common header file. It has been used for mpc85xx and it will
> be used for ARM-based SoC as well.
>
> Signed-off-by: Yangbo Lu <[email protected]>

From a glimpse, looks like proper refactoring. Thanks!

For the I2C part:

Acked-by: Wolfram Sang <[email protected]>


Attachments:
(No filename) (395.00 B)
signature.asc (819.00 B)
Download all attachments

2016-03-13 22:27:36

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

On Wednesday 09 March 2016 18:08:51 Yangbo Lu wrote:
> @@ -567,10 +580,20 @@ static void esdhc_init(struct platform_device *pdev, struct sdhci_host *host)
> struct sdhci_pltfm_host *pltfm_host;
> struct sdhci_esdhc *esdhc;
> u16 host_ver;
> + u32 svr;
>
> pltfm_host = sdhci_priv(host);
> esdhc = sdhci_pltfm_priv(pltfm_host);
>
> + fsl_guts_init();
> + svr = fsl_guts_get_svr();
> + if (svr) {
> + esdhc->soc_ver = SVR_SOC_VER(svr);
> + esdhc->soc_rev = SVR_REV(svr);
> + } else {
> + dev_err(&pdev->dev, "Failed to get SVR value!\n");
> + }
> +

This makes the driver non-portable. Better identify the specific workarounds
based on the compatible string for this device, or add a boolean DT property
for the quirk.

Arnd

2016-03-14 10:03:33

by Yangbo Lu

[permalink] [raw]
Subject: RE: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Monday, March 14, 2016 6:26 AM
> To: [email protected]
> Cc: Yangbo Lu; [email protected]; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]; Zhao Qiang; Russell King; Bhupesh Sharma; Joerg
> Roedel; Santosh Shilimkar; Scott Wood; Rob Herring; Claudiu Manoil; Kumar
> Gala; Yang-Leo Li; Xiaobo Xie
> Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
>
> On Wednesday 09 March 2016 18:08:51 Yangbo Lu wrote:
> > @@ -567,10 +580,20 @@ static void esdhc_init(struct platform_device
> *pdev, struct sdhci_host *host)
> > struct sdhci_pltfm_host *pltfm_host;
> > struct sdhci_esdhc *esdhc;
> > u16 host_ver;
> > + u32 svr;
> >
> > pltfm_host = sdhci_priv(host);
> > esdhc = sdhci_pltfm_priv(pltfm_host);
> >
> > + fsl_guts_init();
> > + svr = fsl_guts_get_svr();
> > + if (svr) {
> > + esdhc->soc_ver = SVR_SOC_VER(svr);
> > + esdhc->soc_rev = SVR_REV(svr);
> > + } else {
> > + dev_err(&pdev->dev, "Failed to get SVR value!\n");
> > + }
> > +
>
> This makes the driver non-portable. Better identify the specific
> workarounds based on the compatible string for this device, or add a
> boolean DT property for the quirk.
>
> Arnd

[Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1 before.
https://patchwork.kernel.org/patch/6834221/

We don't 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 addition, the device tree is stable ABI and errata are often discovered after device tree are deployed.
See the link for details.

So we decide to read SVR from the device-config/guts MMIO block other than using DTS.
Thanks.




2016-03-14 18:19:23

by Scott Wood

[permalink] [raw]
Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

On 03/14/2016 02:29 AM, Yangbo Lu wrote:
>> -----Original Message-----
>> From: Arnd Bergmann [mailto:[email protected]]
>> Sent: Monday, March 14, 2016 6:26 AM
>> To: [email protected]
>> Cc: Yangbo Lu; [email protected]; linux-arm-
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]; [email protected]
>> foundation.org; [email protected]; [email protected];
>> [email protected]; Zhao Qiang; Russell King; Bhupesh Sharma; Joerg
>> Roedel; Santosh Shilimkar; Scott Wood; Rob Herring; Claudiu Manoil; Kumar
>> Gala; Yang-Leo Li; Xiaobo Xie
>> Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-
>> R1.0-R2.0
>>
>> On Wednesday 09 March 2016 18:08:51 Yangbo Lu wrote:
>>> @@ -567,10 +580,20 @@ static void esdhc_init(struct platform_device
>> *pdev, struct sdhci_host *host)
>>> struct sdhci_pltfm_host *pltfm_host;
>>> struct sdhci_esdhc *esdhc;
>>> u16 host_ver;
>>> + u32 svr;
>>>
>>> pltfm_host = sdhci_priv(host);
>>> esdhc = sdhci_pltfm_priv(pltfm_host);
>>>
>>> + fsl_guts_init();
>>> + svr = fsl_guts_get_svr();
>>> + if (svr) {
>>> + esdhc->soc_ver = SVR_SOC_VER(svr);
>>> + esdhc->soc_rev = SVR_REV(svr);
>>> + } else {
>>> + dev_err(&pdev->dev, "Failed to get SVR value!\n");
>>> + }
>>> +
>>
>> This makes the driver non-portable. Better identify the specific
>> workarounds based on the compatible string for this device, or add a
>> boolean DT property for the quirk.
>>
>> Arnd
>
> [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1 before.
> https://patchwork.kernel.org/patch/6834221/
>
> We don?t 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 addition, the device tree is stable ABI and errata are often discovered after device tree are deployed.
> See the link for details.
>
> So we decide to read SVR from the device-config/guts MMIO block other than using DTS.
> Thanks.

Also note that this driver is already only for fsl-specific hardware,
and it will still work even if fsl_guts doesn't find anything to bind to
-- it just wouldn't be able to detect errata based on SVR in that case.

-Scott

2016-03-17 17:01:11

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

On Mon, Mar 14, 2016 at 05:45:43PM +0000, Scott Wood wrote:
> On 03/14/2016 02:29 AM, Yangbo Lu wrote:
> >> -----Original Message-----
> >> From: Arnd Bergmann [mailto:[email protected]]
> >> Sent: Monday, March 14, 2016 6:26 AM
> >> To: [email protected]
> >> Cc: Yangbo Lu; [email protected]; linux-arm-
> >> [email protected]; [email protected]; linux-
> >> [email protected]; [email protected]; [email protected]
> >> foundation.org; [email protected]; [email protected];
> >> [email protected]; Zhao Qiang; Russell King; Bhupesh Sharma; Joerg
> >> Roedel; Santosh Shilimkar; Scott Wood; Rob Herring; Claudiu Manoil; Kumar
> >> Gala; Yang-Leo Li; Xiaobo Xie
> >> Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-
> >> R1.0-R2.0
> >>
> >> On Wednesday 09 March 2016 18:08:51 Yangbo Lu wrote:
> >>> @@ -567,10 +580,20 @@ static void esdhc_init(struct platform_device
> >> *pdev, struct sdhci_host *host)
> >>> struct sdhci_pltfm_host *pltfm_host;
> >>> struct sdhci_esdhc *esdhc;
> >>> u16 host_ver;
> >>> + u32 svr;
> >>>
> >>> pltfm_host = sdhci_priv(host);
> >>> esdhc = sdhci_pltfm_priv(pltfm_host);
> >>>
> >>> + fsl_guts_init();
> >>> + svr = fsl_guts_get_svr();
> >>> + if (svr) {
> >>> + esdhc->soc_ver = SVR_SOC_VER(svr);
> >>> + esdhc->soc_rev = SVR_REV(svr);
> >>> + } else {
> >>> + dev_err(&pdev->dev, "Failed to get SVR value!\n");
> >>> + }
> >>> +
> >>
> >> This makes the driver non-portable. Better identify the specific
> >> workarounds based on the compatible string for this device, or add a
> >> boolean DT property for the quirk.
> >>
> >> Arnd
> >
> > [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1 before.
> > https://patchwork.kernel.org/patch/6834221/
> >
> > We don’t 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 addition, the device tree is stable ABI and errata are often discovered after device tree are deployed.
> > See the link for details.
> >
> > So we decide to read SVR from the device-config/guts MMIO block other than using DTS.
> > Thanks.
>
> Also note that this driver is already only for fsl-specific hardware,
> and it will still work even if fsl_guts doesn't find anything to bind to
> -- it just wouldn't be able to detect errata based on SVR in that case.

IIRC, it is the same IP block as i.MX and Arnd's point is this won't
even compile on !PPC. It is things like this that prevent sharing the
driver. Dealing with Si revs is a common problem. We should have a
common solution. There is soc_device for this purpose.

OTOH, the integration differences may be enough that trying to have a
common driver with i.MX would not be worth it.

Rob

2016-03-17 17:06:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

On Wed, Mar 09, 2016 at 06:08:49PM +0800, Yangbo Lu wrote:
> Move guts devicetree doc to Documentation/devicetree/bindings/soc/fsl/
> since it's used by not only PowerPC but also ARM. And add a specification
> for 'little-endian' property.
>
> Signed-off-by: Yangbo Lu <[email protected]>
> ---
> Changes for v2:
> - None
> Changes for v3:
> - None
> Changes for v4:
> - Added this patch
> Changes for v5:
> - Modified the description for little-endian property
> Changes for v6:
> - None
> ---
> Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt | 3 +++
> 1 file changed, 3 insertions(+)
> rename Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
>
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> similarity index 91%
> rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
> index b71b203..07adca9 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> @@ -25,6 +25,9 @@ Recommended properties:
> - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
> registers, for those SOCs that have a PAMU device.
>
> + - little-endian : Indicates that the global utilities block is little
> + endian. The default is big endian.

The default is "the native endianness of the system". So absence on an
ARM system would be LE. This property is valid for any simple-bus
device, so it isn't really required to document per device. You can, but
your description had better match the documented behaviour.

Rob

2016-03-17 17:07:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

On Thursday 17 March 2016 12:01:01 Rob Herring wrote:
> On Mon, Mar 14, 2016 at 05:45:43PM +0000, Scott Wood wrote:

> > >> This makes the driver non-portable. Better identify the specific
> > >> workarounds based on the compatible string for this device, or add a
> > >> boolean DT property for the quirk.
> > >>
> > >> Arnd
> > >
> > > [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1 before.
> > > https://patchwork.kernel.org/patch/6834221/
> > >
> > > We don’t 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 addition, the device tree is stable ABI and errata are often discovered after device tree are deployed.
> > > See the link for details.
> > >
> > > So we decide to read SVR from the device-config/guts MMIO block other than using DTS.
> > > Thanks.
> >
> > Also note that this driver is already only for fsl-specific hardware,
> > and it will still work even if fsl_guts doesn't find anything to bind to
> > -- it just wouldn't be able to detect errata based on SVR in that case.
>
> IIRC, it is the same IP block as i.MX and Arnd's point is this won't
> even compile on !PPC. It is things like this that prevent sharing the
> driver.

I think the first four patches take care of building for ARM,
but the problem remains if you want to enable COMPILE_TEST as
we need for certain automated checking.

> Dealing with Si revs is a common problem. We should have a
> common solution. There is soc_device for this purpose.

Exactly. The last time this came up, I think we agreed to implement a
helper using glob_match() on the soc_device strings. Unfortunately
this hasn't happened then, but I'd still prefer that over yet another
vendor-specific way of dealing with the generic issue.

Arnd

2016-03-17 17:12:50

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

On Thursday 17 March 2016 12:06:40 Rob Herring wrote:
> > diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> > similarity index 91%
> > rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> > rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
> > index b71b203..07adca9 100644
> > --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> > +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> > @@ -25,6 +25,9 @@ Recommended properties:
> > - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
> > registers, for those SOCs that have a PAMU device.
> >
> > + - little-endian : Indicates that the global utilities block is little
> > + endian. The default is big endian.
>
> The default is "the native endianness of the system".

This may be what is currently documented, but not what we are doing
in practice, as there is no "native endianess" for either PowerPC or
ARM -- both allow running big-endian or little-endian kernels and the
device registers are fixed.

I think the property here is fine.

Arnd

2016-03-17 17:58:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

On Thu, Mar 17, 2016 at 12:11 PM, Arnd Bergmann <[email protected]> wrote:
> On Thursday 17 March 2016 12:06:40 Rob Herring wrote:
>> > diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt b/Documentation/devicetree/bindings/soc/fsl/guts.txt
>> > similarity index 91%
>> > rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
>> > rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
>> > index b71b203..07adca9 100644
>> > --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
>> > +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
>> > @@ -25,6 +25,9 @@ Recommended properties:
>> > - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
>> > registers, for those SOCs that have a PAMU device.
>> >
>> > + - little-endian : Indicates that the global utilities block is little
>> > + endian. The default is big endian.
>>
>> The default is "the native endianness of the system".
>
> This may be what is currently documented, but not what we are doing
> in practice, as there is no "native endianess" for either PowerPC or
> ARM -- both allow running big-endian or little-endian kernels and the
> device registers are fixed.

Notice I said system, not architecture. The way the device registers
are fixed is what I mean by native endianness.

If the purpose of adding this property now is to support GUTS on the
ARM SoCs, then I'd argue using this property is probably wrong. If the
PPC systems are designed with BE device registers and ARM systems with
LE, then this property is not needed.

> I think the property here is fine.

Unless you have studied the FSL ARM based SoCs, then there is not
enough information here to tell.

Rob

2016-03-17 21:33:42

by Yang Li

[permalink] [raw]
Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

On Thu, Mar 17, 2016 at 12:57 PM, Rob Herring <[email protected]> wrote:
> On Thu, Mar 17, 2016 at 12:11 PM, Arnd Bergmann <[email protected]> wrote:
>> On Thursday 17 March 2016 12:06:40 Rob Herring wrote:
>>> > diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt b/Documentation/devicetree/bindings/soc/fsl/guts.txt
>>> > similarity index 91%
>>> > rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
>>> > rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
>>> > index b71b203..07adca9 100644
>>> > --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
>>> > +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
>>> > @@ -25,6 +25,9 @@ Recommended properties:
>>> > - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
>>> > registers, for those SOCs that have a PAMU device.
>>> >
>>> > + - little-endian : Indicates that the global utilities block is little
>>> > + endian. The default is big endian.
>>>
>>> The default is "the native endianness of the system".
>>
>> This may be what is currently documented, but not what we are doing
>> in practice, as there is no "native endianess" for either PowerPC or
>> ARM -- both allow running big-endian or little-endian kernels and the
>> device registers are fixed.
>
> Notice I said system, not architecture. The way the device registers
> are fixed is what I mean by native endianness.

I think sometimes it's also hard to define the native endianess of the
system too. For whatever reason, we have hardware that having
big-endian registers on some on-chip devices but using little-endian
registers on other devices. Even if all the devices on certain
hardware use registers of the same endianess, it is also hard for the
device driver to know what the native endianess really is.

>
> If the purpose of adding this property now is to support GUTS on the
> ARM SoCs, then I'd argue using this property is probably wrong. If the
> PPC systems are designed with BE device registers and ARM systems with
> LE, then this property is not needed.
>
>> I think the property here is fine.
>
> Unless you have studied the FSL ARM based SoCs, then there is not
> enough information here to tell.

Recent FSL ARM SoCs seems to have more weird endianess issue. :( The
same IP could have registers of different endianess on different ARM
SoCs. That why we need to define the endianess explicitly in device
tree.

Regards,
Leo

2016-03-18 18:16:14

by Scott Wood

[permalink] [raw]
Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

On 03/17/2016 12:06 PM, Rob Herring wrote:
> On Wed, Mar 09, 2016 at 06:08:49PM +0800, Yangbo Lu wrote:
>> Move guts devicetree doc to Documentation/devicetree/bindings/soc/fsl/
>> since it's used by not only PowerPC but also ARM. And add a specification
>> for 'little-endian' property.
>>
>> Signed-off-by: Yangbo Lu <[email protected]>
>> ---
>> Changes for v2:
>> - None
>> Changes for v3:
>> - None
>> Changes for v4:
>> - Added this patch
>> Changes for v5:
>> - Modified the description for little-endian property
>> Changes for v6:
>> - None
>> ---
>> Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt | 3 +++
>> 1 file changed, 3 insertions(+)
>> rename Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
>>
>> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt b/Documentation/devicetree/bindings/soc/fsl/guts.txt
>> similarity index 91%
>> rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
>> rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
>> index b71b203..07adca9 100644
>> --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
>> +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
>> @@ -25,6 +25,9 @@ Recommended properties:
>> - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
>> registers, for those SOCs that have a PAMU device.
>>
>> + - little-endian : Indicates that the global utilities block is little
>> + endian. The default is big endian.
>
> The default is "the native endianness of the system". So absence on an
> ARM system would be LE.

No. For this binding, the default is big-endian, because that's what
existed for this device before an endian property was added.

"endianness of the system" is not a well-defined concept.

> This property is valid for any simple-bus device,

Since when does simple-bus mean anything more than that the nodes
underneath it can be used without bus-specific knowledge?

> so it isn't really required to document per device. You can, but
> your description had better match the documented behaviour.

Documented where?

In fact, Documentation/devicetree/bindings/common-properties.txt
explicitly says of the endian properties, "If a binding supports these
properties, then the binding should also specify the default behavior if
none of these properties are present."

-Scott

2016-03-18 18:28:14

by Scott Wood

[permalink] [raw]
Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

On 03/17/2016 12:06 PM, Arnd Bergmann wrote:
> On Thursday 17 March 2016 12:01:01 Rob Herring wrote:
>> On Mon, Mar 14, 2016 at 05:45:43PM +0000, Scott Wood wrote:
>
>>>>> This makes the driver non-portable. Better identify the specific
>>>>> workarounds based on the compatible string for this device, or add a
>>>>> boolean DT property for the quirk.
>>>>>
>>>>> Arnd
>>>>
>>>> [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1 before.
>>>> https://patchwork.kernel.org/patch/6834221/
>>>>
>>>> We don?t 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 addition, the device tree is stable ABI and errata are often discovered after device tree are deployed.
>>>> See the link for details.
>>>>
>>>> So we decide to read SVR from the device-config/guts MMIO block other than using DTS.
>>>> Thanks.
>>>
>>> Also note that this driver is already only for fsl-specific hardware,
>>> and it will still work even if fsl_guts doesn't find anything to bind to
>>> -- it just wouldn't be able to detect errata based on SVR in that case.
>>
>> IIRC, it is the same IP block as i.MX and Arnd's point is this won't
>> even compile on !PPC. It is things like this that prevent sharing the
>> driver.

The whole point of using the MMIO SVR instead of the PPC SPR is so that
it will work on ARM... The guts driver should build on any platform as
long as OF is enabled, and if it doesn't find a node to bind to it will
return 0 for SVR, and the eSDHC driver will continue (after printing an
error that should be removed) without the ability to test for errata
based on SVR.

> I think the first four patches take care of building for ARM,
> but the problem remains if you want to enable COMPILE_TEST as
> we need for certain automated checking.

What specific problem is there with COMPILE_TEST?

>> Dealing with Si revs is a common problem. We should have a
>> common solution. There is soc_device for this purpose.
>
> Exactly. The last time this came up, I think we agreed to implement a
> helper using glob_match() on the soc_device strings. Unfortunately
> this hasn't happened then, but I'd still prefer that over yet another
> vendor-specific way of dealing with the generic issue.

soc_device would require encoding the SVR as a string and then decoding
the string, which is more complicated and error prone than having
platform-specific code test a platform-specific number. And when would
it get registered on arm64, which doesn't have platform code?

-Scott

2016-03-18 23:10:18

by Scott Wood

[permalink] [raw]
Subject: Re: [v6, 2/5] soc: fsl: add GUTS driver for QorIQ platforms

On 03/09/2016 04:18 AM, Yangbo Lu wrote:
> +#ifdef CONFIG_FSL_GUTS
> +u32 fsl_guts_get_svr(void);
> +int fsl_guts_init(void);
> +#endif

Don't ifdef prototypes (when not providing a stub alternative).

-Scott

2016-03-25 06:43:34

by Yangbo Lu

[permalink] [raw]
Subject: RE: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

> -----Original Message-----
> From: Scott Wood
> Sent: Saturday, March 19, 2016 2:28 AM
> To: Arnd Bergmann; Rob Herring
> Cc: Yangbo Lu; [email protected]; [email protected];
> [email protected]; Zhao Qiang; Russell King; Bhupesh Sharma;
> [email protected]; Joerg Roedel; Kumar Gala; linux-
> [email protected]; [email protected]; Yang-Leo Li;
> [email protected]; [email protected]; Claudiu
> Manoil; Santosh Shilimkar; Xiaobo Xie; [email protected]; linux-
> [email protected]
> Subject: Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
>
> On 03/17/2016 12:06 PM, Arnd Bergmann wrote:
> > On Thursday 17 March 2016 12:01:01 Rob Herring wrote:
> >> On Mon, Mar 14, 2016 at 05:45:43PM +0000, Scott Wood wrote:
> >
> >>>>> This makes the driver non-portable. Better identify the specific
> >>>>> workarounds based on the compatible string for this device, or add
> >>>>> a boolean DT property for the quirk.
> >>>>>
> >>>>> Arnd
> >>>>
> >>>> [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS
> in v1 before.
> >>>> https://patchwork.kernel.org/patch/6834221/
> >>>>
> >>>> We don't 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 addition, the device tree is stable ABI and errata are often
> discovered after device tree are deployed.
> >>>> See the link for details.
> >>>>
> >>>> So we decide to read SVR from the device-config/guts MMIO block
> other than using DTS.
> >>>> Thanks.
> >>>
> >>> Also note that this driver is already only for fsl-specific
> >>> hardware, and it will still work even if fsl_guts doesn't find
> >>> anything to bind to
> >>> -- it just wouldn't be able to detect errata based on SVR in that
> case.
> >>
> >> IIRC, it is the same IP block as i.MX and Arnd's point is this won't
> >> even compile on !PPC. It is things like this that prevent sharing the
> >> driver.
>
> The whole point of using the MMIO SVR instead of the PPC SPR is so that
> it will work on ARM... The guts driver should build on any platform as
> long as OF is enabled, and if it doesn't find a node to bind to it will
> return 0 for SVR, and the eSDHC driver will continue (after printing an
> error that should be removed) without the ability to test for errata
> based on SVR.
>
> > I think the first four patches take care of building for ARM, but the
> > problem remains if you want to enable COMPILE_TEST as we need for
> > certain automated checking.
>
> What specific problem is there with COMPILE_TEST?
>
> >> Dealing with Si revs is a common problem. We should have a common
> >> solution. There is soc_device for this purpose.
> >
> > Exactly. The last time this came up, I think we agreed to implement a
> > helper using glob_match() on the soc_device strings. Unfortunately
> > this hasn't happened then, but I'd still prefer that over yet another
> > vendor-specific way of dealing with the generic issue.
>
> soc_device would require encoding the SVR as a string and then decoding
> the string, which is more complicated and error prone than having
> platform-specific code test a platform-specific number. And when would
> it get registered on arm64, which doesn't have platform code?
>
> -Scott

[Lu Yangbo-B47093] Hi Arnd, could you answer Scott's questions?
If you don't oppose this patch, I'd like to rework a new version for merging.
Thanks.

:)




2016-03-25 06:54:17

by Yangbo Lu

[permalink] [raw]
Subject: RE: [v6, 3/5] dt: move guts devicetree doc out of powerpc directory

> -----Original Message-----
> From: Scott Wood
> Sent: Saturday, March 19, 2016 2:16 AM
> To: Rob Herring; Yangbo Lu
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil;
> [email protected]; Bhupesh Sharma; Zhao Qiang; Kumar Gala; Santosh
> Shilimkar; Yang-Leo Li; Xiaobo Xie
> Subject: Re: [v6, 3/5] dt: move guts devicetree doc out of powerpc
> directory
>
> On 03/17/2016 12:06 PM, Rob Herring wrote:
> > On Wed, Mar 09, 2016 at 06:08:49PM +0800, Yangbo Lu wrote:
> >> Move guts devicetree doc to
> >> Documentation/devicetree/bindings/soc/fsl/
> >> since it's used by not only PowerPC but also ARM. And add a
> >> specification for 'little-endian' property.
> >>
> >> Signed-off-by: Yangbo Lu <[email protected]>
> >> ---
> >> Changes for v2:
> >> - None
> >> Changes for v3:
> >> - None
> >> Changes for v4:
> >> - Added this patch
> >> Changes for v5:
> >> - Modified the description for little-endian property Changes for
> >> v6:
> >> - None
> >> ---
> >> Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt | 3
> >> +++
> >> 1 file changed, 3 insertions(+)
> >> rename Documentation/devicetree/bindings/{powerpc =>
> >> soc}/fsl/guts.txt (91%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> >> b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> >> similarity index 91%
> >> rename from Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> >> rename to Documentation/devicetree/bindings/soc/fsl/guts.txt
> >> index b71b203..07adca9 100644
> >> --- a/Documentation/devicetree/bindings/powerpc/fsl/guts.txt
> >> +++ b/Documentation/devicetree/bindings/soc/fsl/guts.txt
> >> @@ -25,6 +25,9 @@ Recommended properties:
> >> - fsl,liodn-bits : Indicates the number of defined bits in the LIODN
> >> registers, for those SOCs that have a PAMU device.
> >>
> >> + - little-endian : Indicates that the global utilities block is
> little
> >> + endian. The default is big endian.
> >
> > The default is "the native endianness of the system". So absence on an
> > ARM system would be LE.
>
> No. For this binding, the default is big-endian, because that's what
> existed for this device before an endian property was added.
>
> "endianness of the system" is not a well-defined concept.
>
> > This property is valid for any simple-bus device,
>
> Since when does simple-bus mean anything more than that the nodes
> underneath it can be used without bus-specific knowledge?
>
> > so it isn't really required to document per device. You can, but your
> > description had better match the documented behaviour.
>
> Documented where?
>
> In fact, Documentation/devicetree/bindings/common-properties.txt
> explicitly says of the endian properties, "If a binding supports these
> properties, then the binding should also specify the default behavior if
> none of these properties are present."
>
> -Scott

[Lu Yangbo-B47093] So, Rob, could you accept this patch after so much discussion?
:)

2016-03-25 07:00:27

by Yangbo Lu

[permalink] [raw]
Subject: RE: [v6, 2/5] soc: fsl: add GUTS driver for QorIQ platforms

> -----Original Message-----
> From: Scott Wood
> Sent: Saturday, March 19, 2016 6:55 AM
> To: Yangbo Lu; [email protected]; linux-arm-
> [email protected]; [email protected]; linuxppc-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Rob Herring; Russell King; Jochen Friedrich; Joerg Roedel; Claudiu
> Manoil; [email protected]; Bhupesh Sharma; Zhao Qiang; Kumar Gala;
> Santosh Shilimkar; Yang-Leo Li; Xiaobo Xie
> Subject: Re: [v6, 2/5] soc: fsl: add GUTS driver for QorIQ platforms
>
> On 03/09/2016 04:18 AM, Yangbo Lu wrote:
> > +#ifdef CONFIG_FSL_GUTS
> > +u32 fsl_guts_get_svr(void);
> > +int fsl_guts_init(void);
> > +#endif
>
> Don't ifdef prototypes (when not providing a stub alternative).
>
> -Scott

[Lu Yangbo-B47093] Ok, will remove the ifdef.
Thank you very much.