[PATCH 0/6] crypto: Add driver for JZ4780 PRNG

[PATCH 0/6] crypto: Add driver for JZ4780 PRNG

[PrasannaKumar Muralidharan]

This patch series adds support of pseudo random number generator found
in Ingenic's JZ4780 and X1000 SoC.

The PRNG hardware block registers are a part of same hardware block
that has clock and power registers which is handled by CGU driver.
Ingenic M200 SoC contains power related registers that are present
after the PRNG registers. So instead of reducing the register range,
syscon interface is used to expose a register map that is used by both
CGU driver and this driver. Changes made to jz4740-cgu.c is only compile
tested.

PrasannaKumar Muralidharan (6):
  crypto: jz4780-rng: Add devicetree bindings for RNG in JZ4780 SoC
  crypto: jz4780-rng: Make ingenic CGU driver use syscon
  crypto: jz4780-rng: Add Ingenic JZ4780 hardware PRNG driver
  crypto: jz4780-rng: Add RNG node to jz4780.dtsi
  crypto: jz4780-rng: Add myself as mainatainer for JZ4780 PRNG driver
  crypto: jz4780-rng: Enable PRNG support in CI20 defconfig

 .../devicetree/bindings/rng/ingenic,jz4780-rng.txt |  24 +++
 MAINTAINERS                                        |   5 +
 arch/mips/boot/dts/ingenic/jz4740.dtsi             |  14 +-
 arch/mips/boot/dts/ingenic/jz4780.dtsi             |  18 ++-
 arch/mips/configs/ci20_defconfig                   |   5 +
 drivers/clk/ingenic/cgu.c                          |  46 +++---
 drivers/clk/ingenic/cgu.h                          |   9 +-
 drivers/clk/ingenic/jz4740-cgu.c                   |  30 ++--
 drivers/clk/ingenic/jz4780-cgu.c                   |  10 +-
 drivers/crypto/Kconfig                             |  19 +++
 drivers/crypto/Makefile                            |   1 +
 drivers/crypto/jz4780-rng.c                        | 173 +++++++++++++++++++++
 12 files changed, 304 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
 create mode 100644 drivers/crypto/jz4780-rng.c

-- 
2.10.0

[PATCH 1/6] crypto: jz4780-rng: Add devicetree bindings for RNG in JZ4780 SoC

[PrasannaKumar Muralidharan]

Add devicetree bindings for hardware pseudo random number generator
present in Ingenic JZ4780 SoC.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 .../devicetree/bindings/rng/ingenic,jz4780-rng.txt | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt

diff --git a/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt b/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
new file mode 100644
index 0000000..88a0a92
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/ingenic,jz4780-rng.txt
@@ -0,0 +1,24 @@
+Ingenic jz4780 RNG driver
+
+Required properties:
+- compatible : Should be "ingenic,jz4780-rng"
+
+Example:
+
+cgu_registers {
+	compatible = "simple-mfd", "syscon";
+	reg = <0x10000000 0x100>;
+
+	cgu: jz4780-cgu@10000000 {
+		compatible = "ingenic,jz4780-cgu";
+
+		clocks = <&ext>, <&rtc>;
+		clock-names = "ext", "rtc";
+
+		#clock-cells = <1>;
+	};
+
+	rng: rng@d8 {
+		compatible = "ingenic,jz480-rng";
+	};
+};
-- 
2.10.0

[PATCH 2/6] crypto: jz4780-rng: Make ingenic CGU driver use syscon

[PrasannaKumar Muralidharan]

Ingenic PRNG registers are a part of the same hardware block as clock
and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power
related registers that are after the PRNG registers. So instead of
shortening the register range use syscon interface to expose regmap.
This regmap is used by the PRNG driver.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++----
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++----
 drivers/clk/ingenic/cgu.c              | 46 +++++++++++++++++++++-------------
 drivers/clk/ingenic/cgu.h              |  9 +++++--
 drivers/clk/ingenic/jz4740-cgu.c       | 30 +++++++++++-----------
 drivers/clk/ingenic/jz4780-cgu.c       | 10 ++++----
 6 files changed, 73 insertions(+), 50 deletions(-)

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index 2ca7ce7..ada5e67 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -34,14 +34,18 @@
 		clock-frequency = <32768>;
 	};
 
-	cgu: jz4740-cgu@10000000 {
-		compatible = "ingenic,jz4740-cgu";
+	cgu_registers {
+		compatible = "simple-mfd", "syscon";
 		reg = <0x10000000 0x100>;
 
-		clocks = <&ext>, <&rtc>;
-		clock-names = "ext", "rtc";
+		cgu: jz4780-cgu@10000000 {
+			compatible = "ingenic,jz4740-cgu";
 
-		#clock-cells = <1>;
+			clocks = <&ext>, <&rtc>;
+			clock-names = "ext", "rtc";
+
+			#clock-cells = <1>;
+		};
 	};
 
 	rtc_dev: rtc@10003000 {
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 4853ef6..1301694 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -34,14 +34,18 @@
 		clock-frequency = <32768>;
 	};
 
-	cgu: jz4780-cgu@10000000 {
-		compatible = "ingenic,jz4780-cgu";
+	cgu_registers {
+		compatible = "simple-mfd", "syscon";
 		reg = <0x10000000 0x100>;
 
-		clocks = <&ext>, <&rtc>;
-		clock-names = "ext", "rtc";
+		cgu: jz4780-cgu@10000000 {
+			compatible = "ingenic,jz4780-cgu";
 
-		#clock-cells = <1>;
+			clocks = <&ext>, <&rtc>;
+			clock-names = "ext", "rtc";
+
+			#clock-cells = <1>;
+		};
 	};
 
 	pinctrl: pin-controller@10010000 {
diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index e8248f9..2f12c7c 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -29,6 +29,18 @@
 
 #define MHZ (1000 * 1000)
 
+unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg)
+{
+	unsigned int val = 0;
+	regmap_read(cgu->regmap, reg, &val);
+	return val;
+}
+
+int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg)
+{
+	return regmap_write(cgu->regmap, reg, val);
+}
+
 /**
  * ingenic_cgu_gate_get() - get the value of clock gate register bit
  * @cgu: reference to the CGU whose registers should be read
@@ -43,7 +55,7 @@ static inline bool
 ingenic_cgu_gate_get(struct ingenic_cgu *cgu,
 		     const struct ingenic_cgu_gate_info *info)
 {
-	return readl(cgu->base + info->reg) & BIT(info->bit);
+	return cgu_readl(cgu, info->reg) & BIT(info->bit);
 }
 
 /**
@@ -60,14 +72,14 @@ static inline void
 ingenic_cgu_gate_set(struct ingenic_cgu *cgu,
 		     const struct ingenic_cgu_gate_info *info, bool val)
 {
-	u32 clkgr = readl(cgu->base + info->reg);
+	u32 clkgr = cgu_readl(cgu, info->reg);
 
 	if (val)
 		clkgr |= BIT(info->bit);
 	else
 		clkgr &= ~BIT(info->bit);
 
-	writel(clkgr, cgu->base + info->reg);
+	cgu_writel(cgu, clkgr, info->reg);
 }
 
 /*
@@ -91,7 +103,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	pll_info = &clk_info->pll;
 
 	spin_lock_irqsave(&cgu->lock, flags);
-	ctl = readl(cgu->base + pll_info->reg);
+	ctl = cgu_readl(cgu, pll_info->reg);
 	spin_unlock_irqrestore(&cgu->lock, flags);
 
 	m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
@@ -190,7 +202,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
 			clk_info->name, req_rate, rate);
 
 	spin_lock_irqsave(&cgu->lock, flags);
-	ctl = readl(cgu->base + pll_info->reg);
+	ctl = cgu_readl(cgu, pll_info->reg);
 
 	ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
 	ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
@@ -204,11 +216,11 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
 	ctl &= ~BIT(pll_info->bypass_bit);
 	ctl |= BIT(pll_info->enable_bit);
 
-	writel(ctl, cgu->base + pll_info->reg);
+	cgu_writel(cgu, ctl, pll_info->reg);
 
 	/* wait for the PLL to stabilise */
 	for (i = 0; i < timeout; i++) {
-		ctl = readl(cgu->base + pll_info->reg);
+		ctl = cgu_readl(cgu, pll_info->reg);
 		if (ctl & BIT(pll_info->stable_bit))
 			break;
 		mdelay(1);
@@ -243,7 +255,7 @@ static u8 ingenic_clk_get_parent(struct clk_hw *hw)
 	clk_info = &cgu->clock_info[ingenic_clk->idx];
 
 	if (clk_info->type & CGU_CLK_MUX) {
-		reg = readl(cgu->base + clk_info->mux.reg);
+		reg = cgu_readl(cgu, clk_info->mux.reg);
 		hw_idx = (reg >> clk_info->mux.shift) &
 			 GENMASK(clk_info->mux.bits - 1, 0);
 
@@ -297,10 +309,10 @@ static int ingenic_clk_set_parent(struct clk_hw *hw, u8 idx)
 		spin_lock_irqsave(&cgu->lock, flags);
 
 		/* write the register */
-		reg = readl(cgu->base + clk_info->mux.reg);
+		reg = cgu_readl(cgu, clk_info->mux.reg);
 		reg &= ~mask;
 		reg |= hw_idx << clk_info->mux.shift;
-		writel(reg, cgu->base + clk_info->mux.reg);
+		cgu_writel(cgu, reg, clk_info->mux.reg);
 
 		spin_unlock_irqrestore(&cgu->lock, flags);
 		return 0;
@@ -321,7 +333,7 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	clk_info = &cgu->clock_info[ingenic_clk->idx];
 
 	if (clk_info->type & CGU_CLK_DIV) {
-		div_reg = readl(cgu->base + clk_info->div.reg);
+		div_reg = cgu_readl(cgu, clk_info->div.reg);
 		div = (div_reg >> clk_info->div.shift) &
 		      GENMASK(clk_info->div.bits - 1, 0);
 		div += 1;
@@ -399,7 +411,7 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 			return -EINVAL;
 
 		spin_lock_irqsave(&cgu->lock, flags);
-		reg = readl(cgu->base + clk_info->div.reg);
+		reg = cgu_readl(cgu, clk_info->div.reg);
 
 		/* update the divide */
 		mask = GENMASK(clk_info->div.bits - 1, 0);
@@ -415,12 +427,12 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long req_rate,
 			reg |= BIT(clk_info->div.ce_bit);
 
 		/* update the hardware */
-		writel(reg, cgu->base + clk_info->div.reg);
+		cgu_writel(cgu, reg, clk_info->div.reg);
 
 		/* wait for the change to take effect */
 		if (clk_info->div.busy_bit != -1) {
 			for (i = 0; i < timeout; i++) {
-				reg = readl(cgu->base + clk_info->div.reg);
+				reg = cgu_readl(cgu, clk_info->div.reg);
 				if (!(reg & BIT(clk_info->div.busy_bit)))
 					break;
 				mdelay(1);
@@ -661,11 +673,9 @@ ingenic_cgu_new(const struct ingenic_cgu_clk_info *clock_info,
 	if (!cgu)
 		goto err_out;
 
-	cgu->base = of_iomap(np, 0);
-	if (!cgu->base) {
-		pr_err("%s: failed to map CGU registers\n", __func__);
+	cgu->regmap = syscon_node_to_regmap(np->parent);
+	if (cgu->regmap == NULL)
 		goto err_out_free;
-	}
 
 	cgu->np = np;
 	cgu->clock_info = clock_info;
diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
index 09700b2..86fd5b0 100644
--- a/drivers/clk/ingenic/cgu.h
+++ b/drivers/clk/ingenic/cgu.h
@@ -19,7 +19,9 @@
 #define __DRIVERS_CLK_INGENIC_CGU_H__
 
 #include <linux/bitops.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
+#include <linux/regmap.h>
 #include <linux/spinlock.h>
 
 /**
@@ -171,14 +173,14 @@ struct ingenic_cgu_clk_info {
 /**
  * struct ingenic_cgu - data about the CGU
  * @np: the device tree node that caused the CGU to be probed
- * @base: the ioremap'ed base address of the CGU registers
+ * @regmap: the regmap object used to access the CGU registers
  * @clock_info: an array containing information about implemented clocks
  * @clocks: used to provide clocks to DT, allows lookup of struct clk*
  * @lock: lock to be held whilst manipulating CGU registers
  */
 struct ingenic_cgu {
 	struct device_node *np;
-	void __iomem *base;
+	struct regmap *regmap;
 
 	const struct ingenic_cgu_clk_info *clock_info;
 	struct clk_onecell_data clocks;
@@ -186,6 +188,9 @@ struct ingenic_cgu {
 	spinlock_t lock;
 };
 
+unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg);
+int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg);
+
 /**
  * struct ingenic_clk - private data for a clock
  * @hw: see Documentation/clk.txt
diff --git a/drivers/clk/ingenic/jz4740-cgu.c b/drivers/clk/ingenic/jz4740-cgu.c
index 510fe7e..3003afb 100644
--- a/drivers/clk/ingenic/jz4740-cgu.c
+++ b/drivers/clk/ingenic/jz4740-cgu.c
@@ -232,7 +232,7 @@ CLK_OF_DECLARE(jz4740_cgu, "ingenic,jz4740-cgu", jz4740_cgu_init);
 
 void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode)
 {
-	uint32_t lcr = readl(cgu->base + CGU_REG_LCR);
+	uint32_t lcr = cgu_readl(cgu, CGU_REG_LCR);
 
 	switch (mode) {
 	case JZ4740_WAIT_MODE_IDLE:
@@ -244,24 +244,24 @@ void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode)
 		break;
 	}
 
-	writel(lcr, cgu->base + CGU_REG_LCR);
+	cgu_writel(cgu, lcr, CGU_REG_LCR);
 }
 
 void jz4740_clock_udc_disable_auto_suspend(void)
 {
-	uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR);
+	uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
 
 	clkgr &= ~CLKGR_UDC;
-	writel(clkgr, cgu->base + CGU_REG_CLKGR);
+	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
 }
 EXPORT_SYMBOL_GPL(jz4740_clock_udc_disable_auto_suspend);
 
 void jz4740_clock_udc_enable_auto_suspend(void)
 {
-	uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR);
+	uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
 
 	clkgr |= CLKGR_UDC;
-	writel(clkgr, cgu->base + CGU_REG_CLKGR);
+	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
 }
 EXPORT_SYMBOL_GPL(jz4740_clock_udc_enable_auto_suspend);
 
@@ -273,31 +273,31 @@ void jz4740_clock_suspend(void)
 {
 	uint32_t clkgr, cppcr;
 
-	clkgr = readl(cgu->base + CGU_REG_CLKGR);
+	clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
 	clkgr |= JZ_CLOCK_GATE_TCU | JZ_CLOCK_GATE_DMAC | JZ_CLOCK_GATE_UART0;
-	writel(clkgr, cgu->base + CGU_REG_CLKGR);
+	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
 
-	cppcr = readl(cgu->base + CGU_REG_CPPCR);
+	cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
 	cppcr &= ~BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit);
-	writel(cppcr, cgu->base + CGU_REG_CPPCR);
+	cgu_writel(cgu, cppcr, CGU_REG_CPPCR);
 }
 
 void jz4740_clock_resume(void)
 {
 	uint32_t clkgr, cppcr, stable;
 
-	cppcr = readl(cgu->base + CGU_REG_CPPCR);
+	cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
 	cppcr |= BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit);
-	writel(cppcr, cgu->base + CGU_REG_CPPCR);
+	cgu_writel(cgu, cppcr, CGU_REG_CPPCR);
 
 	stable = BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.stable_bit);
 	do {
-		cppcr = readl(cgu->base + CGU_REG_CPPCR);
+		cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
 	} while (!(cppcr & stable));
 
-	clkgr = readl(cgu->base + CGU_REG_CLKGR);
+	clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
 	clkgr &= ~JZ_CLOCK_GATE_TCU;
 	clkgr &= ~JZ_CLOCK_GATE_DMAC;
 	clkgr &= ~JZ_CLOCK_GATE_UART0;
-	writel(clkgr, cgu->base + CGU_REG_CLKGR);
+	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
 }
diff --git a/drivers/clk/ingenic/jz4780-cgu.c b/drivers/clk/ingenic/jz4780-cgu.c
index b35d6d9..8f83433 100644
--- a/drivers/clk/ingenic/jz4780-cgu.c
+++ b/drivers/clk/ingenic/jz4780-cgu.c
@@ -113,11 +113,11 @@ static int jz4780_otg_phy_set_parent(struct clk_hw *hw, u8 idx)
 
 	spin_lock_irqsave(&cgu->lock, flags);
 
-	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
+	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
 	usbpcr1 &= ~USBPCR1_REFCLKSEL_MASK;
 	/* we only use CLKCORE */
 	usbpcr1 |= USBPCR1_REFCLKSEL_CORE;
-	writel(usbpcr1, cgu->base + CGU_REG_USBPCR1);
+	cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1);
 
 	spin_unlock_irqrestore(&cgu->lock, flags);
 	return 0;
@@ -129,7 +129,7 @@ static unsigned long jz4780_otg_phy_recalc_rate(struct clk_hw *hw,
 	u32 usbpcr1;
 	unsigned refclk_div;
 
-	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
+	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
 	refclk_div = usbpcr1 & USBPCR1_REFCLKDIV_MASK;
 
 	switch (refclk_div) {
@@ -194,10 +194,10 @@ static int jz4780_otg_phy_set_rate(struct clk_hw *hw, unsigned long req_rate,
 
 	spin_lock_irqsave(&cgu->lock, flags);
 
-	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
+	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
 	usbpcr1 &= ~USBPCR1_REFCLKDIV_MASK;
 	usbpcr1 |= div_bits;
-	writel(usbpcr1, cgu->base + CGU_REG_USBPCR1);
+	cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1);
 
 	spin_unlock_irqrestore(&cgu->lock, flags);
 	return 0;
-- 
2.10.0

[PATCH 3/6] crypto: jz4780-rng: Add Ingenic JZ4780 hardware PRNG driver

[PrasannaKumar Muralidharan]

JZ4780 SoC pseudo random number generator driver using crypto framework.

Adding a delay before reading RNG data and disabling RNG after reading
data was suggested by Jeffery Walton.

Tested-by: Mathieu Malaterre <malat@debian.org>
Suggested-by: Jeffrey Walton <noloader@gmail.com>
Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 drivers/crypto/Kconfig      |  19 +++++
 drivers/crypto/Makefile     |   1 +
 drivers/crypto/jz4780-rng.c | 173 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/crypto/jz4780-rng.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 8fa7e72..8263622 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -614,6 +614,25 @@ config CRYPTO_DEV_IMGTEC_HASH
 	  hardware hash accelerator. Supporting MD5/SHA1/SHA224/SHA256
 	  hashing algorithms.
 
+config CRYPTO_DEV_JZ4780_RNG
+	tristate "JZ4780 HW pseudo random number generator support"
+	depends on MACH_JZ4780 || COMPILE_TEST
+	depends on HAS_IOMEM
+	select CRYPTO_RNG
+	select REGMAP
+	select SYSCON
+	select MFD_SYSCON
+	---help---
+	  This driver provides kernel-side support through the
+	  cryptographic API for the pseudo random number generator
+	  hardware found in ingenic JZ4780 and X1000 SoC. MIPS
+	  Creator CI20 uses JZ4780 SoC.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called jz4780-rng.
+
+	  If unsure, say Y.
+
 config CRYPTO_DEV_SUN4I_SS
 	tristate "Support for Allwinner Security System cryptographic accelerator"
 	depends on ARCH_SUNXI && !64BIT
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index b12eb3c..3a3d970 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_CRYPTO_DEV_GEODE) += geode-aes.o
 obj-$(CONFIG_CRYPTO_DEV_HIFN_795X) += hifn_795x.o
 obj-$(CONFIG_CRYPTO_DEV_IMGTEC_HASH) += img-hash.o
 obj-$(CONFIG_CRYPTO_DEV_IXP4XX) += ixp4xx_crypto.o
+obj-$(CONFIG_CRYPTO_DEV_JZ4780_RNG) += jz4780-rng.o
 obj-$(CONFIG_CRYPTO_DEV_MV_CESA) += mv_cesa.o
 obj-$(CONFIG_CRYPTO_DEV_MARVELL_CESA) += marvell/
 obj-$(CONFIG_CRYPTO_DEV_MEDIATEK) += mediatek/
diff --git a/drivers/crypto/jz4780-rng.c b/drivers/crypto/jz4780-rng.c
new file mode 100644
index 0000000..a03f2a0
--- /dev/null
+++ b/drivers/crypto/jz4780-rng.c
@@ -0,0 +1,173 @@
+/*
+ * jz4780-rng.c - Random Number Generator driver for the jz4780
+ *
+ * Copyright (c) 2017 PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation;
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/crypto.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <crypto/internal/rng.h>
+
+#define REG_RNG_CTRL	0xD8
+#define REG_RNG_DATA	0xDC
+
+/* Context for crypto */
+struct jz4780_rng_ctx {
+	struct jz4780_rng *rng;
+};
+
+/* Device associated memory */
+struct jz4780_rng {
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+static struct jz4780_rng *jz4780_rng;
+
+static int jz4780_rng_readl(struct jz4780_rng *rng, u32 offset)
+{
+	u32 val = 0;
+	int ret;
+
+	ret = regmap_read(rng->regmap, offset, &val);
+	if (!ret)
+		return val;
+
+	return ret;
+}
+
+static int jz4780_rng_writel(struct jz4780_rng *rng, u32 val, u32 offset)
+{
+	return regmap_write(rng->regmap, offset, val);
+}
+
+static int jz4780_rng_generate(struct crypto_rng *tfm,
+			       const u8 *src, unsigned int slen,
+			       u8 *dst, unsigned int dlen)
+{
+	struct jz4780_rng_ctx *ctx = crypto_rng_ctx(tfm);
+	struct jz4780_rng *rng = ctx->rng;
+	u32 data;
+
+	/*
+	 * JZ4780 Programmers manual says the RNG should not run continuously
+	 * for more than 1s. So enable RNG, read data and disable it.
+	 * NOTE: No issue was observed with MIPS creator CI20 board even when
+	 * RNG ran continuously for longer periods. This is just a precaution.
+	 *
+	 * A delay is required so that the current RNG data is not bit shifted
+	 * version of previous RNG data which could happen if random data is
+	 * read continuously from this device.
+	 */
+	jz4780_rng_writel(rng, 1, REG_RNG_CTRL);
+	do {
+		data = jz4780_rng_readl(rng, REG_RNG_DATA);
+		memcpy((void *)dst, (void *)&data, 4);
+		dlen -= 4;
+		dst += 4;
+		udelay(20);
+	} while (dlen >= 4);
+
+	if (dlen > 0) {
+		data = jz4780_rng_readl(rng, REG_RNG_DATA);
+		memcpy((void *)dst, (void *)&data, dlen);
+	}
+	jz4780_rng_writel(rng, 0, REG_RNG_CTRL);
+
+	return 0;
+}
+
+static int jz4780_rng_kcapi_init(struct crypto_tfm *tfm)
+{
+	struct jz4780_rng_ctx *ctx = crypto_tfm_ctx(tfm);
+
+	ctx->rng = jz4780_rng;
+
+	return 0;
+}
+
+static struct rng_alg jz4780_rng_alg = {
+	.generate		= jz4780_rng_generate,
+	.base			= {
+		.cra_name		= "stdrng",
+		.cra_driver_name	= "jz4780_rng",
+		.cra_priority		= 100,
+		.cra_ctxsize		= sizeof(struct jz4780_rng_ctx),
+		.cra_module		= THIS_MODULE,
+		.cra_init		= jz4780_rng_kcapi_init,
+	}
+};
+
+static int jz4780_rng_probe(struct platform_device *pdev)
+{
+	struct jz4780_rng *rng;
+	struct resource *res;
+	int ret;
+
+	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
+	if (!rng)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rng->regmap = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(rng->regmap))
+		return PTR_ERR(rng->regmap);
+
+	jz4780_rng = rng;
+
+	ret = crypto_register_rng(&jz4780_rng_alg);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Couldn't register rng crypto alg: %d\n", ret);
+		jz4780_rng = NULL;
+	}
+
+	return ret;
+}
+
+static int jz4780_rng_remove(struct platform_device *pdev)
+{
+	crypto_unregister_rng(&jz4780_rng_alg);
+
+	jz4780_rng = NULL;
+
+	return 0;
+}
+
+static const struct of_device_id jz4780_rng_dt_match[] = {
+	{
+		.compatible = "ingenic,jz4780-rng",
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, jz4780_rng_dt_match);
+
+static struct platform_driver jz4780_rng_driver = {
+	.driver		= {
+		.name	= "jz4780-rng",
+		.of_match_table = jz4780_rng_dt_match,
+	},
+	.probe		= jz4780_rng_probe,
+	.remove		= jz4780_rng_remove,
+};
+
+module_platform_driver(jz4780_rng_driver);
+
+MODULE_DESCRIPTION("Ingenic JZ4780 H/W Pseudo Random Number Generator driver");
+MODULE_AUTHOR("PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
2.10.0

[PATCH 4/6] crypto: jz4780-rng: Add RNG node to jz4780.dtsi

[PrasannaKumar Muralidharan]

This patch adds RNG node to jz4780.dtsi.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 1301694..865b9bf 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -46,6 +46,10 @@
 
 			#clock-cells = <1>;
 		};
+
+		rng: rng@d8 {
+			compatible = "ingenic,jz480-rng";
+		};
 	};
 
 	pinctrl: pin-controller@10010000 {
-- 
2.10.0

[PATCH 5/6] crypto: jz4780-rng: Add myself as mainatainer for JZ4780 PRNG driver

[PrasannaKumar Muralidharan]

Add myself as the maintainer of JZ4780 SoC's PRNG drvier.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 45ec467..ee8c6f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6733,6 +6733,11 @@ L:	linux-mtd@lists.infradead.org
 S:	Maintained
 F:	drivers/mtd/nand/jz4780_*
 
+INGENIC JZ4780 PRNG DRIVER
+M:	PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
+S:	Maintained
+F:	drivers/crypto/jz4780-rng.c
+
 INOTIFY
 M:	John McCutchan <john@johnmccutchan.com>
 M:	Robert Love <rlove@rlove.org>
-- 
2.10.0

[PATCH 6/6] crypto: jz4780-rng: Enable PRNG support in CI20 defconfig

[PrasannaKumar Muralidharan]

Enable PRNG driver support in MIPS Creator CI20 default config.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 arch/mips/configs/ci20_defconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/mips/configs/ci20_defconfig b/arch/mips/configs/ci20_defconfig
index b42cfa7..9f48f2c 100644
--- a/arch/mips/configs/ci20_defconfig
+++ b/arch/mips/configs/ci20_defconfig
@@ -88,6 +88,11 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=5
 CONFIG_SERIAL_8250_INGENIC=y
 CONFIG_SERIAL_OF_PLATFORM=y
 # CONFIG_HW_RANDOM is not set
+CONFIG_CRYPTO_USER=y
+CONFIG_CRYPTO_USER_API=y
+CONFIG_CRYPTO_USER_API_RNG=y
+CONFIG_CRYPTO_HW=y
+CONFIG_CRYPTO_DEV_JZ4780_RNG=y
 CONFIG_I2C=y
 CONFIG_I2C_JZ4780=y
 CONFIG_GPIO_SYSFS=y
-- 
2.10.0

Re: [PATCH 3/6] crypto: jz4780-rng: Add Ingenic JZ4780 hardware PRNG driver

[Stephan Mueller]

Am Donnerstag, 17. August 2017, 20:25:17 CEST schrieb PrasannaKumar 
Muralidharan:

Hi PrasannaKumar,

> +
> +static int jz4780_rng_generate(struct crypto_rng *tfm,
> +			       const u8 *src, unsigned int slen,
> +			       u8 *dst, unsigned int dlen)
> +{
> +	struct jz4780_rng_ctx *ctx = crypto_rng_ctx(tfm);
> +	struct jz4780_rng *rng = ctx->rng;
> +	u32 data;
> +
> +	/*
> +	 * JZ4780 Programmers manual says the RNG should not run continuously
> +	 * for more than 1s. So enable RNG, read data and disable it.
> +	 * NOTE: No issue was observed with MIPS creator CI20 board even when
> +	 * RNG ran continuously for longer periods. This is just a precaution.
> +	 *
> +	 * A delay is required so that the current RNG data is not bit shifted
> +	 * version of previous RNG data which could happen if random data is
> +	 * read continuously from this device.
> +	 */
> +	jz4780_rng_writel(rng, 1, REG_RNG_CTRL);
> +	do {
> +		data = jz4780_rng_readl(rng, REG_RNG_DATA);
> +		memcpy((void *)dst, (void *)&data, 4);

How do you know that dst is a multiple of 4 bytes? When dlen is only 3, you 
overflow the buffer.

> +		dlen -= 4;
> +		dst += 4;
> +		udelay(20);
> +	} while (dlen >= 4);
> +
> +	if (dlen > 0) {
> +		data = jz4780_rng_readl(rng, REG_RNG_DATA);
> +		memcpy((void *)dst, (void *)&data, dlen);
> +	}
> +	jz4780_rng_writel(rng, 0, REG_RNG_CTRL);
> +
> +	return 0;
> +}

Ciao
Stephan

Re: [PATCH 3/6] crypto: jz4780-rng: Add Ingenic JZ4780 hardware PRNG driver

[PrasannaKumar Muralidharan]

Hi Stephan,

On 18 August 2017 at 00:22, Stephan Mueller <smueller@chronox.de> wrote:
> Am Donnerstag, 17. August 2017, 20:25:17 CEST schrieb PrasannaKumar
> Muralidharan:
>
> Hi PrasannaKumar,
>
>> +
>> +static int jz4780_rng_generate(struct crypto_rng *tfm,
>> +                            const u8 *src, unsigned int slen,
>> +                            u8 *dst, unsigned int dlen)
>> +{
>> +     struct jz4780_rng_ctx *ctx = crypto_rng_ctx(tfm);
>> +     struct jz4780_rng *rng = ctx->rng;
>> +     u32 data;
>> +
>> +     /*
>> +      * JZ4780 Programmers manual says the RNG should not run continuously
>> +      * for more than 1s. So enable RNG, read data and disable it.
>> +      * NOTE: No issue was observed with MIPS creator CI20 board even when
>> +      * RNG ran continuously for longer periods. This is just a precaution.
>> +      *
>> +      * A delay is required so that the current RNG data is not bit shifted
>> +      * version of previous RNG data which could happen if random data is
>> +      * read continuously from this device.
>> +      */
>> +     jz4780_rng_writel(rng, 1, REG_RNG_CTRL);
>> +     do {
>> +             data = jz4780_rng_readl(rng, REG_RNG_DATA);
>> +             memcpy((void *)dst, (void *)&data, 4);
>
> How do you know that dst is a multiple of 4 bytes? When dlen is only 3, you
> overflow the buffer.

You are right. I initially used hw_random framework for this driver.
But later realised that PRNG driver should use crypto framework. When
I started using crypto I reused most of the code. This was because of
porting. Will change it and send next version.

>
>> +             dlen -= 4;
>> +             dst += 4;
>> +             udelay(20);
>> +     } while (dlen >= 4);
>> +
>> +     if (dlen > 0) {
>> +             data = jz4780_rng_readl(rng, REG_RNG_DATA);
>> +             memcpy((void *)dst, (void *)&data, dlen);
>> +     }
>> +     jz4780_rng_writel(rng, 0, REG_RNG_CTRL);
>> +
>> +     return 0;
>> +}
>
> Ciao
> Stephan

Thanks,
PrasannaKumar

Re: [PATCH 2/6] crypto: jz4780-rng: Make ingenic CGU driver use syscon

[Paul Burton]

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

Hi PrasannaKumar,

On Thursday, 17 August 2017 11:25:16 PDT PrasannaKumar Muralidharan wrote:
> Ingenic PRNG registers are a part of the same hardware block as clock
> and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power
> related registers that are after the PRNG registers. So instead of
> shortening the register range use syscon interface to expose regmap.
> This regmap is used by the PRNG driver.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++----
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++----
>  drivers/clk/ingenic/cgu.c              | 46
> +++++++++++++++++++++------------- drivers/clk/ingenic/cgu.h              |
>  9 +++++--
>  drivers/clk/ingenic/jz4740-cgu.c       | 30 +++++++++++-----------
>  drivers/clk/ingenic/jz4780-cgu.c       | 10 ++++----
>  6 files changed, 73 insertions(+), 50 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 2ca7ce7..ada5e67 100644
> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
> @@ -34,14 +34,18 @@
>  		clock-frequency = <32768>;
>  	};
> 
> -	cgu: jz4740-cgu@10000000 {
> -		compatible = "ingenic,jz4740-cgu";
> +	cgu_registers {
> +		compatible = "simple-mfd", "syscon";
>  		reg = <0x10000000 0x100>;
> 
> -		clocks = <&ext>, <&rtc>;
> -		clock-names = "ext", "rtc";
> +		cgu: jz4780-cgu@10000000 {
> +			compatible = "ingenic,jz4740-cgu";
> 
> -		#clock-cells = <1>;
> +			clocks = <&ext>, <&rtc>;
> +			clock-names = "ext", "rtc";
> +
> +			#clock-cells = <1>;
> +		};
>  	};
> 
>  	rtc_dev: rtc@10003000 {
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 4853ef6..1301694 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -34,14 +34,18 @@
>  		clock-frequency = <32768>;
>  	};
> 
> -	cgu: jz4780-cgu@10000000 {
> -		compatible = "ingenic,jz4780-cgu";
> +	cgu_registers {
> +		compatible = "simple-mfd", "syscon";
>  		reg = <0x10000000 0x100>;
> 
> -		clocks = <&ext>, <&rtc>;
> -		clock-names = "ext", "rtc";
> +		cgu: jz4780-cgu@10000000 {
> +			compatible = "ingenic,jz4780-cgu";
> 
> -		#clock-cells = <1>;
> +			clocks = <&ext>, <&rtc>;
> +			clock-names = "ext", "rtc";
> +
> +			#clock-cells = <1>;
> +		};
>  	};
> 
>  	pinctrl: pin-controller@10010000 {
> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
> index e8248f9..2f12c7c 100644
> --- a/drivers/clk/ingenic/cgu.c
> +++ b/drivers/clk/ingenic/cgu.c
> @@ -29,6 +29,18 @@
> 
>  #define MHZ (1000 * 1000)
> 
> +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg)
> +{
> +	unsigned int val = 0;
> +	regmap_read(cgu->regmap, reg, &val);
> +	return val;
> +}
> +
> +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg)
> +{
> +	return regmap_write(cgu->regmap, reg, val);
> +}

This is going to introduce a lot of overhead to the CGU driver - each 
regmap_read() or regmap_write() call will not only add overhead from the 
indirection but also acquire a lock which we really don't need.

Could you instead perhaps:

 - Just add "syscon" as a second compatible string to the CGU node in the
   device tree, but otherwise leave it as-is without the extra cgu_registers
   node.

 - Have your RNG device node as a child of the CGU node, which should let it
   pick up the regmap via syscon_node_to_regmap() as it already does.

 - Leave the CGU driver as-is, so it can continue accessing memory directly
   rather than via regmap.

Thanks,
    Paul

> +
>  /**
>   * ingenic_cgu_gate_get() - get the value of clock gate register bit
>   * @cgu: reference to the CGU whose registers should be read
> @@ -43,7 +55,7 @@ static inline bool
>  ingenic_cgu_gate_get(struct ingenic_cgu *cgu,
>  		     const struct ingenic_cgu_gate_info *info)
>  {
> -	return readl(cgu->base + info->reg) & BIT(info->bit);
> +	return cgu_readl(cgu, info->reg) & BIT(info->bit);
>  }
> 
>  /**
> @@ -60,14 +72,14 @@ static inline void
>  ingenic_cgu_gate_set(struct ingenic_cgu *cgu,
>  		     const struct ingenic_cgu_gate_info *info, bool val)
>  {
> -	u32 clkgr = readl(cgu->base + info->reg);
> +	u32 clkgr = cgu_readl(cgu, info->reg);
> 
>  	if (val)
>  		clkgr |= BIT(info->bit);
>  	else
>  		clkgr &= ~BIT(info->bit);
> 
> -	writel(clkgr, cgu->base + info->reg);
> +	cgu_writel(cgu, clkgr, info->reg);
>  }
> 
>  /*
> @@ -91,7 +103,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long
> parent_rate) pll_info = &clk_info->pll;
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> -	ctl = readl(cgu->base + pll_info->reg);
> +	ctl = cgu_readl(cgu, pll_info->reg);
>  	spin_unlock_irqrestore(&cgu->lock, flags);
> 
>  	m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
> @@ -190,7 +202,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long
> req_rate, clk_info->name, req_rate, rate);
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> -	ctl = readl(cgu->base + pll_info->reg);
> +	ctl = cgu_readl(cgu, pll_info->reg);
> 
>  	ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
>  	ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
> @@ -204,11 +216,11 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long
> req_rate, ctl &= ~BIT(pll_info->bypass_bit);
>  	ctl |= BIT(pll_info->enable_bit);
> 
> -	writel(ctl, cgu->base + pll_info->reg);
> +	cgu_writel(cgu, ctl, pll_info->reg);
> 
>  	/* wait for the PLL to stabilise */
>  	for (i = 0; i < timeout; i++) {
> -		ctl = readl(cgu->base + pll_info->reg);
> +		ctl = cgu_readl(cgu, pll_info->reg);
>  		if (ctl & BIT(pll_info->stable_bit))
>  			break;
>  		mdelay(1);
> @@ -243,7 +255,7 @@ static u8 ingenic_clk_get_parent(struct clk_hw *hw)
>  	clk_info = &cgu->clock_info[ingenic_clk->idx];
> 
>  	if (clk_info->type & CGU_CLK_MUX) {
> -		reg = readl(cgu->base + clk_info->mux.reg);
> +		reg = cgu_readl(cgu, clk_info->mux.reg);
>  		hw_idx = (reg >> clk_info->mux.shift) &
>  			 GENMASK(clk_info->mux.bits - 1, 0);
> 
> @@ -297,10 +309,10 @@ static int ingenic_clk_set_parent(struct clk_hw *hw,
> u8 idx) spin_lock_irqsave(&cgu->lock, flags);
> 
>  		/* write the register */
> -		reg = readl(cgu->base + clk_info->mux.reg);
> +		reg = cgu_readl(cgu, clk_info->mux.reg);
>  		reg &= ~mask;
>  		reg |= hw_idx << clk_info->mux.shift;
> -		writel(reg, cgu->base + clk_info->mux.reg);
> +		cgu_writel(cgu, reg, clk_info->mux.reg);
> 
>  		spin_unlock_irqrestore(&cgu->lock, flags);
>  		return 0;
> @@ -321,7 +333,7 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long
> parent_rate) clk_info = &cgu->clock_info[ingenic_clk->idx];
> 
>  	if (clk_info->type & CGU_CLK_DIV) {
> -		div_reg = readl(cgu->base + clk_info->div.reg);
> +		div_reg = cgu_readl(cgu, clk_info->div.reg);
>  		div = (div_reg >> clk_info->div.shift) &
>  		      GENMASK(clk_info->div.bits - 1, 0);
>  		div += 1;
> @@ -399,7 +411,7 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long
> req_rate, return -EINVAL;
> 
>  		spin_lock_irqsave(&cgu->lock, flags);
> -		reg = readl(cgu->base + clk_info->div.reg);
> +		reg = cgu_readl(cgu, clk_info->div.reg);
> 
>  		/* update the divide */
>  		mask = GENMASK(clk_info->div.bits - 1, 0);
> @@ -415,12 +427,12 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long
> req_rate, reg |= BIT(clk_info->div.ce_bit);
> 
>  		/* update the hardware */
> -		writel(reg, cgu->base + clk_info->div.reg);
> +		cgu_writel(cgu, reg, clk_info->div.reg);
> 
>  		/* wait for the change to take effect */
>  		if (clk_info->div.busy_bit != -1) {
>  			for (i = 0; i < timeout; i++) {
> -				reg = readl(cgu->base + clk_info->div.reg);
> +				reg = cgu_readl(cgu, clk_info->div.reg);
>  				if (!(reg & BIT(clk_info->div.busy_bit)))
>  					break;
>  				mdelay(1);
> @@ -661,11 +673,9 @@ ingenic_cgu_new(const struct ingenic_cgu_clk_info
> *clock_info, if (!cgu)
>  		goto err_out;
> 
> -	cgu->base = of_iomap(np, 0);
> -	if (!cgu->base) {
> -		pr_err("%s: failed to map CGU registers\n", __func__);
> +	cgu->regmap = syscon_node_to_regmap(np->parent);
> +	if (cgu->regmap == NULL)
>  		goto err_out_free;
> -	}
> 
>  	cgu->np = np;
>  	cgu->clock_info = clock_info;
> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
> index 09700b2..86fd5b0 100644
> --- a/drivers/clk/ingenic/cgu.h
> +++ b/drivers/clk/ingenic/cgu.h
> @@ -19,7 +19,9 @@
>  #define __DRIVERS_CLK_INGENIC_CGU_H__
> 
>  #include <linux/bitops.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of.h>
> +#include <linux/regmap.h>
>  #include <linux/spinlock.h>
> 
>  /**
> @@ -171,14 +173,14 @@ struct ingenic_cgu_clk_info {
>  /**
>   * struct ingenic_cgu - data about the CGU
>   * @np: the device tree node that caused the CGU to be probed
> - * @base: the ioremap'ed base address of the CGU registers
> + * @regmap: the regmap object used to access the CGU registers
>   * @clock_info: an array containing information about implemented clocks
>   * @clocks: used to provide clocks to DT, allows lookup of struct clk*
>   * @lock: lock to be held whilst manipulating CGU registers
>   */
>  struct ingenic_cgu {
>  	struct device_node *np;
> -	void __iomem *base;
> +	struct regmap *regmap;
> 
>  	const struct ingenic_cgu_clk_info *clock_info;
>  	struct clk_onecell_data clocks;
> @@ -186,6 +188,9 @@ struct ingenic_cgu {
>  	spinlock_t lock;
>  };
> 
> +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg);
> +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int
> reg); +
>  /**
>   * struct ingenic_clk - private data for a clock
>   * @hw: see Documentation/clk.txt
> diff --git a/drivers/clk/ingenic/jz4740-cgu.c
> b/drivers/clk/ingenic/jz4740-cgu.c index 510fe7e..3003afb 100644
> --- a/drivers/clk/ingenic/jz4740-cgu.c
> +++ b/drivers/clk/ingenic/jz4740-cgu.c
> @@ -232,7 +232,7 @@ CLK_OF_DECLARE(jz4740_cgu, "ingenic,jz4740-cgu",
> jz4740_cgu_init);
> 
>  void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode)
>  {
> -	uint32_t lcr = readl(cgu->base + CGU_REG_LCR);
> +	uint32_t lcr = cgu_readl(cgu, CGU_REG_LCR);
> 
>  	switch (mode) {
>  	case JZ4740_WAIT_MODE_IDLE:
> @@ -244,24 +244,24 @@ void jz4740_clock_set_wait_mode(enum jz4740_wait_mode
> mode) break;
>  	}
> 
> -	writel(lcr, cgu->base + CGU_REG_LCR);
> +	cgu_writel(cgu, lcr, CGU_REG_LCR);
>  }
> 
>  void jz4740_clock_udc_disable_auto_suspend(void)
>  {
> -	uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
> 
>  	clkgr &= ~CLKGR_UDC;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
>  }
>  EXPORT_SYMBOL_GPL(jz4740_clock_udc_disable_auto_suspend);
> 
>  void jz4740_clock_udc_enable_auto_suspend(void)
>  {
> -	uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
> 
>  	clkgr |= CLKGR_UDC;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
>  }
>  EXPORT_SYMBOL_GPL(jz4740_clock_udc_enable_auto_suspend);
> 
> @@ -273,31 +273,31 @@ void jz4740_clock_suspend(void)
>  {
>  	uint32_t clkgr, cppcr;
> 
> -	clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
>  	clkgr |= JZ_CLOCK_GATE_TCU | JZ_CLOCK_GATE_DMAC | JZ_CLOCK_GATE_UART0;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
> 
> -	cppcr = readl(cgu->base + CGU_REG_CPPCR);
> +	cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
>  	cppcr &= ~BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit);
> -	writel(cppcr, cgu->base + CGU_REG_CPPCR);
> +	cgu_writel(cgu, cppcr, CGU_REG_CPPCR);
>  }
> 
>  void jz4740_clock_resume(void)
>  {
>  	uint32_t clkgr, cppcr, stable;
> 
> -	cppcr = readl(cgu->base + CGU_REG_CPPCR);
> +	cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
>  	cppcr |= BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit);
> -	writel(cppcr, cgu->base + CGU_REG_CPPCR);
> +	cgu_writel(cgu, cppcr, CGU_REG_CPPCR);
> 
>  	stable = BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.stable_bit);
>  	do {
> -		cppcr = readl(cgu->base + CGU_REG_CPPCR);
> +		cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
>  	} while (!(cppcr & stable));
> 
> -	clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
>  	clkgr &= ~JZ_CLOCK_GATE_TCU;
>  	clkgr &= ~JZ_CLOCK_GATE_DMAC;
>  	clkgr &= ~JZ_CLOCK_GATE_UART0;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
>  }
> diff --git a/drivers/clk/ingenic/jz4780-cgu.c
> b/drivers/clk/ingenic/jz4780-cgu.c index b35d6d9..8f83433 100644
> --- a/drivers/clk/ingenic/jz4780-cgu.c
> +++ b/drivers/clk/ingenic/jz4780-cgu.c
> @@ -113,11 +113,11 @@ static int jz4780_otg_phy_set_parent(struct clk_hw
> *hw, u8 idx)
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> 
> -	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
> +	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
>  	usbpcr1 &= ~USBPCR1_REFCLKSEL_MASK;
>  	/* we only use CLKCORE */
>  	usbpcr1 |= USBPCR1_REFCLKSEL_CORE;
> -	writel(usbpcr1, cgu->base + CGU_REG_USBPCR1);
> +	cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1);
> 
>  	spin_unlock_irqrestore(&cgu->lock, flags);
>  	return 0;
> @@ -129,7 +129,7 @@ static unsigned long jz4780_otg_phy_recalc_rate(struct
> clk_hw *hw, u32 usbpcr1;
>  	unsigned refclk_div;
> 
> -	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
> +	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
>  	refclk_div = usbpcr1 & USBPCR1_REFCLKDIV_MASK;
> 
>  	switch (refclk_div) {
> @@ -194,10 +194,10 @@ static int jz4780_otg_phy_set_rate(struct clk_hw *hw,
> unsigned long req_rate,
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> 
> -	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
> +	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
>  	usbpcr1 &= ~USBPCR1_REFCLKDIV_MASK;
>  	usbpcr1 |= div_bits;
> -	writel(usbpcr1, cgu->base + CGU_REG_USBPCR1);
> +	cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1);
> 
>  	spin_unlock_irqrestore(&cgu->lock, flags);
>  	return 0;


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/6] crypto: jz4780-rng: Make ingenic CGU driver use syscon

[Paul Burton]

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

Hi PrasannaKumar,

On Thursday, 17 August 2017 11:25:16 PDT PrasannaKumar Muralidharan wrote:
> Ingenic PRNG registers are a part of the same hardware block as clock
> and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power
> related registers that are after the PRNG registers. So instead of
> shortening the register range use syscon interface to expose regmap.
> This regmap is used by the PRNG driver.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++----
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++----
>  drivers/clk/ingenic/cgu.c              | 46
> +++++++++++++++++++++------------- drivers/clk/ingenic/cgu.h              |
>  9 +++++--
>  drivers/clk/ingenic/jz4740-cgu.c       | 30 +++++++++++-----------
>  drivers/clk/ingenic/jz4780-cgu.c       | 10 ++++----
>  6 files changed, 73 insertions(+), 50 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 2ca7ce7..ada5e67 100644
> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
> @@ -34,14 +34,18 @@
>  		clock-frequency = <32768>;
>  	};
> 
> -	cgu: jz4740-cgu@10000000 {
> -		compatible = "ingenic,jz4740-cgu";
> +	cgu_registers {
> +		compatible = "simple-mfd", "syscon";
>  		reg = <0x10000000 0x100>;
> 
> -		clocks = <&ext>, <&rtc>;
> -		clock-names = "ext", "rtc";
> +		cgu: jz4780-cgu@10000000 {
> +			compatible = "ingenic,jz4740-cgu";
> 
> -		#clock-cells = <1>;
> +			clocks = <&ext>, <&rtc>;
> +			clock-names = "ext", "rtc";
> +
> +			#clock-cells = <1>;
> +		};
>  	};
> 
>  	rtc_dev: rtc@10003000 {
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 4853ef6..1301694 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -34,14 +34,18 @@
>  		clock-frequency = <32768>;
>  	};
> 
> -	cgu: jz4780-cgu@10000000 {
> -		compatible = "ingenic,jz4780-cgu";
> +	cgu_registers {
> +		compatible = "simple-mfd", "syscon";
>  		reg = <0x10000000 0x100>;
> 
> -		clocks = <&ext>, <&rtc>;
> -		clock-names = "ext", "rtc";
> +		cgu: jz4780-cgu@10000000 {
> +			compatible = "ingenic,jz4780-cgu";
> 
> -		#clock-cells = <1>;
> +			clocks = <&ext>, <&rtc>;
> +			clock-names = "ext", "rtc";
> +
> +			#clock-cells = <1>;
> +		};
>  	};
> 
>  	pinctrl: pin-controller@10010000 {
> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
> index e8248f9..2f12c7c 100644
> --- a/drivers/clk/ingenic/cgu.c
> +++ b/drivers/clk/ingenic/cgu.c
> @@ -29,6 +29,18 @@
> 
>  #define MHZ (1000 * 1000)
> 
> +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg)
> +{
> +	unsigned int val = 0;
> +	regmap_read(cgu->regmap, reg, &val);
> +	return val;
> +}
> +
> +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg)
> +{
> +	return regmap_write(cgu->regmap, reg, val);
> +}

This is going to introduce a lot of overhead to the CGU driver - each 
regmap_read() or regmap_write() call will not only add overhead from the 
indirection but also acquire a lock which we really don't need.

Could you instead perhaps:

 - Just add "syscon" as a second compatible string to the CGU node in the
   device tree, but otherwise leave it as-is without the extra cgu_registers
   node.

 - Have your RNG device node as a child of the CGU node, which should let it
   pick up the regmap via syscon_node_to_regmap() as it already does.

 - Leave the CGU driver as-is, so it can continue accessing memory directly
   rather than via regmap.

Thanks,
    Paul

> +
>  /**
>   * ingenic_cgu_gate_get() - get the value of clock gate register bit
>   * @cgu: reference to the CGU whose registers should be read
> @@ -43,7 +55,7 @@ static inline bool
>  ingenic_cgu_gate_get(struct ingenic_cgu *cgu,
>  		     const struct ingenic_cgu_gate_info *info)
>  {
> -	return readl(cgu->base + info->reg) & BIT(info->bit);
> +	return cgu_readl(cgu, info->reg) & BIT(info->bit);
>  }
> 
>  /**
> @@ -60,14 +72,14 @@ static inline void
>  ingenic_cgu_gate_set(struct ingenic_cgu *cgu,
>  		     const struct ingenic_cgu_gate_info *info, bool val)
>  {
> -	u32 clkgr = readl(cgu->base + info->reg);
> +	u32 clkgr = cgu_readl(cgu, info->reg);
> 
>  	if (val)
>  		clkgr |= BIT(info->bit);
>  	else
>  		clkgr &= ~BIT(info->bit);
> 
> -	writel(clkgr, cgu->base + info->reg);
> +	cgu_writel(cgu, clkgr, info->reg);
>  }
> 
>  /*
> @@ -91,7 +103,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long
> parent_rate) pll_info = &clk_info->pll;
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> -	ctl = readl(cgu->base + pll_info->reg);
> +	ctl = cgu_readl(cgu, pll_info->reg);
>  	spin_unlock_irqrestore(&cgu->lock, flags);
> 
>  	m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
> @@ -190,7 +202,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long
> req_rate, clk_info->name, req_rate, rate);
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> -	ctl = readl(cgu->base + pll_info->reg);
> +	ctl = cgu_readl(cgu, pll_info->reg);
> 
>  	ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
>  	ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
> @@ -204,11 +216,11 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long
> req_rate, ctl &= ~BIT(pll_info->bypass_bit);
>  	ctl |= BIT(pll_info->enable_bit);
> 
> -	writel(ctl, cgu->base + pll_info->reg);
> +	cgu_writel(cgu, ctl, pll_info->reg);
> 
>  	/* wait for the PLL to stabilise */
>  	for (i = 0; i < timeout; i++) {
> -		ctl = readl(cgu->base + pll_info->reg);
> +		ctl = cgu_readl(cgu, pll_info->reg);
>  		if (ctl & BIT(pll_info->stable_bit))
>  			break;
>  		mdelay(1);
> @@ -243,7 +255,7 @@ static u8 ingenic_clk_get_parent(struct clk_hw *hw)
>  	clk_info = &cgu->clock_info[ingenic_clk->idx];
> 
>  	if (clk_info->type & CGU_CLK_MUX) {
> -		reg = readl(cgu->base + clk_info->mux.reg);
> +		reg = cgu_readl(cgu, clk_info->mux.reg);
>  		hw_idx = (reg >> clk_info->mux.shift) &
>  			 GENMASK(clk_info->mux.bits - 1, 0);
> 
> @@ -297,10 +309,10 @@ static int ingenic_clk_set_parent(struct clk_hw *hw,
> u8 idx) spin_lock_irqsave(&cgu->lock, flags);
> 
>  		/* write the register */
> -		reg = readl(cgu->base + clk_info->mux.reg);
> +		reg = cgu_readl(cgu, clk_info->mux.reg);
>  		reg &= ~mask;
>  		reg |= hw_idx << clk_info->mux.shift;
> -		writel(reg, cgu->base + clk_info->mux.reg);
> +		cgu_writel(cgu, reg, clk_info->mux.reg);
> 
>  		spin_unlock_irqrestore(&cgu->lock, flags);
>  		return 0;
> @@ -321,7 +333,7 @@ ingenic_clk_recalc_rate(struct clk_hw *hw, unsigned long
> parent_rate) clk_info = &cgu->clock_info[ingenic_clk->idx];
> 
>  	if (clk_info->type & CGU_CLK_DIV) {
> -		div_reg = readl(cgu->base + clk_info->div.reg);
> +		div_reg = cgu_readl(cgu, clk_info->div.reg);
>  		div = (div_reg >> clk_info->div.shift) &
>  		      GENMASK(clk_info->div.bits - 1, 0);
>  		div += 1;
> @@ -399,7 +411,7 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long
> req_rate, return -EINVAL;
> 
>  		spin_lock_irqsave(&cgu->lock, flags);
> -		reg = readl(cgu->base + clk_info->div.reg);
> +		reg = cgu_readl(cgu, clk_info->div.reg);
> 
>  		/* update the divide */
>  		mask = GENMASK(clk_info->div.bits - 1, 0);
> @@ -415,12 +427,12 @@ ingenic_clk_set_rate(struct clk_hw *hw, unsigned long
> req_rate, reg |= BIT(clk_info->div.ce_bit);
> 
>  		/* update the hardware */
> -		writel(reg, cgu->base + clk_info->div.reg);
> +		cgu_writel(cgu, reg, clk_info->div.reg);
> 
>  		/* wait for the change to take effect */
>  		if (clk_info->div.busy_bit != -1) {
>  			for (i = 0; i < timeout; i++) {
> -				reg = readl(cgu->base + clk_info->div.reg);
> +				reg = cgu_readl(cgu, clk_info->div.reg);
>  				if (!(reg & BIT(clk_info->div.busy_bit)))
>  					break;
>  				mdelay(1);
> @@ -661,11 +673,9 @@ ingenic_cgu_new(const struct ingenic_cgu_clk_info
> *clock_info, if (!cgu)
>  		goto err_out;
> 
> -	cgu->base = of_iomap(np, 0);
> -	if (!cgu->base) {
> -		pr_err("%s: failed to map CGU registers\n", __func__);
> +	cgu->regmap = syscon_node_to_regmap(np->parent);
> +	if (cgu->regmap == NULL)
>  		goto err_out_free;
> -	}
> 
>  	cgu->np = np;
>  	cgu->clock_info = clock_info;
> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
> index 09700b2..86fd5b0 100644
> --- a/drivers/clk/ingenic/cgu.h
> +++ b/drivers/clk/ingenic/cgu.h
> @@ -19,7 +19,9 @@
>  #define __DRIVERS_CLK_INGENIC_CGU_H__
> 
>  #include <linux/bitops.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of.h>
> +#include <linux/regmap.h>
>  #include <linux/spinlock.h>
> 
>  /**
> @@ -171,14 +173,14 @@ struct ingenic_cgu_clk_info {
>  /**
>   * struct ingenic_cgu - data about the CGU
>   * @np: the device tree node that caused the CGU to be probed
> - * @base: the ioremap'ed base address of the CGU registers
> + * @regmap: the regmap object used to access the CGU registers
>   * @clock_info: an array containing information about implemented clocks
>   * @clocks: used to provide clocks to DT, allows lookup of struct clk*
>   * @lock: lock to be held whilst manipulating CGU registers
>   */
>  struct ingenic_cgu {
>  	struct device_node *np;
> -	void __iomem *base;
> +	struct regmap *regmap;
> 
>  	const struct ingenic_cgu_clk_info *clock_info;
>  	struct clk_onecell_data clocks;
> @@ -186,6 +188,9 @@ struct ingenic_cgu {
>  	spinlock_t lock;
>  };
> 
> +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg);
> +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int
> reg); +
>  /**
>   * struct ingenic_clk - private data for a clock
>   * @hw: see Documentation/clk.txt
> diff --git a/drivers/clk/ingenic/jz4740-cgu.c
> b/drivers/clk/ingenic/jz4740-cgu.c index 510fe7e..3003afb 100644
> --- a/drivers/clk/ingenic/jz4740-cgu.c
> +++ b/drivers/clk/ingenic/jz4740-cgu.c
> @@ -232,7 +232,7 @@ CLK_OF_DECLARE(jz4740_cgu, "ingenic,jz4740-cgu",
> jz4740_cgu_init);
> 
>  void jz4740_clock_set_wait_mode(enum jz4740_wait_mode mode)
>  {
> -	uint32_t lcr = readl(cgu->base + CGU_REG_LCR);
> +	uint32_t lcr = cgu_readl(cgu, CGU_REG_LCR);
> 
>  	switch (mode) {
>  	case JZ4740_WAIT_MODE_IDLE:
> @@ -244,24 +244,24 @@ void jz4740_clock_set_wait_mode(enum jz4740_wait_mode
> mode) break;
>  	}
> 
> -	writel(lcr, cgu->base + CGU_REG_LCR);
> +	cgu_writel(cgu, lcr, CGU_REG_LCR);
>  }
> 
>  void jz4740_clock_udc_disable_auto_suspend(void)
>  {
> -	uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
> 
>  	clkgr &= ~CLKGR_UDC;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
>  }
>  EXPORT_SYMBOL_GPL(jz4740_clock_udc_disable_auto_suspend);
> 
>  void jz4740_clock_udc_enable_auto_suspend(void)
>  {
> -	uint32_t clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	uint32_t clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
> 
>  	clkgr |= CLKGR_UDC;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
>  }
>  EXPORT_SYMBOL_GPL(jz4740_clock_udc_enable_auto_suspend);
> 
> @@ -273,31 +273,31 @@ void jz4740_clock_suspend(void)
>  {
>  	uint32_t clkgr, cppcr;
> 
> -	clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
>  	clkgr |= JZ_CLOCK_GATE_TCU | JZ_CLOCK_GATE_DMAC | JZ_CLOCK_GATE_UART0;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
> 
> -	cppcr = readl(cgu->base + CGU_REG_CPPCR);
> +	cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
>  	cppcr &= ~BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit);
> -	writel(cppcr, cgu->base + CGU_REG_CPPCR);
> +	cgu_writel(cgu, cppcr, CGU_REG_CPPCR);
>  }
> 
>  void jz4740_clock_resume(void)
>  {
>  	uint32_t clkgr, cppcr, stable;
> 
> -	cppcr = readl(cgu->base + CGU_REG_CPPCR);
> +	cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
>  	cppcr |= BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.enable_bit);
> -	writel(cppcr, cgu->base + CGU_REG_CPPCR);
> +	cgu_writel(cgu, cppcr, CGU_REG_CPPCR);
> 
>  	stable = BIT(jz4740_cgu_clocks[JZ4740_CLK_PLL].pll.stable_bit);
>  	do {
> -		cppcr = readl(cgu->base + CGU_REG_CPPCR);
> +		cppcr = cgu_readl(cgu, CGU_REG_CPPCR);
>  	} while (!(cppcr & stable));
> 
> -	clkgr = readl(cgu->base + CGU_REG_CLKGR);
> +	clkgr = cgu_readl(cgu, CGU_REG_CLKGR);
>  	clkgr &= ~JZ_CLOCK_GATE_TCU;
>  	clkgr &= ~JZ_CLOCK_GATE_DMAC;
>  	clkgr &= ~JZ_CLOCK_GATE_UART0;
> -	writel(clkgr, cgu->base + CGU_REG_CLKGR);
> +	cgu_writel(cgu, clkgr, CGU_REG_CLKGR);
>  }
> diff --git a/drivers/clk/ingenic/jz4780-cgu.c
> b/drivers/clk/ingenic/jz4780-cgu.c index b35d6d9..8f83433 100644
> --- a/drivers/clk/ingenic/jz4780-cgu.c
> +++ b/drivers/clk/ingenic/jz4780-cgu.c
> @@ -113,11 +113,11 @@ static int jz4780_otg_phy_set_parent(struct clk_hw
> *hw, u8 idx)
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> 
> -	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
> +	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
>  	usbpcr1 &= ~USBPCR1_REFCLKSEL_MASK;
>  	/* we only use CLKCORE */
>  	usbpcr1 |= USBPCR1_REFCLKSEL_CORE;
> -	writel(usbpcr1, cgu->base + CGU_REG_USBPCR1);
> +	cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1);
> 
>  	spin_unlock_irqrestore(&cgu->lock, flags);
>  	return 0;
> @@ -129,7 +129,7 @@ static unsigned long jz4780_otg_phy_recalc_rate(struct
> clk_hw *hw, u32 usbpcr1;
>  	unsigned refclk_div;
> 
> -	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
> +	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
>  	refclk_div = usbpcr1 & USBPCR1_REFCLKDIV_MASK;
> 
>  	switch (refclk_div) {
> @@ -194,10 +194,10 @@ static int jz4780_otg_phy_set_rate(struct clk_hw *hw,
> unsigned long req_rate,
> 
>  	spin_lock_irqsave(&cgu->lock, flags);
> 
> -	usbpcr1 = readl(cgu->base + CGU_REG_USBPCR1);
> +	usbpcr1 = cgu_readl(cgu, CGU_REG_USBPCR1);
>  	usbpcr1 &= ~USBPCR1_REFCLKDIV_MASK;
>  	usbpcr1 |= div_bits;
> -	writel(usbpcr1, cgu->base + CGU_REG_USBPCR1);
> +	cgu_writel(cgu, usbpcr1, CGU_REG_USBPCR1);
> 
>  	spin_unlock_irqrestore(&cgu->lock, flags);
>  	return 0;


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/6] crypto: jz4780-rng: Make ingenic CGU driver use syscon

[PrasannaKumar Muralidharan]

Hi Paul,

Thanks for your review.

On 19 August 2017 at 02:18, Paul Burton <paul.burton@imgtec.com> wrote:
> Hi PrasannaKumar,
>
> On Thursday, 17 August 2017 11:25:16 PDT PrasannaKumar Muralidharan wrote:
>> Ingenic PRNG registers are a part of the same hardware block as clock
>> and power stuff. It is handled by CGU driver. Ingenic M200 SoC has power
>> related registers that are after the PRNG registers. So instead of
>> shortening the register range use syscon interface to expose regmap.
>> This regmap is used by the PRNG driver.
>>
>> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
>> ---
>>  arch/mips/boot/dts/ingenic/jz4740.dtsi | 14 +++++++----
>>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 14 +++++++----
>>  drivers/clk/ingenic/cgu.c              | 46
>> +++++++++++++++++++++------------- drivers/clk/ingenic/cgu.h              |
>>  9 +++++--
>>  drivers/clk/ingenic/jz4740-cgu.c       | 30 +++++++++++-----------
>>  drivers/clk/ingenic/jz4780-cgu.c       | 10 ++++----
>>  6 files changed, 73 insertions(+), 50 deletions(-)
>>
>> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi
>> b/arch/mips/boot/dts/ingenic/jz4740.dtsi index 2ca7ce7..ada5e67 100644
>> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
>> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
>> @@ -34,14 +34,18 @@
>>               clock-frequency = <32768>;
>>       };
>>
>> -     cgu: jz4740-cgu@10000000 {
>> -             compatible = "ingenic,jz4740-cgu";
>> +     cgu_registers {
>> +             compatible = "simple-mfd", "syscon";
>>               reg = <0x10000000 0x100>;
>>
>> -             clocks = <&ext>, <&rtc>;
>> -             clock-names = "ext", "rtc";
>> +             cgu: jz4780-cgu@10000000 {
>> +                     compatible = "ingenic,jz4740-cgu";
>>
>> -             #clock-cells = <1>;
>> +                     clocks = <&ext>, <&rtc>;
>> +                     clock-names = "ext", "rtc";
>> +
>> +                     #clock-cells = <1>;
>> +             };
>>       };
>>
>>       rtc_dev: rtc@10003000 {
>> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> b/arch/mips/boot/dts/ingenic/jz4780.dtsi index 4853ef6..1301694 100644
>> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>> @@ -34,14 +34,18 @@
>>               clock-frequency = <32768>;
>>       };
>>
>> -     cgu: jz4780-cgu@10000000 {
>> -             compatible = "ingenic,jz4780-cgu";
>> +     cgu_registers {
>> +             compatible = "simple-mfd", "syscon";
>>               reg = <0x10000000 0x100>;
>>
>> -             clocks = <&ext>, <&rtc>;
>> -             clock-names = "ext", "rtc";
>> +             cgu: jz4780-cgu@10000000 {
>> +                     compatible = "ingenic,jz4780-cgu";
>>
>> -             #clock-cells = <1>;
>> +                     clocks = <&ext>, <&rtc>;
>> +                     clock-names = "ext", "rtc";
>> +
>> +                     #clock-cells = <1>;
>> +             };
>>       };
>>
>>       pinctrl: pin-controller@10010000 {
>> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
>> index e8248f9..2f12c7c 100644
>> --- a/drivers/clk/ingenic/cgu.c
>> +++ b/drivers/clk/ingenic/cgu.c
>> @@ -29,6 +29,18 @@
>>
>>  #define MHZ (1000 * 1000)
>>
>> +unsigned int cgu_readl(struct ingenic_cgu *cgu, unsigned int reg)
>> +{
>> +     unsigned int val = 0;
>> +     regmap_read(cgu->regmap, reg, &val);
>> +     return val;
>> +}
>> +
>> +int cgu_writel(struct ingenic_cgu *cgu, unsigned int val, unsigned int reg)
>> +{
>> +     return regmap_write(cgu->regmap, reg, val);
>> +}
>
> This is going to introduce a lot of overhead to the CGU driver - each
> regmap_read() or regmap_write() call will not only add overhead from the
> indirection but also acquire a lock which we really don't need.
>

Indeed.

> Could you instead perhaps:
>
>  - Just add "syscon" as a second compatible string to the CGU node in the
>    device tree, but otherwise leave it as-is without the extra cgu_registers
>    node.
>
>  - Have your RNG device node as a child of the CGU node, which should let it
>    pick up the regmap via syscon_node_to_regmap() as it already does.
>
>  - Leave the CGU driver as-is, so it can continue accessing memory directly
>    rather than via regmap.
>

As per my understanding, CGU driver and syscon will map the same
register set. Is that fine?

Thanks,
PrasannaKumar

Re: [PATCH 2/6] crypto: jz4780-rng: Make ingenic CGU driver use syscon

[Paul Burton]

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

Hi PrasannaKumar,

On Sunday, 20 August 2017 09:12:12 PDT PrasannaKumar Muralidharan wrote:
> > Could you instead perhaps:
> >  - Just add "syscon" as a second compatible string to the CGU node in the
> >  
> >    device tree, but otherwise leave it as-is without the extra
> >    cgu_registers
> >    node.
> >  
> >  - Have your RNG device node as a child of the CGU node, which should let
> >  it
> >  
> >    pick up the regmap via syscon_node_to_regmap() as it already does.
> >  
> >  - Leave the CGU driver as-is, so it can continue accessing memory
> >  directly
> >  
> >    rather than via regmap.
> 
> As per my understanding, CGU driver and syscon will map the same
> register set. Is that fine?

That should work fine.

It's only risky if you have 2 drivers using the same registers in ways that 
might race with one another or clobber one another's data. In this case each 
of the 2 drivers is using a different subset of registers that just happen to 
be in the same address range, so there shouldn't be an issue.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/6] crypto: jz4780-rng: Make ingenic CGU driver use syscon

[Paul Burton]

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

Hi PrasannaKumar,

On Sunday, 20 August 2017 09:12:12 PDT PrasannaKumar Muralidharan wrote:
> > Could you instead perhaps:
> >  - Just add "syscon" as a second compatible string to the CGU node in the
> >  
> >    device tree, but otherwise leave it as-is without the extra
> >    cgu_registers
> >    node.
> >  
> >  - Have your RNG device node as a child of the CGU node, which should let
> >  it
> >  
> >    pick up the regmap via syscon_node_to_regmap() as it already does.
> >  
> >  - Leave the CGU driver as-is, so it can continue accessing memory
> >  directly
> >  
> >    rather than via regmap.
> 
> As per my understanding, CGU driver and syscon will map the same
> register set. Is that fine?

That should work fine.

It's only risky if you have 2 drivers using the same registers in ways that 
might race with one another or clobber one another's data. In this case each 
of the 2 drivers is using a different subset of registers that just happen to 
be in the same address range, so there shouldn't be an issue.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]