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 <[email protected]>
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/
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Mon, Sep 9, 2013 at 6:03 PM, Guennadi Liakhovetski
<[email protected]> 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 <[email protected]>
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
Hi Linus
On Tue, 17 Sep 2013, Linus Walleij wrote:
> On Mon, Sep 9, 2013 at 6:03 PM, Guennadi Liakhovetski
> <[email protected]> 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 <[email protected]>
>
> 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/
On Tue, Sep 17, 2013 at 7:10 PM, Guennadi Liakhovetski
<[email protected]> 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
On Mon, Sep 23, 2013 at 10:50:07AM +0200, Linus Walleij wrote:
> On Tue, Sep 17, 2013 at 7:10 PM, Guennadi Liakhovetski
> <[email protected]> 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.
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 <[email protected]>
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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/
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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi Guennadi,
On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
<[email protected]> 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 <[email protected]>
>> > ---
>> > 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
Hi Magnus
On Thu, 26 Sep 2013, Magnus Damm wrote:
> Hi Guennadi,
>
> On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
> <[email protected]> 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 <[email protected]>
> >> > ---
> >> > 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/
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 <[email protected]>
> > ---
> > 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/
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 <[email protected]>
> > > ---
> > >
> > > 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
Hi Guennadi,
On Thu, Sep 26, 2013 at 12:07 AM, Guennadi Liakhovetski
<[email protected]> wrote:
> Hi Magnus
>
> On Thu, 26 Sep 2013, Magnus Damm wrote:
>
>> Hi Guennadi,
>>
>> On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
>> <[email protected]> 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 <[email protected]>
>> >> > ---
>> >> > 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