[PATCH 0/4] ARM: shmobile: CPUFreq support on Lager

[PATCH 0/4] ARM: shmobile: CPUFreq support on Lager

[Guennadi Liakhovetski]

On Lager a da9210 regulator is used to supply DVFS power to the SoC. This
patch series adds I2C and Z-clock support on r8a7790 and CPUFreq support
for the Lager board. Please note, that patch 4/4 is an RFC as explained
inline.

Developed on top of -next of 09.09.2013 with my recent shmobile patches,
including "ARM: shmobile: lager: disable MMCIF Command Completion Signal,
add CLK_CTRL2" and my two earlier patch series from today:

[PATCH 0/2] regulator: da9210: support Device Tree initialisation
[PATCH 0/5] i2c: rcar: Device Tree support and clock improvements

If these patches are used without da9210 DT support, the driver won't
initialise correctly, otherwise applying these patches without the above
dependencies will compile and shouldn't do any harm.

Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Guennadi Liakhovetski (4):
  pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface
  ARM: shmobile: r8a7790: add I2C support in Device Tree mode
  ARM: shmobile: r8a7790: add CPUFreq clock support
  ARM: shmobile: lager: add CPUFreq support

 arch/arm/boot/dts/r8a7790-lager-reference.dts  |   32 +++++
 arch/arm/boot/dts/r8a7790.dtsi                 |   36 ++++++
 arch/arm/mach-shmobile/Kconfig                 |    2 +
 arch/arm/mach-shmobile/board-lager-reference.c |    4 +-
 arch/arm/mach-shmobile/clock-r8a7790.c         |  160 ++++++++++++++++++++++++
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c           |   28 ++++
 6 files changed, 261 insertions(+), 1 deletions(-)

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Guennadi Liakhovetski]

There are four I2C interfaces on r8a7790, each of them can be connected to
one of the two respective I2C controllers, e.g. interface #0 can be
configured to work with I2C0 or with IIC0. Additionally some of those
interfaces can also use one of several pin sets. Interface #3 is special,
because it can be used in automatic mode for DVFS. It only has one set
of pins available and those pins cannot be used for anything else, they
also lack the GPIO function.

This patch uses the sh-pfc ability to configure pins, not associated with
GPIOs and adds support for I2C3 to the r8a7790 PFC set up.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index 64fcc006..c3c4d9b 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -781,6 +781,8 @@ enum {
 	ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
 	USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
 	TCLK1_B_MARK,
+
+	I2C3_SCL_MARK, I2C3_SDA_MARK,
 	PINMUX_MARK_END,
 };
 
@@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
 	PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
 	PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
 	PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
+
+	PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
+	PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),
 };
 
+/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
+#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
+#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
+#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
+
 static struct sh_pfc_pin pinmux_pins[] = {
 	PINMUX_GPIO_GP_ALL(),
+
+	/* Pins not associated with a GPIO port */
+	SH_PFC_PIN_NAMED(ROW_GROUP_A('J'), 15, AJ15),
+	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
 };
 
 /* - DU RGB ----------------------------------------------------------------- */
@@ -1990,6 +2004,14 @@ static const unsigned int hscif1_ctrl_b_pins[] = {
 static const unsigned int hscif1_ctrl_b_mux[] = {
 	HRTS1_N_B_MARK, HCTS1_N_B_MARK,
 };
+/* - I2C3 ------------------------------------------------------------------- */
+static const unsigned int i2c3_pins[] = {
+	/* SCL, SDA */
+	PIN_A_NUMBER('J', 15), PIN_A_NUMBER('H', 15),
+};
+static const unsigned int i2c3_mux[] = {
+	I2C3_SCL_MARK, I2C3_SDA_MARK,
+};
 /* - INTC ------------------------------------------------------------------- */
 static const unsigned int intc_irq0_pins[] = {
 	/* IRQ */
@@ -3047,6 +3069,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(hscif1_data_b),
 	SH_PFC_PIN_GROUP(hscif1_clk_b),
 	SH_PFC_PIN_GROUP(hscif1_ctrl_b),
+	SH_PFC_PIN_GROUP(i2c3),
 	SH_PFC_PIN_GROUP(intc_irq0),
 	SH_PFC_PIN_GROUP(intc_irq1),
 	SH_PFC_PIN_GROUP(intc_irq2),
@@ -3243,6 +3266,10 @@ static const char * const hscif1_groups[] = {
 	"hscif1_ctrl_b",
 };
 
+static const char * const i2c3_groups[] = {
+	"i2c3",
+};
+
 static const char * const intc_groups[] = {
 	"intc_irq0",
 	"intc_irq1",
@@ -3469,6 +3496,7 @@ static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(eth),
 	SH_PFC_FUNCTION(hscif0),
 	SH_PFC_FUNCTION(hscif1),
+	SH_PFC_FUNCTION(i2c3),
 	SH_PFC_FUNCTION(intc),
 	SH_PFC_FUNCTION(mmc0),
 	SH_PFC_FUNCTION(mmc1),
-- 
1.7.2.5

[PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Guennadi Liakhovetski]

This patch adds clocks and clock lookup entries for the four I2C
controllers on r8a7790 and respective Device Tree nodes.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 arch/arm/boot/dts/r8a7790.dtsi         |   36 ++++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 885f9f4..a5021112 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -127,6 +127,42 @@
 		interrupts = <0 0 4>, <0 1 4>, <0 2 4>,	<0 3 4>;
 	};
 
+	i2c0: i2c@e6508000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "renesas,i2c-rcar-h2";
+		reg = <0 0xe6508000 0 0x40>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 287 0x4>;
+	};
+
+	i2c1: i2c@e6518000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "renesas,i2c-rcar-h2";
+		reg = <0 0xe6518000 0 0x40>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 288 0x4>;
+	};
+
+	i2c2: i2c@e6530000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "renesas,i2c-rcar-h2";
+		reg = <0 0xe6530000 0 0x40>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 286 0x4>;
+	};
+
+	i2c3: i2c@e6540000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "renesas,i2c-rcar-h2";
+		reg = <0 0xe6540000 0 0x40>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 290 0x4>;
+	};
+
 	mmcif0: mmcif@ee200000 {
 		compatible = "renesas,sh-mmcif";
 		reg = <0 0xee200000 0 0x80>;
diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index fc36d3d..8e5e90b 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -52,6 +52,7 @@
 #define SMSTPCR5 0xe6150144
 #define SMSTPCR7 0xe615014c
 #define SMSTPCR8 0xe6150990
+#define SMSTPCR9 0xe6150994
 
 #define SDCKCR		0xE6150074
 #define SD2CKCR		0xE6150078
@@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = {
 
 /* MSTP */
 enum {
+	MSTP931, MSTP930, MSTP929, MSTP928,
 	MSTP813,
 	MSTP721, MSTP720,
 	MSTP717, MSTP716,
@@ -192,6 +194,10 @@ enum {
 };
 
 static struct clk mstp_clks[MSTP_NR] = {
+	[MSTP931] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 31, 0), /* I2C0 */
+	[MSTP930] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 30, 0), /* I2C1 */
+	[MSTP929] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 29, 0), /* I2C2 */
+	[MSTP928] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 28, 0), /* I2C3 */
 	[MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */
 	[MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
 	[MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
@@ -261,6 +267,10 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
 	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
 	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
+	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
+	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
+	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
+	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
 	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
 	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
 	CLKDEV_DEV_ID("ee200000.mmcif", &mstp_clks[MSTP315]),
-- 
1.7.2.5

[PATCH 3/4] ARM: shmobile: r8a7790: add CPUFreq clock support

[Guennadi Liakhovetski]

Add support for the Z clock on r8a7790, driving the four SoC's CA15 cores,
and its parent - PLL0. This is required for CPUFreq support on this SoC.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 arch/arm/mach-shmobile/Kconfig         |    2 +
 arch/arm/mach-shmobile/clock-r8a7790.c |  150 ++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 1f94c31..b9422f2 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -100,6 +100,8 @@ config ARCH_R8A7790
 	select CPU_V7
 	select SH_CLK_CPG
 	select RENESAS_IRQC
+	select ARCH_HAS_CPUFREQ
+	select ARCH_HAS_OPP
 
 config ARCH_EMEV2
 	bool "Emma Mobile EV2"
diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index 8e5e90b..3ef043e 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -54,9 +54,12 @@
 #define SMSTPCR8 0xe6150990
 #define SMSTPCR9 0xe6150994
 
+#define FRQCRB		0xE6150004
 #define SDCKCR		0xE6150074
 #define SD2CKCR		0xE6150078
 #define SD3CKCR		0xE615007C
+#define FRQCRC		0xE61500E0
+#define PLLECR		0xE61500D0
 #define MMC0CKCR	0xE6150240
 #define MMC1CKCR	0xE6150244
 #define SSPCKCR		0xE6150248
@@ -85,6 +88,7 @@ static struct clk main_clk = {
  * clock ratio of these clock will be updated
  * on r8a7790_clock_init()
  */
+SH_FIXED_RATIO_CLK_SET(pll0_clk,		main_clk,	1, 1);
 SH_FIXED_RATIO_CLK_SET(pll1_clk,		main_clk,	1, 1);
 SH_FIXED_RATIO_CLK_SET(pll3_clk,		main_clk,	1, 1);
 SH_FIXED_RATIO_CLK_SET(lb_clk,			pll1_clk,	1, 1);
@@ -113,15 +117,155 @@ SH_FIXED_RATIO_CLK_SET(zb3d2_clk,		pll3_clk,	1, 8);
 SH_FIXED_RATIO_CLK_SET(ddr_clk,			pll3_clk,	1, 8);
 SH_FIXED_RATIO_CLK_SET(mp_clk,			pll1_div2_clk,	1, 15);
 
+/* Locking not needed yet, only one clock is using FRQCR[BC] divisors so far */
+static atomic_t frqcr_lock;
+#define CPG_MAP(o) ((o) - CPG_BASE + cpg_mapping.base)
+
+/* Several clocks need to access FRQCRB, have to lock */
+static bool frqcr_kick_check(struct clk *clk)
+{
+	return !(ioread32(CPG_MAP(FRQCRB)) & BIT(31));
+}
+
+static int frqcr_kick_do(struct clk *clk)
+{
+	int i;
+
+	/* set KICK bit in FRQCRB to update hardware setting, check success */
+	iowrite32(ioread32(CPG_MAP(FRQCRB)) | BIT(31), CPG_MAP(FRQCRB));
+	for (i = 1000; i; i--)
+		if (ioread32(CPG_MAP(FRQCRB)) & BIT(31))
+			cpu_relax();
+		else
+			return 0;
+
+	return -ETIMEDOUT;
+}
+
+static int zclk_set_rate(struct clk *clk, unsigned long rate)
+{
+	void __iomem *frqcrc;
+	int ret;
+	unsigned long step, p_rate;
+	u32 val;
+
+	if (!clk->parent || !__clk_get(clk->parent))
+		return -ENODEV;
+
+	if (!atomic_inc_and_test(&frqcr_lock) || !frqcr_kick_check(clk)) {
+		ret = -EBUSY;
+		goto done;
+	}
+
+	/*
+	 * Users are supposed to first call clk_set_rate() only with
+	 * clk_round_rate() results. So, we don't fix wrong rates here, but
+	 * guard against them anyway
+	 */
+
+	p_rate = clk_get_rate(clk->parent);
+	if (rate == p_rate) {
+		val = 0;
+	} else {
+		step = DIV_ROUND_CLOSEST(p_rate, 32);
+
+		if (rate > p_rate || rate < step) {
+			ret = -EINVAL;
+			goto done;
+		}
+
+		val = 32 - rate / step;
+	}
+
+	frqcrc = clk->mapped_reg + (FRQCRC - (u32)clk->enable_reg);
+
+	iowrite32((ioread32(frqcrc) & ~(clk->div_mask << clk->enable_bit)) |
+		  (val << clk->enable_bit), frqcrc);
+
+	ret = frqcr_kick_do(clk);
+
+done:
+	atomic_dec(&frqcr_lock);
+	__clk_put(clk->parent);
+	return ret;
+}
+
+static long zclk_round_rate(struct clk *clk, unsigned long rate)
+{
+	/*
+	 * theoretical rate = parent rate * multiplier / 32,
+	 * where 1 <= multiplier <= 32. Therefore we should do
+	 * multiplier = rate * 32 / parent rate
+	 * rounded rate = parent rate * multiplier / 32.
+	 * However, multiplication before division won't fit in 32 bits, so
+	 * we sacrifice some precision by first dividing and then multiplying.
+	 * To find the nearest divisor we calculate both and pick up the best
+	 * one. This avoids 64-bit arithmetics.
+	 */
+	unsigned long step, mul_min, mul_max, rate_min, rate_max;
+
+	rate_max = clk_get_rate(clk->parent);
+
+	/* output freq <= parent */
+	if (rate >= rate_max)
+		return rate_max;
+
+	step = DIV_ROUND_CLOSEST(rate_max, 32);
+	/* output freq >= parent / 32 */
+	if (step >= rate)
+		return step;
+
+	mul_min = rate / step;
+	mul_max = DIV_ROUND_UP(rate, step);
+	rate_min = step * mul_min;
+	if (mul_max == mul_min)
+		return rate_min;
+
+	rate_max = step * mul_max;
+
+	if (rate_max - rate <  rate - rate_min)
+		return rate_max;
+
+	return rate_min;
+}
+
+static unsigned long zclk_recalc(struct clk *clk)
+{
+	void __iomem *frqcrc = FRQCRC - (u32)clk->enable_reg + clk->mapped_reg;
+	unsigned int max = clk->div_mask + 1;
+	unsigned long val = ((ioread32(frqcrc) >> clk->enable_bit) &
+			     clk->div_mask);
+
+	return DIV_ROUND_CLOSEST(clk_get_rate(clk->parent), max) *
+		(max - val);
+}
+
+static struct sh_clk_ops zclk_ops = {
+	.recalc = zclk_recalc,
+	.set_rate = zclk_set_rate,
+	.round_rate = zclk_round_rate,
+};
+
+static struct clk z_clk = {
+	.parent = &pll0_clk,
+	.div_mask = 0x1f,
+	.enable_bit = 8,
+	/* We'll need to access FRQCRB and FRQCRC */
+	.enable_reg = (void __iomem *)FRQCRB,
+	.ops = &zclk_ops,
+};
+
 static struct clk *main_clks[] = {
 	&extal_clk,
 	&extal_div2_clk,
 	&main_clk,
+	&pll0_clk,
 	&pll1_clk,
 	&pll1_div2_clk,
 	&pll3_clk,
 	&lb_clk,
 	&qspi_clk,
+	&z_clk,
 	&zg_clk,
 	&zx_clk,
 	&zs_clk,
@@ -249,6 +393,9 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_CON_ID("qspi",		&qspi_clk),
 	CLKDEV_CON_ID("cp",		&cp_clk),
 
+	/* CPU clock */
+	CLKDEV_DEV_ID("cpufreq-cpu0",	&z_clk),
+
 	/* DIV4 */
 	CLKDEV_CON_ID("sdh",		&div4_clks[DIV4_SDH]),
 
@@ -291,6 +438,7 @@ static struct clk_lookup lookups[] = {
 #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
 	extal_clk.rate	= e * 1000 * 1000;			\
 	main_clk.parent	= m;					\
+	SH_CLK_SET_RATIO(&pll0_clk_ratio, p0 / 2, 1);		\
 	SH_CLK_SET_RATIO(&pll1_clk_ratio, p1 / 2, 1);		\
 	if (mode & MD(19))					\
 		SH_CLK_SET_RATIO(&pll3_clk_ratio, p31, 1);	\
@@ -303,6 +451,8 @@ void __init r8a7790_clock_init(void)
 	u32 mode = r8a7790_read_mode_pins();
 	int k, ret = 0;
 
+	atomic_set(&frqcr_lock, -1);
+
 	switch (mode & (MD(14) | MD(13))) {
 	case 0:
 		R8A7790_CLOCK_ROOT(15, &extal_clk, 172, 208, 106, 88);
-- 
1.7.2.5

[PATCH/RFC 4/4] ARM: shmobile: lager: add CPUFreq support

[Guennadi Liakhovetski]

The Lager board uses a DA9210 voltage regulator to supply DVFS power to the
CA15 cores on the r8a7790 SoC. This patch adds CPUFreq support for that
board using the cpufreq-cpu0 driver.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---

This patch is marked as RFC, because so far I don't have data about r8a7790
valid OPPs. The Lager system boots with 1.3GHz, supplying 1.0V to SoC's
DVFS. Apart from that I don't have information what voltage is required for
other clock frequencies. Here I'm using a "safe guess" of 0.95V for 700MHz,
but we might either wait with this patch until exact data is available, or
actually commit these values - this can be decided.

 arch/arm/boot/dts/r8a7790-lager-reference.dts  |   32 ++++++++++++++++++++++++
 arch/arm/mach-shmobile/board-lager-reference.c |    4 ++-
 2 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager-reference.dts b/arch/arm/boot/dts/r8a7790-lager-reference.dts
index c462ef1..7a41419 100644
--- a/arch/arm/boot/dts/r8a7790-lager-reference.dts
+++ b/arch/arm/boot/dts/r8a7790-lager-reference.dts
@@ -43,3 +43,35 @@
 		};
 	};
 };
+
+&i2c3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c3_pins>;
+
+	vdd_dvfs: da9210@68 {
+		compatible = "diasemi,da9210";
+		reg = <0x68>;
+
+		regulator-min-microvolt = <900000>;
+		regulator-max-microvolt = <1000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+};
+
+&cpu0 {
+	cpu0-supply = <&vdd_dvfs>;
+	operating-points = <
+		/* kHz  uV - OPs unknown yet */
+		1300000 1000000
+		 700000  950000
+	>;
+	voltage-tolerance = <1>; /* 1% */
+};
+
+&pfc {
+	i2c3_pins: i2c3 {
+		renesas,groups = "i2c3";
+		renesas,function = "i2c3";
+	};
+};
diff --git a/arch/arm/mach-shmobile/board-lager-reference.c b/arch/arm/mach-shmobile/board-lager-reference.c
index 9c316a1..442d854 100644
--- a/arch/arm/mach-shmobile/board-lager-reference.c
+++ b/arch/arm/mach-shmobile/board-lager-reference.c
@@ -20,6 +20,7 @@
 
 #include <linux/init.h>
 #include <linux/of_platform.h>
+#include <linux/platform_device.h>
 #include <mach/r8a7790.h>
 #include <asm/mach/arch.h>
 
@@ -29,7 +30,8 @@ static void __init lager_add_standard_devices(void)
 	r8a7790_clock_init();
 
 	r8a7790_add_dt_devices();
-        of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	platform_device_register_simple("cpufreq-cpu0", -1, NULL, 0);
 }
 
 static const char *lager_boards_compat_dt[] __initdata = {
-- 
1.7.2.5

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Linus Walleij]

On Mon, Sep 9, 2013 at 6:03 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:

> There are four I2C interfaces on r8a7790, each of them can be connected to
> one of the two respective I2C controllers, e.g. interface #0 can be
> configured to work with I2C0 or with IIC0. Additionally some of those
> interfaces can also use one of several pin sets. Interface #3 is special,
> because it can be used in automatic mode for DVFS. It only has one set
> of pins available and those pins cannot be used for anything else, they
> also lack the GPIO function.
>
> This patch uses the sh-pfc ability to configure pins, not associated with
> GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Pls CC Laurent who is main reviewer on all sh-pfc stuff.

> +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
> +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)

You add these #defines but do not use them.

Yours,
Linus Walleij

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Guennadi Liakhovetski]

Hi Linus

On Tue, 17 Sep 2013, Linus Walleij wrote:

> On Mon, Sep 9, 2013 at 6:03 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> 
> > There are four I2C interfaces on r8a7790, each of them can be connected to
> > one of the two respective I2C controllers, e.g. interface #0 can be
> > configured to work with I2C0 or with IIC0. Additionally some of those
> > interfaces can also use one of several pin sets. Interface #3 is special,
> > because it can be used in automatic mode for DVFS. It only has one set
> > of pins available and those pins cannot be used for anything else, they
> > also lack the GPIO function.
> >
> > This patch uses the sh-pfc ability to configure pins, not associated with
> > GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> 
> Pls CC Laurent who is main reviewer on all sh-pfc stuff.

Sure, sorry, thanks for adding him.

> > +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> > +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> 
> You add these #defines but do not use them.

ehm, actually I do:

+/* - I2C3 ------------------------------------------------------------------- */
+static const unsigned int i2c3_pins[] = {
+	/* SCL, SDA */
+	PIN_A_NUMBER('J', 15), PIN_A_NUMBER('H', 15),
+};

Besides, the PIN_NUMBER() macro is used in sh_pfc.h in the definition of 
SH_PFC_PIN_NAMED(), and that macro is also used in this patch:

+	/* Pins not associated with a GPIO port */
+	SH_PFC_PIN_NAMED(ROW_GROUP_A('J'), 15, AJ15),
+	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),


> Yours,
> Linus Walleij

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Linus Walleij]

On Tue, Sep 17, 2013 at 7:10 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Tue, 17 Sep 2013, Linus Walleij wrote:

>> > +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
>> > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
>> > +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
>> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
>>
>> You add these #defines but do not use them.
>
> ehm, actually I do:

Argh I was just wrong, forget about this.

Woudl really like Laurent's ACK on this before I apply it though.

Yours,
Linus Walleij

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Simon Horman]

On Mon, Sep 23, 2013 at 10:50:07AM +0200, Linus Walleij wrote:
> On Tue, Sep 17, 2013 at 7:10 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > On Tue, 17 Sep 2013, Linus Walleij wrote:
> 
> >> > +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> >> > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> >> > +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
> >> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> >>
> >> You add these #defines but do not use them.
> >
> > ehm, actually I do:
> 
> Argh I was just wrong, forget about this.
> 
> Woudl really like Laurent's ACK on this before I apply it though.

Laurent, ping.

Re: [PATCH 3/4] ARM: shmobile: r8a7790: add CPUFreq clock support

[Simon Horman]

On Mon, Sep 09, 2013 at 06:03:55PM +0200, Guennadi Liakhovetski wrote:
> Add support for the Z clock on r8a7790, driving the four SoC's CA15 cores,
> and its parent - PLL0. This is required for CPUFreq support on this SoC.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Magnus, I think this needs a review from you.

> ---
>  arch/arm/mach-shmobile/Kconfig         |    2 +
>  arch/arm/mach-shmobile/clock-r8a7790.c |  150 ++++++++++++++++++++++++++++++++
>  2 files changed, 152 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 1f94c31..b9422f2 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -100,6 +100,8 @@ config ARCH_R8A7790
>  	select CPU_V7
>  	select SH_CLK_CPG
>  	select RENESAS_IRQC
> +	select ARCH_HAS_CPUFREQ
> +	select ARCH_HAS_OPP
>  
>  config ARCH_EMEV2
>  	bool "Emma Mobile EV2"
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> index 8e5e90b..3ef043e 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -54,9 +54,12 @@
>  #define SMSTPCR8 0xe6150990
>  #define SMSTPCR9 0xe6150994
>  
> +#define FRQCRB		0xE6150004
>  #define SDCKCR		0xE6150074
>  #define SD2CKCR		0xE6150078
>  #define SD3CKCR		0xE615007C
> +#define FRQCRC		0xE61500E0
> +#define PLLECR		0xE61500D0
>  #define MMC0CKCR	0xE6150240
>  #define MMC1CKCR	0xE6150244
>  #define SSPCKCR		0xE6150248
> @@ -85,6 +88,7 @@ static struct clk main_clk = {
>   * clock ratio of these clock will be updated
>   * on r8a7790_clock_init()
>   */
> +SH_FIXED_RATIO_CLK_SET(pll0_clk,		main_clk,	1, 1);
>  SH_FIXED_RATIO_CLK_SET(pll1_clk,		main_clk,	1, 1);
>  SH_FIXED_RATIO_CLK_SET(pll3_clk,		main_clk,	1, 1);
>  SH_FIXED_RATIO_CLK_SET(lb_clk,			pll1_clk,	1, 1);
> @@ -113,15 +117,155 @@ SH_FIXED_RATIO_CLK_SET(zb3d2_clk,		pll3_clk,	1, 8);
>  SH_FIXED_RATIO_CLK_SET(ddr_clk,			pll3_clk,	1, 8);
>  SH_FIXED_RATIO_CLK_SET(mp_clk,			pll1_div2_clk,	1, 15);
>  
> +/* Locking not needed yet, only one clock is using FRQCR[BC] divisors so far */
> +static atomic_t frqcr_lock;
> +#define CPG_MAP(o) ((o) - CPG_BASE + cpg_mapping.base)
> +
> +/* Several clocks need to access FRQCRB, have to lock */
> +static bool frqcr_kick_check(struct clk *clk)
> +{
> +	return !(ioread32(CPG_MAP(FRQCRB)) & BIT(31));
> +}
> +
> +static int frqcr_kick_do(struct clk *clk)
> +{
> +	int i;
> +
> +	/* set KICK bit in FRQCRB to update hardware setting, check success */
> +	iowrite32(ioread32(CPG_MAP(FRQCRB)) | BIT(31), CPG_MAP(FRQCRB));
> +	for (i = 1000; i; i--)
> +		if (ioread32(CPG_MAP(FRQCRB)) & BIT(31))
> +			cpu_relax();
> +		else
> +			return 0;
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int zclk_set_rate(struct clk *clk, unsigned long rate)
> +{
> +	void __iomem *frqcrc;
> +	int ret;
> +	unsigned long step, p_rate;
> +	u32 val;
> +
> +	if (!clk->parent || !__clk_get(clk->parent))
> +		return -ENODEV;
> +
> +	if (!atomic_inc_and_test(&frqcr_lock) || !frqcr_kick_check(clk)) {
> +		ret = -EBUSY;
> +		goto done;
> +	}
> +
> +	/*
> +	 * Users are supposed to first call clk_set_rate() only with
> +	 * clk_round_rate() results. So, we don't fix wrong rates here, but
> +	 * guard against them anyway
> +	 */
> +
> +	p_rate = clk_get_rate(clk->parent);
> +	if (rate == p_rate) {
> +		val = 0;
> +	} else {
> +		step = DIV_ROUND_CLOSEST(p_rate, 32);
> +
> +		if (rate > p_rate || rate < step) {
> +			ret = -EINVAL;
> +			goto done;
> +		}
> +
> +		val = 32 - rate / step;
> +	}
> +
> +	frqcrc = clk->mapped_reg + (FRQCRC - (u32)clk->enable_reg);
> +
> +	iowrite32((ioread32(frqcrc) & ~(clk->div_mask << clk->enable_bit)) |
> +		  (val << clk->enable_bit), frqcrc);
> +
> +	ret = frqcr_kick_do(clk);
> +
> +done:
> +	atomic_dec(&frqcr_lock);
> +	__clk_put(clk->parent);
> +	return ret;
> +}
> +
> +static long zclk_round_rate(struct clk *clk, unsigned long rate)
> +{
> +	/*
> +	 * theoretical rate = parent rate * multiplier / 32,
> +	 * where 1 <= multiplier <= 32. Therefore we should do
> +	 * multiplier = rate * 32 / parent rate
> +	 * rounded rate = parent rate * multiplier / 32.
> +	 * However, multiplication before division won't fit in 32 bits, so
> +	 * we sacrifice some precision by first dividing and then multiplying.
> +	 * To find the nearest divisor we calculate both and pick up the best
> +	 * one. This avoids 64-bit arithmetics.
> +	 */
> +	unsigned long step, mul_min, mul_max, rate_min, rate_max;
> +
> +	rate_max = clk_get_rate(clk->parent);
> +
> +	/* output freq <= parent */
> +	if (rate >= rate_max)
> +		return rate_max;
> +
> +	step = DIV_ROUND_CLOSEST(rate_max, 32);
> +	/* output freq >= parent / 32 */
> +	if (step >= rate)
> +		return step;
> +
> +	mul_min = rate / step;
> +	mul_max = DIV_ROUND_UP(rate, step);
> +	rate_min = step * mul_min;
> +	if (mul_max == mul_min)
> +		return rate_min;
> +
> +	rate_max = step * mul_max;
> +
> +	if (rate_max - rate <  rate - rate_min)
> +		return rate_max;
> +
> +	return rate_min;
> +}
> +
> +static unsigned long zclk_recalc(struct clk *clk)
> +{
> +	void __iomem *frqcrc = FRQCRC - (u32)clk->enable_reg + clk->mapped_reg;
> +	unsigned int max = clk->div_mask + 1;
> +	unsigned long val = ((ioread32(frqcrc) >> clk->enable_bit) &
> +			     clk->div_mask);
> +
> +	return DIV_ROUND_CLOSEST(clk_get_rate(clk->parent), max) *
> +		(max - val);
> +}
> +
> +static struct sh_clk_ops zclk_ops = {
> +	.recalc = zclk_recalc,
> +	.set_rate = zclk_set_rate,
> +	.round_rate = zclk_round_rate,
> +};
> +
> +static struct clk z_clk = {
> +	.parent = &pll0_clk,
> +	.div_mask = 0x1f,
> +	.enable_bit = 8,
> +	/* We'll need to access FRQCRB and FRQCRC */
> +	.enable_reg = (void __iomem *)FRQCRB,
> +	.ops = &zclk_ops,
> +};
> +
>  static struct clk *main_clks[] = {
>  	&extal_clk,
>  	&extal_div2_clk,
>  	&main_clk,
> +	&pll0_clk,
>  	&pll1_clk,
>  	&pll1_div2_clk,
>  	&pll3_clk,
>  	&lb_clk,
>  	&qspi_clk,
> +	&z_clk,
>  	&zg_clk,
>  	&zx_clk,
>  	&zs_clk,
> @@ -249,6 +393,9 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_CON_ID("qspi",		&qspi_clk),
>  	CLKDEV_CON_ID("cp",		&cp_clk),
>  
> +	/* CPU clock */
> +	CLKDEV_DEV_ID("cpufreq-cpu0",	&z_clk),
> +
>  	/* DIV4 */
>  	CLKDEV_CON_ID("sdh",		&div4_clks[DIV4_SDH]),
>  
> @@ -291,6 +438,7 @@ static struct clk_lookup lookups[] = {
>  #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31)		\
>  	extal_clk.rate	= e * 1000 * 1000;			\
>  	main_clk.parent	= m;					\
> +	SH_CLK_SET_RATIO(&pll0_clk_ratio, p0 / 2, 1);		\
>  	SH_CLK_SET_RATIO(&pll1_clk_ratio, p1 / 2, 1);		\
>  	if (mode & MD(19))					\
>  		SH_CLK_SET_RATIO(&pll3_clk_ratio, p31, 1);	\
> @@ -303,6 +451,8 @@ void __init r8a7790_clock_init(void)
>  	u32 mode = r8a7790_read_mode_pins();
>  	int k, ret = 0;
>  
> +	atomic_set(&frqcr_lock, -1);
> +
>  	switch (mode & (MD(14) | MD(13))) {
>  	case 0:
>  		R8A7790_CLOCK_ROOT(15, &extal_clk, 172, 208, 106, 88);
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Laurent Pinchart]

Hi Guennadi,

Thank you for the patch.

On Monday 09 September 2013 18:03:53 Guennadi Liakhovetski wrote:
> There are four I2C interfaces on r8a7790, each of them can be connected to
> one of the two respective I2C controllers, e.g. interface #0 can be
> configured to work with I2C0 or with IIC0. Additionally some of those
> interfaces can also use one of several pin sets. Interface #3 is special,
> because it can be used in automatic mode for DVFS. It only has one set
> of pins available and those pins cannot be used for anything else, they
> also lack the GPIO function.
> 
> This patch uses the sh-pfc ability to configure pins, not associated with
> GPIOs and adds support for I2C3 to the r8a7790 PFC set up.

Ulrich Hecht sent a patch titled "sh-pfc: r8a7790: Add I2C pin groups and 
functions" that added pin groups for I2C1 and I2C2. The patch is available 
from

	git://linuxtv.org/pinchartl/fbdev.git pinmux/next

If you need to resubmit this patch due to my comments below, could you please 
rebase it on top of that branch ?

> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..c3c4d9b 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -781,6 +781,8 @@ enum {
>  	ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
>  	USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
>  	TCLK1_B_MARK,
> +
> +	I2C3_SCL_MARK, I2C3_SDA_MARK,
>  	PINMUX_MARK_END,
>  };
> 
> @@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
>  	PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
>  	PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
>  	PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
> +
> +	PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
> +	PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),

You introduce a way to mux the I2C3 function on those two pins, but no way to 
select the IICDVFS back. I don't think it's an issue, we can always add that 
later when (if) needed. Linus, is that fine with you ?

>  };
> 
> +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)

The BGA package has 31 columns, shouldn't you multiply the row number by 31 
instead of 16 ?

As we have 192 GPIOs, shouldn't you use an offset of 192 instead of 200 ? This 
doesn't matter too much I guess.

> +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> +
>  static struct sh_pfc_pin pinmux_pins[] = {
>  	PINMUX_GPIO_GP_ALL(),
> +
> +	/* Pins not associated with a GPIO port */
> +	SH_PFC_PIN_NAMED(ROW_GROUP_A('J'), 15, AJ15),
> +	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
>  };
> 
>  /* - DU RGB
> ----------------------------------------------------------------- */ @@
> -1990,6 +2004,14 @@ static const unsigned int hscif1_ctrl_b_pins[] = {
> static const unsigned int hscif1_ctrl_b_mux[] = {
>  	HRTS1_N_B_MARK, HCTS1_N_B_MARK,
>  };
> +/* - I2C3
> ------------------------------------------------------------------- */
> +static const unsigned int i2c3_pins[] = {
> +	/* SCL, SDA */
> +	PIN_A_NUMBER('J', 15), PIN_A_NUMBER('H', 15),
> +};
> +static const unsigned int i2c3_mux[] = {
> +	I2C3_SCL_MARK, I2C3_SDA_MARK,
> +};
>  /* - INTC
> ------------------------------------------------------------------- */
> static const unsigned int intc_irq0_pins[] = {
>  	/* IRQ */
> @@ -3047,6 +3069,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] =
> { SH_PFC_PIN_GROUP(hscif1_data_b),
>  	SH_PFC_PIN_GROUP(hscif1_clk_b),
>  	SH_PFC_PIN_GROUP(hscif1_ctrl_b),
> +	SH_PFC_PIN_GROUP(i2c3),
>  	SH_PFC_PIN_GROUP(intc_irq0),
>  	SH_PFC_PIN_GROUP(intc_irq1),
>  	SH_PFC_PIN_GROUP(intc_irq2),
> @@ -3243,6 +3266,10 @@ static const char * const hscif1_groups[] = {
>  	"hscif1_ctrl_b",
>  };
> 
> +static const char * const i2c3_groups[] = {
> +	"i2c3",
> +};
> +
>  static const char * const intc_groups[] = {
>  	"intc_irq0",
>  	"intc_irq1",
> @@ -3469,6 +3496,7 @@ static const struct sh_pfc_function pinmux_functions[]
> = { SH_PFC_FUNCTION(eth),
>  	SH_PFC_FUNCTION(hscif0),
>  	SH_PFC_FUNCTION(hscif1),
> +	SH_PFC_FUNCTION(i2c3),
>  	SH_PFC_FUNCTION(intc),
>  	SH_PFC_FUNCTION(mmc0),
>  	SH_PFC_FUNCTION(mmc1),
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Laurent Pinchart]

Hi Guennadi,

Thank you for the patch.

On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
> This patch adds clocks and clock lookup entries for the four I2C
> controllers on r8a7790 and respective Device Tree nodes.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi         |   36 +++++++++++++++++++++++++++++
>  arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 885f9f4..a5021112 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -127,6 +127,42 @@
>  		interrupts = <0 0 4>, <0 1 4>, <0 2 4>,	<0 3 4>;
>  	};
> 
> +	i2c0: i2c@e6508000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6508000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 287 0x4>;

Shouldn't you add state = "disabled" to all I2C controllers in order not to 
enable the unused controllers by default ?

> +	};
> +
> +	i2c1: i2c@e6518000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6518000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 288 0x4>;
> +	};
> +
> +	i2c2: i2c@e6530000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6530000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 286 0x4>;
> +	};
> +
> +	i2c3: i2c@e6540000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6540000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 290 0x4>;
> +	};
> +
>  	mmcif0: mmcif@ee200000 {
>  		compatible = "renesas,sh-mmcif";
>  		reg = <0 0xee200000 0 0x80>;
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c
> b/arch/arm/mach-shmobile/clock-r8a7790.c index fc36d3d..8e5e90b 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -52,6 +52,7 @@
>  #define SMSTPCR5 0xe6150144
>  #define SMSTPCR7 0xe615014c
>  #define SMSTPCR8 0xe6150990
> +#define SMSTPCR9 0xe6150994
> 
>  #define SDCKCR		0xE6150074
>  #define SD2CKCR		0xE6150078
> @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = {
> 
>  /* MSTP */
>  enum {
> +	MSTP931, MSTP930, MSTP929, MSTP928,
>  	MSTP813,
>  	MSTP721, MSTP720,
>  	MSTP717, MSTP716,
> @@ -192,6 +194,10 @@ enum {
>  };
> 
>  static struct clk mstp_clks[MSTP_NR] = {
> +	[MSTP931] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 31, 0), /* I2C0 */
> +	[MSTP930] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 30, 0), /* I2C1 */
> +	[MSTP929] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 29, 0), /* I2C2 */
> +	[MSTP928] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 28, 0), /* I2C3 */
>  	[MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */
>  	[MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
>  	[MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
> @@ -261,6 +267,10 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
>  	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
>  	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
> +	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
> +	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
> +	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
> +	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
>  	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
>  	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
>  	CLKDEV_DEV_ID("ee200000.mmcif", &mstp_clks[MSTP315]),
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Guennadi Liakhovetski]

Hi Laurent

On Wed, 25 Sep 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> Thank you for the patch.
> 
> On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
> > This patch adds clocks and clock lookup entries for the four I2C
> > controllers on r8a7790 and respective Device Tree nodes.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> >  arch/arm/boot/dts/r8a7790.dtsi         |   36 +++++++++++++++++++++++++++++
> >  arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
> >  2 files changed, 46 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> > index 885f9f4..a5021112 100644
> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > @@ -127,6 +127,42 @@
> >  		interrupts = <0 0 4>, <0 1 4>, <0 2 4>,	<0 3 4>;
> >  	};
> > 
> > +	i2c0: i2c@e6508000 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "renesas,i2c-rcar-h2";
> > +		reg = <0 0xe6508000 0 0x40>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 287 0x4>;
> 
> Shouldn't you add state = "disabled" to all I2C controllers in order not to 
> enable the unused controllers by default ?

It would be logical, yes, and I seem to remember having discussed this 
earlier with someone (with Magnus, IIRC), and the outcome was, that all 
Renesas .dtsi files so far define all I2C directly in enabled mode and 
that it's intentional, so, I just followed this pattern here. You can 
indeed check in other .dtsi files - all seem to do exactly the same.

Thanks
Guennadi

> 
> > +	};
> > +
> > +	i2c1: i2c@e6518000 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "renesas,i2c-rcar-h2";
> > +		reg = <0 0xe6518000 0 0x40>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 288 0x4>;
> > +	};
> > +
> > +	i2c2: i2c@e6530000 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "renesas,i2c-rcar-h2";
> > +		reg = <0 0xe6530000 0 0x40>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 286 0x4>;
> > +	};
> > +
> > +	i2c3: i2c@e6540000 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "renesas,i2c-rcar-h2";
> > +		reg = <0 0xe6540000 0 0x40>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 290 0x4>;
> > +	};
> > +
> >  	mmcif0: mmcif@ee200000 {
> >  		compatible = "renesas,sh-mmcif";
> >  		reg = <0 0xee200000 0 0x80>;
> > diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c
> > b/arch/arm/mach-shmobile/clock-r8a7790.c index fc36d3d..8e5e90b 100644
> > --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> > +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> > @@ -52,6 +52,7 @@
> >  #define SMSTPCR5 0xe6150144
> >  #define SMSTPCR7 0xe615014c
> >  #define SMSTPCR8 0xe6150990
> > +#define SMSTPCR9 0xe6150994
> > 
> >  #define SDCKCR		0xE6150074
> >  #define SD2CKCR		0xE6150078
> > @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = {
> > 
> >  /* MSTP */
> >  enum {
> > +	MSTP931, MSTP930, MSTP929, MSTP928,
> >  	MSTP813,
> >  	MSTP721, MSTP720,
> >  	MSTP717, MSTP716,
> > @@ -192,6 +194,10 @@ enum {
> >  };
> > 
> >  static struct clk mstp_clks[MSTP_NR] = {
> > +	[MSTP931] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 31, 0), /* I2C0 */
> > +	[MSTP930] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 30, 0), /* I2C1 */
> > +	[MSTP929] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 29, 0), /* I2C2 */
> > +	[MSTP928] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 28, 0), /* I2C3 */
> >  	[MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */
> >  	[MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
> >  	[MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
> > @@ -261,6 +267,10 @@ static struct clk_lookup lookups[] = {
> >  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
> >  	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
> >  	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
> > +	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
> > +	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
> > +	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
> > +	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
> >  	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
> >  	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
> >  	CLKDEV_DEV_ID("ee200000.mmcif", &mstp_clks[MSTP315]),
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Simon Horman]

On Mon, Sep 09, 2013 at 06:03:54PM +0200, Guennadi Liakhovetski wrote:
> This patch adds clocks and clock lookup entries for the four I2C
> controllers on r8a7790 and respective Device Tree nodes.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi         |   36 ++++++++++++++++++++++++++++++++
>  arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)

Hi Guennadi,

please split this patch into two patches.
1. A SoC patch which updates clock-r8a7790.c
2. A DT patch which updates r8a7790.dtsi

Thanks

> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 885f9f4..a5021112 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -127,6 +127,42 @@
>  		interrupts = <0 0 4>, <0 1 4>, <0 2 4>,	<0 3 4>;
>  	};
>  
> +	i2c0: i2c@e6508000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6508000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 287 0x4>;
> +	};
> +
> +	i2c1: i2c@e6518000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6518000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 288 0x4>;
> +	};
> +
> +	i2c2: i2c@e6530000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6530000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 286 0x4>;
> +	};
> +
> +	i2c3: i2c@e6540000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "renesas,i2c-rcar-h2";
> +		reg = <0 0xe6540000 0 0x40>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 290 0x4>;
> +	};
> +
>  	mmcif0: mmcif@ee200000 {
>  		compatible = "renesas,sh-mmcif";
>  		reg = <0 0xee200000 0 0x80>;
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
> index fc36d3d..8e5e90b 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -52,6 +52,7 @@
>  #define SMSTPCR5 0xe6150144
>  #define SMSTPCR7 0xe615014c
>  #define SMSTPCR8 0xe6150990
> +#define SMSTPCR9 0xe6150994
>  
>  #define SDCKCR		0xE6150074
>  #define SD2CKCR		0xE6150078
> @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = {
>  
>  /* MSTP */
>  enum {
> +	MSTP931, MSTP930, MSTP929, MSTP928,
>  	MSTP813,
>  	MSTP721, MSTP720,
>  	MSTP717, MSTP716,
> @@ -192,6 +194,10 @@ enum {
>  };
>  
>  static struct clk mstp_clks[MSTP_NR] = {
> +	[MSTP931] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 31, 0), /* I2C0 */
> +	[MSTP930] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 30, 0), /* I2C1 */
> +	[MSTP929] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 29, 0), /* I2C2 */
> +	[MSTP928] = SH_CLK_MSTP32(&hp_clk, SMSTPCR9, 28, 0), /* I2C3 */
>  	[MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */
>  	[MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */
>  	[MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */
> @@ -261,6 +267,10 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_DEV_ID("sh-sci.7", &mstp_clks[MSTP720]),
>  	CLKDEV_DEV_ID("sh-sci.8", &mstp_clks[MSTP717]),
>  	CLKDEV_DEV_ID("sh-sci.9", &mstp_clks[MSTP716]),
> +	CLKDEV_DEV_ID("e6508000.i2c", &mstp_clks[MSTP931]),
> +	CLKDEV_DEV_ID("e6518000.i2c", &mstp_clks[MSTP930]),
> +	CLKDEV_DEV_ID("e6530000.i2c", &mstp_clks[MSTP929]),
> +	CLKDEV_DEV_ID("e6540000.i2c", &mstp_clks[MSTP928]),
>  	CLKDEV_DEV_ID("r8a7790-ether", &mstp_clks[MSTP813]),
>  	CLKDEV_DEV_ID("rcar_thermal", &mstp_clks[MSTP522]),
>  	CLKDEV_DEV_ID("ee200000.mmcif", &mstp_clks[MSTP315]),
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Magnus Damm]

Hi Guennadi,

On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi Laurent
>
> On Wed, 25 Sep 2013, Laurent Pinchart wrote:
>
>> Hi Guennadi,
>>
>> Thank you for the patch.
>>
>> On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
>> > This patch adds clocks and clock lookup entries for the four I2C
>> > controllers on r8a7790 and respective Device Tree nodes.
>> >
>> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
>> > ---
>> >  arch/arm/boot/dts/r8a7790.dtsi         |   36 +++++++++++++++++++++++++++++
>> >  arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
>> >  2 files changed, 46 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
>> > index 885f9f4..a5021112 100644
>> > --- a/arch/arm/boot/dts/r8a7790.dtsi
>> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> > @@ -127,6 +127,42 @@
>> >             interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
>> >     };
>> >
>> > +   i2c0: i2c@e6508000 {
>> > +           #address-cells = <1>;
>> > +           #size-cells = <0>;
>> > +           compatible = "renesas,i2c-rcar-h2";
>> > +           reg = <0 0xe6508000 0 0x40>;
>> > +           interrupt-parent = <&gic>;
>> > +           interrupts = <0 287 0x4>;
>>
>> Shouldn't you add state = "disabled" to all I2C controllers in order not to
>> enable the unused controllers by default ?
>
> It would be logical, yes, and I seem to remember having discussed this
> earlier with someone (with Magnus, IIRC), and the outcome was, that all
> Renesas .dtsi files so far define all I2C directly in enabled mode and
> that it's intentional, so, I just followed this pattern here. You can
> indeed check in other .dtsi files - all seem to do exactly the same.

Uhm, I think you mix up platform device use case and DT use case. In
the platform device case we define all devices by default, but that's
not how it should be for the DT case. In the case of DT then default
should most likely be "disabled". Please follow the direction of
Laurent.

Also, I mentioned this 25 times before already so once more cannot
hurt: "renesas,i2c-rcar-h2" needs to be replaced with something more
standard.

/ magnus

Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Guennadi Liakhovetski]

Hi Magnus

On Thu, 26 Sep 2013, Magnus Damm wrote:

> Hi Guennadi,
> 
> On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > Hi Laurent
> >
> > On Wed, 25 Sep 2013, Laurent Pinchart wrote:
> >
> >> Hi Guennadi,
> >>
> >> Thank you for the patch.
> >>
> >> On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
> >> > This patch adds clocks and clock lookup entries for the four I2C
> >> > controllers on r8a7790 and respective Device Tree nodes.
> >> >
> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> >> > ---
> >> >  arch/arm/boot/dts/r8a7790.dtsi         |   36 +++++++++++++++++++++++++++++
> >> >  arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
> >> >  2 files changed, 46 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> >> > index 885f9f4..a5021112 100644
> >> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> >> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> >> > @@ -127,6 +127,42 @@
> >> >             interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
> >> >     };
> >> >
> >> > +   i2c0: i2c@e6508000 {
> >> > +           #address-cells = <1>;
> >> > +           #size-cells = <0>;
> >> > +           compatible = "renesas,i2c-rcar-h2";
> >> > +           reg = <0 0xe6508000 0 0x40>;
> >> > +           interrupt-parent = <&gic>;
> >> > +           interrupts = <0 287 0x4>;
> >>
> >> Shouldn't you add state = "disabled" to all I2C controllers in order not to
> >> enable the unused controllers by default ?
> >
> > It would be logical, yes, and I seem to remember having discussed this
> > earlier with someone (with Magnus, IIRC), and the outcome was, that all
> > Renesas .dtsi files so far define all I2C directly in enabled mode and
> > that it's intentional, so, I just followed this pattern here. You can
> > indeed check in other .dtsi files - all seem to do exactly the same.
> 
> Uhm, I think you mix up platform device use case and DT use case.

I'm sure I don't.

> In
> the platform device case we define all devices by default, but that's
> not how it should be for the DT case. In the case of DT then default
> should most likely be "disabled". Please follow the direction of
> Laurent.

To put the record straight - originally I suggested to use status 
"disabled" for mmc devices in .dtsi, before that the status property 
wasn't used in Renesas .dts(i) files, and that's when all I2C nodes were 
already present in .dtsi in enabled state.

> Also, I mentioned this 25 times before already so once more cannot
> hurt: "renesas,i2c-rcar-h2" needs to be replaced with something more
> standard.

I didn't count, but yes, it has been discussed and that's been fixed in v2 
of my i2c patches. Now that I've got comments to the ARM patches I can 
also update them with a correct compatibility string.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Guennadi Liakhovetski]

Hi Laurent

On Wed, 25 Sep 2013, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> Thank you for the patch.
> 
> On Monday 09 September 2013 18:03:53 Guennadi Liakhovetski wrote:
> > There are four I2C interfaces on r8a7790, each of them can be connected to
> > one of the two respective I2C controllers, e.g. interface #0 can be
> > configured to work with I2C0 or with IIC0. Additionally some of those
> > interfaces can also use one of several pin sets. Interface #3 is special,
> > because it can be used in automatic mode for DVFS. It only has one set
> > of pins available and those pins cannot be used for anything else, they
> > also lack the GPIO function.
> > 
> > This patch uses the sh-pfc ability to configure pins, not associated with
> > GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
> 
> Ulrich Hecht sent a patch titled "sh-pfc: r8a7790: Add I2C pin groups and 
> functions" that added pin groups for I2C1 and I2C2. The patch is available 
> from
> 
> 	git://linuxtv.org/pinchartl/fbdev.git pinmux/next
> 
> If you need to resubmit this patch due to my comments below, could you please 
> rebase it on top of that branch ?
> 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |   28 ++++++++++++++++++++++++++++
> >  1 files changed, 28 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..c3c4d9b 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > @@ -781,6 +781,8 @@ enum {
> >  	ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
> >  	USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
> >  	TCLK1_B_MARK,
> > +
> > +	I2C3_SCL_MARK, I2C3_SDA_MARK,
> >  	PINMUX_MARK_END,
> >  };
> > 
> > @@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
> >  	PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
> >  	PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
> >  	PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
> > +
> > +	PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
> > +	PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),
> 
> You introduce a way to mux the I2C3 function on those two pins, but no way to 
> select the IICDVFS back. I don't think it's an issue, we can always add that 
> later when (if) needed. Linus, is that fine with you ?

I did it on purpose, since I didn't have a use case for IICDVFS. I prefer 
not to add too many things, that cannot be tested.

> >  };
> > 
> > +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> > +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
> 
> The BGA package has 31 columns, shouldn't you multiply the row number by 31 
> instead of 16 ?

Oops, you're right - the pin table is continued on the second page...

> As we have 192 GPIOs, shouldn't you use an offset of 192 instead of 200 ? This 
> doesn't matter too much I guess.

On one of Renesas SoCs I've seen an offset of 2000 used. I thought it 
would be an exaggeration in this case ;) but I followed the pattern of 
using a round number for the offset, is this ok?

Thanks
Guennadi

> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> > +
> >  static struct sh_pfc_pin pinmux_pins[] = {
> >  	PINMUX_GPIO_GP_ALL(),
> > +
> > +	/* Pins not associated with a GPIO port */
> > +	SH_PFC_PIN_NAMED(ROW_GROUP_A('J'), 15, AJ15),
> > +	SH_PFC_PIN_NAMED(ROW_GROUP_A('H'), 15, AH15),
> >  };
> > 
> >  /* - DU RGB
> > ----------------------------------------------------------------- */ @@
> > -1990,6 +2004,14 @@ static const unsigned int hscif1_ctrl_b_pins[] = {
> > static const unsigned int hscif1_ctrl_b_mux[] = {
> >  	HRTS1_N_B_MARK, HCTS1_N_B_MARK,
> >  };
> > +/* - I2C3
> > ------------------------------------------------------------------- */
> > +static const unsigned int i2c3_pins[] = {
> > +	/* SCL, SDA */
> > +	PIN_A_NUMBER('J', 15), PIN_A_NUMBER('H', 15),
> > +};
> > +static const unsigned int i2c3_mux[] = {
> > +	I2C3_SCL_MARK, I2C3_SDA_MARK,
> > +};
> >  /* - INTC
> > ------------------------------------------------------------------- */
> > static const unsigned int intc_irq0_pins[] = {
> >  	/* IRQ */
> > @@ -3047,6 +3069,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] =
> > { SH_PFC_PIN_GROUP(hscif1_data_b),
> >  	SH_PFC_PIN_GROUP(hscif1_clk_b),
> >  	SH_PFC_PIN_GROUP(hscif1_ctrl_b),
> > +	SH_PFC_PIN_GROUP(i2c3),
> >  	SH_PFC_PIN_GROUP(intc_irq0),
> >  	SH_PFC_PIN_GROUP(intc_irq1),
> >  	SH_PFC_PIN_GROUP(intc_irq2),
> > @@ -3243,6 +3266,10 @@ static const char * const hscif1_groups[] = {
> >  	"hscif1_ctrl_b",
> >  };
> > 
> > +static const char * const i2c3_groups[] = {
> > +	"i2c3",
> > +};
> > +
> >  static const char * const intc_groups[] = {
> >  	"intc_irq0",
> >  	"intc_irq1",
> > @@ -3469,6 +3496,7 @@ static const struct sh_pfc_function pinmux_functions[]
> > = { SH_PFC_FUNCTION(eth),
> >  	SH_PFC_FUNCTION(hscif0),
> >  	SH_PFC_FUNCTION(hscif1),
> > +	SH_PFC_FUNCTION(i2c3),
> >  	SH_PFC_FUNCTION(intc),
> >  	SH_PFC_FUNCTION(mmc0),
> >  	SH_PFC_FUNCTION(mmc1),
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/4] pinctrl: sh-pfc: r8a7790: add pin definitions for the I2C3 interface

[Laurent Pinchart]

Hi Guennadi,

On Thursday 26 September 2013 11:24:26 Guennadi Liakhovetski wrote:
> On Wed, 25 Sep 2013, Laurent Pinchart wrote:
> > On Monday 09 September 2013 18:03:53 Guennadi Liakhovetski wrote:
> > > There are four I2C interfaces on r8a7790, each of them can be connected
> > > to one of the two respective I2C controllers, e.g. interface #0 can be
> > > configured to work with I2C0 or with IIC0. Additionally some of those
> > > interfaces can also use one of several pin sets. Interface #3 is
> > > special, because it can be used in automatic mode for DVFS. It only has
> > > one set of pins available and those pins cannot be used for anything
> > > else, they also lack the GPIO function.
> > > 
> > > This patch uses the sh-pfc ability to configure pins, not associated
> > > with GPIOs and adds support for I2C3 to the r8a7790 PFC set up.
> > 
> > Ulrich Hecht sent a patch titled "sh-pfc: r8a7790: Add I2C pin groups and
> > functions" that added pin groups for I2C1 and I2C2. The patch is available
> > from
> > 
> > 	git://linuxtv.org/pinchartl/fbdev.git pinmux/next
> > 
> > If you need to resubmit this patch due to my comments below, could you
> > please rebase it on top of that branch ?
> > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > > ---
> > > 
> > >  drivers/pinctrl/sh-pfc/pfc-r8a7790.c |   28
> > >  ++++++++++++++++++++++++++++
> > >  1 files changed, 28 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..c3c4d9b 100644
> > > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > @@ -781,6 +781,8 @@ enum {
> > >  	ADICS_SAMP_MARK, DU2_CDE_MARK, QPOLB_MARK, SCIFA2_RXD_B_MARK,
> > >  	USB1_PWEN_MARK, AUDIO_CLKOUT_D_MARK, USB1_OVC_MARK,
> > >  	TCLK1_B_MARK,
> > > +
> > > +	I2C3_SCL_MARK, I2C3_SDA_MARK,
> > > 
> > >  	PINMUX_MARK_END,
> > >  };
> > > @@ -1719,10 +1721,22 @@ static const u16 pinmux_data[] = {
> > >  	PINMUX_IPSR_DATA(IP16_6, AUDIO_CLKOUT_D),
> > >  	PINMUX_IPSR_DATA(IP16_7, USB1_OVC),
> > >  	PINMUX_IPSR_MODSEL_DATA(IP16_7, TCLK1_B, SEL_TMU1_1),
> > > +
> > > +	PINMUX_DATA(I2C3_SCL_MARK, FN_SEL_IICDVFS_1),
> > > +	PINMUX_DATA(I2C3_SDA_MARK, FN_SEL_IICDVFS_1),
> > 
> > You introduce a way to mux the I2C3 function on those two pins, but no way
> > to select the IICDVFS back. I don't think it's an issue, we can always
> > add that later when (if) needed. Linus, is that fine with you ?
> 
> I did it on purpose, since I didn't have a use case for IICDVFS. I prefer
> not to add too many things, that cannot be tested.

Just for the record, I'm fine with that.

> > >  };
> > > 
> > > +/* R8A7790 has 6 banks with 32 GPIOs in each = 192 GPIOs */
> > > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> > > +#define PIN_NUMBER(r, c) (((r) - 'A') * 16 + (c) + 200)
> > 
> > The BGA package has 31 columns, shouldn't you multiply the row number by
> > 31 instead of 16 ?
> 
> Oops, you're right - the pin table is continued on the second page...

Could you then submit a v2 based on git://linuxtv.org/pinchartl/fbdev.git 
pinmux/next ?

> > As we have 192 GPIOs, shouldn't you use an offset of 192 instead of 200 ?
> > This doesn't matter too much I guess.
> 
> On one of Renesas SoCs I've seen an offset of 2000 used. I thought it
> would be an exaggeration in this case ;) but I followed the pattern of
> using a round number for the offset, is this ok?

I suppose that's fine, yes.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode

[Magnus Damm]

Hi Guennadi,

On Thu, Sep 26, 2013 at 12:07 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi Magnus
>
> On Thu, 26 Sep 2013, Magnus Damm wrote:
>
>> Hi Guennadi,
>>
>> On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
>> <g.liakhovetski@gmx.de> wrote:
>> > Hi Laurent
>> >
>> > On Wed, 25 Sep 2013, Laurent Pinchart wrote:
>> >
>> >> Hi Guennadi,
>> >>
>> >> Thank you for the patch.
>> >>
>> >> On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
>> >> > This patch adds clocks and clock lookup entries for the four I2C
>> >> > controllers on r8a7790 and respective Device Tree nodes.
>> >> >
>> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
>> >> > ---
>> >> >  arch/arm/boot/dts/r8a7790.dtsi         |   36 +++++++++++++++++++++++++++++
>> >> >  arch/arm/mach-shmobile/clock-r8a7790.c |   10 ++++++++
>> >> >  2 files changed, 46 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
>> >> > index 885f9f4..a5021112 100644
>> >> > --- a/arch/arm/boot/dts/r8a7790.dtsi
>> >> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> >> > @@ -127,6 +127,42 @@
>> >> >             interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
>> >> >     };
>> >> >
>> >> > +   i2c0: i2c@e6508000 {
>> >> > +           #address-cells = <1>;
>> >> > +           #size-cells = <0>;
>> >> > +           compatible = "renesas,i2c-rcar-h2";
>> >> > +           reg = <0 0xe6508000 0 0x40>;
>> >> > +           interrupt-parent = <&gic>;
>> >> > +           interrupts = <0 287 0x4>;
>> >>
>> >> Shouldn't you add state = "disabled" to all I2C controllers in order not to
>> >> enable the unused controllers by default ?
>> >
>> > It would be logical, yes, and I seem to remember having discussed this
>> > earlier with someone (with Magnus, IIRC), and the outcome was, that all
>> > Renesas .dtsi files so far define all I2C directly in enabled mode and
>> > that it's intentional, so, I just followed this pattern here. You can
>> > indeed check in other .dtsi files - all seem to do exactly the same.
>>
>> Uhm, I think you mix up platform device use case and DT use case.
>
> I'm sure I don't.

Ok, but it sure sounds like that.

>> In
>> the platform device case we define all devices by default, but that's
>> not how it should be for the DT case. In the case of DT then default
>> should most likely be "disabled". Please follow the direction of
>> Laurent.
>
> To put the record straight - originally I suggested to use status
> "disabled" for mmc devices in .dtsi, before that the status property
> wasn't used in Renesas .dts(i) files, and that's when all I2C nodes were
> already present in .dtsi in enabled state.

Regarding "disabled", using that in a coherent way probably makes
sense. As for MMC, despite my efforts the DT binding development
turned out far from perfect. So during that time the good ideas
proposed may have accidentally been shot down together with the rest,
my apologies for that.

Now, would it be possible to make sure that all mach-shmobile I2C
descriptions follow the same style? I would like the SoCs to be
supported in the same way, not randomly - both then it comes to
compatbile string format and if "disabled" is used or not.

>> Also, I mentioned this 25 times before already so once more cannot
>> hurt: "renesas,i2c-rcar-h2" needs to be replaced with something more
>> standard.
>
> I didn't count, but yes, it has been discussed and that's been fixed in v2
> of my i2c patches. Now that I've got comments to the ARM patches I can
> also update them with a correct compatibility string.

Good, thanks.

/ magnus