Devicetree
 help / color / mirror / Atom feed
* [PATCH v3 2/6] clk: ti: dra7: Add clkctrl clock data for the mcan clocks
From: Faiz Abbas @ 2018-06-06  6:08 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-omap, linux-arm-kernel, linux-clk
  Cc: robh+dt, bcousson, tony, paul, t-kristo, faiz_abbas, mark.rutland
In-Reply-To: <20180606060826.14671-1-faiz_abbas@ti.com>

Add clkctrl data for the m_can clocks and register it within the
clkctrl driver

CC: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 drivers/clk/ti/clk-7xx.c         | 1 +
 include/dt-bindings/clock/dra7.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index fb249a1637a5..71a122b2dc67 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -708,6 +708,7 @@ static const struct omap_clkctrl_reg_data dra7_wkupaon_clkctrl_regs[] __initcons
 	{ DRA7_COUNTER_32K_CLKCTRL, NULL, 0, "wkupaon_iclk_mux" },
 	{ DRA7_UART10_CLKCTRL, dra7_uart10_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0060:24" },
 	{ DRA7_DCAN1_CLKCTRL, dra7_dcan1_bit_data, CLKF_SW_SUP, "wkupaon_cm:clk:0068:24" },
+	{ DRA7_ADC_CLKCTRL, NULL, CLKF_SW_SUP, "mcan_clk"},
 	{ 0 },
 };
 
diff --git a/include/dt-bindings/clock/dra7.h b/include/dt-bindings/clock/dra7.h
index 5e1061b15aed..d7549c57cac3 100644
--- a/include/dt-bindings/clock/dra7.h
+++ b/include/dt-bindings/clock/dra7.h
@@ -168,5 +168,6 @@
 #define DRA7_COUNTER_32K_CLKCTRL	DRA7_CLKCTRL_INDEX(0x50)
 #define DRA7_UART10_CLKCTRL	DRA7_CLKCTRL_INDEX(0x80)
 #define DRA7_DCAN1_CLKCTRL	DRA7_CLKCTRL_INDEX(0x88)
+#define DRA7_ADC_CLKCTRL	DRA7_CLKCTRL_INDEX(0xa0)
 
 #endif
-- 
2.17.0

^ permalink raw reply related

* [PATCH v3 3/6] bus: ti-sysc: Add support for using ti-sysc for MCAN on dra76x
From: Faiz Abbas @ 2018-06-06  6:08 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-omap, linux-arm-kernel, linux-clk
  Cc: robh+dt, bcousson, tony, paul, t-kristo, faiz_abbas, mark.rutland
In-Reply-To: <20180606060826.14671-1-faiz_abbas@ti.com>

The dra76x MCAN generic interconnect module has a its own
format for the bits in the control registers.

Therefore add a new module type, new regbits and new capabilities
specific to the MCAN module.

Acked-by: Rob Herring <robh@kernel.org>
CC: Tony Lindgren <tony@atomide.com>
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 .../devicetree/bindings/bus/ti-sysc.txt        |  1 +
 drivers/bus/ti-sysc.c                          | 18 ++++++++++++++++++
 include/dt-bindings/bus/ti-sysc.h              |  2 ++
 include/linux/platform_data/ti-sysc.h          |  1 +
 4 files changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/ti-sysc.txt b/Documentation/devicetree/bindings/bus/ti-sysc.txt
index 2957a9ae291f..ebbb11144b7b 100644
--- a/Documentation/devicetree/bindings/bus/ti-sysc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-sysc.txt
@@ -36,6 +36,7 @@ Required standard properties:
 		"ti,sysc-omap-aes"
 		"ti,sysc-mcasp"
 		"ti,sysc-usb-host-fs"
+		"ti,sysc-dra7-mcan"
 
 - reg		shall have register areas implemented for the interconnect
 		target module in question such as revision, sysc and syss
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 7cd2fd04b212..4a2244419b9b 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1262,6 +1262,23 @@ static const struct sysc_capabilities sysc_omap4_usb_host_fs = {
 	.regbits = &sysc_regbits_omap4_usb_host_fs,
 };
 
+static const struct sysc_regbits sysc_regbits_dra7_mcan = {
+	.dmadisable_shift = -ENODEV,
+	.midle_shift = -ENODEV,
+	.sidle_shift = -ENODEV,
+	.clkact_shift = -ENODEV,
+	.enwkup_shift = 4,
+	.srst_shift = 0,
+	.emufree_shift = -ENODEV,
+	.autoidle_shift = -ENODEV,
+};
+
+static const struct sysc_capabilities sysc_dra7_mcan = {
+	.type = TI_SYSC_DRA7_MCAN,
+	.sysc_mask = SYSC_DRA7_MCAN_ENAWAKEUP | SYSC_OMAP4_SOFTRESET,
+	.regbits = &sysc_regbits_dra7_mcan,
+};
+
 static int sysc_init_pdata(struct sysc *ddata)
 {
 	struct ti_sysc_platform_data *pdata = dev_get_platdata(ddata->dev);
@@ -1441,6 +1458,7 @@ static const struct of_device_id sysc_match[] = {
 	{ .compatible = "ti,sysc-mcasp", .data = &sysc_omap4_mcasp, },
 	{ .compatible = "ti,sysc-usb-host-fs",
 	  .data = &sysc_omap4_usb_host_fs, },
+	{ .compatible = "ti,sysc-dra7-mcan", .data = &sysc_dra7_mcan, },
 	{  },
 };
 MODULE_DEVICE_TABLE(of, sysc_match);
diff --git a/include/dt-bindings/bus/ti-sysc.h b/include/dt-bindings/bus/ti-sysc.h
index 2c005376ac0e..7138384e2ef9 100644
--- a/include/dt-bindings/bus/ti-sysc.h
+++ b/include/dt-bindings/bus/ti-sysc.h
@@ -15,6 +15,8 @@
 /* SmartReflex sysc found on 36xx and later */
 #define SYSC_OMAP3_SR_ENAWAKEUP		(1 << 26)
 
+#define SYSC_DRA7_MCAN_ENAWAKEUP	(1 << 4)
+
 /* SYSCONFIG STANDBYMODE/MIDLEMODE/SIDLEMODE supported by hardware */
 #define SYSC_IDLE_FORCE			0
 #define SYSC_IDLE_NO			1
diff --git a/include/linux/platform_data/ti-sysc.h b/include/linux/platform_data/ti-sysc.h
index 80ce28d40832..1ea3aab972b4 100644
--- a/include/linux/platform_data/ti-sysc.h
+++ b/include/linux/platform_data/ti-sysc.h
@@ -14,6 +14,7 @@ enum ti_sysc_module_type {
 	TI_SYSC_OMAP4_SR,
 	TI_SYSC_OMAP4_MCASP,
 	TI_SYSC_OMAP4_USB_HOST_FS,
+	TI_SYSC_DRA7_MCAN,
 };
 
 struct ti_sysc_cookie {
-- 
2.17.0

^ permalink raw reply related

* [PATCH v3 4/6] bus: ti-sysc: Add support for software reset
From: Faiz Abbas @ 2018-06-06  6:08 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-omap, linux-arm-kernel, linux-clk
  Cc: robh+dt, bcousson, tony, paul, t-kristo, faiz_abbas, mark.rutland
In-Reply-To: <20180606060826.14671-1-faiz_abbas@ti.com>

Add support for the software reset of a target interconnect
module using its sysconfig and sysstatus registers.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 drivers/bus/ti-sysc.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 4a2244419b9b..74d716a7bd6e 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -22,11 +22,14 @@
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <linux/slab.h>
+#include <linux/iopoll.h>
 
 #include <linux/platform_data/ti-sysc.h>
 
 #include <dt-bindings/bus/ti-sysc.h>
 
+#define MAX_MODULE_SOFTRESET_WAIT		10000
+
 static const char * const reg_names[] = { "rev", "sysc", "syss", };
 
 enum sysc_clocks {
@@ -74,6 +77,11 @@ struct sysc {
 	struct delayed_work idle_work;
 };
 
+void sysc_write(struct sysc *ddata, int offset, u32 value)
+{
+	writel_relaxed(value, ddata->module_va + offset);
+}
+
 static u32 sysc_read(struct sysc *ddata, int offset)
 {
 	if (ddata->cfg.quirks & SYSC_QUIRK_16BIT) {
@@ -700,6 +708,26 @@ static void sysc_init_revision_quirks(struct sysc *ddata)
 	}
 }
 
+static int sysc_reset(struct sysc *ddata)
+{
+	int offset = ddata->offsets[SYSC_SYSCONFIG];
+	int val = sysc_read(ddata, offset);
+
+	val |= (0x1 << ddata->cap->regbits->srst_shift);
+	sysc_write(ddata, offset, val);
+
+	/* Poll on reset status */
+	if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
+		offset = ddata->offsets[SYSC_SYSSTATUS];
+
+		return readl_poll_timeout(ddata->module_va + offset, val,
+				(val & ddata->cfg.syss_mask) == 0x0,
+				100, MAX_MODULE_SOFTRESET_WAIT);
+	}
+
+	return 0;
+}
+
 /* At this point the module is configured enough to read the revision */
 static int sysc_init_module(struct sysc *ddata)
 {
@@ -716,6 +744,18 @@ static int sysc_init_module(struct sysc *ddata)
 
 		return 0;
 	}
+
+	if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT) &&
+	    !ddata->legacy_mode) {
+		error = sysc_reset(ddata);
+		if (error) {
+			dev_err(ddata->dev, "Reset failed with %d\n", error);
+			pm_runtime_put_sync(ddata->dev);
+
+			return error;
+		}
+	}
+
 	ddata->revision = sysc_read_revision(ddata);
 	pm_runtime_put_sync(ddata->dev);
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH v3 5/6] ARM: dts: Add generic interconnect target module node for MCAN
From: Faiz Abbas @ 2018-06-06  6:08 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-omap, linux-arm-kernel, linux-clk
  Cc: robh+dt, bcousson, tony, paul, t-kristo, faiz_abbas, mark.rutland
In-Reply-To: <20180606060826.14671-1-faiz_abbas@ti.com>

The ti-sysc driver provides support for manipulating the idle modes
and interconnect level resets.

Add the generic interconnect target module node for MCAN to support
the same.

CC: Tony Lindgren <tony@atomide.com>
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 arch/arm/boot/dts/dra76x.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index bfc82636999c..5157cc431574 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -11,6 +11,24 @@
 / {
 	compatible = "ti,dra762", "ti,dra7";
 
+	ocp {
+		target-module@42c01900 {
+			compatible = "ti,sysc-dra7-mcan", "ti,sysc";
+			ranges = <0x0 0x42c00000 0x2000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x42c01900 0x4>,
+			      <0x42c01904 0x4>,
+			      <0x42c01908 0x4>;
+			reg-names = "rev", "sysc", "syss";
+			ti,sysc-mask = <(SYSC_OMAP4_SOFTRESET |
+					 SYSC_DRA7_MCAN_ENAWAKEUP)>;
+			ti,syss-mask = <1>;
+			clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>;
+			clock-names = "fck";
+		};
+	};
+
 };
 
 /* MCAN interrupts are hard-wired to irqs 67, 68 */
-- 
2.17.0

^ permalink raw reply related

* [PATCH v3 6/6] ARM: dts: dra76x: Add MCAN node
From: Faiz Abbas @ 2018-06-06  6:08 UTC (permalink / raw)
  To: linux-kernel, devicetree, linux-omap, linux-arm-kernel, linux-clk
  Cc: robh+dt, bcousson, tony, paul, t-kristo, faiz_abbas, mark.rutland
In-Reply-To: <20180606060826.14671-1-faiz_abbas@ti.com>

From: Franklin S Cooper Jr <fcooper@ti.com>

Add support for the MCAN peripheral which supports both classic
CAN messages along with the new CAN-FD message.

Add MCAN node to evm and enable it with a maximum datarate of 5 mbps

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 arch/arm/boot/dts/dra76-evm.dts |  6 ++++++
 arch/arm/boot/dts/dra76x.dtsi   | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
index 2deb96405d06..b6ac2b6ffc6e 100644
--- a/arch/arm/boot/dts/dra76-evm.dts
+++ b/arch/arm/boot/dts/dra76-evm.dts
@@ -404,3 +404,9 @@
 	phys = <&pcie1_phy>, <&pcie2_phy>;
 	phy-names = "pcie-phy0", "pcie-phy1";
 };
+
+&m_can0 {
+	can-transceiver {
+		max-bitrate = <5000000>;
+	};
+};
diff --git a/arch/arm/boot/dts/dra76x.dtsi b/arch/arm/boot/dts/dra76x.dtsi
index 5157cc431574..613e4dc0ed3e 100644
--- a/arch/arm/boot/dts/dra76x.dtsi
+++ b/arch/arm/boot/dts/dra76x.dtsi
@@ -26,6 +26,19 @@
 			ti,syss-mask = <1>;
 			clocks = <&wkupaon_clkctrl DRA7_ADC_CLKCTRL 0>;
 			clock-names = "fck";
+
+			m_can0: mcan@1a00 {
+				compatible = "bosch,m_can";
+				reg = <0x1a00 0x4000>, <0x0 0x18FC>;
+				reg-names = "m_can", "message_ram";
+				interrupt-parent = <&gic>;
+				interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-names = "int0", "int1";
+				clocks = <&mcan_clk>, <&l3_iclk_div>;
+				clock-names = "cclk", "hclk";
+				bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
+			};
 		};
 	};
 
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH V2 1/4] mmc: sdhci-msm: Define new Register address map
From: Adrian Hunter @ 2018-06-06  6:30 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, robh+dt, mark.rutland
  Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov,
	devicetree, asutoshd, stummala, venkatg, jeremymc,
	bjorn.andersson, riteshh, vbadigan, dianders, sayalil
In-Reply-To: <1527587561-27448-2-git-send-email-vviswana@codeaurora.org>

On 29/05/18 12:52, Vijay Viswanath wrote:
> From: Sayali Lokhande <sayalil@codeaurora.org>
> 
> For SDCC version 5.0.0, MCI registers are removed from SDCC
> interface and some registers are moved to HC.
> Define a new data structure where we can statically define
> the address offsets for the registers in different SDCC versions.
> 
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

It would be prettier to line up the '=' and use BIT() not << but nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>


> ---
>  drivers/mmc/host/sdhci-msm.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index bb11916..4050c99 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -137,6 +137,95 @@
>  /* Timeout value to avoid infinite waiting for pwr_irq */
>  #define MSM_PWR_IRQ_TIMEOUT_MS 5000
>  
> +struct sdhci_msm_offset {
> +	u32 core_hc_mode;
> +	u32 core_mci_data_cnt;
> +	u32 core_mci_status;
> +	u32 core_mci_fifo_cnt;
> +	u32 core_mci_version;
> +	u32 core_generics;
> +	u32 core_testbus_config;
> +	u32 core_testbus_sel2_bit;
> +	u32 core_testbus_ena;
> +	u32 core_testbus_sel2;
> +	u32 core_pwrctl_status;
> +	u32 core_pwrctl_mask;
> +	u32 core_pwrctl_clear;
> +	u32 core_pwrctl_ctl;
> +	u32 core_sdcc_debug_reg;
> +	u32 core_dll_config;
> +	u32 core_dll_status;
> +	u32 core_vendor_spec;
> +	u32 core_vendor_spec_adma_err_addr0;
> +	u32 core_vendor_spec_adma_err_addr1;
> +	u32 core_vendor_spec_func2;
> +	u32 core_vendor_spec_capabilities0;
> +	u32 core_ddr_200_cfg;
> +	u32 core_vendor_spec3;
> +	u32 core_dll_config_2;
> +	u32 core_ddr_config;
> +	u32 core_ddr_config_2;
> +};
> +
> +static const struct sdhci_msm_offset sdhci_msm_v5_offset = {
> +	.core_mci_data_cnt = 0x35c,
> +	.core_mci_status = 0x324,
> +	.core_mci_fifo_cnt = 0x308,
> +	.core_mci_version = 0x318,
> +	.core_generics = 0x320,
> +	.core_testbus_config = 0x32c,
> +	.core_testbus_sel2_bit = 3,
> +	.core_testbus_ena = (1 << 31),
> +	.core_testbus_sel2 = (1 << 3),
> +	.core_pwrctl_status = 0x240,
> +	.core_pwrctl_mask = 0x244,
> +	.core_pwrctl_clear = 0x248,
> +	.core_pwrctl_ctl = 0x24c,
> +	.core_sdcc_debug_reg = 0x358,
> +	.core_dll_config = 0x200,
> +	.core_dll_status = 0x208,
> +	.core_vendor_spec = 0x20c,
> +	.core_vendor_spec_adma_err_addr0 = 0x214,
> +	.core_vendor_spec_adma_err_addr1 = 0x218,
> +	.core_vendor_spec_func2 = 0x210,
> +	.core_vendor_spec_capabilities0 = 0x21c,
> +	.core_ddr_200_cfg = 0x224,
> +	.core_vendor_spec3 = 0x250,
> +	.core_dll_config_2 = 0x254,
> +	.core_ddr_config = 0x258,
> +	.core_ddr_config_2 = 0x25c,
> +};
> +
> +static const struct sdhci_msm_offset sdhci_msm_mci_offset = {
> +	.core_hc_mode = 0x78,
> +	.core_mci_data_cnt = 0x30,
> +	.core_mci_status = 0x34,
> +	.core_mci_fifo_cnt = 0x44,
> +	.core_mci_version = 0x050,
> +	.core_generics = 0x70,
> +	.core_testbus_config = 0x0cc,
> +	.core_testbus_sel2_bit = 4,
> +	.core_testbus_ena = (1 << 3),
> +	.core_testbus_sel2 = (1 << 4),
> +	.core_pwrctl_status = 0xdc,
> +	.core_pwrctl_mask = 0xe0,
> +	.core_pwrctl_clear = 0xe4,
> +	.core_pwrctl_ctl = 0xe8,
> +	.core_sdcc_debug_reg = 0x124,
> +	.core_dll_config = 0x100,
> +	.core_dll_status = 0x108,
> +	.core_vendor_spec = 0x10c,
> +	.core_vendor_spec_adma_err_addr0 = 0x114,
> +	.core_vendor_spec_adma_err_addr1 = 0x118,
> +	.core_vendor_spec_func2 = 0x110,
> +	.core_vendor_spec_capabilities0 = 0x11c,
> +	.core_ddr_200_cfg = 0x184,
> +	.core_vendor_spec3 = 0x1b0,
> +	.core_dll_config_2 = 0x1b4,
> +	.core_ddr_config = 0x1b8,
> +	.core_ddr_config_2 = 0x1bc,
> +};
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> 

^ permalink raw reply

* Re: [PATCH V2 2/4] mmc: sdhci-msm: Add msm version specific ops and data structures
From: Adrian Hunter @ 2018-06-06  6:31 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, robh+dt, mark.rutland
  Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov,
	devicetree, asutoshd, stummala, venkatg, jeremymc,
	bjorn.andersson, riteshh, vbadigan, dianders, sayalil
In-Reply-To: <1527587561-27448-3-git-send-email-vviswana@codeaurora.org>

On 29/05/18 12:52, Vijay Viswanath wrote:
> In addition to offsets of certain registers changing, the registers in
> core_mem have been shifted to HC mem as well. To access these
> registers, define msm version specific functions. These functions can
> be loaded into the function pointers at the time of probe based on
> the msm version detected.
> 
> Also defind new data structure to hold version specific Ops and
> register addresses.
> 
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 77 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4050c99..2a66aa0 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -226,6 +226,24 @@ struct sdhci_msm_offset {
>  	.core_ddr_config_2 = 0x1bc,
>  };
>  
> +struct sdhci_msm_variant_ops {
> +	u8 (*msm_readb_relaxed)(struct sdhci_host *host, u32 offset);
> +	u32 (*msm_readl_relaxed)(struct sdhci_host *host, u32 offset);
> +	void (*msm_writeb_relaxed)(u8 val, struct sdhci_host *host, u32 offset);
> +	void (*msm_writel_relaxed)(u32 val, struct sdhci_host *host,
> +			u32 offset);
> +};
> +
> +/*
> + * From V5, register spaces have changed. Wrap this info in a structure
> + * and choose the data_structure based on version info mentioned in DT.
> + */
> +struct sdhci_msm_variant_info {
> +	bool mci_removed;
> +	const struct sdhci_msm_variant_ops *var_ops;
> +	const struct sdhci_msm_offset *offset;
> +};
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> @@ -245,8 +263,45 @@ struct sdhci_msm_host {
>  	wait_queue_head_t pwr_irq_wait;
>  	bool pwr_irq_flag;
>  	u32 caps_0;
> +	bool mci_removed;
> +	const struct sdhci_msm_variant_ops *var_ops;
> +	const struct sdhci_msm_offset *offset;
>  };
>  
> +/*
> + * APIs to read/write to vendor specific registers which were there in the
> + * core_mem region before MCI was removed.
> + */
> +static u32 sdhci_msm_mci_variant_readl_relaxed(struct sdhci_host *host,
> +		u32 offset)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	return readl_relaxed(msm_host->core_mem + offset);
> +}
> +
> +static u32 sdhci_msm_v5_variant_readl_relaxed(struct sdhci_host *host,
> +		u32 offset)
> +{
> +	return readl_relaxed(host->ioaddr + offset);
> +}
> +
> +static void sdhci_msm_mci_variant_writel_relaxed(u32 val,
> +		struct sdhci_host *host, u32 offset)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	writel_relaxed(val, msm_host->core_mem + offset);
> +}
> +
> +static void sdhci_msm_v5_variant_writel_relaxed(u32 val,
> +		struct sdhci_host *host, u32 offset)
> +{
> +	writel_relaxed(val, host->ioaddr + offset);
> +}
> +
>  static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
>  						    unsigned int clock)
>  {
> @@ -1481,6 +1536,28 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
>  	pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
>  }
>  
> +static const struct sdhci_msm_variant_ops mci_var_ops = {
> +	.msm_readl_relaxed = sdhci_msm_mci_variant_readl_relaxed,
> +	.msm_writel_relaxed = sdhci_msm_mci_variant_writel_relaxed,
> +};
> +
> +static const struct sdhci_msm_variant_ops v5_var_ops = {
> +	.msm_readl_relaxed = sdhci_msm_v5_variant_readl_relaxed,
> +	.msm_writel_relaxed = sdhci_msm_v5_variant_writel_relaxed,
> +};
> +
> +static const struct sdhci_msm_variant_info sdhci_msm_mci_var = {
> +	.mci_removed = 0,
> +	.var_ops = &mci_var_ops,
> +	.offset = &sdhci_msm_mci_offset,
> +};
> +
> +static const struct sdhci_msm_variant_info sdhci_msm_v5_var = {
> +	.mci_removed = 1,
> +	.var_ops = &v5_var_ops,
> +	.offset = &sdhci_msm_v5_offset,
> +};
> +
>  static const struct of_device_id sdhci_msm_dt_match[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4" },
>  	{},
> 

^ permalink raw reply

* Re: [PATCH V2 4/4] mmc: host: Register changes for sdcc V5
From: Adrian Hunter @ 2018-06-06  6:31 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, robh+dt, mark.rutland
  Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov,
	devicetree, asutoshd, stummala, venkatg, jeremymc,
	bjorn.andersson, riteshh, vbadigan, dianders, sayalil
In-Reply-To: <1527587561-27448-5-git-send-email-vviswana@codeaurora.org>

On 29/05/18 12:52, Vijay Viswanath wrote:
> Add support to use the new compatible string "qcom,sdhci-msm-v5".
> 
> Based on the msm variant, pick the relevant variant data and
> use it for register read/write to msm specific registers.
> 
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 347 +++++++++++++++++++++++++++----------------
>  1 file changed, 221 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 2a66aa0..4d0fd2d 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -33,16 +33,11 @@
>  #define CORE_MCI_GENERICS		0x70
>  #define SWITCHABLE_SIGNALING_VOLTAGE	BIT(29)
>  
> -#define CORE_HC_MODE		0x78
>  #define HC_MODE_EN		0x1
>  #define CORE_POWER		0x0
>  #define CORE_SW_RST		BIT(7)
>  #define FF_CLK_SW_RST_DIS	BIT(13)
>  
> -#define CORE_PWRCTL_STATUS	0xdc
> -#define CORE_PWRCTL_MASK	0xe0
> -#define CORE_PWRCTL_CLEAR	0xe4
> -#define CORE_PWRCTL_CTL		0xe8
>  #define CORE_PWRCTL_BUS_OFF	BIT(0)
>  #define CORE_PWRCTL_BUS_ON	BIT(1)
>  #define CORE_PWRCTL_IO_LOW	BIT(2)
> @@ -63,17 +58,13 @@
>  #define CORE_CDR_EXT_EN		BIT(19)
>  #define CORE_DLL_PDN		BIT(29)
>  #define CORE_DLL_RST		BIT(30)
> -#define CORE_DLL_CONFIG		0x100
>  #define CORE_CMD_DAT_TRACK_SEL	BIT(0)
> -#define CORE_DLL_STATUS		0x108
>  
> -#define CORE_DLL_CONFIG_2	0x1b4
>  #define CORE_DDR_CAL_EN		BIT(0)
>  #define CORE_FLL_CYCLE_CNT	BIT(18)
>  #define CORE_DLL_CLOCK_DISABLE	BIT(21)
>  
> -#define CORE_VENDOR_SPEC	0x10c
> -#define CORE_VENDOR_SPEC_POR_VAL	0xa1c
> +#define CORE_VENDOR_SPEC_POR_VAL 0xa1c
>  #define CORE_CLK_PWRSAVE	BIT(1)
>  #define CORE_HC_MCLK_SEL_DFLT	(2 << 8)
>  #define CORE_HC_MCLK_SEL_HS400	(3 << 8)
> @@ -111,17 +102,14 @@
>  #define CORE_CDC_SWITCH_BYPASS_OFF	BIT(0)
>  #define CORE_CDC_SWITCH_RC_EN		BIT(1)
>  
> -#define CORE_DDR_200_CFG		0x184
>  #define CORE_CDC_T4_DLY_SEL		BIT(0)
>  #define CORE_CMDIN_RCLK_EN		BIT(1)
>  #define CORE_START_CDC_TRAFFIC		BIT(6)
> -#define CORE_VENDOR_SPEC3	0x1b0
> +
>  #define CORE_PWRSAVE_DLL	BIT(3)
>  
> -#define CORE_DDR_CONFIG		0x1b8
>  #define DDR_CONFIG_POR_VAL	0x80040853
>  
> -#define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
>  
>  #define INVALID_TUNING_PHASE	-1
>  #define SDHCI_MSM_MIN_CLOCK	400000
> @@ -137,6 +125,12 @@
>  /* Timeout value to avoid infinite waiting for pwr_irq */
>  #define MSM_PWR_IRQ_TIMEOUT_MS 5000
>  
> +#define MSM_HOST_READL(msm_host, host, offset) \
> +	msm_host->var_ops->msm_readl_relaxed(host, offset)
> +
> +#define MSM_HOST_WRITEL(msm_host, val, host, offset) \
> +	msm_host->var_ops->msm_writel_relaxed(val, host, offset)
> +
>  struct sdhci_msm_offset {
>  	u32 core_hc_mode;
>  	u32 core_mci_data_cnt;
> @@ -268,6 +262,14 @@ struct sdhci_msm_host {
>  	const struct sdhci_msm_offset *offset;
>  };
>  
> +const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	return msm_host->offset;
> +}
> +
>  /*
>   * APIs to read/write to vendor specific registers which were there in the
>   * core_mem region before MCI was removed.
> @@ -349,10 +351,12 @@ static inline int msm_dll_poll_ck_out_en(struct sdhci_host *host, u8 poll)
>  	u32 wait_cnt = 50;
>  	u8 ck_out_en;
>  	struct mmc_host *mmc = host->mmc;
> +	const struct sdhci_msm_offset *msm_offset =
> +					sdhci_priv_msm_offset(host);
>  
>  	/* Poll for CK_OUT_EN bit.  max. poll time = 50us */
> -	ck_out_en = !!(readl_relaxed(host->ioaddr + CORE_DLL_CONFIG) &
> -			CORE_CK_OUT_EN);
> +	ck_out_en = !!(readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config) & CORE_CK_OUT_EN);
>  
>  	while (ck_out_en != poll) {
>  		if (--wait_cnt == 0) {
> @@ -362,8 +366,8 @@ static inline int msm_dll_poll_ck_out_en(struct sdhci_host *host, u8 poll)
>  		}
>  		udelay(1);
>  
> -		ck_out_en = !!(readl_relaxed(host->ioaddr + CORE_DLL_CONFIG) &
> -				CORE_CK_OUT_EN);
> +		ck_out_en = !!(readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config) & CORE_CK_OUT_EN);
>  	}
>  
>  	return 0;
> @@ -379,16 +383,18 @@ static int msm_config_cm_dll_phase(struct sdhci_host *host, u8 phase)
>  	unsigned long flags;
>  	u32 config;
>  	struct mmc_host *mmc = host->mmc;
> +	const struct sdhci_msm_offset *msm_offset =
> +					sdhci_priv_msm_offset(host);
>  
>  	if (phase > 0xf)
>  		return -EINVAL;
>  
>  	spin_lock_irqsave(&host->lock, flags);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config);
>  	config &= ~(CORE_CDR_EN | CORE_CK_OUT_EN);
>  	config |= (CORE_CDR_EXT_EN | CORE_DLL_EN);
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config);
>  
>  	/* Wait until CK_OUT_EN bit of DLL_CONFIG register becomes '0' */
>  	rc = msm_dll_poll_ck_out_en(host, 0);
> @@ -399,24 +405,24 @@ static int msm_config_cm_dll_phase(struct sdhci_host *host, u8 phase)
>  	 * Write the selected DLL clock output phase (0 ... 15)
>  	 * to CDR_SELEXT bit field of DLL_CONFIG register.
>  	 */
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config);
>  	config &= ~CDR_SELEXT_MASK;
>  	config |= grey_coded_phase_table[phase] << CDR_SELEXT_SHIFT;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config);
>  	config |= CORE_CK_OUT_EN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config);
>  
>  	/* Wait until CK_OUT_EN bit of DLL_CONFIG register becomes '1' */
>  	rc = msm_dll_poll_ck_out_en(host, 1);
>  	if (rc)
>  		goto err_out;
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config);
>  	config |= CORE_CDR_EN;
>  	config &= ~CORE_CDR_EXT_EN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config);
>  	goto out;
>  
>  err_out:
> @@ -542,6 +548,8 @@ static int msm_find_most_appropriate_phase(struct sdhci_host *host,
>  static inline void msm_cm_dll_set_freq(struct sdhci_host *host)
>  {
>  	u32 mclk_freq = 0, config;
> +	const struct sdhci_msm_offset *msm_offset =
> +					sdhci_priv_msm_offset(host);
>  
>  	/* Program the MCLK value to MCLK_FREQ bit field */
>  	if (host->clock <= 112000000)
> @@ -561,10 +569,10 @@ static inline void msm_cm_dll_set_freq(struct sdhci_host *host)
>  	else if (host->clock <= 200000000)
>  		mclk_freq = 7;
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config);
>  	config &= ~CMUX_SHIFT_PHASE_MASK;
>  	config |= mclk_freq << CMUX_SHIFT_PHASE_SHIFT;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config);
>  }
>  
>  /* Initialize the DLL (Programmable Delay Line) */
> @@ -576,6 +584,8 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  	int wait_cnt = 50;
>  	unsigned long flags;
>  	u32 config;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	spin_lock_irqsave(&host->lock, flags);
>  
> @@ -584,34 +594,43 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  	 * tuning is in progress. Keeping PWRSAVE ON may
>  	 * turn off the clock.
>  	 */
> -	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
>  	config &= ~CORE_CLK_PWRSAVE;
> -	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
>  
>  	if (msm_host->use_14lpp_dll_reset) {
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config);
>  		config &= ~CORE_CK_OUT_EN;
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config);
>  
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  		config |= CORE_DLL_CLOCK_DISABLE;
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config);
>  	config |= CORE_DLL_RST;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr +
> +			msm_offset->core_dll_config);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config);
>  	config |= CORE_DLL_PDN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr +
> +			msm_offset->core_dll_config);
>  	msm_cm_dll_set_freq(host);
>  
>  	if (msm_host->use_14lpp_dll_reset &&
>  	    !IS_ERR_OR_NULL(msm_host->xo_clk)) {
>  		u32 mclk_freq = 0;
>  
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  		config &= CORE_FLL_CYCLE_CNT;
>  		if (config)
>  			mclk_freq = DIV_ROUND_CLOSEST_ULL((host->clock * 8),
> @@ -620,40 +639,52 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>  			mclk_freq = DIV_ROUND_CLOSEST_ULL((host->clock * 4),
>  					clk_get_rate(msm_host->xo_clk));
>  
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  		config &= ~(0xFF << 10);
>  		config |= mclk_freq << 10;
>  
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  		/* wait for 5us before enabling DLL clock */
>  		udelay(5);
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config);
>  	config &= ~CORE_DLL_RST;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr +
> +			msm_offset->core_dll_config);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config);
>  	config &= ~CORE_DLL_PDN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr +
> +			msm_offset->core_dll_config);
>  
>  	if (msm_host->use_14lpp_dll_reset) {
>  		msm_cm_dll_set_freq(host);
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  		config &= ~CORE_DLL_CLOCK_DISABLE;
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config_2);
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config);
>  	config |= CORE_DLL_EN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr +
> +			msm_offset->core_dll_config);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr +
> +			msm_offset->core_dll_config);
>  	config |= CORE_CK_OUT_EN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr +
> +			msm_offset->core_dll_config);
>  
>  	/* Wait until DLL_LOCK bit of DLL_STATUS register becomes '1' */
> -	while (!(readl_relaxed(host->ioaddr + CORE_DLL_STATUS) &
> +	while (!(readl_relaxed(host->ioaddr + msm_offset->core_dll_status) &
>  		 CORE_DLL_LOCK)) {
>  		/* max. wait for 50us sec for LOCK bit to be set */
>  		if (--wait_cnt == 0) {
> @@ -674,19 +705,21 @@ static void msm_hc_select_default(struct sdhci_host *host)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u32 config;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	if (!msm_host->use_cdclp533) {
>  		config = readl_relaxed(host->ioaddr +
> -				CORE_VENDOR_SPEC3);
> +				msm_offset->core_vendor_spec3);
>  		config &= ~CORE_PWRSAVE_DLL;
>  		writel_relaxed(config, host->ioaddr +
> -				CORE_VENDOR_SPEC3);
> +				msm_offset->core_vendor_spec3);
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
>  	config &= ~CORE_HC_MCLK_SEL_MASK;
>  	config |= CORE_HC_MCLK_SEL_DFLT;
> -	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
>  
>  	/*
>  	 * Disable HC_SELECT_IN to be able to use the UHS mode select
> @@ -695,10 +728,10 @@ static void msm_hc_select_default(struct sdhci_host *host)
>  	 * Write 0 to HC_SELECT_IN and HC_SELECT_IN_EN field
>  	 * in VENDOR_SPEC_FUNC
>  	 */
> -	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
>  	config &= ~CORE_HC_SELECT_IN_EN;
>  	config &= ~CORE_HC_SELECT_IN_MASK;
> -	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
>  
>  	/*
>  	 * Make sure above writes impacting free running MCLK are completed
> @@ -714,32 +747,36 @@ static void msm_hc_select_hs400(struct sdhci_host *host)
>  	struct mmc_ios ios = host->mmc->ios;
>  	u32 config, dll_lock;
>  	int rc;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	/* Select the divided clock (free running MCLK/2) */
> -	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec);
>  	config &= ~CORE_HC_MCLK_SEL_MASK;
>  	config |= CORE_HC_MCLK_SEL_HS400;
>  
> -	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec);
>  	/*
>  	 * Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC
>  	 * register
>  	 */
>  	if ((msm_host->tuning_done || ios.enhanced_strobe) &&
>  	    !msm_host->calibration_done) {
> -		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_vendor_spec);
>  		config |= CORE_HC_SELECT_IN_HS400;
>  		config |= CORE_HC_SELECT_IN_EN;
> -		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_vendor_spec);
>  	}
>  	if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
>  		/*
>  		 * Poll on DLL_LOCK or DDR_DLL_LOCK bits in
> -		 * CORE_DLL_STATUS to be set.  This should get set
> +		 * core_dll_status to be set. This should get set
>  		 * within 15 us at 200 MHz.
>  		 */
>  		rc = readl_relaxed_poll_timeout(host->ioaddr +
> -						CORE_DLL_STATUS,
> +						msm_offset->core_dll_status,
>  						dll_lock,
>  						(dll_lock &
>  						(CORE_DLL_LOCK |
> @@ -791,6 +828,8 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u32 config, calib_done;
>  	int ret;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>  
> @@ -807,13 +846,13 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	if (ret)
>  		goto out;
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config);
>  	config |= CORE_CMD_DAT_TRACK_SEL;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_ddr_200_cfg);
>  	config &= ~CORE_CDC_T4_DLY_SEL;
> -	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_ddr_200_cfg);
>  
>  	config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>  	config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
> @@ -823,9 +862,9 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  	config |= CORE_CDC_SWITCH_RC_EN;
>  	writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_ddr_200_cfg);
>  	config &= ~CORE_START_CDC_TRAFFIC;
> -	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_ddr_200_cfg);
>  
>  	/* Perform CDC Register Initialization Sequence */
>  
> @@ -877,9 +916,9 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>  		goto out;
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_ddr_200_cfg);
>  	config |= CORE_START_CDC_TRAFFIC;
> -	writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_ddr_200_cfg);
>  out:
>  	pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
>  		 __func__, ret);
> @@ -891,32 +930,38 @@ static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>  	struct mmc_host *mmc = host->mmc;
>  	u32 dll_status, config;
>  	int ret;
> +	const struct sdhci_msm_offset *msm_offset =
> +					sdhci_priv_msm_offset(host);
>  
>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>  
>  	/*
> -	 * Currently the CORE_DDR_CONFIG register defaults to desired
> +	 * Currently the core_ddr_config register defaults to desired
>  	 * configuration on reset. Currently reprogramming the power on
>  	 * reset (POR) value in case it might have been modified by
>  	 * bootloaders. In the future, if this changes, then the desired
>  	 * values will need to be programmed appropriately.
>  	 */
> -	writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
> +	writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr +
> +			msm_offset->core_ddr_config);
>  
>  	if (mmc->ios.enhanced_strobe) {
> -		config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_ddr_200_cfg);
>  		config |= CORE_CMDIN_RCLK_EN;
> -		writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_ddr_200_cfg);
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config_2);
>  	config |= CORE_DDR_CAL_EN;
> -	writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_2);
>  
> -	ret = readl_relaxed_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
> -					 dll_status,
> -					 (dll_status & CORE_DDR_DLL_LOCK),
> -					 10, 1000);
> +	ret = readl_relaxed_poll_timeout(host->ioaddr +
> +					msm_offset->core_dll_status,
> +					dll_status,
> +					(dll_status & CORE_DDR_DLL_LOCK),
> +					10, 1000);
>  
>  	if (ret == -ETIMEDOUT) {
>  		pr_err("%s: %s: CM_DLL_SDC4 calibration was not completed\n",
> @@ -924,9 +969,9 @@ static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>  		goto out;
>  	}
>  
> -	config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
> +	config = readl_relaxed(host->ioaddr + msm_offset->core_vendor_spec3);
>  	config |= CORE_PWRSAVE_DLL;
> -	writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
> +	writel_relaxed(config, host->ioaddr + msm_offset->core_vendor_spec3);
>  
>  	/*
>  	 * Drain writebuffer to ensure above DLL calibration
> @@ -946,6 +991,8 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>  	struct mmc_host *mmc = host->mmc;
>  	int ret;
>  	u32 config;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
>  
> @@ -963,9 +1010,11 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>  					      msm_host->saved_tuning_phase);
>  		if (ret)
>  			goto out;
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config);
>  		config |= CORE_CMD_DAT_TRACK_SEL;
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config);
>  	}
>  
>  	if (msm_host->use_cdclp533)
> @@ -1095,6 +1144,8 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u16 ctrl_2;
>  	u32 config;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>  	/* Select Bus Speed Mode for host */
> @@ -1135,13 +1186,17 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  		 * DLL is not required for clock <= 100MHz
>  		 * Thus, make sure DLL it is disabled when not required
>  		 */
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config);
>  		config |= CORE_DLL_RST;
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config);
>  
> -		config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_dll_config);
>  		config |= CORE_DLL_PDN;
> -		writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +		writel_relaxed(config, host->ioaddr +
> +				msm_offset->core_dll_config);
>  
>  		/*
>  		 * The DLL needs to be restored and CDCLP533 recalibrated
> @@ -1183,7 +1238,9 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	bool done = false;
> -	u32 val;
> +	u32 val = SWITCHABLE_SIGNALING_VOLTAGE;
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	pr_debug("%s: %s: request %d curr_pwr_state %x curr_io_level %x\n",
>  			mmc_hostname(host->mmc), __func__, req_type,
> @@ -1192,8 +1249,12 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
>  	/*
>  	 * The power interrupt will not be generated for signal voltage
>  	 * switches if SWITCHABLE_SIGNALING_VOLTAGE in MCI_GENERICS is not set.
> +	 * Since sdhci-msm-v5, this bit has been removed and SW must consider
> +	 * it as always set.
>  	 */
> -	val = readl(msm_host->core_mem + CORE_MCI_GENERICS);
> +	if (!msm_host->mci_removed)
> +		val = MSM_HOST_READL(msm_host, host,
> +				msm_offset->core_generics);
>  	if ((req_type & REQ_IO_HIGH || req_type & REQ_IO_LOW) &&
>  	    !(val & SWITCHABLE_SIGNALING_VOLTAGE)) {
>  		return;
> @@ -1241,12 +1302,14 @@ static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	const struct sdhci_msm_offset *msm_offset =
> +					msm_host->offset;
>  
>  	pr_err("%s: PWRCTL_STATUS: 0x%08x | PWRCTL_MASK: 0x%08x | PWRCTL_CTL: 0x%08x\n",
> -			mmc_hostname(host->mmc),
> -			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS),
> -			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_MASK),
> -			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL));
> +		mmc_hostname(host->mmc),
> +		MSM_HOST_READL(msm_host, host, msm_offset->core_pwrctl_status),
> +		MSM_HOST_READL(msm_host, host, msm_offset->core_pwrctl_mask),
> +		MSM_HOST_READL(msm_host, host, msm_offset->core_pwrctl_ctl));
>  }
>  
>  static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
> @@ -1257,11 +1320,14 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	int retry = 10;
>  	u32 pwr_state = 0, io_level = 0;
>  	u32 config;
> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>  
> -	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
> +	irq_status = MSM_HOST_READL(msm_host, host,
> +			msm_offset->core_pwrctl_status);
>  	irq_status &= INT_MASK;
>  
> -	writel_relaxed(irq_status, msm_host->core_mem + CORE_PWRCTL_CLEAR);
> +	MSM_HOST_WRITEL(msm_host, irq_status, host,
> +			msm_offset->core_pwrctl_clear);
>  
>  	/*
>  	 * There is a rare HW scenario where the first clear pulse could be
> @@ -1270,8 +1336,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	 * sure status register is cleared. Otherwise, this will result in
>  	 * a spurious power IRQ resulting in system instability.
>  	 */
> -	while (irq_status & readl_relaxed(msm_host->core_mem +
> -				CORE_PWRCTL_STATUS)) {
> +	while (irq_status & MSM_HOST_READL(msm_host, host,
> +				msm_offset->core_pwrctl_status)) {
>  		if (retry == 0) {
>  			pr_err("%s: Timedout clearing (0x%x) pwrctl status register\n",
>  					mmc_hostname(host->mmc), irq_status);
> @@ -1279,8 +1345,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  			WARN_ON(1);
>  			break;
>  		}
> -		writel_relaxed(irq_status,
> -				msm_host->core_mem + CORE_PWRCTL_CLEAR);
> +		MSM_HOST_WRITEL(msm_host, irq_status, host,
> +			msm_offset->core_pwrctl_clear);
>  		retry--;
>  		udelay(10);
>  	}
> @@ -1311,7 +1377,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	 * report back if it succeded or not to this register. The voltage
>  	 * switches are handled by the sdhci core, so just report success.
>  	 */
> -	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
> +	MSM_HOST_WRITEL(msm_host, irq_ack, host,
> +			msm_offset->core_pwrctl_ctl);
>  
>  	/*
>  	 * If we don't have info regarding the voltage levels supported by
> @@ -1330,7 +1397,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  		 * controllers with only 1.8V, we will set the IO PAD bit
>  		 * without waiting for a REQ_IO_LOW.
>  		 */
> -		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_vendor_spec);
>  		new_config = config;
>  
>  		if ((io_level & REQ_IO_HIGH) &&
> @@ -1341,8 +1409,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  			new_config |= CORE_IO_PAD_PWR_SWITCH;
>  
>  		if (config ^ new_config)
> -			writel_relaxed(new_config,
> -					host->ioaddr + CORE_VENDOR_SPEC);
> +			writel_relaxed(new_config, host->ioaddr +
> +					msm_offset->core_vendor_spec);
>  	}
>  
>  	if (pwr_state)
> @@ -1503,6 +1571,7 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
>  	struct regulator *supply = mmc->supply.vqmmc;
>  	u32 caps = 0, config;
>  	struct sdhci_host *host = mmc_priv(mmc);
> +	const struct sdhci_msm_offset *msm_offset = msm_host->offset;
>  
>  	if (!IS_ERR(mmc->supply.vqmmc)) {
>  		if (regulator_is_supported_voltage(supply, 1700000, 1950000))
> @@ -1522,7 +1591,8 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
>  		 */
>  		u32 io_level = msm_host->curr_io_level;
>  
> -		config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> +		config = readl_relaxed(host->ioaddr +
> +				msm_offset->core_vendor_spec);
>  		config |= CORE_IO_PAD_PWR_SWITCH_EN;
>  
>  		if ((io_level & REQ_IO_HIGH) && (caps &	CORE_3_0V_SUPPORT))
> @@ -1530,7 +1600,8 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
>  		else if ((io_level & REQ_IO_LOW) || (caps & CORE_1_8V_SUPPORT))
>  			config |= CORE_IO_PAD_PWR_SWITCH;
>  
> -		writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +		writel_relaxed(config,
> +				host->ioaddr + msm_offset->core_vendor_spec);
>  	}
>  	msm_host->caps_0 |= caps;
>  	pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
> @@ -1559,7 +1630,8 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
>  };
>  
>  static const struct of_device_id sdhci_msm_dt_match[] = {
> -	{ .compatible = "qcom,sdhci-msm-v4" },
> +	{.compatible = "qcom,sdhci-msm-v4", .data = &sdhci_msm_mci_var},
> +	{.compatible = "qcom,sdhci-msm-v5", .data = &sdhci_msm_v5_var},
>  	{},
>  };
>  
> @@ -1596,6 +1668,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	u16 host_version, core_minor;
>  	u32 core_version, config;
>  	u8 core_major;
> +	const struct sdhci_msm_offset *msm_offset;
> +	const struct sdhci_msm_variant_info *var_info;
>  
>  	host = sdhci_pltfm_init(pdev, &sdhci_msm_pdata, sizeof(*msm_host));
>  	if (IS_ERR(host))
> @@ -1611,6 +1685,18 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto pltfm_free;
>  
> +	/*
> +	 * Based on the compatible string, load the required msm host info from
> +	 * the data associated with the version info.
> +	 */
> +	var_info = of_device_get_match_data(&pdev->dev);
> +
> +	msm_host->mci_removed = var_info->mci_removed;
> +	msm_host->var_ops = var_info->var_ops;
> +	msm_host->offset = var_info->offset;
> +
> +	msm_offset = msm_host->offset;
> +
>  	sdhci_get_of_property(pdev);
>  
>  	msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
> @@ -1675,32 +1761,40 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		dev_warn(&pdev->dev, "TCXO clk not present (%d)\n", ret);
>  	}
>  
> -	core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	msm_host->core_mem = devm_ioremap_resource(&pdev->dev, core_memres);
> +	if (!msm_host->mci_removed) {
> +		core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +		msm_host->core_mem = devm_ioremap_resource(&pdev->dev,
> +				core_memres);
>  
> -	if (IS_ERR(msm_host->core_mem)) {
> -		dev_err(&pdev->dev, "Failed to remap registers\n");
> -		ret = PTR_ERR(msm_host->core_mem);
> -		goto clk_disable;
> +		if (IS_ERR(msm_host->core_mem)) {
> +			dev_err(&pdev->dev, "Failed to remap registers\n");
> +			ret = PTR_ERR(msm_host->core_mem);
> +			goto clk_disable;
> +		}
>  	}
>  
>  	/* Reset the vendor spec register to power on reset state */
>  	writel_relaxed(CORE_VENDOR_SPEC_POR_VAL,
> -		       host->ioaddr + CORE_VENDOR_SPEC);
> -
> -	/* Set HC_MODE_EN bit in HC_MODE register */
> -	writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
> -
> -	config = readl_relaxed(msm_host->core_mem + CORE_HC_MODE);
> -	config |= FF_CLK_SW_RST_DIS;
> -	writel_relaxed(config, msm_host->core_mem + CORE_HC_MODE);
> +			host->ioaddr + msm_offset->core_vendor_spec);
> +
> +	if (!msm_host->mci_removed) {
> +		/* Set HC_MODE_EN bit in HC_MODE register */
> +		MSM_HOST_WRITEL(msm_host, HC_MODE_EN, host,
> +				msm_offset->core_hc_mode);
> +		config = MSM_HOST_READL(msm_host, host,
> +				msm_offset->core_hc_mode);
> +		config |= FF_CLK_SW_RST_DIS;
> +		MSM_HOST_WRITEL(msm_host, config, host,
> +				msm_offset->core_hc_mode);
> +	}
>  
>  	host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
>  	dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n",
>  		host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >>
>  			       SDHCI_VENDOR_VER_SHIFT));
>  
> -	core_version = readl_relaxed(msm_host->core_mem + CORE_MCI_VERSION);
> +	core_version = MSM_HOST_READL(msm_host, host,
> +			msm_offset->core_mci_version);
>  	core_major = (core_version & CORE_VERSION_MAJOR_MASK) >>
>  		      CORE_VERSION_MAJOR_SHIFT;
>  	core_minor = core_version & CORE_VERSION_MINOR_MASK;
> @@ -1725,7 +1819,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		config = readl_relaxed(host->ioaddr + SDHCI_CAPABILITIES);
>  		config |= SDHCI_CAN_VDD_300 | SDHCI_CAN_DO_8BIT;
>  		writel_relaxed(config, host->ioaddr +
> -			       CORE_VENDOR_SPEC_CAPABILITIES0);
> +				msm_offset->core_vendor_spec_capabilities0);
>  	}
>  
>  	/*
> @@ -1754,7 +1848,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  
>  	sdhci_msm_init_pwr_irq_wait(msm_host);
>  	/* Enable pwr irq interrupts */
> -	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
> +	MSM_HOST_WRITEL(msm_host, INT_MASK, host,
> +		msm_offset->core_pwrctl_mask);
>  
>  	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
>  					sdhci_msm_pwr_irq, IRQF_ONESHOT,
> 

^ permalink raw reply

* RE: [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Michel Pollet @ 2018-06-06  6:36 UTC (permalink / raw)
  To: Frank Rowand, linux-renesas-soc@vger.kernel.org, Simon Horman
  Cc: Michel Pollet, Mark Rutland, Phil Edworthy, Florian Fainelli,
	Rajendra Nayak, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Stefan Wahren, Magnus Damm,
	Russell King, Douglas Anderson, Chen-Yu Tsai, Rob Herring,
	Carlo Caione, Andreas Färber, Frank Rowand,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <0481173f-6384-98d6-707c-89dc5ef103f0@gmail.com>

Hi Frank,

On 05 June 2018 18:34, Frank wrote:
> On 06/05/18 04:28, Michel Pollet wrote:
> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
> > it requires a special enable method to get it started.
> >
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> > ---
> >  arch/arm/mach-shmobile/Makefile        |  1 +
> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
> > ++++++++++++++++++++++++++++++++++
> >  2 files changed, 80 insertions(+)
> >  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
> >
> > diff --git a/arch/arm/mach-shmobile/Makefile
> > b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
> > --- a/arch/arm/mach-shmobile/Makefile
> > +++ b/arch/arm/mach-shmobile/Makefile
> > @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
> headsmp-scu.o platsmp-scu.o
> >  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
> platsmp-scu.o
> >  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
> >  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
> > +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
> >  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
> platsmp-scu.o
> >
> >  # PM objects
> > diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
> > b/arch/arm/mach-shmobile/smp-r9a06g032.c
> > new file mode 100644
> > index 0000000..cd40e6e
> > --- /dev/null
> > +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> > @@ -0,0 +1,79 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * R9A06G032 Second CA7 enabler.
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Europe Limited
> > + *
> > + * Michel Pollet <michel.pollet@bp.renesas.com>,
> <buserror@gmail.com>
> > + * Derived from action,s500-smp
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/smp.h>
> > +
> > +/*
> > + * The second CPU is parked in ROM at boot time. It requires waking
> > +it after
> > + * writing an address into the BOOTADDR register of sysctrl.
> > + *
> > + * So the default value of the "cpu-release-addr" corresponds to
> BOOTADDR...
> > + *
> > + * *However* the BOOTADDR register is not available when the kernel
> > + * starts in NONSEC mode.
> > + *
> > + * So for NONSEC mode, the bootloader re-parks the second CPU into a
> > +pen
> > + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
> > +SRAM address,
> > + * which is not restricted.
>
> The binding document for cpu-release-addr does not have a definition for 32
> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
> for 32 bit arm to patch 1.

Hmmm I do find a definition in
Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
added my 'enable-method' -- And it is already used as 32 bits in at least
arch/arm/boot/dts/stih407-family.dtsi.

What do you want me to add to this exactly? Do you want me to just
change "required for systems that have an "enable-method" property
value of "spin-table" to also specify renesas,r9a06g032 ?

Thanks!
Michel

>
> -Frank
>
>
> > + */
> > +
> > +static void __iomem *cpu_bootaddr;
> > +
> > +static DEFINE_SPINLOCK(cpu_lock);
> > +
> > +static int r9a06g032_smp_boot_secondary(unsigned int cpu, struct
> > +task_struct *idle) {
> > +if (!cpu_bootaddr)
> > +return -ENODEV;
> > +
> > +spin_lock(&cpu_lock);
> > +
> > +writel(__pa_symbol(secondary_startup), cpu_bootaddr);
> > +arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> > +
> > +spin_unlock(&cpu_lock);
> > +
> > +return 0;
> > +}
> > +
> > +static void __init r9a06g032_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > +struct device_node *dn;
> > +int ret;
> > +u32 bootaddr;
> > +
> > +dn = of_get_cpu_node(1, NULL);
> > +if (!dn) {
> > +pr_err("CPU#1: missing device tree node\n");
> > +return;
> > +}
> > +/*
> > + * Determine the address from which the CPU is polling.
> > + * The bootloader *does* change this property
> > + */
> > +ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
> > +of_node_put(dn);
> > +if (ret) {
> > +pr_err("CPU#1: invalid cpu-release-addr property\n");
> > +return;
> > +}
> > +pr_info("CPU#1: cpu-release-addr %08x\n", bootaddr);
> > +
> > +cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr)); }
> > +
> > +static const struct smp_operations r9a06g032_smp_ops __initconst = {
> > +.smp_prepare_cpus = r9a06g032_smp_prepare_cpus,
> > +.smp_boot_secondary = r9a06g032_smp_boot_secondary, };
> > +CPU_METHOD_OF_DECLARE(r9a06g032_smp, "renesas,r9a06g032-smp",
> > +&r9a06g032_smp_ops);
> >




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

^ permalink raw reply

* Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
From: Sricharan R @ 2018-06-06  6:39 UTC (permalink / raw)
  To: Vinod Koul
  Cc: bjorn.andersson, ohad, robh+dt, mark.rutland, andy.gross,
	david.brown, linux-remoteproc, devicetree, linux-kernel,
	linux-arm-msm, linux-soc, sibis
In-Reply-To: <20180605164031.GZ16230@vkoul-mobl>

Hi Vinod,

On 6/5/2018 10:10 PM, Vinod Koul wrote:
> On 05-06-18, 18:26, Sricharan R wrote:
>> Hi Vinod,
>>
>> On 6/5/2018 11:49 AM, Vinod wrote:
>>> On 05-06-18, 11:12, Sricharan R wrote:
>>>
>>>> +config QCOM_Q6V5_WCSS
>>>> +	tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader"
>>>> +	depends on OF && ARCH_QCOM
>>>> +	depends on QCOM_SMEM
>>>> +	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>>>> +	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>>>
>>> Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would
>>> happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM
>>>
>>   RPMSG_QCOM_GLINK_SMEM=n should be for the COMPILE_TEST. Probably that
> 
> why would that be a limitation? I am more worried about
> RPMSG_QCOM_GLINK_SMEM=n being the condition here. In new drivers we
> should not typically have dependency on some symbol being not there
> 

Without that, if RPMSG_QCOM_GLINK_SMEM=m is compiled as a module, then
it would break the build.

>>   means that it should be corrected here and for ADSP, Q6V5_PIL as well.
>>   Bjorn, is that correct ?, should it be, below ?
>>  
>>   depends on (RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)) || (RPMSG_QCOM_GLINK_SMEM || (COMPILE_TEST && RPMSG_QCOM_GLINK_SMEM=n))
> 
> that doesnt really sound good :(
> 

 Hmm, but i was thinking it should functionally depend on either SMD or GLINK and not both.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

^ permalink raw reply

* [PATCH] dt-bindings: rcar-gen3-phy-usb2: Add bindings for r8a77990
From: Yoshihiro Shimoda @ 2018-06-06  6:42 UTC (permalink / raw)
  To: kishon, robh+dt, mark.rutland
  Cc: linux-kernel, devicetree, linux-renesas-soc, Yoshihiro Shimoda

This patch adds suuport for r8a77990 (R-Car E3).

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
index dbd137c..fb4a204 100644
--- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
+++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
@@ -10,6 +10,8 @@ Required properties:
 	      SoC.
 	      "renesas,usb2-phy-r8a77965" if the device is a part of an
 	      R8A77965 SoC.
+	      "renesas,usb2-phy-r8a77990" if the device is a part of an
+	      R8A77990 SoC.
 	      "renesas,usb2-phy-r8a77995" if the device is a part of an
 	      R8A77995 SoC.
 	      "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3 compatible device.
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 01/20] coresight: Fix memory leak in coresight_register
From: Arvind Yadav @ 2018-06-06  6:44 UTC (permalink / raw)
  To: Suzuki K Poulose, linux-arm-kernel
  Cc: mathieu.poirier, robh, frowand.list, mark.rutland, sudeep.holla,
	arm, linux-kernel, matt.sealey, john.horley, charles.garcia-tobin,
	coresight, devicetree, mike.leach
In-Reply-To: <1528235011-30691-2-git-send-email-suzuki.poulose@arm.com>

Hi Suzuki,


On Wednesday 06 June 2018 03:13 AM, Suzuki K Poulose wrote:
> commit 6403587a930c ("coresight: use put_device() instead of kfree()")
> introduced a memory leak where, if we fail to register the device
> for coresight_device, we don't free the "coresight_device" object,
> which was allocated via kzalloc(). Fix this by jumping to the
> appropriate error path.
put_device() will decrement the last reference and then
free the memory by calling dev->release.  Internally
put_device() -> kobject_put() -> kobject_cleanup() which is
responsible to call 'dev -> release' and also free other kobject
resources. If you will see the coresight_device_release. There
we are releasing all allocated memory. Still if you call kfree() again
then it'll be redundancy.

~arvind
>
> Fixes: commit 6403587a930c ("coresight: use put_device() instead of kfree()")
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index 4969b32..2893cfe 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -1020,7 +1020,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   	ret = device_register(&csdev->dev);
>   	if (ret) {
>   		put_device(&csdev->dev);
> -		goto err_kzalloc_csdev;
> +		goto err_kzalloc_refcnts;
>   	}
>   
>   	mutex_lock(&coresight_mutex);

^ permalink raw reply

* Re: [PATCH] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
From: Vinod @ 2018-06-06  6:49 UTC (permalink / raw)
  To: Sricharan R
  Cc: bjorn.andersson, ohad, robh+dt, mark.rutland, andy.gross,
	david.brown, linux-remoteproc, devicetree, linux-kernel,
	linux-arm-msm, linux-soc, sibis
In-Reply-To: <1b376fc2-6a1b-1457-ffff-332954d1bef8@codeaurora.org>

Hi Sricharan,

On 06-06-18, 12:09, Sricharan R wrote:

> >>>> +config QCOM_Q6V5_WCSS
> >>>> +	tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader"
> >>>> +	depends on OF && ARCH_QCOM
> >>>> +	depends on QCOM_SMEM
> >>>> +	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
> >>>> +	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
> >>>
> >>> Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would
> >>> happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM
> >>>
> >>   RPMSG_QCOM_GLINK_SMEM=n should be for the COMPILE_TEST. Probably that
> > 
> > why would that be a limitation? I am more worried about
> > RPMSG_QCOM_GLINK_SMEM=n being the condition here. In new drivers we
> > should not typically have dependency on some symbol being not there
> 
> Without that, if RPMSG_QCOM_GLINK_SMEM=m is compiled as a module, then
> it would break the build.

Okay I do not know the details, but that doesn't sound correct to me.
Breaking build sounds a bit extreme to me. Can you give details on this
part..

> >>   means that it should be corrected here and for ADSP, Q6V5_PIL as well.
> >>   Bjorn, is that correct ?, should it be, below ?
> >>  
> >>   depends on (RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)) || (RPMSG_QCOM_GLINK_SMEM || (COMPILE_TEST && RPMSG_QCOM_GLINK_SMEM=n))
> > 
> > that doesnt really sound good :(
> 
>  Hmm, but i was thinking it should functionally depend on either SMD or GLINK and not both.

If you are depedent upon a symbol provided by a module you should say
depends on. If a machine is not supposed to have both SMD or GLINK then
the driver will not get probed.

-- 
~Vinod

^ permalink raw reply

* [PATCH 2/4] arm64: dts: renesas: r8a77990: add USB2.0 host device nodes
From: Yoshihiro Shimoda @ 2018-06-06  6:56 UTC (permalink / raw)
  To: horms, magnus.damm, robh+dt, mark.rutland
  Cc: devicetree, linux-renesas-soc, Yoshihiro Shimoda
In-Reply-To: <1528268171-19276-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>

This patch adds USB2.0 host(EHCI/OHCI) device nodes for r8a77990.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 121c4a0..0b2bec3 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -248,6 +248,31 @@
 			status = "disabled";
 		};
 
+		ohci0: usb@ee080000 {
+			compatible = "generic-ohci";
+			reg = <0 0xee080000 0 0x100>;
+			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 703>;
+			phys = <&usb2_phy0>;
+			phy-names = "usb";
+			power-domains = <&sysc 32>;
+			resets = <&cpg 703>;
+			status = "disabled";
+		};
+
+		ehci0: usb@ee080100 {
+			compatible = "generic-ehci";
+			reg = <0 0xee080100 0 0x100>;
+			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 703>;
+			phys = <&usb2_phy0>;
+			phy-names = "usb";
+			companion = <&ohci0>;
+			power-domains = <&sysc 32>;
+			resets = <&cpg 703>;
+			status = "disabled";
+		};
+
 		usb2_phy0: usb-phy@ee080200 {
 			compatible = "renesas,usb2-phy-r8a77990",
 				     "renesas,rcar-gen3-usb2-phy";
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/4] arm64: dts: renesas: r8a77990: ebisu: enable usb2_phy0
From: Yoshihiro Shimoda @ 2018-06-06  6:56 UTC (permalink / raw)
  To: horms, magnus.damm, robh+dt, mark.rutland
  Cc: devicetree, linux-renesas-soc, Yoshihiro Shimoda
In-Reply-To: <1528268171-19276-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>

This patch enables usb2_phy0 for r8a77990 Ebisu board.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 7a09d05..1be10c9 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -58,8 +58,20 @@
 			function = "avb";
 		};
 	};
+
+	usb0_pins: usb {
+		groups = "usb0_b";
+		function = "usb0";
+	};
 };
 
 &scif2 {
 	status = "okay";
 };
+
+&usb2_phy0 {
+	pinctrl-0 = <&usb0_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/4] arm64: dts: renesas: r8a77990 and ebisu: Enable USB2.0 host
From: Yoshihiro Shimoda @ 2018-06-06  6:56 UTC (permalink / raw)
  To: horms, magnus.damm, robh+dt, mark.rutland
  Cc: devicetree, linux-renesas-soc, Yoshihiro Shimoda

This patch set is based on renesas-drivers.git /
renesas-drivers-2018-06-05-v4.17 tag.

About dt-bindings of "renesas,usb2-phy-r8a77990", I submitted a patch:
https://patchwork.kernel.org/patch/10449723/

But, the phy-rcar-gen3-usb2 driver works on the board without the bindings
because "renesas,rcar-gen3-usb2-phy" can bind the driver.

Also, for now, I don't try to add/enable USB2.0 peripheral on the board
because the phy driver needs some modifications.

Yoshihiro Shimoda (4):
  arm64: dts: renesas: r8a77990: add usb2_phy device node
  arm64: dts: renesas: r8a77990: add USB2.0 host device nodes
  arm64: dts: renesas: r8a77990: ebisu: enable usb2_phy0
  arm64: dts: renesas: r8a77990: ebisu: enable USB2.0 host

 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 20 ++++++++++++++
 arch/arm64/boot/dts/renesas/r8a77990.dtsi      | 37 ++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

-- 
1.9.1


^ permalink raw reply

* [PATCH 1/4] arm64: dts: renesas: r8a77990: add usb2_phy device node
From: Yoshihiro Shimoda @ 2018-06-06  6:56 UTC (permalink / raw)
  To: horms, magnus.damm, robh+dt, mark.rutland
  Cc: devicetree, linux-renesas-soc, Yoshihiro Shimoda
In-Reply-To: <1528268171-19276-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>

This patch adds USB2.0 phy device node for r8a77990.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index be4f519..121c4a0 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -248,6 +248,18 @@
 			status = "disabled";
 		};
 
+		usb2_phy0: usb-phy@ee080200 {
+			compatible = "renesas,usb2-phy-r8a77990",
+				     "renesas,rcar-gen3-usb2-phy";
+			reg = <0 0xee080200 0 0x700>;
+			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 703>;
+			power-domains = <&sysc 32>;
+			resets = <&cpg 703>;
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
 		gic: interrupt-controller@f1010000 {
 			compatible = "arm,gic-400";
 			#interrupt-cells = <3>;
-- 
1.9.1


^ permalink raw reply related

* [PATCH 4/4] arm64: dts: renesas: r8a77990: ebisu: enable USB2.0 host
From: Yoshihiro Shimoda @ 2018-06-06  6:56 UTC (permalink / raw)
  To: horms, magnus.damm, robh+dt, mark.rutland
  Cc: devicetree, linux-renesas-soc, Yoshihiro Shimoda
In-Reply-To: <1528268171-19276-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>

This patch enables USB2.0 host (EHCI/OHCI) for r8a77990 Ebisu board.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 1be10c9..76fa244 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -47,10 +47,18 @@
 	};
 };
 
+&ehci0 {
+	status = "okay";
+};
+
 &extal_clk {
 	clock-frequency = <48000000>;
 };
 
+&ohci0 {
+	status = "okay";
+};
+
 &pfc {
 	avb_pins: avb {
 		mux {
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH V2 2/2] arm: dts: sunxi: Add missing cooling device properties for CPUs
From: Maxime Ripard @ 2018-06-06  6:58 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Viresh Kumar, arm-soc, Rob Herring, Mark Rutland, Vincent Guittot,
	ionela.voinescu, Daniel Lezcano, chris.redpath, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <CAGb2v65EX2RwKjxPw+EK4i6rkfgOJoCU2amuFkF-tHQT40isxw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

On Wed, Jun 06, 2018 at 12:02:20AM +0800, Chen-Yu Tsai wrote:
> On Tue, Jun 5, 2018 at 3:11 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Tue, Jun 05, 2018 at 10:17:49AM +0530, Viresh Kumar wrote:
> >> The cooling device properties, like "#cooling-cells" and
> >> "dynamic-power-coefficient", should either be present for all the CPUs
> >> of a cluster or none. If these are present only for a subset of CPUs of
> >> a cluster then things will start falling apart as soon as the CPUs are
> >> brought online in a different order. For example, this will happen
> >> because the operating system looks for such properties in the CPU node
> >> it is trying to bring up, so that it can register a cooling device.
> >>
> >> Add such missing properties.
> >>
> >> Fix other missing properties (clocks, OPP, clock latency) as well to
> >> make it all work.
> >>
> >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >
> > Applied both, thanks!
> 
> Please fix the "ARM" prefix when applying. :)

Done, thanks for the reminder :)

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] dt-bindings: display: renesas: du: document R8A77980 bindings
From: Geert Uytterhoeven @ 2018-06-06  7:04 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Sergei Shtylyov, David Airlie, DRI Development, Linux-Renesas,
	Rob Herring
In-Reply-To: <4427404.jlkXCGIJ77@avalon>

Hi Laurent,

On Tue, Jun 5, 2018 at 10:24 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday, 5 June 2018 22:49:57 EEST Sergei Shtylyov wrote:
>> On 06/05/2018 10:16 PM, Laurent Pinchart wrote:
>> >>>> Document the R-Car V3H (R8A77980) SoC in the R-Car DU bindings; the DU
>> >>>> hardware seems the same as in the R-Car V3M (R8A77970).
>> >>>
>> >>> How about "the DU hardware has the same topology as in the R-Car V3M
>> >>> (R8A77970)" ? "seems" sounds like we're very unsure :-)
>> >>
>> >> That's probably better, indeed.
>> >>
>> >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>> >>>>
>> >>>> ---
>> >>>> The patch is against the 'drm-next' branch of David Airlie's
>> >>>> 'linux.git' repo.
>> >>>
>> >>> Then you might want to switch to git://anongit.freedesktop.org/drm/drm
>> >>> :-)
>> >>
>> >> No, the corresponding MAINTAINERS records don't include
>> >> drivers/gpu/drm/rcar-du/ or worse yet, the DU bindings. :-)
>>
>> Well, there is still no corresponding record, actually...
>>
>> > My point is that Dave's tree has moved.
>>
>> How am I supposed to learn about it, from gossips? :-)
>
> I learnt it the hard way when a pull request I had prepared against the old
> tree conflicted (at compile time) with patches merged in the new tree. This
> broke the build, and I learnt about the new tree from a kbuild bot report.

Updated repo list for renesas-drivers.
Thanks for letting me know!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v3 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device
From: Boris Brezillon @ 2018-06-06  7:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: Stefan Agner, dwmw2, computersforpeace, marek.vasut, mark.rutland,
	thierry.reding, dev, miquel.raynal, richard, marcel, krzk, digetx,
	benjamin.lindqvist, jonathanh, pdeschrijver, pgaikwad, mirza.krak,
	linux-mtd, linux-tegra, devicetree, linux-kernel
In-Reply-To: <20180605201102.GA22487@rob-hp-laptop>

Hi Rob,

On Tue, 5 Jun 2018 14:11:02 -0600
Rob Herring <robh@kernel.org> wrote:

> On Fri, Jun 01, 2018 at 12:16:33AM +0200, Stefan Agner wrote:
> > Allow to define a NAND chip as a boot device. This can be helpful
> > for the selection of the ECC algorithm and strength in case the boot
> > ROM supports only a subset of controller provided options.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> >  Documentation/devicetree/bindings/mtd/nand.txt | 4 ++++
> >  drivers/mtd/nand/raw/nand_base.c               | 3 +++
> >  include/linux/mtd/rawnand.h                    | 6 ++++++
> >  3 files changed, 13 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> > index 8bb11d809429..8daf81b9748c 100644
> > --- a/Documentation/devicetree/bindings/mtd/nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> > @@ -43,6 +43,10 @@ Optional NAND chip properties:
> >  		     This is particularly useful when only the in-band area is
> >  		     used by the upper layers, and you want to make your NAND
> >  		     as reliable as possible.
> > +- nand-is-boot-medium: Whether the NAND chip is a boot medium. Drivers might use
> > +		       this information to select ECC algorithms supported by
> > +		       the boot ROM or similar restrictions.
> > +  
> 
> Shouldn't this be a partition level option? You could conceivably do one 
> ECC type for boot area and something else for the rest of the NAND.

I tried that a long time ago [1]. The result was far from perfect. I'm
not saying it's impossible to do it, but it definitely requires a lot
of work if we want to do it properly.

Also, what about boards that are not defining their partitions in the
DT but through the command line (using mtdparts)? That means patching
the mtdparts part parser to also take the ECC setup into account.

Regards,

Boris

[1]http://lists.infradead.org/pipermail/linux-mtd/2015-July/060600.html

^ permalink raw reply

* Re: [PATCH v4 2/6] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-06-06  7:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: Matti Vaittinen, Matti Vaittinen, Michael Turquette, Stephen Boyd,
	Mark Rutland, Lee Jones, Liam Girdwood, Mark Brown, linux-clk,
	devicetree, linux-kernel@vger.kernel.org, mikko.mutanen,
	heikki.haikola
In-Reply-To: <CAL_JsqL3Z_o2CFeYDvb3PqgiskTSHuOVqwdUP+gFymLDTKhn2g@mail.gmail.com>

On Tue, Jun 05, 2018 at 10:46:14AM -0500, Rob Herring wrote:
> On Mon, Jun 4, 2018 at 6:32 AM, Matti Vaittinen
> <mazziesaccount@gmail.com> wrote:
> > On Fri, Jun 01, 2018 at 12:32:16PM -0500, Rob Herring wrote:
> >> On Fri, Jun 1, 2018 at 1:25 AM, Matti Vaittinen
> >> <mazziesaccount@gmail.com> wrote:
> >> > On Thu, May 31, 2018 at 09:07:24AM -0500, Rob Herring wrote:
> >> >> On Thu, May 31, 2018 at 5:23 AM, Matti Vaittinen
> >> >> <mazziesaccount@gmail.com> wrote:
> >> >> > On Thu, May 31, 2018 at 10:17:17AM +0300, Matti Vaittinen wrote:
> >> >> >> On Wed, May 30, 2018 at 10:01:29PM -0500, Rob Herring wrote:
> >> >> >> > On Wed, May 30, 2018 at 11:42:03AM +0300, Matti Vaittinen wrote:
> >> >> >> > > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> >> >> >> > > + - interrupts            : The interrupt line the device is connected to.
> >> >> >> > > + - interrupt-controller  : Marks the device node as an interrupt controller.
> >> >> >> >
> >> >> >> > What sub blocks have interrupts?
> >> >> >>
> >> >> >> The PMIC can generate interrupts from events which cause it to reset.
> >> >> >> Eg, irq from watchdog line change, power button pushes, reset request
> >> >> >> via register interface etc. I don't know any generic handling for these
> >> >> >> interrupts. In "normal" use-case this PMIC is powering the processor
> >> >> >> where driver is running and I do not see reasonable handling because
> >> >> >> power-reset is going to follow the irq.
> >> >> >>
> >> >> >
> >> >> > Oh, but when reading this I understand that the interrupt-controller
> >> >> > property should at least be optional.
> >> >>
> >> >> I don't think it should. The h/w either has an interrupt controller or
> >> >> it doesn't.
> >> >
> >> > I hope this explains why I did this interrupt controller - please tell
> >> > me if this is legitimate use-case and what you think of following:
> >> >
> >> > +Optional properties:
> >> > + - interrupt-controller        : Marks the device node as an interrupt controller.
> >> > +                         BD71837MWV can report different power state change
> >> > +                         events to other devices. Different events can be seen
> >> > +                         as separate BD71837 domain interrupts.
> >>
> >> To what other devices?
> >
> > Would it be better if I wrote "other drivers" instead? I think I've seen
> > examples where MFD driver is just providing the interrupts for other
> > drivers - like power-button input driver. Currently I have no such "irq
> > consumer" drivers written. Still I would like to expose these interrupts
> > so that they are ready for using if any platform using PMIC needs them.
> 
> No, worse. Interrupt binding describes interrupt connections between a
> controller and devices (which could be sub-blocks in a device), not to
> drivers.

Ok.
 
> I'm just curious as to what sub-blocks/devices there are. You don't
> have to have a driver (yet) to define the devices.

Right. I should have done this from the start. I just thought everyone
is busy with other things and pushing people to read data sheets might
be considered as lazines. "Go and read from data sheet what my driver
does, I am too lazy to try to explain what I am doing" - type of thing
you know... But as people asked me for more information about HW:

Datasheet:
https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
(Would it be good idea to add this link to comments in MFD driver or to
binding document(s)?) Page 8 contains roughly the same picture I drew
below. Page 69 shows the interrupt registers. And for interested ones,
HW state transitions are described on page 24.

                    +--------------------------------------------------+
                    |                                                  |
VSYS           +-----------------+                    +-----------+    |
                    |            |                    |           |    |
                    |  +-------+ |                    | BUCKS 1-4 +-------->
                    |  |       | |                    |           |    |
I2C IF         +------->   H   | +--------------------+  DVS      +-------->
                    |  |   O   | |                    |  Support  |    |
PWRON_B        +------->   S   | |                    |           +-------->
                    |  |   T   | |                    |           |    |
PMIC_STBY_REQ  +------->       | |                    |           +-------->
                    |  |   I   | |                    |           |    |
PMIC_ON_REQ    +------->   /   | |                    +-----------+    |
                    |  |   F   | |                                     |
WDOG_B         +------->       | |                    +-----------+    |
                    |  |       | |                    |           +-------->
                    |  |       | +--------------------+ BUCKS 5,8 |    |
                    |  |       | |                    |           +-------->
                    |  |       | |                    +-----------+    |
                    |  |       | |                                     |
                    |  |       | |                     +----------+    |
IRQ_OUT        <-------+       | |                     |          |    |
                    |  |       | +---------------------+ LDO1     +-------->
C32K_OUT       <-------+       | |                     |          |    |
                    |  |       | |                     +----------+    |
                    |  |       | |                                     |
                    |  |       | |                     +----------+    |
                    |  |       | |                     |          |    |
                    |  |       | +---------------------+ LDO2     +-------->
                    |  |       | |                     |          |    |
                    |  |       | |                     +----------+    |
                    |  |       | |                                     |
                    |  |       | |                     +----------+    |
                    |  |       | |                     |          |    |
                    |  |       | +---------------------+ LDO7     +-------->
                    |  +-------+ |                     |          |    |
                    |            |                     +----------+    |
                    |            |                                     |
                    |            | +----------+ +-------------------------->
                    |            | |          | |                      |
                    |            +-+ BUCK6    +-+      +----------+    |
                    |            | |          | |      |          |    |
                    |            | +----------+ +------> LDO5     +-------->
                    |            |              |      |          |    |
                    |            |              |      +----------+    |
                    |            |              |                      |
                    |  +-------+ |              |      +----------+    |
                    |  |       | |              o      |          |    |
 XIN         +---------+ 32K   | |               \-----> LDO3     +-------->
                    |  |Crystal| +--------------o      |          |    |
                    |  |Driver | |  +---------+        +----------+    |
 XOUT        +---------+       | |  |         |                        |
                    |  |       | +--+ BUCK7   +-+-------------------------->
                    |  +-------+ |  |         | |                      |
                    |            |  +---------+ |      +-----------+   |
                    |            |              |      |           |   |
                    |            |              +------>   LDO6    +------->
                    |            |              |      |           |   |
                    |            |              |      +-----------+   |
                    |            |              |                      |
                    |            |              |      +-----------+   |
                    |            |              o      |           |   |
                    |            |               \----->   LDO4    +------->
                    |            +--------------o      |           |   |
                    |                                  +-----------+   |
                    |                                                  |
                    +--------------------------------------------------+

On the left we see input lines to PMIC. PWRON_B intended to be connected
to power button. PMIC_STBY_REQ and PMIC_ON_REQ lines for controlling HW
state machine PMIC has. And WDOG_B from watch dog.

PMIC has control register for controlling what happens to BUCK/LDO
outputs when input line states change. PMIC reports change in input
lines via the IRQ_OUT line and IRQ status register.

So HW mapping for interrup(s) from PMIC would be:

(HW) event => BD71837 domain IRQ

PMIC_STBY_REQ line level change		=> 0
PMIC_ON_REQ line level change		=> 1
PMIC_WDOG_B line level change		=> 2
PMIC_PWRON_B line level change		=> 3
PMIC_PWRON_B line/long push detected	=> 4
PMIC_PWRON_B line/short push detected	=> 5
SWRESET register on PMIC written	=> 6

> > "The BD71837 driver only provides the infrastructure for the IRQs. The
> > users can write his own driver to convert the IRQ into the event they
> > wish. The IRQ can be used with the standard
> > request_irq/enable_irq/disable_irq API inside the kernel." (I found this
> > text from NXP forums and ruthlessly copied and modified it over here)
> 
> That's all OS details that have nothing to do with the binding. The
> binding describes the h/w.

Right. I'll drop it.

> 
> > If this is not feasible, then I will remove the irq handling from MFD
> > (or leave code there but remove the binding information?) as I don't
> > know what the irq handles should do in generic case.
> 
> I don't understand what you mean by generic. An IRQ has to be wired to
> something. The only generic IRQs are GPIOs.

By generic case I mean for example when PMIC_WDOG_B line changes. In
example use-case I have seen, this would be cutting the power from
processor. But this is not necessarily the case. This can be configured
from PMIC register so that action can be warm or cold reset, or no
action. Finally, I'd rather not expect that the BUCKs are supplying
power to processor which is controlling the PMIC. Thus I do not know how
to do generic _handler_ for these interrupts.

So from PMIC HW point of view I know that the interrupt is tied to
PMIC_WDOG_B line change. And this can be described in binding document.
(I tried doing this to v5 patch). Still from system/SW point of view I
don't know what action should be taken (or by which driver) when such
change happens. Hence I liked the idea of hiding the irq register
details in MFD driver by declaring it as interrupt controller - and
leaving the interrupts to be used by what ever drivers need the change
information is system the PMIC is used.

> >> > + - #interrupt-cells    : The number of cells to describe an IRQ should be 1.
> >> > +                           The first cell is the IRQ number.
> >> > +                           masks from ../interrupt-controller/interrupts.txt.
> >
> > Sorry this "masks from ../interrupt-controller/interrupts.txt." was
> > accidentally pasted here. I should have deleted it.
> >
> >> I'm still not clear. Generally in a PMIC, you'd define an interrupt
> >> controller when there's a common set of registers to manage sub-block
> >> interrupts (typical mask/unmask, ack regs) and the subblocks
> >> themselves have control of masking/unmasking interrupts. If there's
> >> not a need to have these 2 levels of interrupt handling, then you
> >> don't really need to define an interrupt controller.
> >
> > And to clarify - the PMIC can generate irq via one irq line. This is
> > typical ACTIVE_LOW irq with 8 bit "write 1 to clear" status register and
> > 8 bit mask register. The role of interrupt-controller code here is just
> > to allow these 8 irq reasons to be seen as individual BD71837 domain
> > interrupts. I just don't have the driver(s) for handling these
> > interrupts.
> 
> If what I'm asking for above is still not clear, what are the 8 bits
> defined as or what are those 8 lines connected to?

I am sorry - there were only 7 bits. One bit was unused. I hope my
explanation abowe did clarify this.

> >  Or should I just
> > somehow state that irq X in BD71837 is a "power button short push"
> > event and power button driver should be the consumer for it?
> 
> Yes, at least, but who is the consumer is an OS detail that is not
> relevant to the binding. Ideally, you would describe the node with the
> interrupts property for "irq X".

I think I need to try changing my mind set. I tend to think the DT nodes
are for drivers so that drivers can get the information they need. An as
I don't know what kind of driver would be handling the irq, I don't know
what kind of DT node would be good for it. Hence I would rather leave
constructing the node who consumes the IRQ to someone who knows what
they want to do with this IRQ information.

> 
> > Rest of the interrupts are not so obvious. I have no idea how I should
> > handle rest of the interrupts. Those are interrupts which cause the PMIC
> > to reset and cut the powers from most of the regulators too. I can
> > easily think setup where one processor is controlling PMIC which powers
> > for the other processor. And getting IRQ if for example watchdog reset
> > the other processor would probably be very usefull. But doing any
> > 'de-facto' handler for this is hard. Only generally usefull thing would
> > be notifying the user-space but I don't think I should invent any new
> > kernelspace - userspace interfaces for this. I believe that when such
> > are needed those should be implemented by ones knowing the platform.
> 
> Don't think about the OS or driver details. Think about sub-blocks of
> the hardware and the connections between them (like irqs) and to board
> that need to be described in DT.
> 
> If you can't describe that, then you just probably shouldn't have
> sub-nodes in DT (ever).

This is why I did not add any "irq consumer" nodes in example DT. But I
believe someone can think of a board/setup where such are needed. Thus I
liked the idea of providing the interrupt-controller.

> >
> > So please bear with me but do you mean I should
> > a) document what conditions generate which IRQ
> >    or
> > b)  should I tell what kind of driver is needed for handling the IRQs
> >    or
> > c) should I first write code using IRQs before addinf MFD binding?
> 
> A.


So what do you think of this:

+Optional properties:
+ - interrupt-controller	: Marks the device node as an interrupt controller.
+			  BD71837MWV can report input line state change and SW
+			  reset events via interrupts. Different events can be seen
+			  as separate BD71837 domain interrupts.
+ - #interrupt-cells	: The number of cells to describe an IRQ should be 1.
+			  The value in cell is the IRQ number.
+			    Meaningfull numbers are:
+			      0 => PMIC_STBY_REQ level change
+			      1 => PMIC_ON_REQ level change
+			      2 => WDOG_B level change
+			      3 => Power Button level change
+			      4 => Power Button Long Push
+			      5 => Power Button Short Push
+			      6 => SWRESET register is written 1

Would this be getting closer to what is needed from binding document?

Oh, and thanks for the patience =)

Br,
	Matti Vaittinen

^ permalink raw reply

* Re: [PATCH v7 3/3] gpio: pca953x: fix address calculation for pcal6524
From: Pavel Machek @ 2018-06-06  7:35 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Andy Shevchenko, Kumar Gala, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Linus Walleij, Alexandre Courbot,
	devicetree, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
	Discussions about the Letux Kernel, kernel
In-Reply-To: <A639885E-0E5D-4FB0-BE51-AFF35BF4F76B@computer.org>

[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]

On Wed 2018-06-06 07:33:32, H. Nikolaus Schaller wrote:
> Hi,
> 
> > Am 05.06.2018 um 22:39 schrieb Pavel Machek <pavel@ucw.cz>:
> > 
> > On Tue 2018-06-05 18:37:21, Andy Shevchenko wrote:
> >> On Wed, May 23, 2018 at 5:06 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >>> On Thu 2018-05-17 06:59:49, H. Nikolaus Schaller wrote:
> >>>> The register constants are so far defined in a way that they fit
> >>>> for the pcal9555a when shifted by the number of banks, i.e. are
> >>>> multiplied by 2 in the accessor function.
> >>>> 
> >>>> Now, the pcal6524 has 3 banks which means the relative offset
> >>>> is multiplied by 4 for the standard registers.
> >>>> 
> >>>> Simply applying the bit shift to the extended registers gives
> >>>> a wrong result, since the base offset is already included in
> >>>> the offset.
> >>>> 
> >>>> Therefore, we have to add code to the 24 bit accessor functions
> >>>> that adjusts the register number for these exended registers.
> >>>> 
> >>>> The formula finally used was developed and proposed by
> >>>> Andy Shevchenko <andy.shevchenko@gmail.com>.
> >> 
> >>>>      int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
> >>>> +     int addr = (reg & PCAL_GPIO_MASK) << bank_shift;
> >>>> +     int pinctrl = (reg & PCAL_PINCTRL_MASK) << 1;
> >> 
> >>> Is this reasonable to do on each register access? Compiler will not be
> >>> able to optimize out fls and shifts, right?
> >> 
> >> On modern CPUs fls() is one assembly command. OTOH, any proposal to do
> >> this better?
> >> 
> >> What I can see is that bank_shift is invariant to the function, and
> >> maybe cached.
> > 
> > Yes, I thought that caching bank_shift might be good idea. I thought
> > it was constant for given chip...
> 
> Yes, it is an f(chip), but the question that comes to my mind is if
> optimization is worth any effort. This is an accessor method over

It will also be less ugly. Copy&pasted complex exprepsion all over the
driver is not nice.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v3 6/6] regulator: bd71837: BD71837 PMIC regulator driver
From: Matti Vaittinen @ 2018-06-06  7:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Matti Vaittinen, Michael Turquette, Stephen Boyd, Rob Herring,
	Mark Rutland, Lee Jones, Liam Girdwood, Mark Brown,
	mazziesaccount, linux-clk, devicetree, Linux Kernel Mailing List,
	mikko.mutanen, heikki.haikola
In-Reply-To: <CAHp75VeVSbZd12vXew9f0z2m1=BFz+cOKomga3SZEzg=kx9CFw@mail.gmail.com>


On Tue, Jun 05, 2018 at 04:58:43PM +0300, Andy Shevchenko wrote:
> On Tue, May 29, 2018 at 1:02 PM, Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> > Support for controlling the 8 bucks and 7 LDOs the PMIC contains.
> 

Thanks for the comments Andy. The regulator part of patch set v4 was
already applied by Mark but I am going to do some further work on this
afer I get the MFD and clk portions done. I'll store these comments and
address issues in next set of patches.

Br,
	Matti Vaittinen

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: renesas: condor: add I2C0 support
From: Geert Uytterhoeven @ 2018-06-06  7:46 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Magnus Damm, Catalin Marinas, Will Deacon, Rob Herring,
	Linux-Renesas, Simon Horman, linux-arm-kernel@lists.infradead.org
In-Reply-To: <61f6f4a4-e55c-06e0-cba1-7d90a556950a@cogentembedded.com>

Hi Sergei,

On Mon, May 28, 2018 at 10:14 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Define the Condor board dependent part of the I2C0 device node.
>
> The I2C0 bus is populated by 2 ON Semiconductor PCA9654 I/O expanders
> and Analog Devices  ADV7511W HDMI transmitter (but we're only describing
> the former chips now).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Suggestion for future improvement below.

> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> @@ -80,6 +80,28 @@
>         clock-frequency = <32768>;
>  };
>
> +&i2c0 {
> +       pinctrl-0 = <&i2c0_pins>;
> +       pinctrl-names = "default";
> +
> +       status = "okay";
> +       clock-frequency = <400000>;
> +
> +       io_expander0: gpio@20 {
> +               compatible = "onnn,pca9654";
> +               reg = <0x20>;
> +               gpio-controller;
> +               #gpio-cells = <2>;

The bindings don't mention this (the example does have it) the optional
interrupts prototype. As this is wired to INTC-EX IRQ4, you may want to
add that. Perhaps later, as r8a77980.dtsi doesn't have INTC-EX yet.

> +       };
> +
> +       io_expander1: gpio@21 {
> +               compatible = "onnn,pca9654";
> +               reg = <0x21>;
> +               gpio-controller;
> +               #gpio-cells = <2>;

Same for IRQ5.

> +       };
> +};

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox