[v10, 0/7] Fix eSDHC host version register bug

[v10, 0/7] Fix eSDHC host version register bug

[Yangbo Lu]

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 four patches are to add the GUTS driver.
The following patches except the updating MAINTAINERS patch are to enable
GUTS driver support to get SVR in eSDHC driver and fix host version for T4240.

Yangbo Lu (7):
  Documentation: DT: update Freescale DCFG compatible
  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
  MAINTAINERS: add entry for Freescale SoC drivers
  mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

 Documentation/devicetree/bindings/arm/fsl.txt      |   6 +-
 .../bindings/{powerpc => soc}/fsl/guts.txt         |   3 +
 MAINTAINERS                                        |  11 +-
 arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi     |   6 +
 arch/powerpc/kernel/cpu_setup_fsl_booke.S          |   2 +-
 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                           | 126 +++++++++++++--------
 .../asm/mpc85xx.h => include/linux/fsl/svr.h       |   4 +-
 17 files changed, 262 insertions(+), 60 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

[v10, 1/7] Documentation: DT: update Freescale DCFG compatible

[Yangbo Lu]

Update Freescale DCFG compatible with 'fsl,<chip>-dcfg' instead
of 'fsl,ls1021a-dcfg' to include more chips such as ls1021a,
ls1043a, and ls2080a.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v8:
	- Added this patch
Changes for v9:
	- Added a list for the possible compatibles
Changes for v10:
	- None
---
 Documentation/devicetree/bindings/arm/fsl.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/fsl.txt b/Documentation/devicetree/bindings/arm/fsl.txt
index 752a685..465cba1 100644
--- a/Documentation/devicetree/bindings/arm/fsl.txt
+++ b/Documentation/devicetree/bindings/arm/fsl.txt
@@ -119,7 +119,11 @@ Freescale DCFG
 configuration and status for the device. Such as setting the secondary
 core start address and release the secondary core from holdoff and startup.
   Required properties:
-  - compatible: should be "fsl,ls1021a-dcfg"
+  - compatible: should be "fsl,<chip>-dcfg"
+    Possible compatibles:
+	"fsl,ls1021a-dcfg"
+	"fsl,ls1043a-dcfg"
+	"fsl,ls2080a-dcfg"
   - reg : should contain base address and length of DCFG memory-mapped registers
 
 Example:
-- 
2.1.0.27.g96db324

Re: [v10, 1/7] Documentation: DT: update Freescale DCFG compatible

[Rob Herring]

On Thu, May 05, 2016 at 11:12:24AM +0800, Yangbo Lu wrote:
> Update Freescale DCFG compatible with 'fsl,<chip>-dcfg' instead
> of 'fsl,ls1021a-dcfg' to include more chips such as ls1021a,
> ls1043a, and ls2080a.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v8:
> 	- Added this patch
> Changes for v9:
> 	- Added a list for the possible compatibles
> Changes for v10:
> 	- None
> ---
>  Documentation/devicetree/bindings/arm/fsl.txt | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

[v10, 2/7] ARM64: dts: ls2080a: add device configuration node

[Yangbo Lu]

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

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Acked-by: Scott Wood <oss@buserror.net>
---
Changes for v5:
	- Added this patch
Changes for v6:
	- None
Changes for v7:
	- None
Changes for v8:
	- Added 'Acked-by: Scott Wood'
Changes for v9:
	- None
Changes for v10:
	- 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 9d746c6..8724cf1 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

[v10, 3/7] soc: fsl: add GUTS driver for QorIQ platforms

[Yangbo Lu]

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 <yangbo.lu@nxp.com>
Acked-by: Scott Wood <oss@buserror.net>
---
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
Changes for v7:
	- Removed 'ifdef' for function declaration in guts.h
Changes for v8:
	- Fixes lines longer than 80 characters checkpatch issue
	- Added 'Acked-by: Scott Wood'
Changes for v9:
	- None
Changes for v10:
	- None
---
 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 | 126 +++++++++++++++++++++++++++++------------------
 5 files changed, 207 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 cb58ef0..7106463 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 649e917..7e1e22b 100644
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -29,83 +29,113 @@
  * #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));
 
+u32 fsl_guts_get_svr(void);
+int fsl_guts_init(void);
 
 /* Alternate function signal multiplex control */
 #define MPC85xx_PMUXCR_QE(x) (0x8000 >> (x))
-- 
2.1.0.27.g96db324

Re: [v10, 3/7] soc: fsl: add GUTS driver for QorIQ platforms

[Paul Gortmaker]

On Wed, May 4, 2016 at 11:12 PM, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> 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 <yangbo.lu@nxp.com>
> Acked-by: Scott Wood <oss@buserror.net>
> ---
> 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
> Changes for v7:
>         - Removed 'ifdef' for function declaration in guts.h
> Changes for v8:
>         - Fixes lines longer than 80 characters checkpatch issue
>         - Added 'Acked-by: Scott Wood'
> Changes for v9:
>         - None
> Changes for v10:
>         - None
> ---
>  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 | 126 +++++++++++++++++++++++++++++------------------
>  5 files changed, 207 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 cb58ef0..7106463 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>

Seems there was lots of discussion on this.  If it does end up being
resent, it would be nice to get the module.h and other modular stuff
gone since it is a bool Kconfig.

Thanks,
Paul.
--

> +#include <linux/slab.h>
> +#include <linux/mutex.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/fsl/guts.h>
> +
>

Re: [v10, 3/7] soc: fsl: add GUTS driver for QorIQ platforms

[Scott Wood]

On Fri, 2016-07-15 at 12:43 -0400, Paul Gortmaker wrote:
> On Wed, May 4, 2016 at 11:12 PM, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> > 
> > 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 <yangbo.lu@nxp.com>
> > Acked-by: Scott Wood <oss@buserror.net>
> > ---
> > 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
> > Changes for v7:
> >         - Removed 'ifdef' for function declaration in guts.h
> > Changes for v8:
> >         - Fixes lines longer than 80 characters checkpatch issue
> >         - Added 'Acked-by: Scott Wood'
> > Changes for v9:
> >         - None
> > Changes for v10:
> >         - None
> > ---
> >  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 | 126 +++++++++++++++++++++++++++++-------------
> > -----
> >  5 files changed, 207 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 cb58ef0..7106463 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>
> Seems there was lots of discussion on this.  If it does end up being
> resent, it would be nice to get the module.h and other modular stuff
> gone since it is a bool Kconfig.

I plan to resend just the GUTS driver portion and send it through the PPC
tree.

I don't see any modular stuff in there besides the linux/module.h include.

-Scott

Re: [v10, 3/7] soc: fsl: add GUTS driver for QorIQ platforms

[Paul Gortmaker]

[Re: [v10, 3/7] soc: fsl: add GUTS driver for QorIQ platforms] On 15/07/2016 (Fri 14:12) Scott Wood wrote:

> On Fri, 2016-07-15 at 12:43 -0400, Paul Gortmaker wrote:
> > > +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>
> > Seems there was lots of discussion on this.  If it does end up being
> > resent, it would be nice to get the module.h and other modular stuff
> > gone since it is a bool Kconfig.
> 
> I plan to resend just the GUTS driver portion and send it through the PPC
> tree.
> 
> I don't see any modular stuff in there besides the linux/module.h include.

Great.  Normally I'm seeing the MODULE_DEVICE_TABLE and MODULE_AUTHOR
and MODULE_LICENSE etc, so it has (unfortunately) become a knee jerk
reaction to assume the latter follows a module.h presence...  thanks for
removing the extraneous include.

Paul.
--

> 
> -Scott
> 
> 

[v10, 4/7] dt: move guts devicetree doc out of powerpc directory

[Yangbo Lu]

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 <yangbo.lu@nxp.com>
Acked-by: Scott Wood <oss@buserror.net>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes for v4:
	- Added this patch
Changes for v5:
	- Modified the description for little-endian property
Changes for v6:
	- None
Changes for v7:
	- None
Changes for v8:
	- Added 'Acked-by: Scott Wood'
	- Added 'Acked-by: Rob Herring'
Changes for v9:
	- None
Changes for v10:
	- 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

[v10, 5/7] powerpc/fsl: move mpc85xx.h to include/linux/fsl

[Yangbo Lu]

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 <yangbo.lu@nxp.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Acked-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Scott Wood <oss@buserror.net>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
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
Changes for v7:
	- Added 'Acked-by: Wolfram Sang' for I2C part
	- Also applied to arch/powerpc/kernel/cpu_setup_fsl_booke.S
Changes for v8:
	- Added 'Acked-by: Stephen Boyd' for clk part
	- Added 'Acked-by: Scott Wood'
	- Added 'Acked-by: Joerg Roedel' for iommu part
Changes for v9:
	- None
Changes for v10:
	- None
---
 arch/powerpc/kernel/cpu_setup_fsl_booke.S                     | 2 +-
 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 ++--
 6 files changed, 7 insertions(+), 9 deletions(-)
 rename arch/powerpc/include/asm/mpc85xx.h => include/linux/fsl/svr.h (97%)

diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
index 462aed9..2b0284e 100644
--- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
+++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
@@ -13,13 +13,13 @@
  *
  */
 
+#include <linux/fsl/svr.h>
 #include <asm/page.h>
 #include <asm/processor.h>
 #include <asm/cputable.h>
 #include <asm/ppc_asm.h>
 #include <asm/mmu-book3e.h>
 #include <asm/asm-offsets.h>
-#include <asm/mpc85xx.h>
 
 _GLOBAL(__e500_icache_setup)
 	mfspr	r0, SPRN_L1CSR1
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 d2f917a..2224b10 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

[v10, 6/7] MAINTAINERS: add entry for Freescale SoC drivers

[Yangbo Lu]

Add maintainer entry for Freescale SoC drivers including
the QE library and the GUTS driver now. Also add maintainer
for QE library.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Acked-by: Scott Wood <oss@buserror.net>
Acked-by: Qiang Zhao <qiang.zhao@nxp.com>
---
Changes for v8:
	- Added this patch
Changes for v9:
	- Added linux-arm mail list
	- Removed GUTS driver entry
Changes for v10:
	- Changed 'DRIVER' to 'DRIVERS'
	- Added 'Acked-by' of Scott and Qiang
---
 MAINTAINERS | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 42e65d1..85c4b8b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4622,9 +4622,18 @@ F:	drivers/net/ethernet/freescale/fec_ptp.c
 F:	drivers/net/ethernet/freescale/fec.h
 F:	Documentation/devicetree/bindings/net/fsl-fec.txt
 
+FREESCALE SOC DRIVERS
+M:	Scott Wood <oss@buserror.net>
+L:	linuxppc-dev@lists.ozlabs.org
+L:	linux-arm-kernel@lists.infradead.org
+S:	Maintained
+F:	drivers/soc/fsl/
+F:	include/linux/fsl/
+
 FREESCALE QUICC ENGINE LIBRARY
+M:	Qiang Zhao <qiang.zhao@nxp.com>
 L:	linuxppc-dev@lists.ozlabs.org
-S:	Orphan
+S:	Maintained
 F:	drivers/soc/fsl/qe/
 F:	include/soc/fsl/*qe*.h
 F:	include/soc/fsl/*ucc*.h
-- 
2.1.0.27.g96db324

[v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Yangbo Lu]

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 <yangbo.lu@nxp.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Scott Wood <oss@buserror.net>
---
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'
Changes for v7:
	- None
Changes for v8:
	- Added 'Acked-by: Scott Wood'
Changes for v9:
	- None
Changes for v10:
	- None
---
 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 0aa484c..e15e836 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -143,6 +143,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

Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Arnd Bergmann]

On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> 
> +       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");
> +       }
> +
> 


Sorry for jumping in again after not participating in the discussion for the
past few versions.

What happened to my suggestion of making this a platform-independent interface
to avoid the link time dependency?

Specifically, why not add an exported function to drivers/base/soc.c that
uses glob_match() for comparing a string in the device driver to the ID
of the SoC that is set by whatever SoC identifying driver the platform
has?

	Arnd

RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Yangbo Lu]

Hi Arnd,


> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Thursday, May 05, 2016 4:32 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Yangbo Lu; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> linux-clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> foundation.org; netdev@vger.kernel.org; Mark Rutland;
> ulf.hansson@linaro.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> Kumar Gala; Xiaobo Xie; Qiang Zhao
> Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
> 
> On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> >
> > +       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");
> > +       }
> > +
> >
> 
> 
> Sorry for jumping in again after not participating in the discussion for
> the past few versions.
> 
> What happened to my suggestion of making this a platform-independent
> interface to avoid the link time dependency?
> 
> Specifically, why not add an exported function to drivers/base/soc.c that
> uses glob_match() for comparing a string in the device driver to the ID
> of the SoC that is set by whatever SoC identifying driver the platform
> has?
> 
> 	Arnd

[Lu Yangbo-B47093] I think this has been discussed in v6.
You can find Scott's comments about this in below link.
https://patchwork.kernel.org/patch/8544501/

Thanks.

Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Arnd Bergmann]

On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: Thursday, May 05, 2016 4:32 PM
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Yangbo Lu; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > linux-clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> > foundation.org; netdev@vger.kernel.org; Mark Rutland;
> > ulf.hansson@linaro.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> > Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> > Kumar Gala; Xiaobo Xie; Qiang Zhao
> > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> > R1.0-R2.0
> > 
> > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > >
> > > +       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");
> > > +       }
> > > +
> > >
> > 
> > 
> > Sorry for jumping in again after not participating in the discussion for
> > the past few versions.
> > 
> > What happened to my suggestion of making this a platform-independent
> > interface to avoid the link time dependency?
> > 
> > Specifically, why not add an exported function to drivers/base/soc.c that
> > uses glob_match() for comparing a string in the device driver to the ID
> > of the SoC that is set by whatever SoC identifying driver the platform
> > has?
> 
> [Lu Yangbo-B47093] I think this has been discussed in v6.
> You can find Scott's comments about this in below link.
> https://patchwork.kernel.org/patch/8544501/

Ah, thanks for bearing with me and digging this out again. Let me follow
up on Scott's older replies here then:

> >> 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.

It feels like a bad design to have to come up with a different
method for each SoC type here when they all do the same thing
and want to identify some variant of the chip to do device
specific quirks.

As far as I'm concerned, every driver in drivers/soc that needs to
export a symbol to be used by a device driver is an indication that
we don't have the right set of abstractions yet. There are cases
that are not worth abstracting because the functionality is rather
obscure and only a couple of drivers for one particular chip
ever need it.

Finding out the version of the SoC does not look like this case.

> > 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?

COMPILE_TEST is solvable here and the way it is implemented in this
case (selecting FSL_GUTS from the driver) indeed looks like it works
correctly, but it's still awkward that this means building the
SoC specific ID stuff into the vmlinux binary for any driver that
uses something like that for a particular SoC.

> >> 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. 

You already need to encode it as a string to register the soc_device,
and the driver just needs to pass a glob string, so the only part that
is missing is the generic function that takes the string from the
driver and passes that to glob_match for the soc_device.

> And when would it get registered on arm64, which doesn't have
> platform code?

Whenever the soc driver is loaded, as is the case now. The match
function can return -EPROBE_DEFER if no SoC device is registered
yet.

	Arnd

Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Scott Wood]

On Thu, 2016-05-05 at 13:10 +0200, Arnd Bergmann wrote:
> On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: Thursday, May 05, 2016 4:32 PM
> > > To: linuxppc-dev@lists.ozlabs.org
> > > Cc: Yangbo Lu; linux-mmc@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > linux-clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> > > foundation.org; netdev@vger.kernel.org; Mark Rutland;
> > > ulf.hansson@linaro.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> > > Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> > > Kumar Gala; Xiaobo Xie; Qiang Zhao
> > > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> > > R1.0-R2.0
> > > 
> > > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > > > 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.
> 
> It feels like a bad design to have to come up with a different
> method for each SoC type here when they all do the same thing
> and want to identify some variant of the chip to do device
> specific quirks.
> 
> As far as I'm concerned, every driver in drivers/soc that needs to
> export a symbol to be used by a device driver is an indication that
> we don't have the right set of abstractions yet. There are cases
> that are not worth abstracting because the functionality is rather
> obscure and only a couple of drivers for one particular chip
> ever need it.
> 
> Finding out the version of the SoC does not look like this case.

I'm open to new ways of abstracting this, but can that please be discussed
after these patches are merged?  This patchset is fixing a problem, the
existing abstraction is unappealing and not widely adopted, a new abstraction
is not ready, and we're only touching code for our hardware.

Oh, and the existing abstraction isn't even "existing".  I don't see any
examples where soc_device is being used like this -- or even any way for a
driver (the one consuming the information, not the soc "driver") to get a
reference to the soc_device that's been registered short of searching for the
device object by name -- and you're asking for new functionality in
drivers/base/soc.c.

> > > 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?
> 
> COMPILE_TEST is solvable here and the way it is implemented in this
> case (selecting FSL_GUTS from the driver) indeed looks like it works
> correctly, but it's still awkward that this means building the
> SoC specific ID stuff into the vmlinux binary for any driver that
> uses something like that for a particular SoC.

Please keep in mind that this is a Freescale-specific driver... it's not as if
we're attaching this dependency to common SDHCI code.

> 
> > > > 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. 
> 
> You already need to encode it as a string to register the soc_device,

No we don't, because we don't already register a soc_device on arm64 or ppc
(and it looks like whatever does get registered on at least some relevant
arm32 chips is not particularly useful).

> and the driver just needs to pass a glob string, so the only part that
> is missing is the generic function that takes the string from the
> driver and passes that to glob_match for the soc_device.

"just"

And what would the glob look like?

I'd rather not write kernel code as if it were a shell/Perl script.

> > And when would it get registered on arm64, which doesn't have
> > platform code?
> 
> Whenever the soc driver is loaded, as is the case now. The match
> function can return -EPROBE_DEFER if no SoC device is registered
> yet.

That's too late for some places where we need access to SVR, e.g. clock
drivers (which use CLK_OF_DECLARE and are initialized very early, not as part
of the driver model and thus can't defer).  Currently we have an #ifdef
CONFIG_PPC for this in drivers/clk/clk-qoriq.c... Maybe we should have done
that here as well, and saved some grief. :-)  At least until an erratum pops
up on an ARM-based chip.

And what happens if we're running on arm32, and thus the arch code already
registered an soc_device with a different (and less useful) encoding?

-Scott

RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Yangbo Lu]

Hi Arnd,

Any comments? 
Please reply when you see the email since we want this eSDHC issue to be fixed soon.

All the patches are Freescale-specific and is to fix the eSDHC driver.
Could we let them merged first if you were talking about a new way of abstracting getting SoC version.


Thanks :)


Best regards,
Yangbo Lu

> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Wednesday, May 11, 2016 11:26 AM
> To: Arnd Bergmann; linux-arm-kernel@lists.infradead.org
> Cc: Yangbo Lu; linuxppc-dev@lists.ozlabs.org; Mark Rutland;
> devicetree@vger.kernel.org; ulf.hansson@linaro.org; Russell King; Bhupesh
> Sharma; netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
> iommu@lists.linux-foundation.org; Rob Herring; linux-i2c@vger.kernel.org;
> Claudiu Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org;
> Qiang Zhao
> Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
> 
> On Thu, 2016-05-05 at 13:10 +0200, Arnd Bergmann wrote:
> > On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > > > -----Original Message-----
> > > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > > Sent: Thursday, May 05, 2016 4:32 PM
> > > > To: linuxppc-dev@lists.ozlabs.org
> > > > Cc: Yangbo Lu; linux-mmc@vger.kernel.org;
> > > > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; linux-clk@vger.kernel.org;
> > > > linux-i2c@vger.kernel.org; iommu@lists.linux- foundation.org;
> > > > netdev@vger.kernel.org; Mark Rutland; ulf.hansson@linaro.org;
> > > > Russell King; Bhupesh Sharma; Joerg Roedel; Santosh Shilimkar;
> > > > Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil; Kumar Gala;
> > > > Xiaobo Xie; Qiang Zhao
> > > > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for
> > > > T4240-
> > > > R1.0-R2.0
> > > >
> > > > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > > > > 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.
> >
> > It feels like a bad design to have to come up with a different method
> > for each SoC type here when they all do the same thing and want to
> > identify some variant of the chip to do device specific quirks.
> >
> > As far as I'm concerned, every driver in drivers/soc that needs to
> > export a symbol to be used by a device driver is an indication that we
> > don't have the right set of abstractions yet. There are cases that are
> > not worth abstracting because the functionality is rather obscure and
> > only a couple of drivers for one particular chip ever need it.
> >
> > Finding out the version of the SoC does not look like this case.
> 
> I'm open to new ways of abstracting this, but can that please be
> discussed after these patches are merged?  This patchset is fixing a
> problem, the existing abstraction is unappealing and not widely adopted,
> a new abstraction is not ready, and we're only touching code for our
> hardware.
> 
> Oh, and the existing abstraction isn't even "existing".  I don't see any
> examples where soc_device is being used like this -- or even any way for
> a driver (the one consuming the information, not the soc "driver") to get
> a reference to the soc_device that's been registered short of searching
> for the device object by name -- and you're asking for new functionality
> in drivers/base/soc.c.
> 
> > > > 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?
> >
> > COMPILE_TEST is solvable here and the way it is implemented in this
> > case (selecting FSL_GUTS from the driver) indeed looks like it works
> > correctly, but it's still awkward that this means building the SoC
> > specific ID stuff into the vmlinux binary for any driver that uses
> > something like that for a particular SoC.
> 
> Please keep in mind that this is a Freescale-specific driver... it's not
> as if we're attaching this dependency to common SDHCI code.
> 
> >
> > > > > 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.
> >
> > You already need to encode it as a string to register the soc_device,
> 
> No we don't, because we don't already register a soc_device on arm64 or
> ppc (and it looks like whatever does get registered on at least some
> relevant
> arm32 chips is not particularly useful).
> 
> > and the driver just needs to pass a glob string, so the only part that
> > is missing is the generic function that takes the string from the
> > driver and passes that to glob_match for the soc_device.
> 
> "just"
> 
> And what would the glob look like?
> 
> I'd rather not write kernel code as if it were a shell/Perl script.
> 
> > > And when would it get registered on arm64, which doesn't have
> > > platform code?
> >
> > Whenever the soc driver is loaded, as is the case now. The match
> > function can return -EPROBE_DEFER if no SoC device is registered yet.
> 
> That's too late for some places where we need access to SVR, e.g. clock
> drivers (which use CLK_OF_DECLARE and are initialized very early, not as
> part of the driver model and thus can't defer).  Currently we have an
> #ifdef CONFIG_PPC for this in drivers/clk/clk-qoriq.c... Maybe we should
> have done that here as well, and saved some grief. :-)  At least until an
> erratum pops up on an ARM-based chip.
> 
> And what happens if we're running on arm32, and thus the arch code
> already registered an soc_device with a different (and less useful)
> encoding?
> 
> -Scott

RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Yangbo Lu]

Hi Uffe,

Could we merge this patchset? ...
It has been a long time to wait for Arnd's response...
 
Thanks a lot.


Best regards,
Yangbo Lu


> -----Original Message-----
> From: Yangbo Lu
> Sent: Friday, May 20, 2016 2:06 PM
> To: 'Scott Wood'; Arnd Bergmann; linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org; Mark Rutland;
> devicetree@vger.kernel.org; ulf.hansson@linaro.org; Russell King; Bhupesh
> Sharma; netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
> iommu@lists.linux-foundation.org; Rob Herring; linux-i2c@vger.kernel.org;
> Claudiu Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org;
> Qiang Zhao
> Subject: RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> R1.0-R2.0
> 
> Hi Arnd,
> 
> Any comments?
> Please reply when you see the email since we want this eSDHC issue to be
> fixed soon.
> 
> All the patches are Freescale-specific and is to fix the eSDHC driver.
> Could we let them merged first if you were talking about a new way of
> abstracting getting SoC version.
> 
> 
> Thanks :)
> 
> 
> Best regards,
> Yangbo Lu
> 

Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Ulf Hansson]

On 26 May 2016 at 06:05, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> Hi Uffe,
>
> Could we merge this patchset? ...
> It has been a long time to wait for Arnd's response...
>
> Thanks a lot.
>
>

As we are still in the merge window I won't queue anything but fixes.
Let's give Arnd another week or so to respond.

Kind regards
Uffe

> Best regards,
> Yangbo Lu
>
>
>> -----Original Message-----
>> From: Yangbo Lu
>> Sent: Friday, May 20, 2016 2:06 PM
>> To: 'Scott Wood'; Arnd Bergmann; linux-arm-kernel@lists.infradead.org
>> Cc: linuxppc-dev@lists.ozlabs.org; Mark Rutland;
>> devicetree@vger.kernel.org; ulf.hansson@linaro.org; Russell King; Bhupesh
>> Sharma; netdev@vger.kernel.org; Joerg Roedel; Kumar Gala; linux-
>> mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Yang-Leo Li;
>> iommu@lists.linux-foundation.org; Rob Herring; linux-i2c@vger.kernel.org;
>> Claudiu Manoil; Santosh Shilimkar; Xiaobo Xie; linux-clk@vger.kernel.org;
>> Qiang Zhao
>> Subject: RE: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
>> R1.0-R2.0
>>
>> Hi Arnd,
>>
>> Any comments?
>> Please reply when you see the email since we want this eSDHC issue to be
>> fixed soon.
>>
>> All the patches are Freescale-specific and is to fix the eSDHC driver.
>> Could we let them merged first if you were talking about a new way of
>> abstracting getting SoC version.
>>
>>
>> Thanks :)
>>
>>
>> Best regards,
>> Yangbo Lu
>>
>

Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Arnd Bergmann]

On Thursday, May 26, 2016 9:44:10 AM CEST Ulf Hansson wrote:
> On 26 May 2016 at 06:05, Yangbo Lu <yangbo.lu@nxp.com> wrote:
> > Hi Uffe,
> >
> > Could we merge this patchset? ...
> > It has been a long time to wait for Arnd's response...
> >
> > Thanks a lot.
> >
> >
> 
> As we are still in the merge window I won't queue anything but fixes.
> Let's give Arnd another week or so to respond.

I've got a patch series now that implements a method for matching
the soc ID, see the following emails.

	Arnd

[PATCH 1/4] base: soc: introduce soc_device_match() interface

[Arnd Bergmann]

We keep running into cases where device drivers want to know the exact
version of the SoC a they are currently running on. In the past, this
has usually been done through a vendor specific API that can be called
by a driver, or by directly accessing some kind of version register
that is not part of the device itself but that belongs to a global
register area of the chip.

Common reasons for doing this include:

- A machine is not using devicetree or similar for passing data
  about on-chip devices, but just announces their presence using
  boot-time platform devices, and the machine code itself does
  not care about the revision.

- There is existing firmware or boot loaders with existing DT
  binaries with generic compatible strings that do not identify
  the particular revision of each device, but the driver knows
  which SoC revisions include which part

- A prerelease version of a chip has some quirks and we are
  using the same version of the bootloader and the DT blob
  on both the prerelease and the final version. An update of
  the DT binding seems inappropriate because that would involve
  maintaining multiple copies of the dts and/or bootloader.

This introduces the soc_device_match() interface that is meant
to work like of_match_node() but instead of identifying the
version of a device, it identifies the SoC itself using a
vendor-agnostic interface.

Unlike soc_device_match(), we do not do an exact string compare
but instead use glob_match() to allow wildcards in strings.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 246acdafedb6..fc7613cc7fd5 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -225,6 +225,7 @@ config GENERIC_CPU_AUTOPROBE
 
 config SOC_BUS
 	bool
+	select GLOB
 
 source "drivers/base/regmap/Kconfig"
 
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index 75b98aad6faf..e9623c6674a5 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -14,6 +14,7 @@
 #include <linux/spinlock.h>
 #include <linux/sys_soc.h>
 #include <linux/err.h>
+#include <linux/glob.h>
 
 static DEFINE_IDA(soc_ida);
 
@@ -168,3 +169,60 @@ static void __exit soc_bus_unregister(void)
 	bus_unregister(&soc_bus_type);
 }
 module_exit(soc_bus_unregister);
+
+static int soc_device_match_one(struct device *dev, void *arg)
+{
+	struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
+	struct soc_device_attribute *match = arg;
+
+	if (match->machine && !glob_match(match->machine, soc_dev->attr->machine))
+		return 0;
+
+	if (match->family && !glob_match(match->family, soc_dev->attr->family))
+		return 0;
+
+	if (match->revision && !glob_match(match->revision, soc_dev->attr->revision))
+		return 0;
+
+	if (match->soc_id && !glob_match(match->soc_id, soc_dev->attr->revision))
+		return 0;
+
+	return 1;
+}
+
+/*
+ * soc_device_match - identify the SoC in the machine
+ * @matches: zero-terminated array of possible matches
+ *
+ * returns the first matching entry of the argument array, or NULL
+ * if none of them match.
+ *
+ * This function is meant as a helper in place of of_match_node()
+ * in cases where either no device tree is available or the information
+ * in a device node is insufficient to identify a particular variant
+ * by its compatible strings or other properties. For new devices,
+ * the DT binding should always provide unique compatible strings
+ * that allow the use of of_match_node() instead.
+ *
+ * The calling function can use the .data entry of the
+ * soc_device_attribute to pass a structure or function pointer for
+ * each entry.
+ */
+struct soc_device_attribute *soc_device_match(struct soc_device_attribute *matches)
+{
+	struct device *dev;
+	int ret;
+
+	for (ret = 0; ret == 0; matches++) {
+		if (matches->machine || matches->family ||
+		    matches->revision || matches->soc_id)
+			return NULL;
+
+		dev = NULL;
+		ret = bus_for_each_dev(&soc_bus_type, dev, matches,
+					 soc_device_match_one);
+	}
+
+	return matches;
+}
+EXPORT_SYMBOL_GPL(soc_device_match);
diff --git a/include/linux/sys_soc.h b/include/linux/sys_soc.h
index 2739ccb69571..02c48c76052b 100644
--- a/include/linux/sys_soc.h
+++ b/include/linux/sys_soc.h
@@ -13,6 +13,8 @@ struct soc_device_attribute {
 	const char *family;
 	const char *revision;
 	const char *soc_id;
+
+	const void *data;
 };
 
 /**
@@ -34,4 +36,6 @@ void soc_device_unregister(struct soc_device *soc_dev);
  */
 struct device *soc_device_to_device(struct soc_device *soc);
 
+struct soc_device_attribute *soc_device_match(struct soc_device_attribute *matches);
+
 #endif /* __SOC_BUS_H */

Re: [PATCH 1/4] base: soc: introduce soc_device_match() interface

[Arnd Bergmann]

On Monday, May 30, 2016 3:14:38 PM CEST Arnd Bergmann wrote:
> We keep running into cases where device drivers want to know the exact
> version of the SoC a they are currently running on. In the past, this
> has usually been done through a vendor specific API that can be called
> by a driver, or by directly accessing some kind of version register
> that is not part of the device itself but that belongs to a global
> register area of the chip.
> 
> Common reasons for doing this include:
> 
> - A machine is not using devicetree or similar for passing data
>   about on-chip devices, but just announces their presence using
>   boot-time platform devices, and the machine code itself does
>   not care about the revision.
> 
> - There is existing firmware or boot loaders with existing DT
>   binaries with generic compatible strings that do not identify
>   the particular revision of each device, but the driver knows
>   which SoC revisions include which part
> 
> - A prerelease version of a chip has some quirks and we are
>   using the same version of the bootloader and the DT blob
>   on both the prerelease and the final version. An update of
>   the DT binding seems inappropriate because that would involve
>   maintaining multiple copies of the dts and/or bootloader.
> 
> This introduces the soc_device_match() interface that is meant
> to work like of_match_node() but instead of identifying the
> version of a device, it identifies the SoC itself using a
> vendor-agnostic interface.
> 
> Unlike soc_device_match(), we do not do an exact string compare
> but instead use glob_match() to allow wildcards in strings.

I'm sorry the series introduced build failures (I had done some changes
after testing), here is a quick fixup. I'll resend the whole thing
after someone has looked at it as none of the changes below should
have any influence on the review.

	Arnd

diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index e9623c6674a5..c38573249777 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -173,7 +173,7 @@ module_exit(soc_bus_unregister);
 static int soc_device_match_one(struct device *dev, void *arg)
 {
 	struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
-	struct soc_device_attribute *match = arg;
+	const struct soc_device_attribute *match = arg;
 
 	if (match->machine && !glob_match(match->machine, soc_dev->attr->machine))
 		return 0;
@@ -208,7 +208,7 @@ static int soc_device_match_one(struct device *dev, void *arg)
  * soc_device_attribute to pass a structure or function pointer for
  * each entry.
  */
-struct soc_device_attribute *soc_device_match(struct soc_device_attribute *matches)
+const struct soc_device_attribute *soc_device_match(const struct soc_device_attribute *matches)
 {
 	struct device *dev;
 	int ret;
@@ -219,7 +219,7 @@ struct soc_device_attribute *soc_device_match(struct soc_device_attribute *match
 			return NULL;
 
 		dev = NULL;
-		ret = bus_for_each_dev(&soc_bus_type, dev, matches,
+		ret = bus_for_each_dev(&soc_bus_type, dev, (void *)matches,
 					 soc_device_match_one);
 	}
 
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 1d4814fe4cb2..a7b8b05a13e8 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -19,6 +19,8 @@
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/mmc/host.h>
+#include <linux/sys_soc.h>
+
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
 
@@ -75,7 +77,6 @@ static u16 esdhc_readw_fixup(struct sdhci_host *host,
 {
 	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;
 
 	if (spec_reg == SDHCI_HOST_VERSION)
@@ -565,7 +566,7 @@ static const struct sdhci_pltfm_data sdhci_esdhc_le_pdata = {
 };
 
 #define T4240_HOST_VER ((VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200)
-static const struct soc_device_attribute esdhc_t4240_quirk = {
+static const struct soc_device_attribute esdhc_t4240_quirk[] = {
 	/* T4240 revision < 0x20 uses vendor version 23, SDHCI version 200 */
 	{ .soc_id = "T4*(0x824000)", .revision = "0x[01]?",
 	  .data = (void *)(uintptr_t)(T4240_HOST_VER) },
@@ -576,6 +577,7 @@ static void esdhc_init(struct platform_device *pdev, struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_esdhc *esdhc;
+	u32 host_ver;
 
 	pltfm_host = sdhci_priv(host);
 	esdhc = sdhci_pltfm_priv(pltfm_host);
@@ -583,9 +585,9 @@ static void esdhc_init(struct platform_device *pdev, struct sdhci_host *host)
 	host_ver = sdhci_readw(host, SDHCI_HOST_VERSION);
 
 	if (of_device_is_compatible(pdev->dev.of_node, "fsl,t4240-esdhc")) {
-		struct soc_device_attribute *match;
+		const struct soc_device_attribute *match;
 
-		match = soc_device_match(&esdhc_t4240_quirk);
+		match = soc_device_match(esdhc_t4240_quirk);
 		if (match)
 			host_ver = (uintptr_t)match->data;
 	}
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 2f30698f5bcf..476969644bed 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -34,14 +34,6 @@ static u32 fsl_guts_get_svr(struct guts *guts)
 		return ioread32be(guts->regs + GUTS_SVR);
 }
 
-static u32 fsl_guts_get_pvr(struct guts *guts)
-{
-	if (guts->little_endian)
-		return ioread32(guts->regs + GUTS_PVR);
-	else
-		return ioread32be(guts->regs + GUTS_PVR);
-}
-
 /*
  * Table for matching compatible strings, for device tree
  * guts node, for Freescale QorIQ SOCs.
@@ -76,13 +68,15 @@ static const struct of_device_id fsl_guts_of_match[] = {
 
 static void fsl_guts_init(struct device *dev, struct guts *guts)
 {
-	const struct of_device_id *id;
 	u32 svr = fsl_guts_get_svr(guts);
+	const struct of_device_id *id;
+	const char *socname;
 
 	guts->soc.family = "NXP QorIQ";
 	id = of_match_node(fsl_guts_of_match, dev->of_node);
-	guts->soc.soc_id = devm_kasprintf(dev, "%s (ver 0x%06x)" id->data,
-					  svr >> 8;
+	socname = id->data;
+	guts->soc.soc_id = devm_kasprintf(dev, GFP_KERNEL, "%s (ver 0x%06x)",
+					  socname, svr >> 8);
 	guts->soc.revision = devm_kasprintf(dev, GFP_KERNEL, "0x%02x",
 					    svr & 0xff);
 }
diff --git a/include/linux/sys_soc.h b/include/linux/sys_soc.h
index 02c48c76052b..3dfc8714a88c 100644
--- a/include/linux/sys_soc.h
+++ b/include/linux/sys_soc.h
@@ -36,6 +36,6 @@ void soc_device_unregister(struct soc_device *soc_dev);
  */
 struct device *soc_device_to_device(struct soc_device *soc);
 
-struct soc_device_attribute *soc_device_match(struct soc_device_attribute *matches);
+const struct soc_device_attribute *soc_device_match(const struct soc_device_attribute *matches);
 
 #endif /* __SOC_BUS_H */

[PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Arnd Bergmann]

From: Yangbo Lu <yangbo.lu@nxp.com>

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.

[arnd turned this into a platform_driver registering a soc_device
 rather than providing an ad-hoc interface for soc-id]

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index cb58ef0d9b2c..7106463f118e 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 000000000000..33d331cac8d6
--- /dev/null
+++ b/drivers/soc/fsl/Kconfig
@@ -0,0 +1,15 @@
+#
+# Freescale SOC drivers
+#
+
+source "drivers/soc/fsl/qe/Kconfig"
+
+config FSL_GUTS
+	bool "NXP Layerscale SoC identification"
+	depends on PPC_FSL || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST
+	default PPC_FSL || SOC_LS1021A || ARCH_LAYERSCAPE
+	select SOC_BUS
+	help
+	  This registers a SoC device for NXP (formerly Freescale)
+	  Layerscape devices, making information about the system
+	  available in /sys/devices/soc/
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 203307fd92c1..02afb7f980f6 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 000000000000..2f30698f5bcf
--- /dev/null
+++ b/drivers/soc/fsl/guts.c
@@ -0,0 +1,130 @@
+/*
+ * 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/platform_device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/sys_soc.h>
+
+#define GUTS_PVR	0x0a0
+#define GUTS_SVR	0x0a4
+
+struct guts {
+	void __iomem *regs;
+	bool little_endian;
+	struct soc_device_attribute soc;
+};
+
+static u32 fsl_guts_get_svr(struct guts *guts)
+{
+	if (guts->little_endian)
+		return ioread32(guts->regs + GUTS_SVR);
+	else
+		return ioread32be(guts->regs + GUTS_SVR);
+}
+
+static u32 fsl_guts_get_pvr(struct guts *guts)
+{
+	if (guts->little_endian)
+		return ioread32(guts->regs + GUTS_PVR);
+	else
+		return ioread32be(guts->regs + GUTS_PVR);
+}
+
+/*
+ * Table for matching compatible strings, for device tree
+ * guts node, for Freescale QorIQ SOCs.
+ */
+static const struct of_device_id fsl_guts_of_match[] = {
+	/* For T4 & B4 Series SOCs */
+	{ .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4 series" },
+	/* For P Series SOCs */
+	{ .compatible = "fsl,p1010-guts", .data = "P1010/P1014" },
+	{ .compatible = "fsl,p1020-guts", .data = "P1020/P1011" },
+	{ .compatible = "fsl,p1021-guts", .data = "P1021/P1012" },
+	{ .compatible = "fsl,p1022-guts", .data = "P1022/P1013" },
+	{ .compatible = "fsl,p1023-guts", .data = "P1013/P1017" },
+	{ .compatible = "fsl,p2020-guts", .data = "P2010/P2020" },
+	{ .compatible = "fsl,qoriq-device-config-2.0", .data = "P series" },
+	/* For BSC Series SOCs */
+	{ .compatible = "fsl,bsc9131-guts", .data = "BSC9131 Qonverge" },
+	{ .compatible = "fsl,bsc9132-guts", .data = "BSC9132 Qonverge" },
+	/* For MPC85xx Series SOCs */
+	{ .compatible = "fsl,mpc8536-guts", .data = "PowerPC MPC8536" },
+	{ .compatible = "fsl,mpc8544-guts", .data = "PowerPC MPC8544" },
+	{ .compatible = "fsl,mpc8548-guts", .data = "PowerPC MPC8548" },
+	{ .compatible = "fsl,mpc8568-guts", .data = "PowerPC MPC8568" },
+	{ .compatible = "fsl,mpc8569-guts", .data = "PowerPC MPC8569" },
+	{ .compatible = "fsl,mpc8572-guts", .data = "PowerPC MPC8572" },
+	/* For Layerscape Series SOCs */
+	{ .compatible = "fsl,ls1021a-dcfg", .data = "Layerscape LS1021A" },
+	{ .compatible = "fsl,ls1043a-dcfg", .data = "Layerscape LS1043A" },
+	{ .compatible = "fsl,ls2080a-dcfg", .data = "Layerscape LS2080A" },
+	{}
+};
+
+static void fsl_guts_init(struct device *dev, struct guts *guts)
+{
+	const struct of_device_id *id;
+	u32 svr = fsl_guts_get_svr(guts);
+
+	guts->soc.family = "NXP QorIQ";
+	id = of_match_node(fsl_guts_of_match, dev->of_node);
+	guts->soc.soc_id = devm_kasprintf(dev, "%s (ver 0x%06x)" id->data,
+					  svr >> 8;
+	guts->soc.revision = devm_kasprintf(dev, GFP_KERNEL, "0x%02x",
+					    svr & 0xff);
+}
+
+static int fsl_guts_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct guts *guts;
+	int ret;
+
+	guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
+	if (!guts) {
+		ret = -ENOMEM;
+		goto out;
+
+	}
+
+	/*
+	 * syscon devices default to little-endian, but on powerpc we have
+	 * existing device trees with big-endian maps and an absent endianess
+	 * "big-property"
+	 */
+	if (!IS_ENABLED(CONFIG_POWERPC) &&
+	    !of_property_read_bool(dev->of_node, "big-endian"))
+		guts->little_endian = true;
+
+	guts->regs = devm_ioremap_resource(dev, 0);
+	if (!guts->regs) {
+		ret = -ENOMEM;
+		kfree(guts);
+		goto out;
+	}
+
+	fsl_guts_init(dev, guts);
+	ret = 0;
+out:
+	return ret;
+}
+
+static struct platform_driver fsl_soc_guts = {
+	.probe = fsl_guts_probe,
+	.driver.of_match_table = fsl_guts_of_match,
+};
+
+module_platform_driver(fsl_soc_guts);

Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Scott Wood]

On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> new file mode 100644
> index 000000000000..2f30698f5bcf
> --- /dev/null
> +++ b/drivers/soc/fsl/guts.c
> @@ -0,0 +1,130 @@
> +/*
> + * 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/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/sys_soc.h>
> +
> +#define GUTS_PVR	0x0a0
> +#define GUTS_SVR	0x0a4
> +
> +struct guts {
> +	void __iomem *regs;

We already have a struct to define guts.  Why are you not using it?  Why do
you consider using it to be "abuse"?  What if we want to move more guts
functionality into this driver?

> +	bool little_endian;
> +	struct soc_device_attribute soc;
> +};
> +
> +static u32 fsl_guts_get_svr(struct guts *guts)
> +{
> +	if (guts->little_endian)
> +		return ioread32(guts->regs + GUTS_SVR);
> +	else
> +		return ioread32be(guts->regs + GUTS_SVR);
> +}
> +
> +static u32 fsl_guts_get_pvr(struct guts *guts)
> +{
> +	if (guts->little_endian)
> +		return ioread32(guts->regs + GUTS_PVR);
> +	else
> +		return ioread32be(guts->regs + GUTS_PVR);
> +}

You've removed the fallback to mfspr() on PPC, which would be helpful in some
virtualized environments where we don't have the guts node (but do have other
directly assigned devices).  Of course, this is a consequence of the
conversion into a platform device.

> +
> +/*
> + * Table for matching compatible strings, for device tree
> + * guts node, for Freescale QorIQ SOCs.
> + */
> +static const struct of_device_id fsl_guts_of_match[] = {
> +	/* For T4 & B4 Series SOCs */
> +	{ .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> series" },
[snip]
> +	{ .compatible = "fsl,qoriq-device-config-2.0", .data = "P series"

As noted in my comment on patch 3/4, these descriptions are reversed.

They're also incomplete.  t2080 has device config 2.0.  t1040 is described as
2.0 though it should probably be 2.1 (or better, drop the generic compatible
altogether).

> +	/*
> +	 * syscon devices default to little-endian, but on powerpc we have
> +	 * existing device trees with big-endian maps and an absent
> endianess
> +	 * "big-property"
> +	 */
> +	if (!IS_ENABLED(CONFIG_POWERPC) &&
> +	    !of_property_read_bool(dev->of_node, "big-endian"))
> +		guts->little_endian = true;

This is not a syscon device (Yangbo's patch to add a guts node on ls2080 is
the only guts node that says "syscon", and that was a leftover from earlier
revisions and should probably be removed).  Even if it were, where is it
documented that syscon defaults to little-endian?  

Documentation/devicetree/bindings/common-properties.txt says that the
individual binding specifies the default.  The default for this node should be
big-endian because that's what existed before there was a need to describe the
endianness.  And we need an update to the guts binding to specify that.

> +
> +	guts->regs = devm_ioremap_resource(dev, 0);
> +	if (!guts->regs) {
> +		ret = -ENOMEM;
> +		kfree(guts);
> +		goto out;
> +	}
> +
> +	fsl_guts_init(dev, guts);
> +	ret = 0;
> +out:
> +	return ret;
> +}
> +
> +static struct platform_driver fsl_soc_guts = {
> +	.probe = fsl_guts_probe,
> +	.driver.of_match_table = fsl_guts_of_match,
> +};
> +
> +module_platform_driver(fsl_soc_guts);

Again, this means that the information is not available during early boot,
such as in the clock driver.  Thus we would not be able to convert clk-qoriq's
direct mfspr(SPRN_SVR) into an soc_device_match() (or anything else that makes
use of this file), nor would we be able to move its access of the guts RCW
registers into this driver.

-Scott

Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Arnd Bergmann]

On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> > new file mode 100644
> > index 000000000000..2f30698f5bcf
> > --- /dev/null
> > +++ b/drivers/soc/fsl/guts.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * 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/platform_device.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/sys_soc.h>
> > +
> > +#define GUTS_PVR	0x0a0
> > +#define GUTS_SVR	0x0a4
> > +
> > +struct guts {
> > +	void __iomem *regs;
> 
> We already have a struct to define guts.  Why are you not using it?  Why do
> you consider using it to be "abuse"?  What if we want to move more guts
> functionality into this driver?

This structure was in the original patch, I left it in there, only
removed the inclusion of the powerpc header file, which seemed to
be misplaced.

> > +	bool little_endian;
> > +	struct soc_device_attribute soc;
> > +};
> > +
> > +static u32 fsl_guts_get_svr(struct guts *guts)
> > +{
> > +	if (guts->little_endian)
> > +		return ioread32(guts->regs + GUTS_SVR);
> > +	else
> > +		return ioread32be(guts->regs + GUTS_SVR);
> > +}
> > +
> > +static u32 fsl_guts_get_pvr(struct guts *guts)
> > +{
> > +	if (guts->little_endian)
> > +		return ioread32(guts->regs + GUTS_PVR);
> > +	else
> > +		return ioread32be(guts->regs + GUTS_PVR);
> > +}
> 
> You've removed the fallback to mfspr() on PPC, which would be helpful in some
> virtualized environments where we don't have the guts node (but do have other
> directly assigned devices).  Of course, this is a consequence of the
> conversion into a platform device.

Right, it just didn't make any sense after the conversion. I couldn't
figure out what the intention was for the fallback. Virtualized environments
are an interesting case, but I'd also argue that a virtualized system is
not the original SoC anyway, so reporting the physical SoC as the
main system in the guest is a bit strange and we probably want to
come up with a different solution there.

In soc_device, we probably want to just report the type of hypervisor
in this case, which is what most users would expect there. For the
specific case of the mmc driver (are there any real use cases where
you'd pass on the mmc controller to a guest, as opposed to passing either
an emulated block device or the mmc device on an emulated mmc host?),
that means we have to come up with a different solution, but then
we'd be free to work around this by modifying the DT node of the mmc
device.

> > +/*
> > + * Table for matching compatible strings, for device tree
> > + * guts node, for Freescale QorIQ SOCs.
> > + */
> > +static const struct of_device_id fsl_guts_of_match[] = {
> > +	/* For T4 & B4 Series SOCs */
> > +	{ .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > series" },
> [snip]
> > +	{ .compatible = "fsl,qoriq-device-config-2.0", .data = "P series"
> 
> As noted in my comment on patch 3/4, these descriptions are reversed.
> 
> They're also incomplete.  t2080 has device config 2.0.  t1040 is described as
> 2.0 though it should probably be 2.1 (or better, drop the generic compatible
> altogether).

Ok. Ideally I think we'd even look up the specific SoC names from the
SVC rather than the compatible string. I just didn't have a good list
for those to put in the driver.

> > +	/*
> > +	 * syscon devices default to little-endian, but on powerpc we have
> > +	 * existing device trees with big-endian maps and an absent
> > endianess
> > +	 * "big-property"
> > +	 */
> > +	if (!IS_ENABLED(CONFIG_POWERPC) &&
> > +	    !of_property_read_bool(dev->of_node, "big-endian"))
> > +		guts->little_endian = true;
> 
> This is not a syscon device (Yangbo's patch to add a guts node on ls2080 is
> the only guts node that says "syscon", and that was a leftover from earlier
> revisions and should probably be removed).  Even if it were, where is it
> documented that syscon defaults to little-endian?

Documentation/devicetree/bindings/regmap/regmap.txt

We had a little screwup here, basically regmap (and by consequence, syscon)
always defaulted to little-endian way before that was documented, so it's
too late to change it, although I agree it would have made sense to document
regmap to default to big-endian on powerpc.

> Documentation/devicetree/bindings/common-properties.txt says that the
> individual binding specifies the default.  The default for this node should be
> big-endian because that's what existed before there was a need to describe the
> endianness.  And we need an update to the guts binding to specify that.

Good point. This proably means that specifying both the "guts" and "syscon"
compatible strings implies having to also specify the endianess explicitly
both ways, because otherwise we break one of the two bindings.

> > +
> > +	guts->regs = devm_ioremap_resource(dev, 0);
> > +	if (!guts->regs) {
> > +		ret = -ENOMEM;
> > +		kfree(guts);
> > +		goto out;
> > +	}
> > +
> > +	fsl_guts_init(dev, guts);
> > +	ret = 0;
> > +out:
> > +	return ret;
> > +}
> > +
> > +static struct platform_driver fsl_soc_guts = {
> > +	.probe = fsl_guts_probe,
> > +	.driver.of_match_table = fsl_guts_of_match,
> > +};
> > +
> > +module_platform_driver(fsl_soc_guts);
> 
> Again, this means that the information is not available during early boot,
> such as in the clock driver.  Thus we would not be able to convert clk-qoriq's
> direct mfspr(SPRN_SVR) into an soc_device_match() (or anything else that makes
> use of this file), nor would we be able to move its access of the guts RCW
> registers into this driver.

Correct. Do we have a reason to convert the mfspr() though? I don't really
see an improvement over the current state if we do that, and for new devices
that might need the erratum workaround, we could add a DT property that would
be preferred to both.

	Arnd

Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Scott Wood]

On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> > > new file mode 100644
> > > index 000000000000..2f30698f5bcf
> > > --- /dev/null
> > > +++ b/drivers/soc/fsl/guts.c
> > > @@ -0,0 +1,130 @@
> > > +/*
> > > + * 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/platform_device.h>
> > > +#include <linux/module.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/sys_soc.h>
> > > +
> > > +#define GUTS_PVR	0x0a0
> > > +#define GUTS_SVR	0x0a4
> > > +
> > > +struct guts {
> > > +	void __iomem *regs;
> > 
> > We already have a struct to define guts.  Why are you not using it?  Why
> > do
> > you consider using it to be "abuse"?  What if we want to move more guts
> > functionality into this driver?
> 
> This structure was in the original patch, I left it in there, only
> removed the inclusion of the powerpc header file, which seemed to
> be misplaced.

I'm not refering "struct guts".  I'm referring to changing
"struct ccsr_guts __iomem *regs" into "void __iomem *regs".

And it's not a powerpc header file.

> > > +/*
> > > + * Table for matching compatible strings, for device tree
> > > + * guts node, for Freescale QorIQ SOCs.
> > > + */
> > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > +	/* For T4 & B4 Series SOCs */
> > > +	{ .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > series" },
> > [snip]
> > > +	{ .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > series"
> > 
> > As noted in my comment on patch 3/4, these descriptions are reversed.
> > 
> > They're also incomplete.  t2080 has device config 2.0.  t1040 is described
> > as
> > 2.0 though it should probably be 2.1 (or better, drop the generic
> > compatible
> > altogether).
> 
> Ok. Ideally I think we'd even look up the specific SoC names from the
> SVC rather than the compatible string. I just didn't have a good list
> for those to put in the driver.

The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why we need
to convert it to a string in the first place.

> 
> > > +	/*
> > > +	 * syscon devices default to little-endian, but on powerpc we
> > > have
> > > +	 * existing device trees with big-endian maps and an absent
> > > endianess
> > > +	 * "big-property"
> > > +	 */
> > > +	if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > +	    !of_property_read_bool(dev->of_node, "big-endian"))
> > > +		guts->little_endian = true;
> > 
> > This is not a syscon device (Yangbo's patch to add a guts node on ls2080
> > is
> > the only guts node that says "syscon", and that was a leftover from
> > earlier
> > revisions and should probably be removed).  Even if it were, where is it
> > documented that syscon defaults to little-endian?
> 
> Documentation/devicetree/bindings/regmap/regmap.txt
> 
> We had a little screwup here, basically regmap (and by consequence, syscon)
> always defaulted to little-endian way before that was documented, so it's
> too late to change it, 

What causes a device node to fall under the jurisdiction of regmap.txt? 
 Again, these nodes do not claim "syscon" compatibility.

> although I agree it would have made sense to document
> regmap to default to big-endian on powerpc.

Please don't.  It's enough of a mess as is; no need to start throwing in
architecture ifdefs.

> > Documentation/devicetree/bindings/common-properties.txt says that the
> > individual binding specifies the default.  The default for this node
> > should be
> > big-endian because that's what existed before there was a need to describe
> > the
> > endianness.  And we need an update to the guts binding to specify that.
> 
> Good point. This proably means that specifying both the "guts" and "syscon"
> compatible strings implies having to also specify the endianess explicitly
> both ways, because otherwise we break one of the two bindings.

Yes, but the node should only specify "guts".

> 
> > > +
> > > +	guts->regs = devm_ioremap_resource(dev, 0);
> > > +	if (!guts->regs) {
> > > +		ret = -ENOMEM;
> > > +		kfree(guts);
> > > +		goto out;
> > > +	}
> > > +
> > > +	fsl_guts_init(dev, guts);
> > > +	ret = 0;
> > > +out:
> > > +	return ret;
> > > +}
> > > +
> > > +static struct platform_driver fsl_soc_guts = {
> > > +	.probe = fsl_guts_probe,
> > > +	.driver.of_match_table = fsl_guts_of_match,
> > > +};
> > > +
> > > +module_platform_driver(fsl_soc_guts);
> > 
> > Again, this means that the information is not available during early boot,
> > such as in the clock driver.  Thus we would not be able to convert clk
> > -qoriq's
> > direct mfspr(SPRN_SVR) into an soc_device_match() (or anything else that
> > makes
> > use of this file), nor would we be able to move its access of the guts RCW
> > registers into this driver.
> 
> Correct. Do we have a reason to convert the mfspr() though? I don't really
> see an improvement over the current state if we do that, 

Then should we drop this patchset and put a similar PPC ifdef in
 drivers/mmc/host/sdhci-of-esdhc.c?

There's also the RCW access.  You said in the patch 4/4 discussion that you di
dn't like any random driver ioremapping the registers...

> and for new devices
> that might need the erratum workaround, we could add a DT property that
> would
> be preferred to both.

It's unlikely that we would know the erratum exists at the time the device
tree is created.  We also generally don't have separate device trees for each
revision of a chip (and if we did, we'd have users that use the wrong one).

-Scott

RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Yangbo Lu]

Hi Arnd,

Could you comment on these?
Thanks.


Best regards,
Yangbo Lu


> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Saturday, June 11, 2016 9:51 AM
> To: Arnd Bergmann; linuxppc-dev@lists.ozlabs.org
> Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> i2c@vger.kernel.org; linux-clk@vger.kernel.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicetree@vger.kernel.org;
> Kumar Gala; Rob Herring; Santosh Shilimkar; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org; linux-
> mmc@vger.kernel.org; Xiaobo Xie; Yang-Leo Li; iommu@lists.linux-
> foundation.org; Yangbo Lu
> Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
> 
> On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> > On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c new
> > > > file mode 100644 index 000000000000..2f30698f5bcf
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/guts.c
> > > > @@ -0,0 +1,130 @@
> > > > +/*
> > > > + * 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/platform_device.h> #include <linux/module.h>
> > > > +#include <linux/slab.h> #include <linux/of_address.h> #include
> > > > +<linux/of_platform.h> #include <linux/sys_soc.h>
> > > > +
> > > > +#define GUTS_PVR	0x0a0
> > > > +#define GUTS_SVR	0x0a4
> > > > +
> > > > +struct guts {
> > > > +	void __iomem *regs;
> > >
> > > We already have a struct to define guts.  Why are you not using it?
> > > Why do you consider using it to be "abuse"?  What if we want to move
> > > more guts functionality into this driver?
> >
> > This structure was in the original patch, I left it in there, only
> > removed the inclusion of the powerpc header file, which seemed to be
> > misplaced.
> 
> I'm not refering "struct guts".  I'm referring to changing "struct
> ccsr_guts __iomem *regs" into "void __iomem *regs".
> 
> And it's not a powerpc header file.
> 
> > > > +/*
> > > > + * Table for matching compatible strings, for device tree
> > > > + * guts node, for Freescale QorIQ SOCs.
> > > > + */
> > > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > > +	/* For T4 & B4 Series SOCs */
> > > > +	{ .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > > series" },
> > > [snip]
> > > > +	{ .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > > series"
> > >
> > > As noted in my comment on patch 3/4, these descriptions are reversed.
> > >
> > > They're also incomplete.  t2080 has device config 2.0.  t1040 is
> > > described as
> > > 2.0 though it should probably be 2.1 (or better, drop the generic
> > > compatible altogether).
> >
> > Ok. Ideally I think we'd even look up the specific SoC names from the
> > SVC rather than the compatible string. I just didn't have a good list
> > for those to put in the driver.
> 
> The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why we
> need to convert it to a string in the first place.
> 
> >
> > > > +	/*
> > > > +	 * syscon devices default to little-endian, but on powerpc we
> > > > have
> > > > +	 * existing device trees with big-endian maps and an absent
> > > > endianess
> > > > +	 * "big-property"
> > > > +	 */
> > > > +	if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > > +	    !of_property_read_bool(dev->of_node, "big-endian"))
> > > > +		guts->little_endian = true;
> > >
> > > This is not a syscon device (Yangbo's patch to add a guts node on
> > > ls2080 is the only guts node that says "syscon", and that was a
> > > leftover from earlier revisions and should probably be removed).
> > > Even if it were, where is it documented that syscon defaults to
> > > little-endian?
> >
> > Documentation/devicetree/bindings/regmap/regmap.txt
> >
> > We had a little screwup here, basically regmap (and by consequence,
> > syscon) always defaulted to little-endian way before that was
> > documented, so it's too late to change it,
> 
> What causes a device node to fall under the jurisdiction of regmap.txt?
>  Again, these nodes do not claim "syscon" compatibility.
> 
> > although I agree it would have made sense to document regmap to
> > default to big-endian on powerpc.
> 
> Please don't.  It's enough of a mess as is; no need to start throwing in
> architecture ifdefs.
> 
> > > Documentation/devicetree/bindings/common-properties.txt says that
> > > the individual binding specifies the default.  The default for this
> > > node should be big-endian because that's what existed before there
> > > was a need to describe the endianness.  And we need an update to the
> > > guts binding to specify that.
> >
> > Good point. This proably means that specifying both the "guts" and
> "syscon"
> > compatible strings implies having to also specify the endianess
> > explicitly both ways, because otherwise we break one of the two
> bindings.
> 
> Yes, but the node should only specify "guts".
> 
> >
> > > > +
> > > > +	guts->regs = devm_ioremap_resource(dev, 0);
> > > > +	if (!guts->regs) {
> > > > +		ret = -ENOMEM;
> > > > +		kfree(guts);
> > > > +		goto out;
> > > > +	}
> > > > +
> > > > +	fsl_guts_init(dev, guts);
> > > > +	ret = 0;
> > > > +out:
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static struct platform_driver fsl_soc_guts = {
> > > > +	.probe = fsl_guts_probe,
> > > > +	.driver.of_match_table = fsl_guts_of_match, };
> > > > +
> > > > +module_platform_driver(fsl_soc_guts);
> > >
> > > Again, this means that the information is not available during early
> > > boot, such as in the clock driver.  Thus we would not be able to
> > > convert clk -qoriq's direct mfspr(SPRN_SVR) into an
> > > soc_device_match() (or anything else that makes use of this file),
> > > nor would we be able to move its access of the guts RCW registers
> > > into this driver.
> >
> > Correct. Do we have a reason to convert the mfspr() though? I don't
> > really see an improvement over the current state if we do that,
> 
> Then should we drop this patchset and put a similar PPC ifdef in
> drivers/mmc/host/sdhci-of-esdhc.c?
> 
> There's also the RCW access.  You said in the patch 4/4 discussion that
> you di dn't like any random driver ioremapping the registers...
> 
> > and for new devices
> > that might need the erratum workaround, we could add a DT property
> > that would be preferred to both.
> 
> It's unlikely that we would know the erratum exists at the time the
> device tree is created.  We also generally don't have separate device
> trees for each revision of a chip (and if we did, we'd have users that
> use the wrong one).
> 
> -Scott

RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Yangbo Lu]

Hi Arnd,

Could you reply when you see the email?
If your method doesn’t resolve the problem, we still want to use our old patchset.

This guts driver had been discussed about one year and blocked many workaround upstream.
So please help to review and comment soon.

Thanks a lot.


Best regards,
Yangbo Lu

> -----Original Message-----
> From: Yangbo Lu
> Sent: Thursday, June 23, 2016 10:46 AM
> To: 'Scott Wood'; Arnd Bergmann; linuxppc-dev@lists.ozlabs.org
> Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> i2c@vger.kernel.org; linux-clk@vger.kernel.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicetree@vger.kernel.org;
> Kumar Gala; Rob Herring; Santosh Shilimkar; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org; linux-
> mmc@vger.kernel.org; Xiaobo Xie; Yang-Leo Li; iommu@lists.linux-
> foundation.org
> Subject: RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
> 
> Hi Arnd,
> 
> Could you comment on these?
> Thanks.
> 
> 
> Best regards,
> Yangbo Lu
> 
> 
> > -----Original Message-----
> > From: Scott Wood [mailto:oss@buserror.net]
> > Sent: Saturday, June 11, 2016 9:51 AM
> > To: Arnd Bergmann; linuxppc-dev@lists.ozlabs.org
> > Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> > i2c@vger.kernel.org; linux-clk@vger.kernel.org; Qiang Zhao; Russell
> > King; Bhupesh Sharma; Joerg Roedel; Claudiu Manoil;
> > devicetree@vger.kernel.org; Kumar Gala; Rob Herring; Santosh
> > Shilimkar; linux-arm- kernel@lists.infradead.org;
> > netdev@vger.kernel.org; linux- mmc@vger.kernel.org; Xiaobo Xie;
> > Yang-Leo Li; iommu@lists.linux- foundation.org; Yangbo Lu
> > Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
> >
> > On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> > > On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > > > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c new
> > > > > file mode 100644 index 000000000000..2f30698f5bcf
> > > > > --- /dev/null
> > > > > +++ b/drivers/soc/fsl/guts.c
> > > > > @@ -0,0 +1,130 @@
> > > > > +/*
> > > > > + * 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/platform_device.h> #include <linux/module.h>
> > > > > +#include <linux/slab.h> #include <linux/of_address.h> #include
> > > > > +<linux/of_platform.h> #include <linux/sys_soc.h>
> > > > > +
> > > > > +#define GUTS_PVR	0x0a0
> > > > > +#define GUTS_SVR	0x0a4
> > > > > +
> > > > > +struct guts {
> > > > > +	void __iomem *regs;
> > > >
> > > > We already have a struct to define guts.  Why are you not using it?
> > > > Why do you consider using it to be "abuse"?  What if we want to
> > > > move more guts functionality into this driver?
> > >
> > > This structure was in the original patch, I left it in there, only
> > > removed the inclusion of the powerpc header file, which seemed to be
> > > misplaced.
> >
> > I'm not refering "struct guts".  I'm referring to changing "struct
> > ccsr_guts __iomem *regs" into "void __iomem *regs".
> >
> > And it's not a powerpc header file.
> >
> > > > > +/*
> > > > > + * Table for matching compatible strings, for device tree
> > > > > + * guts node, for Freescale QorIQ SOCs.
> > > > > + */
> > > > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > > > +	/* For T4 & B4 Series SOCs */
> > > > > +	{ .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > > > series" },
> > > > [snip]
> > > > > +	{ .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > > > series"
> > > >
> > > > As noted in my comment on patch 3/4, these descriptions are
> reversed.
> > > >
> > > > They're also incomplete.  t2080 has device config 2.0.  t1040 is
> > > > described as
> > > > 2.0 though it should probably be 2.1 (or better, drop the generic
> > > > compatible altogether).
> > >
> > > Ok. Ideally I think we'd even look up the specific SoC names from
> > > the SVC rather than the compatible string. I just didn't have a good
> > > list for those to put in the driver.
> >
> > The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why
> > we need to convert it to a string in the first place.
> >
> > >
> > > > > +	/*
> > > > > +	 * syscon devices default to little-endian, but on powerpc we
> > > > > have
> > > > > +	 * existing device trees with big-endian maps and an absent
> > > > > endianess
> > > > > +	 * "big-property"
> > > > > +	 */
> > > > > +	if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > > > +	    !of_property_read_bool(dev->of_node, "big-endian"))
> > > > > +		guts->little_endian = true;
> > > >
> > > > This is not a syscon device (Yangbo's patch to add a guts node on
> > > > ls2080 is the only guts node that says "syscon", and that was a
> > > > leftover from earlier revisions and should probably be removed).
> > > > Even if it were, where is it documented that syscon defaults to
> > > > little-endian?
> > >
> > > Documentation/devicetree/bindings/regmap/regmap.txt
> > >
> > > We had a little screwup here, basically regmap (and by consequence,
> > > syscon) always defaulted to little-endian way before that was
> > > documented, so it's too late to change it,
> >
> > What causes a device node to fall under the jurisdiction of regmap.txt?
> >  Again, these nodes do not claim "syscon" compatibility.
> >
> > > although I agree it would have made sense to document regmap to
> > > default to big-endian on powerpc.
> >
> > Please don't.  It's enough of a mess as is; no need to start throwing
> > in architecture ifdefs.
> >
> > > > Documentation/devicetree/bindings/common-properties.txt says that
> > > > the individual binding specifies the default.  The default for
> > > > this node should be big-endian because that's what existed before
> > > > there was a need to describe the endianness.  And we need an
> > > > update to the guts binding to specify that.
> > >
> > > Good point. This proably means that specifying both the "guts" and
> > "syscon"
> > > compatible strings implies having to also specify the endianess
> > > explicitly both ways, because otherwise we break one of the two
> > bindings.
> >
> > Yes, but the node should only specify "guts".
> >
> > >
> > > > > +
> > > > > +	guts->regs = devm_ioremap_resource(dev, 0);
> > > > > +	if (!guts->regs) {
> > > > > +		ret = -ENOMEM;
> > > > > +		kfree(guts);
> > > > > +		goto out;
> > > > > +	}
> > > > > +
> > > > > +	fsl_guts_init(dev, guts);
> > > > > +	ret = 0;
> > > > > +out:
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static struct platform_driver fsl_soc_guts = {
> > > > > +	.probe = fsl_guts_probe,
> > > > > +	.driver.of_match_table = fsl_guts_of_match, };
> > > > > +
> > > > > +module_platform_driver(fsl_soc_guts);
> > > >
> > > > Again, this means that the information is not available during
> > > > early boot, such as in the clock driver.  Thus we would not be
> > > > able to convert clk -qoriq's direct mfspr(SPRN_SVR) into an
> > > > soc_device_match() (or anything else that makes use of this file),
> > > > nor would we be able to move its access of the guts RCW registers
> > > > into this driver.
> > >
> > > Correct. Do we have a reason to convert the mfspr() though? I don't
> > > really see an improvement over the current state if we do that,
> >
> > Then should we drop this patchset and put a similar PPC ifdef in
> > drivers/mmc/host/sdhci-of-esdhc.c?
> >
> > There's also the RCW access.  You said in the patch 4/4 discussion
> > that you di dn't like any random driver ioremapping the registers...
> >
> > > and for new devices
> > > that might need the erratum workaround, we could add a DT property
> > > that would be preferred to both.
> >
> > It's unlikely that we would know the erratum exists at the time the
> > device tree is created.  We also generally don't have separate device
> > trees for each revision of a chip (and if we did, we'd have users that
> > use the wrong one).
> >
> > -Scott

Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Arnd Bergmann]

On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote:
> Hi Arnd,
> 
> Could you reply when you see the email?
> If your method doesn’t resolve the problem, we still want to use our old patchset.
> 
> This guts driver had been discussed about one year and blocked many workaround upstream.
> So please help to review and comment soon.
> 

I don't really see how more discussion is going to help us here. I think
I've made it pretty clear that I don't want to see another platform
specific way to read an SoC revision and I've even sent a proof-of-concept
patch to show how the interface can work, now it's up to you to fit the
guts hardware into that and send a new patch series.

	Arnd

Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Scott Wood]

On Thu, 2016-07-07 at 10:30 +0200, Arnd Bergmann wrote:
> On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote:
> > 
> > Hi Arnd,
> > 
> > Could you reply when you see the email?
> > If your method doesn’t resolve the problem, we still want to use our old
> > patchset.
> > 
> > This guts driver had been discussed about one year and blocked many
> > workaround upstream.
> > So please help to review and comment soon.
> > 
> I don't really see how more discussion is going to help us here. I think
> I've made it pretty clear that I don't want to see another platform
> specific way to read an SoC revision and I've even sent a proof-of-concept
> patch to show how the interface can work, now it's up to you to fit the
> guts hardware into that and send a new patch series.

In which relevant maintainership capacity are you NACKing it?

-Scott

RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms

[Yangbo Lu]

Hi Arnd,


> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Thursday, July 07, 2016 4:30 PM
> To: Yangbo Lu
> Cc: Scott Wood; linuxppc-dev@lists.ozlabs.org; Mark Rutland; Ulf Hansson;
> linux-kernel@vger.kernel.org; linux-i2c@vger.kernel.org; linux-
> clk@vger.kernel.org; Qiang Zhao; Russell King; Bhupesh Sharma; Joerg
> Roedel; Claudiu Manoil; devicetree@vger.kernel.org; Kumar Gala; Rob
> Herring; Santosh Shilimkar; linux-arm-kernel@lists.infradead.org;
> netdev@vger.kernel.org; linux-mmc@vger.kernel.org; Xiaobo Xie; Yang-Leo
> Li; iommu@lists.linux-foundation.org
> Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
> 
> On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote:
> > Hi Arnd,
> >
> > Could you reply when you see the email?
> > If your method doesn’t resolve the problem, we still want to use our
> old patchset.
> >
> > This guts driver had been discussed about one year and blocked many
> workaround upstream.
> > So please help to review and comment soon.
> >
> 
> I don't really see how more discussion is going to help us here. I think
> I've made it pretty clear that I don't want to see another platform
> specific way to read an SoC revision and I've even sent a proof-of-
> concept patch to show how the interface can work, now it's up to you to
> fit the guts hardware into that and send a new patch series.
> 
> 	Arnd

I think your proof-of-concept patch is still in discussion. Some answers are needed from you to
address Scott's comments on your patchset. Have you reached an agreement?

Thanks.

- Yangbo Lu

[PATCH 3/4] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Arnd Bergmann]

This is a rewrite of an earlier patch from Yangbo Lu, adding a quirk
for the NXP QorIQ T4240 in the detection of the host device version.

Unfortunately, this device cannot be detected using the compatible
string, as we have to support existing DTS files that use the generic
"fsl,t4240-esdhc" identifier but that have other host versions that
are correctly detected.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 3f34d354f1fc..1d4814fe4cb2 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -73,14 +73,16 @@ 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;
 
 	if (spec_reg == SDHCI_HOST_VERSION)
-		ret = value & 0xffff;
-	else
-		ret = (value >> shift) & 0xffff;
-	return ret;
+		return esdhc->vendor_ver << SDHCI_VENDOR_VER_SHIFT |
+		       esdhc->spec_ver;
+
+	return (value >> shift) & 0xffff;
 }
 
 static u8 esdhc_readb_fixup(struct sdhci_host *host,
@@ -562,16 +564,32 @@ static const struct sdhci_pltfm_data sdhci_esdhc_le_pdata = {
 	.ops = &sdhci_esdhc_le_ops,
 };
 
+#define T4240_HOST_VER ((VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) | SDHCI_SPEC_200)
+static const struct soc_device_attribute esdhc_t4240_quirk = {
+	/* T4240 revision < 0x20 uses vendor version 23, SDHCI version 200 */
+	{ .soc_id = "T4*(0x824000)", .revision = "0x[01]?",
+	  .data = (void *)(uintptr_t)(T4240_HOST_VER) },
+	{ },
+};
+
 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;
 
 	pltfm_host = sdhci_priv(host);
 	esdhc = sdhci_pltfm_priv(pltfm_host);
 
 	host_ver = sdhci_readw(host, SDHCI_HOST_VERSION);
+
+	if (of_device_is_compatible(pdev->dev.of_node, "fsl,t4240-esdhc")) {
+		struct soc_device_attribute *match;
+
+		match = soc_device_match(&esdhc_t4240_quirk);
+		if (match)
+			host_ver = (uintptr_t)match->data;
+	}
+
 	esdhc->vendor_ver = (host_ver & SDHCI_VENDOR_VER_MASK) >>
 			     SDHCI_VENDOR_VER_SHIFT;
 	esdhc->spec_ver = host_ver & SDHCI_SPEC_VER_MASK;

Re: [PATCH 3/4] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Scott Wood]

On Mon, 2016-05-30 at 15:16 +0200, Arnd Bergmann wrote:
> This is a rewrite of an earlier patch from Yangbo Lu, adding a quirk
> for the NXP QorIQ T4240 in the detection of the host device version.
> 
> Unfortunately, this device cannot be detected using the compatible
> string, as we have to support existing DTS files that use the generic
> "fsl,t4240-esdhc" identifier but that have other host versions that
> are correctly detected.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of
> -esdhc.c
> index 3f34d354f1fc..1d4814fe4cb2 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -73,14 +73,16 @@ 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;
>  
>  	if (spec_reg == SDHCI_HOST_VERSION)
> -		ret = value & 0xffff;
> -	else
> -		ret = (value >> shift) & 0xffff;
> -	return ret;
> +		return esdhc->vendor_ver << SDHCI_VENDOR_VER_SHIFT |
> +		       esdhc->spec_ver;
> +
> +	return (value >> shift) & 0xffff;
>  }
>  
>  static u8 esdhc_readb_fixup(struct sdhci_host *host,
> @@ -562,16 +564,32 @@ static const struct sdhci_pltfm_data
> sdhci_esdhc_le_pdata = {
>  	.ops = &sdhci_esdhc_le_ops,
>  };
>  
> +#define T4240_HOST_VER ((VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) |
> SDHCI_SPEC_200)
> +static const struct soc_device_attribute esdhc_t4240_quirk = {
> +	/* T4240 revision < 0x20 uses vendor version 23, SDHCI version 200
> */
> +	{ .soc_id = "T4*(0x824000)", .revision = "0x[01]?",
> +	  .data = (void *)(uintptr_t)(T4240_HOST_VER) },

Why should this code need to care that the string begins with "T4"?  This
creates dual maintenance if that were to change.  It's also broken because
T4240 has compatible = "fsl,t4240-device-config", "fsl,qoriq-device-config
-2.0" and thus with these patches it would incorrectly show up as "P series
(0x824000)".  The compatible string of this node was never meant to be a key
for choosing a string to describe the system to userspace.

0x824000 is a magic number which should be represented symbolically.

If T4240 is affected, then so are the reduced-core variants T4160 and T4080,
but 0x824000 doesn't match them (Yangbo's patch had the same problem).  And
please don't respond with "0x824*"

You also didn't strip out the E bit of SVR which indicates encryption
capability and nothing else (Yangbo's patch did not have this problem because
it used SVR_SOC_VER).

What happens if the revision condition is more complicated, such as <= 0x20
with 0x21 being fine?  Multiple quirk entries where before we had as simple
comparison?

I fail to see how this approach is an improvement (much less one that needs to
hold up a patchset that is fixing a problem and is not touching any generic
code).  Why does this need to be a string?

-Scott

Re: [PATCH 3/4] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Arnd Bergmann]

On Wednesday, June 1, 2016 8:11:14 PM CEST Scott Wood wrote:
> > +#define T4240_HOST_VER ((VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) |
> > SDHCI_SPEC_200)
> > +static const struct soc_device_attribute esdhc_t4240_quirk = {
> > +     /* T4240 revision < 0x20 uses vendor version 23, SDHCI version 200
> > */
> > +     { .soc_id = "T4*(0x824000)", .revision = "0x[01]?",
> > +       .data = (void *)(uintptr_t)(T4240_HOST_VER) },
> 
> Why should this code need to care that the string begins with "T4"?  This
> creates dual maintenance if that were to change.  It's also broken because
> T4240 has compatible = "fsl,t4240-device-config", "fsl,qoriq-device-config
> -2.0" and thus with these patches it would incorrectly show up as "P series
> (0x824000)".  The compatible string of this node was never meant to be a key
> for choosing a string to describe the system to userspace.

This is an artifact of not knowing the specific SoC name, and we can change
that by looking up the name from the SVR value in the soc_device driver.
 
> 0x824000 is a magic number which should be represented symbolically.

Sure, feel free to change the format of the soc_device string in any
name, I just converted the information I had available in the comments.
The only thing that is important here is that the string matches what
the soc_device driver calls it.

> If T4240 is affected, then so are the reduced-core variants T4160 and T4080,
> but 0x824000 doesn't match them (Yangbo's patch had the same problem).  And
> please don't respond with "0x824*"
> 
> You also didn't strip out the E bit of SVR which indicates encryption
> capability and nothing else (Yangbo's patch did not have this problem because
> it used SVR_SOC_VER).

Ok, that should be easy enough to fix in the soc_device driver.

> What happens if the revision condition is more complicated, such as <= 0x20
> with 0x21 being fine?  Multiple quirk entries where before we had as simple
> comparison?

I guess yes. I would really hope that there is no need to use this interface
pervasively, it's really just to work around the cases where there is no
way to pass the information in DT otherwise.

> I fail to see how this approach is an improvement (much less one that needs to
> hold up a patchset that is fixing a problem and is not touching any generic
> code).  Why does this need to be a string?

A string is what user space gets in /sys/devices/soc/*, and we already have
code that does the same things there to work around quirks, here we just
use the same interface in a completely generic way. Note that not every
SoC family uses numbers in the same way, some have multiple subrevisions,
some have names etc.

	Arnd

Re: [PATCH 3/4] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0

[Scott Wood]

On Thu, 2016-06-02 at 10:52 +0200, Arnd Bergmann wrote:
> On Wednesday, June 1, 2016 8:11:14 PM CEST Scott Wood wrote:
> > > +#define T4240_HOST_VER ((VENDOR_V_23 << SDHCI_VENDOR_VER_SHIFT) |
> > > SDHCI_SPEC_200)
> > > +static const struct soc_device_attribute esdhc_t4240_quirk = {
> > > +     /* T4240 revision < 0x20 uses vendor version 23, SDHCI version 200
> > > */
> > > +     { .soc_id = "T4*(0x824000)", .revision = "0x[01]?",
> > > +       .data = (void *)(uintptr_t)(T4240_HOST_VER) },
> > 
> > Why should this code need to care that the string begins with "T4"?  This
> > creates dual maintenance if that were to change.  It's also broken because
> > T4240 has compatible = "fsl,t4240-device-config", "fsl,qoriq-device-config
> > -2.0" and thus with these patches it would incorrectly show up as "P
> > series
> > (0x824000)".  The compatible string of this node was never meant to be a
> > key
> > for choosing a string to describe the system to userspace.
> 
> This is an artifact of not knowing the specific SoC name, and we can change
> that by looking up the name from the SVR value in the soc_device driver.

...or we could keep it simple and just match the number.

> > 0x824000 is a magic number which should be represented symbolically.
> 
> Sure, feel free to change the format of the soc_device string in any
> name,

That's not what I was asking for...  The match should be numeric but the
knowledge of what the number is should come from a symbolic #define.

> > If T4240 is affected, then so are the reduced-core variants T4160 and
> > T4080,
> > but 0x824000 doesn't match them (Yangbo's patch had the same problem). 
> >  And
> > please don't respond with "0x824*"
> > 
> > You also didn't strip out the E bit of SVR which indicates encryption
> > capability and nothing else (Yangbo's patch did not have this problem
> > because
> > it used SVR_SOC_VER).
> 
> Ok, that should be easy enough to fix in the soc_device driver.

No, because the soc_device driver doesn't know whether the consumer of the ID
cares about the E bit.

> > What happens if the revision condition is more complicated, such as <=
> > 0x20
> > with 0x21 being fine?  Multiple quirk entries where before we had as
> > simple
> > comparison?
> 
> I guess yes. I would really hope that there is no need to use this interface
> pervasively, it's really just to work around the cases where there is no
> way to pass the information in DT otherwise.

How does putting it in the DT work when you have multiple versions of the same
SoC, some of which have the bug and some which don't?

> > I fail to see how this approach is an improvement (much less one that
> > needs to
> > hold up a patchset that is fixing a problem and is not touching any
> > generic
> > code).  Why does this need to be a string?
> 
> A string is what user space gets in /sys/devices/soc/*,

It is rare that the kernel accesses information in the exact same way that
userspace does.  And once we expose this to userspace we're stuck with it, so
exporting anything other than a simple number is even less desirable.

>  and we already have
> code that does the same things there to work around quirks, here we just
> use the same interface in a completely generic way. Note that not every
> SoC family uses numbers in the same way, some have multiple subrevisions,
> some have names etc.

Where is the need for a "completely generic way" for one piece of vendor
-specific code to get information that is inherently specific to that vendor,
that is supplied by code specific to that vendor?

-Scott

[PATCH 4/4] Revert "powerpc/fsl: Move fsl_guts.h out of arch/powerpc"

[Arnd Bergmann]

All users of this driver are PowerPC specific and the header file
has no business in the global include/linux/ hierarchy, so move
it back before anyone starts using it on ARM.

This reverts commit 948486544713492f00ac8a9572909101ea892cb0.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This part of the series is not required for the eSDHC quirk,
but it restores the asm/fsl_guts.h header so it doesn't accidentally
get abused for this in the future. I found two drivers outside of
arch/powerpc that already accessed the registers directly, but the
functions look fairly contained, and can be easily hidden in an
#ifdef CONFIG_PPC

diff --git a/include/linux/fsl/guts.h b/arch/powerpc/include/asm/fsl_guts.h
similarity index 99%
rename from include/linux/fsl/guts.h
rename to arch/powerpc/include/asm/fsl_guts.h
index 649e9171a9b3..a67413c52701 100644
--- a/include/linux/fsl/guts.h
+++ b/arch/powerpc/include/asm/fsl_guts.h
@@ -12,10 +12,9 @@
  * option) any later version.
  */
 
-#ifndef __FSL_GUTS_H__
-#define __FSL_GUTS_H__
-
-#include <linux/types.h>
+#ifndef __ASM_POWERPC_FSL_GUTS_H__
+#define __ASM_POWERPC_FSL_GUTS_H__
+#ifdef __KERNEL__
 
 /**
  * Global Utility Registers.
@@ -295,3 +294,4 @@ struct ccsr_rcpm_v2 {
 };
 
 #endif
+#endif
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index f61cbe235581..00f052e9f2a2 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -34,7 +34,6 @@
 #include <linux/of_device.h>
 #include <linux/phy.h>
 #include <linux/memblock.h>
-#include <linux/fsl/guts.h>
 
 #include <linux/atomic.h>
 #include <asm/time.h>
@@ -52,6 +51,7 @@
 #include <soc/fsl/qe/qe_ic.h>
 #include <asm/mpic.h>
 #include <asm/swiotlb.h>
+#include <asm/fsl_guts.h>
 #include "smp.h"
 
 #include "mpc85xx.h"
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c
index f05325f0cc03..a812c0511252 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c
@@ -14,8 +14,8 @@
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
-#include <linux/fsl/guts.h>
 
+#include <asm/fsl_guts.h>
 #include <asm/io.h>
 #include <asm/fsl_pm.h>
 
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
index 3f4dad133338..453ddda00fce 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
@@ -17,7 +17,6 @@
 #include <linux/seq_file.h>
 #include <linux/interrupt.h>
 #include <linux/of_platform.h>
-#include <linux/fsl/guts.h>
 
 #include <asm/time.h>
 #include <asm/machdep.h>
@@ -28,6 +27,7 @@
 #include <asm/mpic.h>
 #include <soc/fsl/qe/qe.h>
 #include <soc/fsl/qe/qe_ic.h>
+#include <asm/fsl_guts.h>
 
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
index 371df822e88e..6ac986d3f8a3 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -16,7 +16,6 @@
  * kind, whether express or implied.
  */
 
-#include <linux/fsl/guts.h>
 #include <linux/pci.h>
 #include <linux/of_platform.h>
 #include <asm/div64.h>
@@ -26,6 +25,7 @@
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 #include <asm/udbg.h>
+#include <asm/fsl_guts.h>
 #include <asm/fsl_lbc.h>
 #include "smp.h"
 
diff --git a/arch/powerpc/platforms/85xx/p1022_rdk.c b/arch/powerpc/platforms/85xx/p1022_rdk.c
index 5087becaa8bc..680232d6ba48 100644
--- a/arch/powerpc/platforms/85xx/p1022_rdk.c
+++ b/arch/powerpc/platforms/85xx/p1022_rdk.c
@@ -12,7 +12,6 @@
  * kind, whether express or implied.
  */
 
-#include <linux/fsl/guts.h>
 #include <linux/pci.h>
 #include <linux/of_platform.h>
 #include <asm/div64.h>
@@ -22,6 +21,7 @@
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 #include <asm/udbg.h>
+#include <asm/fsl_guts.h>
 #include "smp.h"
 
 #include "mpc85xx.h"
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index fe9f19e5e935..6bd3a292e790 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -18,7 +18,6 @@
 #include <linux/kexec.h>
 #include <linux/highmem.h>
 #include <linux/cpu.h>
-#include <linux/fsl/guts.h>
 
 #include <asm/machdep.h>
 #include <asm/pgtable.h>
@@ -26,6 +25,7 @@
 #include <asm/mpic.h>
 #include <asm/cacheflush.h>
 #include <asm/dbell.h>
+#include <asm/fsl_guts.h>
 #include <asm/code-patching.h>
 #include <asm/cputhreads.h>
 #include <asm/fsl_pm.h>
diff --git a/arch/powerpc/platforms/85xx/twr_p102x.c b/arch/powerpc/platforms/85xx/twr_p102x.c
index 71bc255b4324..2cac45f72d24 100644
--- a/arch/powerpc/platforms/85xx/twr_p102x.c
+++ b/arch/powerpc/platforms/85xx/twr_p102x.c
@@ -15,7 +15,6 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/errno.h>
-#include <linux/fsl/guts.h>
 #include <linux/pci.h>
 #include <linux/of_platform.h>
 
@@ -24,6 +23,7 @@
 #include <asm/mpic.h>
 #include <soc/fsl/qe/qe.h>
 #include <soc/fsl/qe/qe_ic.h>
+#include <asm/fsl_guts.h>
 
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 957473e5c8e5..761c81476957 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -24,7 +24,6 @@
 #include <linux/delay.h>
 #include <linux/seq_file.h>
 #include <linux/of.h>
-#include <linux/fsl/guts.h>
 
 #include <asm/time.h>
 #include <asm/machdep.h>
@@ -39,6 +38,7 @@
 #include <sysdev/fsl_pci.h>
 #include <sysdev/fsl_soc.h>
 #include <sysdev/simple_gpio.h>
+#include <asm/fsl_guts.h>
 
 #include "mpc86xx.h"
 
diff --git a/arch/powerpc/sysdev/fsl_rcpm.c b/arch/powerpc/sysdev/fsl_rcpm.c
index 9259a94f70e1..8af22187cb25 100644
--- a/arch/powerpc/sysdev/fsl_rcpm.c
+++ b/arch/powerpc/sysdev/fsl_rcpm.c
@@ -19,7 +19,7 @@
 #include <linux/export.h>
 
 #include <asm/io.h>
-#include <linux/fsl/guts.h>
+#include <asm/fsl_guts.h>
 #include <asm/cputhreads.h>
 #include <asm/fsl_pm.h>
 #include <asm/smp.h>
diff --git a/drivers/clk/clk-qoriq.c b/drivers/clk/clk-qoriq.c
index 58566a17944a..f311bd399672 100644
--- a/drivers/clk/clk-qoriq.c
+++ b/drivers/clk/clk-qoriq.c
@@ -12,7 +12,6 @@
 
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
-#include <linux/fsl/guts.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -21,6 +20,10 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 
+#ifdef CONFIG_PPC
+#include <asm/fsl_guts.h>
+#endif
+
 #define PLL_DIV1	0
 #define PLL_DIV2	1
 #define PLL_DIV3	2
@@ -341,6 +344,8 @@ static const struct clockgen_muxinfo t4240_hwa5 = {
 	},
 };
 
+#ifdef CONFIG_PPC
+
 #define RCWSR7_FM1_CLK_SEL	0x40000000
 #define RCWSR7_FM2_CLK_SEL	0x20000000
 #define RCWSR7_HWA_ASYNC_DIV	0x04000000
@@ -408,6 +413,7 @@ static void __init p5040_init_periph(struct clockgen *cg)
 	else
 		cg->fman[1] = cg->pll[PLATFORM_PLL].div[PLL_DIV2].clk;
 }
+#endif
 
 static void __init t1023_init_periph(struct clockgen *cg)
 {
@@ -499,6 +505,7 @@ static const struct clockgen_chipinfo chipinfo[] = {
 		.pll_mask = 0x37,
 		.flags = CG_VER3 | CG_LITTLE_ENDIAN,
 	},
+#ifdef CONFIG_PPC
 	{
 		.compat = "fsl,p2041-clockgen",
 		.guts_compat = "fsl,qoriq-device-config-1.0",
@@ -559,6 +566,7 @@ static const struct clockgen_chipinfo chipinfo[] = {
 		},
 		.pll_mask = 0x0f,
 	},
+#endif
 	{
 		.compat = "fsl,t1023-clockgen",
 		.guts_compat = "fsl,t1023-device-config",
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index a34355fca37a..2570f2a25dc4 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -20,11 +20,11 @@
 
 #include "fsl_pamu.h"
 
-#include <linux/fsl/guts.h>
 #include <linux/interrupt.h>
 #include <linux/genalloc.h>
 
 #include <asm/mpc85xx.h>
+#include <asm/fsl_guts.h>
 
 /* define indexes for each operation mapping scenario */
 #define OMI_QMAN        0x00
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 1de2e1e51c2b..dabee93fcadd 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -35,7 +35,6 @@
 #include "fman.h"
 #include "fman_muram.h"
 
-#include <linux/fsl/guts.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/module.h>
@@ -46,6 +45,10 @@
 #include <linux/interrupt.h>
 #include <linux/libfdt_env.h>
 
+#ifdef CONFIG_PPC
+#include <asm/fsl_guts.h>
+#endif
+
 /* General defines */
 #define FMAN_LIODN_TBL			64	/* size of LIODN table */
 #define MAX_NUM_OF_MACS			10
@@ -1889,7 +1892,9 @@ static int fman_reset(struct fman *fman)
 			err = -EBUSY;
 
 		goto _return;
-	} else {
+	}
+#ifdef CONFIG_PPC
+	else {
 		struct device_node *guts_node;
 		struct ccsr_guts __iomem *guts_regs;
 		u32 devdisr2, reg;
@@ -1952,6 +1957,7 @@ guts_node:
 		dev_dbg(fman->dev, "%s: Didn't perform FManV3 reset due to Errata A007273!\n",
 			__func__);
 	}
+#endif
 _return:
 	return err;
 }
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index ddf49f30b23f..73b2b89d4a3e 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -12,11 +12,11 @@
 
 #include <linux/module.h>
 #include <linux/interrupt.h>
-#include <linux/fsl/guts.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
 #include <sound/soc.h>
+#include <asm/fsl_guts.h>
 
 #include "fsl_dma.h"
 #include "fsl_ssi.h"
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index a1f780ecadf5..a24e04919f71 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -11,12 +11,12 @@
  */
 
 #include <linux/module.h>
-#include <linux/fsl/guts.h>
 #include <linux/interrupt.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
 #include <sound/soc.h>
+#include <asm/fsl_guts.h>
 
 #include "fsl_dma.h"
 #include "fsl_ssi.h"
diff --git a/sound/soc/fsl/p1022_rdk.c b/sound/soc/fsl/p1022_rdk.c
index d4d88a8cb9c0..d8448887bfda 100644
--- a/sound/soc/fsl/p1022_rdk.c
+++ b/sound/soc/fsl/p1022_rdk.c
@@ -18,12 +18,12 @@
  */
 
 #include <linux/module.h>
-#include <linux/fsl/guts.h>
 #include <linux/interrupt.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/slab.h>
 #include <sound/soc.h>
+#include <asm/fsl_guts.h>
 
 #include "fsl_dma.h"
 #include "fsl_ssi.h"

Re: [PATCH 4/4] Revert "powerpc/fsl: Move fsl_guts.h out of arch/powerpc"

[Scott Wood]

On Mon, 2016-05-30 at 15:18 +0200, Arnd Bergmann wrote:
> All users of this driver are PowerPC specific and the header file
> has no business in the global include/linux/ hierarchy, so move
> it back before anyone starts using it on ARM.
> 
> This reverts commit 948486544713492f00ac8a9572909101ea892cb0.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This part of the series is not required for the eSDHC quirk,
> but it restores the asm/fsl_guts.h header so it doesn't accidentally
> get abused for this in the future. I found two drivers outside of
> arch/powerpc that already accessed the registers directly, but the
> functions look fairly contained, and can be easily hidden in an
> #ifdef CONFIG_PPC

NACK

Besides adding ifdef pollution for no good reason, this register block is used
on some ARM chips as well.  Why is it a problem if "anyone starts using it on
ARM"?

BTW, of all the mailing lists you included on this CC, you seem to have left
off the PPC list (I've added it).

-Scott

Re: [PATCH 4/4] Revert "powerpc/fsl: Move fsl_guts.h out of arch/powerpc"

[Arnd Bergmann]

On Wednesday, June 1, 2016 8:24:20 PM CEST Scott Wood wrote:
> On Mon, 2016-05-30 at 15:18 +0200, Arnd Bergmann wrote:
> > All users of this driver are PowerPC specific and the header file
> > has no business in the global include/linux/ hierarchy, so move
> > it back before anyone starts using it on ARM.
> > 
> > This reverts commit 948486544713492f00ac8a9572909101ea892cb0.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This part of the series is not required for the eSDHC quirk,
> > but it restores the asm/fsl_guts.h header so it doesn't accidentally
> > get abused for this in the future. I found two drivers outside of
> > arch/powerpc that already accessed the registers directly, but the
> > functions look fairly contained, and can be easily hidden in an
> > #ifdef CONFIG_PPC
> 
> NACK
> 
> Besides adding ifdef pollution for no good reason, this register block is used
> on some ARM chips as well.  Why is it a problem if "anyone starts using it on
> ARM"?

It's just not a good interface when it's defined as "this is the layout of
a register area that any driver can ioremap() if they can figure out the
device node". It's not uncommon to have register areas like that, but
normally you have at the minimum a 'syscon' device to handle locking
between drivers accessing the same registers and to avoid having to map
the same area multiple times.

If we need to use 'guts' registers on ARM, we can find a way to abstract
them properly for the given use cases, using a syscon or a driver with
exported functions, but just making a PowerPC platform specific header
global to all Linux drivers by putting it into include/linux doesn't seem
right.

Note that the header file uses a structure definition rather than the more
common macros with register offsets, which is fine for a driver that has
its own registers and abstracts them, but it doesn't really work with
the regmap interface, so if we want to use it with syscon, it also needs to
be rewritten.

> BTW, of all the mailing lists you included on this CC, you seem to have left
> off the PPC list (I've added it).

Sorry, my mistake.

	Arnd

Re: [PATCH 4/4] Revert "powerpc/fsl: Move fsl_guts.h out of arch/powerpc"

[Scott Wood]

On Thu, 2016-06-02 at 11:01 +0200, Arnd Bergmann wrote:
> On Wednesday, June 1, 2016 8:24:20 PM CEST Scott Wood wrote:
> > On Mon, 2016-05-30 at 15:18 +0200, Arnd Bergmann wrote:
> > > All users of this driver are PowerPC specific and the header file
> > > has no business in the global include/linux/ hierarchy, so move
> > > it back before anyone starts using it on ARM.
> > > 
> > > This reverts commit 948486544713492f00ac8a9572909101ea892cb0.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > This part of the series is not required for the eSDHC quirk,
> > > but it restores the asm/fsl_guts.h header so it doesn't accidentally
> > > get abused for this in the future. I found two drivers outside of
> > > arch/powerpc that already accessed the registers directly, but the
> > > functions look fairly contained, and can be easily hidden in an
> > > #ifdef CONFIG_PPC
> > 
> > NACK
> > 
> > Besides adding ifdef pollution for no good reason, this register block is
> > used
> > on some ARM chips as well.  Why is it a problem if "anyone starts using it
> > on
> > ARM"?
> 
> It's just not a good interface when it's defined as "this is the layout of
> a register area that any driver can ioremap() if they can figure out the
> device node".

That's why I want to move accesses into one guts driver.

>  It's not uncommon to have register areas like that, but
> normally you have at the minimum a 'syscon' device to handle locking
> between drivers accessing the same registers and to avoid having to map
> the same area multiple times.

syscon requires device tree changes.

I don't see read-modify-write operations in regmap -- how does locking around
an individual, inherently-atomic load or store help?

> If we need to use 'guts' registers on ARM, we can find a way to abstract
> them properly for the given use cases, using a syscon or a driver with
> exported functions, but just making a PowerPC platform specific header
> global to all Linux drivers by putting it into include/linux doesn't seem
> right.

Again, it's not PowerPC-specific!  It started that way but then the same
register block got put onto some ARM chips.

It's not global to "all Linux drivers", just the ones that choose to include
an fsl-specific header.

If and when all uses of guts are moved into the guts driver, the header can be
moved into drivers/soc/fsl.

> Note that the header file uses a structure definition rather than the more
> common macros with register offsets, which is fine for a driver that has
> its own registers and abstracts them, but it doesn't really work with
> the regmap interface, so if we want to use it with syscon, it also needs to
> be rewritten.

We don't want to use it with syscon.  If we did, the solution wouldn't be to
move the header back to arch/powerpc, but to convert the struct into offsets.

-Scott