[PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish

[PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish

[David Wu]

As rk3368 contained two separated iodomain areas, this was
determined to use which regmap base address.

Signed-off-by: David Wu <david.wu@rock-chips.com>
---
 drivers/power/avs/rockchip-io-domain.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/power/avs/rockchip-io-domain.c b/drivers/power/avs/rockchip-io-domain.c
index 8099456..b17aeb7 100644
--- a/drivers/power/avs/rockchip-io-domain.c
+++ b/drivers/power/avs/rockchip-io-domain.c
@@ -47,6 +47,11 @@
 #define RK3368_SOC_CON15_FLASH0		BIT(14)
 #define RK3368_SOC_FLASH_SUPPLY_NUM	2
 
+enum rockchip_iodomain_grf_type {
+	GRF,
+	PMUGRF
+};
+
 struct rockchip_iodomain;
 
 /**
@@ -54,6 +59,7 @@ struct rockchip_iodomain;
  */
 struct rockchip_iodomain_soc_data {
 	int grf_offset;
+	enum rockchip_iodomain_grf_type type;
 	const char *supply_names[MAX_SUPPLIES];
 	void (*init)(struct rockchip_iodomain *iod);
 };
@@ -67,7 +73,7 @@ struct rockchip_iodomain_supply {
 
 struct rockchip_iodomain {
 	struct device *dev;
-	struct regmap *grf;
+	struct regmap *base;
 	struct rockchip_iodomain_soc_data *soc_data;
 	struct rockchip_iodomain_supply supplies[MAX_SUPPLIES];
 };
@@ -86,7 +92,7 @@ static int rockchip_iodomain_write(struct rockchip_iodomain_supply *supply,
 	/* apply hiword-mask */
 	val |= (BIT(supply->idx) << 16);
 
-	ret = regmap_write(iod->grf, iod->soc_data->grf_offset, val);
+	ret = regmap_write(iod->base, iod->soc_data->grf_offset, val);
 	if (ret)
 		dev_err(iod->dev, "Couldn't write to GRF\n");
 
@@ -157,7 +163,7 @@ static void rk3288_iodomain_init(struct rockchip_iodomain *iod)
 	 * instead of a special gpio.
 	 */
 	val = RK3288_SOC_CON2_FLASH0 | (RK3288_SOC_CON2_FLASH0 << 16);
-	ret = regmap_write(iod->grf, RK3288_SOC_CON2, val);
+	ret = regmap_write(iod->base, RK3288_SOC_CON2, val);
 	if (ret < 0)
 		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
 }
@@ -176,7 +182,7 @@ static void rk3368_iodomain_init(struct rockchip_iodomain *iod)
 	 * instead of a special gpio.
 	 */
 	val = RK3368_SOC_CON15_FLASH0 | (RK3368_SOC_CON15_FLASH0 << 16);
-	ret = regmap_write(iod->grf, RK3368_SOC_CON15, val);
+	ret = regmap_write(iod->base, RK3368_SOC_CON15, val);
 	if (ret < 0)
 		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
 }
@@ -187,6 +193,7 @@ static void rk3368_iodomain_init(struct rockchip_iodomain *iod)
  */
 static const struct rockchip_iodomain_soc_data soc_data_rk3188 = {
 	.grf_offset = 0x104,
+	.type = GRF,
 	.supply_names = {
 		NULL,
 		NULL,
@@ -209,6 +216,7 @@ static const struct rockchip_iodomain_soc_data soc_data_rk3188 = {
 
 static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
 	.grf_offset = 0x380,
+	.type = GRF,
 	.supply_names = {
 		"lcdc",		/* LCDC_VDD */
 		"dvp",		/* DVPIO_VDD */
@@ -226,6 +234,7 @@ static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
 
 static const struct rockchip_iodomain_soc_data soc_data_rk3368 = {
 	.grf_offset = 0x900,
+	.type = GRF,
 	.supply_names = {
 		NULL,		/* reserved */
 		"dvp",		/* DVPIO_VDD */
@@ -242,6 +251,7 @@ static const struct rockchip_iodomain_soc_data soc_data_rk3368 = {
 
 static const struct rockchip_iodomain_soc_data soc_data_rk3368_pmu = {
 	.grf_offset = 0x100,
+	.type = PMUGRF,
 	.supply_names = {
 		NULL,
 		NULL,
@@ -293,10 +303,16 @@ static int rockchip_iodomain_probe(struct platform_device *pdev)
 	match = of_match_node(rockchip_iodomain_match, np);
 	iod->soc_data = (struct rockchip_iodomain_soc_data *)match->data;
 
-	iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
-	if (IS_ERR(iod->grf)) {
-		dev_err(&pdev->dev, "couldn't find grf regmap\n");
-		return PTR_ERR(iod->grf);
+	if (iod->soc_data->type == PMUGRF)
+		iod->base = syscon_regmap_lookup_by_phandle(
+				np, "rockchip,pmugrf");
+	else
+		iod->base = syscon_regmap_lookup_by_phandle(
+				np, "rockchip,grf");
+	if (IS_ERR(iod->base)) {
+		dev_err(&pdev->dev, "couldn't find %s regmap\n",
+			(iod->soc_data->type == PMUGRF) ? "pmugrf" : "grf");
+		return PTR_ERR(iod->base);
 	}
 
 	for (i = 0; i < MAX_SUPPLIES; i++) {
-- 
1.9.1

Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish

[Heiko Stuebner]

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

Hi David,

Am Samstag, 30. Januar 2016, 20:01:45 schrieb David Wu:
> As rk3368 contained two separated iodomain areas, this was
> determined to use which regmap base address.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>

I don't think we need to specify this on a driver level. Both GRF areas are 
"General register files" only located in two separate power-domains.
So the rockchip,grf property should work for both. Especially as nothing 
keeps designers from introducing yet another GRF-area somewhere else ;-)

>From when I started working on the rk3368, I still have a preliminary 
patches for that sitting here, so I've attached on how I envisoned that to 
work.


Heiko

> ---
>  drivers/power/avs/rockchip-io-domain.c | 32
> ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8
> deletions(-)
> 
> diff --git a/drivers/power/avs/rockchip-io-domain.c
> b/drivers/power/avs/rockchip-io-domain.c index 8099456..b17aeb7 100644
> --- a/drivers/power/avs/rockchip-io-domain.c
> +++ b/drivers/power/avs/rockchip-io-domain.c
> @@ -47,6 +47,11 @@
>  #define RK3368_SOC_CON15_FLASH0		BIT(14)
>  #define RK3368_SOC_FLASH_SUPPLY_NUM	2
> 
> +enum rockchip_iodomain_grf_type {
> +	GRF,
> +	PMUGRF
> +};
> +
>  struct rockchip_iodomain;
> 
>  /**
> @@ -54,6 +59,7 @@ struct rockchip_iodomain;
>   */
>  struct rockchip_iodomain_soc_data {
>  	int grf_offset;
> +	enum rockchip_iodomain_grf_type type;
>  	const char *supply_names[MAX_SUPPLIES];
>  	void (*init)(struct rockchip_iodomain *iod);
>  };
> @@ -67,7 +73,7 @@ struct rockchip_iodomain_supply {
> 
>  struct rockchip_iodomain {
>  	struct device *dev;
> -	struct regmap *grf;
> +	struct regmap *base;
>  	struct rockchip_iodomain_soc_data *soc_data;
>  	struct rockchip_iodomain_supply supplies[MAX_SUPPLIES];
>  };
> @@ -86,7 +92,7 @@ static int rockchip_iodomain_write(struct
> rockchip_iodomain_supply *supply, /* apply hiword-mask */
>  	val |= (BIT(supply->idx) << 16);
> 
> -	ret = regmap_write(iod->grf, iod->soc_data->grf_offset, val);
> +	ret = regmap_write(iod->base, iod->soc_data->grf_offset, val);
>  	if (ret)
>  		dev_err(iod->dev, "Couldn't write to GRF\n");
> 
> @@ -157,7 +163,7 @@ static void rk3288_iodomain_init(struct
> rockchip_iodomain *iod) * instead of a special gpio.
>  	 */
>  	val = RK3288_SOC_CON2_FLASH0 | (RK3288_SOC_CON2_FLASH0 << 16);
> -	ret = regmap_write(iod->grf, RK3288_SOC_CON2, val);
> +	ret = regmap_write(iod->base, RK3288_SOC_CON2, val);
>  	if (ret < 0)
>  		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
>  }
> @@ -176,7 +182,7 @@ static void rk3368_iodomain_init(struct
> rockchip_iodomain *iod) * instead of a special gpio.
>  	 */
>  	val = RK3368_SOC_CON15_FLASH0 | (RK3368_SOC_CON15_FLASH0 << 16);
> -	ret = regmap_write(iod->grf, RK3368_SOC_CON15, val);
> +	ret = regmap_write(iod->base, RK3368_SOC_CON15, val);
>  	if (ret < 0)
>  		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
>  }
> @@ -187,6 +193,7 @@ static void rk3368_iodomain_init(struct
> rockchip_iodomain *iod) */
>  static const struct rockchip_iodomain_soc_data soc_data_rk3188 = {
>  	.grf_offset = 0x104,
> +	.type = GRF,
>  	.supply_names = {
>  		NULL,
>  		NULL,
> @@ -209,6 +216,7 @@ static const struct rockchip_iodomain_soc_data
> soc_data_rk3188 = {
> 
>  static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
>  	.grf_offset = 0x380,
> +	.type = GRF,
>  	.supply_names = {
>  		"lcdc",		/* LCDC_VDD */
>  		"dvp",		/* DVPIO_VDD */
> @@ -226,6 +234,7 @@ static const struct rockchip_iodomain_soc_data
> soc_data_rk3288 = {
> 
>  static const struct rockchip_iodomain_soc_data soc_data_rk3368 = {
>  	.grf_offset = 0x900,
> +	.type = GRF,
>  	.supply_names = {
>  		NULL,		/* reserved */
>  		"dvp",		/* DVPIO_VDD */
> @@ -242,6 +251,7 @@ static const struct rockchip_iodomain_soc_data
> soc_data_rk3368 = {
> 
>  static const struct rockchip_iodomain_soc_data soc_data_rk3368_pmu = {
>  	.grf_offset = 0x100,
> +	.type = PMUGRF,
>  	.supply_names = {
>  		NULL,
>  		NULL,
> @@ -293,10 +303,16 @@ static int rockchip_iodomain_probe(struct
> platform_device *pdev) match = of_match_node(rockchip_iodomain_match,
> np);
>  	iod->soc_data = (struct rockchip_iodomain_soc_data *)match->data;
> 
> -	iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> -	if (IS_ERR(iod->grf)) {
> -		dev_err(&pdev->dev, "couldn't find grf regmap\n");
> -		return PTR_ERR(iod->grf);
> +	if (iod->soc_data->type == PMUGRF)
> +		iod->base = syscon_regmap_lookup_by_phandle(
> +				np, "rockchip,pmugrf");
> +	else
> +		iod->base = syscon_regmap_lookup_by_phandle(
> +				np, "rockchip,grf");
> +	if (IS_ERR(iod->base)) {
> +		dev_err(&pdev->dev, "couldn't find %s regmap\n",
> +			(iod->soc_data->type == PMUGRF) ? "pmugrf" : "grf");
> +		return PTR_ERR(iod->base);
>  	}
> 
>  	for (i = 0; i < MAX_SUPPLIES; i++) {

[-- Attachment #2: 0001-PM-AVS-rockchip-io-add-io-selectors-and-supplies-for.patch --]
[-- Type: text/x-patch, Size: 4768 bytes --]

>From cb71dd1bc8afb825e200c0c116186fed43df83ea Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Wed, 17 Jun 2015 13:33:18 +0200
Subject: [PATCH 1/2] PM / AVS: rockchip-io: add io selectors and supplies for
 rk3368

This adds the necessary data for handling io voltage domains on the rk3368.
As interesting tidbit, the rk3368 contains two separate iodomain areas.
One in the regular General Register Files (GRF) and one in PMUGRF in the
pmu power domain.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../bindings/power/rockchip-io-domain.txt          | 14 +++++
 drivers/power/avs/rockchip-io-domain.c             | 59 ++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/rockchip-io-domain.txt b/Documentation/devicetree/bindings/power/rockchip-io-domain.txt
index 8b70db1..b8627e7 100644
--- a/Documentation/devicetree/bindings/power/rockchip-io-domain.txt
+++ b/Documentation/devicetree/bindings/power/rockchip-io-domain.txt
@@ -33,6 +33,8 @@ Required properties:
 - compatible: should be one of:
   - "rockchip,rk3188-io-voltage-domain" for rk3188
   - "rockchip,rk3288-io-voltage-domain" for rk3288
+  - "rockchip,rk3368-io-voltage-domain" for rk3368
+  - "rockchip,rk3368-pmu-io-voltage-domain" for rk3368 pmu-domains
 - rockchip,grf: phandle to the syscon managing the "general register files"
 
 
@@ -64,6 +66,18 @@ Possible supplies for rk3288:
 - sdcard-supply: The supply connected to SDMMC0_VDD.
 - wifi-supply:   The supply connected to APIO3_VDD.  Also known as SDIO0.
 
+Possible supplies for rk3368:
+- audio-supply:  The supply connected to APIO3_VDD.
+- dvp-supply:    The supply connected to DVPIO_VDD.
+- flash0-supply: The supply connected to FLASH0_VDD.  Typically for eMMC
+- gpio30-supply: The supply connected to APIO1_VDD.
+- gpio1830       The supply connected to APIO4_VDD.
+- sdcard-supply: The supply connected to SDMMC0_VDD.
+- wifi-supply:   The supply connected to APIO2_VDD.  Also known as SDIO0.
+
+Possible supplies for rk3368 pmu-domains:
+- pmu-supply:    The supply connected to PMUIO_VDD.
+- vop-supply:    The supply connected to LCDC_VDD.
 
 Example:
 
diff --git a/drivers/power/avs/rockchip-io-domain.c b/drivers/power/avs/rockchip-io-domain.c
index 3ae35d0..2e30002 100644
--- a/drivers/power/avs/rockchip-io-domain.c
+++ b/drivers/power/avs/rockchip-io-domain.c
@@ -43,6 +43,10 @@
 #define RK3288_SOC_CON2_FLASH0		BIT(7)
 #define RK3288_SOC_FLASH_SUPPLY_NUM	2
 
+#define RK3368_SOC_CON15		0x43c
+#define RK3368_SOC_CON15_FLASH0		BIT(14)
+#define RK3368_SOC_FLASH_SUPPLY_NUM	2
+
 struct rockchip_iodomain;
 
 /**
@@ -158,6 +162,25 @@ static void rk3288_iodomain_init(struct rockchip_iodomain *iod)
 		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
 }
 
+static void rk3368_iodomain_init(struct rockchip_iodomain *iod)
+{
+	int ret;
+	u32 val;
+
+	/* if no flash supply we should leave things alone */
+	if (!iod->supplies[RK3368_SOC_FLASH_SUPPLY_NUM].reg)
+		return;
+
+	/*
+	 * set flash0 iodomain to also use this framework
+	 * instead of a special gpio.
+	 */
+	val = RK3368_SOC_CON15_FLASH0 | (RK3368_SOC_CON15_FLASH0 << 16);
+	ret = regmap_write(iod->grf, RK3368_SOC_CON15, val);
+	if (ret < 0)
+		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
+}
+
 /*
  * On the rk3188 the io-domains are handled by a shared register with the
  * lower 8 bits being still being continuing drive-strength settings.
@@ -201,6 +224,34 @@ static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
 	.init = rk3288_iodomain_init,
 };
 
+static const struct rockchip_iodomain_soc_data soc_data_rk3368 = {
+	.grf_offset = 0x900,
+	.supply_names = {
+		NULL,		/* reserved */
+		"dvp",		/* DVPIO_VDD */
+		"flash0",	/* FLASH0_VDD (emmc) */
+		"wifi",		/* APIO2_VDD (sdio0) */
+		NULL,
+		"audio",	/* APIO3_VDD */
+		"sdcard",	/* SDMMC0_VDD (sdmmc) */
+		"gpio30",	/* APIO1_VDD */
+		"gpio1830",	/* APIO4_VDD (gpujtag) */
+	},
+	.init = rk3368_iodomain_init,
+};
+
+static const struct rockchip_iodomain_soc_data soc_data_rk3368_pmu = {
+	.grf_offset = 0x100,
+	.supply_names = {
+		NULL,
+		NULL,
+		NULL,
+		NULL,
+		"pmu",		/*PMU IO domain*/
+		"vop",		/*LCDC IO domain*/
+	},
+};
+
 static const struct of_device_id rockchip_iodomain_match[] = {
 	{
 		.compatible = "rockchip,rk3188-io-voltage-domain",
@@ -210,6 +261,14 @@ static const struct of_device_id rockchip_iodomain_match[] = {
 		.compatible = "rockchip,rk3288-io-voltage-domain",
 		.data = (void *)&soc_data_rk3288
 	},
+	{
+		.compatible = "rockchip,rk3368-io-voltage-domain",
+		.data = (void *)&soc_data_rk3368
+	},
+	{
+		.compatible = "rockchip,rk3368-pmu-io-voltage-domain",
+		.data = (void *)&soc_data_rk3368_pmu
+	},
 	{ /* sentinel */ },
 };
 
-- 
2.6.4


[-- Attachment #3: 0002-ARM64-dts-rockchip-add-iodomains-and-stuff.patch --]
[-- Type: text/x-patch, Size: 1036 bytes --]

>From 76fc704626e611180223ac99e4067c30c464dd23 Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Wed, 29 Jul 2015 16:13:35 +0200
Subject: [PATCH 2/2] arm64: dts: rockchip: add rk3368 iodomains


probably unfinished
---

diff --git a/arch/arm64/boot/dts/rockchip/rk3368-r88.dts b/arch/arm64/boot/dts/rockchip/rk3368-r88.dts
index 401a812..6547f1e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-r88.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3368-r88.dts
@@ -89,6 +89,25 @@
 		};
 	};
 
+	io-domains {
+		compatible = "rockchip,rk3368-io-voltage-domain";
+		rockchip,grf = <&grf>;
+
+		audio-supply = <&vcc_io>;
+		gpio30-supply = <&vcc_io>;
+		gpio1830-supply = <&vcc_io>;
+		sdcard-supply = <&vccio_sd>;
+		wifi-supply = <&vccio_wl>;
+	};
+
+	pmu-io-domains {
+		compatible = "rockchip,rk3368-pmu-io-voltage-domain";
+		rockchip,grf = <&pmugrf>;
+
+		pmu-supply = <&vcc_io>;
+		vop-supply = <&vcc_io>;
+	};
+
 	ir: ir-receiver {
 		compatible = "gpio-ir-receiver";
 		gpios = <&gpio3 30 GPIO_ACTIVE_LOW>;
-- 
2.6.4

Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish

[David.Wu]

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

Hi Heiko,

在 2016/1/30 20:39, Heiko Stuebner 写道:
> Hi David,
>
> Am Samstag, 30. Januar 2016, 20:01:45 schrieb David Wu:
>> As rk3368 contained two separated iodomain areas, this was
>> determined to use which regmap base address.
>>
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
> I don't think we need to specify this on a driver level. Both GRF areas are
> "General register files" only located in two separate power-domains.
> So the rockchip,grf property should work for both. Especially as nothing
> keeps designers from introducing yet another GRF-area somewhere else ;-)
>
> >From when I started working on the rk3368, I still have a preliminary
> patches for that sitting here, so I've attached on how I envisoned that to
> work.

Okay, i agree to you, but it make someone a little confused just from 
the drive code,
not DT file,  about  pmugrf regmap base address.:-)

How do you feel about intergating GRF and PMU drivers in one driver?
Thanks!

>
> Heiko
>
>> ---
>>   drivers/power/avs/rockchip-io-domain.c | 32
>> ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8
>> deletions(-)
>>
>> diff --git a/drivers/power/avs/rockchip-io-domain.c
>> b/drivers/power/avs/rockchip-io-domain.c index 8099456..b17aeb7 100644
>> --- a/drivers/power/avs/rockchip-io-domain.c
>> +++ b/drivers/power/avs/rockchip-io-domain.c
>> @@ -47,6 +47,11 @@
>>   #define RK3368_SOC_CON15_FLASH0		BIT(14)
>>   #define RK3368_SOC_FLASH_SUPPLY_NUM	2
>>
>> +enum rockchip_iodomain_grf_type {
>> +	GRF,
>> +	PMUGRF
>> +};
>> +
>>   struct rockchip_iodomain;
>>
>>   /**
>> @@ -54,6 +59,7 @@ struct rockchip_iodomain;
>>    */
>>   struct rockchip_iodomain_soc_data {
>>   	int grf_offset;
>> +	enum rockchip_iodomain_grf_type type;
>>   	const char *supply_names[MAX_SUPPLIES];
>>   	void (*init)(struct rockchip_iodomain *iod);
>>   };
>> @@ -67,7 +73,7 @@ struct rockchip_iodomain_supply {
>>
>>   struct rockchip_iodomain {
>>   	struct device *dev;
>> -	struct regmap *grf;
>> +	struct regmap *base;
>>   	struct rockchip_iodomain_soc_data *soc_data;
>>   	struct rockchip_iodomain_supply supplies[MAX_SUPPLIES];
>>   };
>> @@ -86,7 +92,7 @@ static int rockchip_iodomain_write(struct
>> rockchip_iodomain_supply *supply, /* apply hiword-mask */
>>   	val |= (BIT(supply->idx) << 16);
>>
>> -	ret = regmap_write(iod->grf, iod->soc_data->grf_offset, val);
>> +	ret = regmap_write(iod->base, iod->soc_data->grf_offset, val);
>>   	if (ret)
>>   		dev_err(iod->dev, "Couldn't write to GRF\n");
>>
>> @@ -157,7 +163,7 @@ static void rk3288_iodomain_init(struct
>> rockchip_iodomain *iod) * instead of a special gpio.
>>   	 */
>>   	val = RK3288_SOC_CON2_FLASH0 | (RK3288_SOC_CON2_FLASH0 << 16);
>> -	ret = regmap_write(iod->grf, RK3288_SOC_CON2, val);
>> +	ret = regmap_write(iod->base, RK3288_SOC_CON2, val);
>>   	if (ret < 0)
>>   		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
>>   }
>> @@ -176,7 +182,7 @@ static void rk3368_iodomain_init(struct
>> rockchip_iodomain *iod) * instead of a special gpio.
>>   	 */
>>   	val = RK3368_SOC_CON15_FLASH0 | (RK3368_SOC_CON15_FLASH0 << 16);
>> -	ret = regmap_write(iod->grf, RK3368_SOC_CON15, val);
>> +	ret = regmap_write(iod->base, RK3368_SOC_CON15, val);
>>   	if (ret < 0)
>>   		dev_warn(iod->dev, "couldn't update flash0 ctrl\n");
>>   }
>> @@ -187,6 +193,7 @@ static void rk3368_iodomain_init(struct
>> rockchip_iodomain *iod) */
>>   static const struct rockchip_iodomain_soc_data soc_data_rk3188 = {
>>   	.grf_offset = 0x104,
>> +	.type = GRF,
>>   	.supply_names = {
>>   		NULL,
>>   		NULL,
>> @@ -209,6 +216,7 @@ static const struct rockchip_iodomain_soc_data
>> soc_data_rk3188 = {
>>
>>   static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
>>   	.grf_offset = 0x380,
>> +	.type = GRF,
>>   	.supply_names = {
>>   		"lcdc",		/* LCDC_VDD */
>>   		"dvp",		/* DVPIO_VDD */
>> @@ -226,6 +234,7 @@ static const struct rockchip_iodomain_soc_data
>> soc_data_rk3288 = {
>>
>>   static const struct rockchip_iodomain_soc_data soc_data_rk3368 = {
>>   	.grf_offset = 0x900,
>> +	.type = GRF,
>>   	.supply_names = {
>>   		NULL,		/* reserved */
>>   		"dvp",		/* DVPIO_VDD */
>> @@ -242,6 +251,7 @@ static const struct rockchip_iodomain_soc_data
>> soc_data_rk3368 = {
>>
>>   static const struct rockchip_iodomain_soc_data soc_data_rk3368_pmu = {
>>   	.grf_offset = 0x100,
>> +	.type = PMUGRF,
>>   	.supply_names = {
>>   		NULL,
>>   		NULL,
>> @@ -293,10 +303,16 @@ static int rockchip_iodomain_probe(struct
>> platform_device *pdev) match = of_match_node(rockchip_iodomain_match,
>> np);
>>   	iod->soc_data = (struct rockchip_iodomain_soc_data *)match->data;
>>
>> -	iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
>> -	if (IS_ERR(iod->grf)) {
>> -		dev_err(&pdev->dev, "couldn't find grf regmap\n");
>> -		return PTR_ERR(iod->grf);
>> +	if (iod->soc_data->type == PMUGRF)
>> +		iod->base = syscon_regmap_lookup_by_phandle(
>> +				np, "rockchip,pmugrf");
>> +	else
>> +		iod->base = syscon_regmap_lookup_by_phandle(
>> +				np, "rockchip,grf");
>> +	if (IS_ERR(iod->base)) {
>> +		dev_err(&pdev->dev, "couldn't find %s regmap\n",
>> +			(iod->soc_data->type == PMUGRF) ? "pmugrf" : "grf");
>> +		return PTR_ERR(iod->base);
>>   	}
>>
>>   	for (i = 0; i < MAX_SUPPLIES; i++) {


[-- Attachment #2: 0001-rk3368-io-domain-add-io-domain-volt-sel.patch --]
[-- Type: text/plain, Size: 9726 bytes --]

From 73cc56277c42fd13bed3921b07c84ac4b030321a Mon Sep 17 00:00:00 2001
From: David Wu <wdc@rock-chips.com>
Date: Wed, 24 Dec 2014 16:31:09 +0800
Subject: [PATCH 1/5] rk3368: io-domain: add io domain volt sel
Content-Type: text/plain; charset="utf-8"

Signed-off-by: David Wu <wdc@rock-chips.com>
---
 drivers/power/avs/rockchip-io-domain.c | 184 +++++++++++++++++++++++----------
 1 file changed, 130 insertions(+), 54 deletions(-)

diff --git a/drivers/power/avs/rockchip-io-domain.c b/drivers/power/avs/rockchip-io-domain.c
index b160132..d096457 100755
--- a/drivers/power/avs/rockchip-io-domain.c
+++ b/drivers/power/avs/rockchip-io-domain.c
@@ -43,19 +43,28 @@
 #define RK3288_SOC_CON2_FLASH0		BIT(7)
 #define RK3288_SOC_FLASH_SUPPLY_NUM	2
 
-#define RK3368_GRF_SOC_CON15			0x43c
-#define RK3368_GRF_SOC_CON15_FLASH0		BIT(14)
+#define RK3368_GRF_SOC_CON15		0x43c
+#define RK3368_GRF_SOC_CON15_FLASH0	BIT(14)
 #define RK3368_SOC_FLASH_SUPPLY_NUM	2
 
+#define MAX_ROCKCHIP_GRF_NUM        2
 
 struct rockchip_iodomain;
 
 /**
  * @supplies: voltage settings matching the register bits.
  */
+
+enum rockchip_iodomain_grf_type {
+	GRF,
+	PMU_GRF,
+};
+
 struct rockchip_iodomain_soc_data {
 	int grf_offset;
-	const char *supply_names[MAX_SUPPLIES];
+	int pmugrf_offset;
+	const char *grf_supply_names[MAX_SUPPLIES];
+	const char *pmugrf_supply_names[MAX_SUPPLIES];
 	void (*init)(struct rockchip_iodomain *iod);
 };
 
@@ -64,19 +73,24 @@ struct rockchip_iodomain_supply {
 	struct regulator *reg;
 	struct notifier_block nb;
 	int idx;
+	enum rockchip_iodomain_grf_type type;
 };
 
 struct rockchip_iodomain {
 	struct device *dev;
 	struct regmap *grf;
+	struct regmap *pmu;
 	struct rockchip_iodomain_soc_data *soc_data;
-	struct rockchip_iodomain_supply supplies[MAX_SUPPLIES];
+	struct rockchip_iodomain_supply grf_supplies[MAX_SUPPLIES];
+	struct rockchip_iodomain_supply pmugrf_supplies[MAX_SUPPLIES];
 };
 
 static int rockchip_iodomain_write(struct rockchip_iodomain_supply *supply,
 				   int uV)
 {
 	struct rockchip_iodomain *iod = supply->iod;
+	struct regmap *reg;
+	int offset;
 	u32 val;
 	int ret;
 
@@ -87,7 +101,15 @@ static int rockchip_iodomain_write(struct rockchip_iodomain_supply *supply,
 	/* apply hiword-mask */
 	val |= (BIT(supply->idx) << 16);
 
-	ret = regmap_write(iod->grf, iod->soc_data->grf_offset, val);
+	if (supply->type == GRF) {
+		reg = iod->grf;
+		offset = iod->soc_data->grf_offset;
+	} else if (supply->type == PMU_GRF) {
+		reg = iod->pmu;
+		offset = iod->soc_data->pmugrf_offset;
+	}
+
+	ret = regmap_write(reg, offset, val);
 	if (ret)
 		dev_err(iod->dev, "Couldn't write to GRF\n");
 
@@ -150,7 +172,7 @@ static void rk3288_iodomain_init(struct rockchip_iodomain *iod)
 	u32 val;
 
 	/* if no flash supply we should leave things alone */
-	if (!iod->supplies[RK3288_SOC_FLASH_SUPPLY_NUM].reg)
+	if (!iod->grf_supplies[RK3288_SOC_FLASH_SUPPLY_NUM].reg)
 		return;
 
 	/*
@@ -169,7 +191,7 @@ static void rk3368_iodomain_init(struct rockchip_iodomain *iod)
 	u32 val;
 
 	/* if no flash supply we should leave things alone */
-	if (!iod->supplies[RK3368_SOC_FLASH_SUPPLY_NUM].reg)
+	if (!iod->grf_supplies[RK3368_SOC_FLASH_SUPPLY_NUM].reg)
 		return;
 
 	/*
@@ -189,7 +211,7 @@ static void rk3368_iodomain_init(struct rockchip_iodomain *iod)
  */
 static const struct rockchip_iodomain_soc_data soc_data_rk3188 = {
 	.grf_offset = 0x104,
-	.supply_names = {
+	.grf_supply_names = {
 		NULL,
 		NULL,
 		NULL,
@@ -211,7 +233,7 @@ static const struct rockchip_iodomain_soc_data soc_data_rk3188 = {
 
 static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
 	.grf_offset = 0x380,
-	.supply_names = {
+	.grf_supply_names = {
 		"lcdc",		/* LCDC_VDD */
 		"dvp",		/* DVPIO_VDD */
 		"flash0",	/* FLASH0_VDD (emmc) */
@@ -228,16 +250,25 @@ static const struct rockchip_iodomain_soc_data soc_data_rk3288 = {
 
 static const struct rockchip_iodomain_soc_data soc_data_rk3368 = {
 	.grf_offset = 0x900,
-	.supply_names = {
+	.pmugrf_offset = 0x100,
+	.grf_supply_names = {
+		NULL,
+		"dvp",		/*DVP IO domain*/
+		"flash0",	/*FLASH0 IO domain*/
+		"wifi",         /*APIO2 IO domain*/
+		NULL,
+		"audio",	/*APIO3 IO domain*/
+		"sdcard",	/*SDCARD IO domain*/
+		"gpio30",	/*APIO1 IO domain*/
+		"gpio1830",	/*ADIO4 IO domain*/
+	},
+	.pmugrf_supply_names = {
+		NULL,
+		NULL,
 		NULL,
-		"dvp_v18sel",		/*DVP IO domain*/
-		"flash0_v18sel",		/*FLASH0 IO domain*/
-		"wifi_v18sel",	/*WIFI IO domain*/
 		NULL,
-		"audio_v18sel",	/*AUDIO IO domain*/
-		"sdcard_v18sel",		/*SDCARD IO domain*/
-		"gpio30_v18sel",		/*GPIO30 IO domain*/
-		"gpio1830_v18sel",	/*GPIO1830 IO domain*/
+		"pmu",	        /*PMU IO domain*/
+		"vop",	        /*LCDC IO domain*/
 	},
 	.init = rk3368_iodomain_init,
 };
@@ -259,47 +290,34 @@ static const struct of_device_id rockchip_iodomain_match[] = {
 	{ /* sentinel */ },
 };
 
-static int rockchip_iodomain_probe(struct platform_device *pdev)
+static int rockchip_iodomain_parse_supply(struct rockchip_iodomain *iod,
+					  struct device_node *np,
+					  enum rockchip_iodomain_grf_type type)
 {
-	struct device_node *np = pdev->dev.of_node;
-	const struct of_device_id *match;
-	struct rockchip_iodomain *iod;
+	struct rockchip_iodomain_supply *group_supply;
+	const char **group_supply_names;
 	int i, ret = 0;
 
-	if (!np)
-		return -ENODEV;
-
-	iod = devm_kzalloc(&pdev->dev, sizeof(*iod), GFP_KERNEL);
-	if (!iod)
-		return -ENOMEM;
-
-	iod->dev = &pdev->dev;
-	platform_set_drvdata(pdev, iod);
-
-	match = of_match_node(rockchip_iodomain_match, np);
-	iod->soc_data = (struct rockchip_iodomain_soc_data *)match->data;
-
-	iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
-	if (IS_ERR(iod->grf)) {
-		dev_err(&pdev->dev, "couldn't find grf regmap\n");
-		return PTR_ERR(iod->grf);
+	if (type == GRF) {
+		group_supply_names =
+			(const char **)iod->soc_data->grf_supply_names;
+		group_supply = iod->grf_supplies;
+	} else if (type == PMU_GRF) {
+		group_supply_names =
+			(const char **)iod->soc_data->pmugrf_supply_names;
+		group_supply = iod->pmugrf_supplies;
 	}
 
 	for (i = 0; i < MAX_SUPPLIES; i++) {
-		const char *supply_name = iod->soc_data->supply_names[i];
-		struct rockchip_iodomain_supply *supply = &iod->supplies[i];
+		const char *supply_name = group_supply_names[i];
+		struct rockchip_iodomain_supply *supply = &group_supply[i];
 		struct regulator *reg;
 		int uV;
-		const char *regulator_name = NULL;
 
 		if (!supply_name)
 			continue;
 
-		of_property_read_string(np, supply_name, &regulator_name);
-		if (!regulator_name)
-			continue;
-
-		reg = regulator_get(NULL, regulator_name);
+		reg = devm_regulator_get_optional(iod->dev, supply_name);
 		if (IS_ERR(reg)) {
 			ret = PTR_ERR(reg);
 
@@ -334,6 +352,7 @@ static int rockchip_iodomain_probe(struct platform_device *pdev)
 		supply->idx = i;
 		supply->iod = iod;
 		supply->reg = reg;
+		supply->type = type;
 		supply->nb.notifier_call = rockchip_iodomain_notify;
 
 		ret = rockchip_iodomain_write(supply, uV);
@@ -345,21 +364,16 @@ static int rockchip_iodomain_probe(struct platform_device *pdev)
 		/* register regulator notifier */
 		ret = regulator_register_notifier(reg, &supply->nb);
 		if (ret) {
-			dev_err(&pdev->dev,
+			dev_err(iod->dev,
 				"regulator notifier request failed\n");
 			supply->reg = NULL;
 			goto unreg_notify;
 		}
 	}
 
-	if (iod->soc_data->init)
-		iod->soc_data->init(iod);
-
-	return 0;
-
 unreg_notify:
 	for (i = MAX_SUPPLIES - 1; i >= 0; i--) {
-		struct rockchip_iodomain_supply *io_supply = &iod->supplies[i];
+		struct rockchip_iodomain_supply *io_supply = &group_supply[i];
 
 		if (io_supply->reg)
 			regulator_unregister_notifier(io_supply->reg,
@@ -369,13 +383,75 @@ unreg_notify:
 	return ret;
 }
 
+static int rockchip_iodomain_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node, *node;
+	const struct of_device_id *match;
+	struct rockchip_iodomain *iod;
+	int ret = 0;
+
+	if (!np)
+		return -ENODEV;
+
+	iod = devm_kzalloc(&pdev->dev, sizeof(*iod), GFP_KERNEL);
+	if (!iod)
+		return -ENOMEM;
+
+	iod->dev = &pdev->dev;
+	platform_set_drvdata(pdev, iod);
+
+	match = of_match_node(rockchip_iodomain_match, np);
+	iod->soc_data = (struct rockchip_iodomain_soc_data *)match->data;
+
+	iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+	if (IS_ERR(iod->grf)) {
+		dev_err(&pdev->dev, "couldn't find grf regmap\n");
+		return PTR_ERR(iod->grf);
+	}
+	ret = rockchip_iodomain_parse_supply(iod, np, GRF);
+	if (ret) {
+		dev_err(iod->dev,
+			"rockchip iodomain parse grf supply failed\n");
+		return ret;
+	}
+
+	/* try to find the optional reference to the pmu syscon */
+	node = of_parse_phandle(np, "rockchip,pmu", 0);
+	if (node) {
+		iod->pmu = syscon_node_to_regmap(node);
+		if (IS_ERR(iod->pmu))
+			return PTR_ERR(iod->pmu);
+		ret = rockchip_iodomain_parse_supply(iod, np, PMU_GRF);
+		if (ret) {
+			dev_err(iod->dev,
+				"rockchip iodomain parse pmu_grf supply failed\n");
+			return ret;
+		}
+	}
+
+	if (iod->soc_data->init)
+		iod->soc_data->init(iod);
+
+	return ret;
+}
+
 static int rockchip_iodomain_remove(struct platform_device *pdev)
 {
 	struct rockchip_iodomain *iod = platform_get_drvdata(pdev);
 	int i;
 
 	for (i = MAX_SUPPLIES - 1; i >= 0; i--) {
-		struct rockchip_iodomain_supply *io_supply = &iod->supplies[i];
+		struct rockchip_iodomain_supply *io_supply
+			= &iod->grf_supplies[i];
+
+		if (io_supply->reg)
+			regulator_unregister_notifier(io_supply->reg,
+						      &io_supply->nb);
+	}
+
+	for (i = MAX_SUPPLIES - 1; i >= 0; i--) {
+		struct rockchip_iodomain_supply *io_supply =
+			&iod->pmugrf_supplies[i];
 
 		if (io_supply->reg)
 			regulator_unregister_notifier(io_supply->reg,
-- 
1.9.1

Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish

[Heiko Stübner]

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

Hi David,

Am Montag, 1. Februar 2016, 16:54:38 schrieb David.Wu:
> 在 2016/1/30 20:39, Heiko Stuebner 写道:
> > Am Samstag, 30. Januar 2016, 20:01:45 schrieb David Wu:
> >> As rk3368 contained two separated iodomain areas, this was
> >> determined to use which regmap base address.
> >> 
> >> Signed-off-by: David Wu <david.wu@rock-chips.com>
> > 
> > I don't think we need to specify this on a driver level. Both GRF areas
> > are
> > "General register files" only located in two separate power-domains.
> > So the rockchip,grf property should work for both. Especially as nothing
> > keeps designers from introducing yet another GRF-area somewhere else ;-)
> > 
> > >From when I started working on the rk3368, I still have a preliminary
> > 
> > patches for that sitting here, so I've attached on how I envisoned that to
> > work.
> 
> Okay, i agree to you, but it make someone a little confused just from
> the drive code,
> not DT file,  about  pmugrf regmap base address.:-)
> 
> How do you feel about intergating GRF and PMU drivers in one driver?
> Thanks!

I will very strongly disagree here ;-) .
Similar to the power-domains being part of the pmu, the io-domains are
part of their individual GRFs. So if you want it really clean and tidy the way 
to go foward will be the attached patches. Compile-tested only.

Other things like the usbphy control should move there as well in the long 
run. But that's not immediate necessary.


Heiko

[-- Attachment #2: 0001-PM-AVS-rockchip-io-make-io-domains-a-child-of-the-GR.patch --]
[-- Type: text/x-patch, Size: 2790 bytes --]

>From 7893f35ee24c8aa921a480a1fd3045d9dbe48a15 Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Mon, 1 Feb 2016 16:54:14 +0100
Subject: [PATCH 1/3] PM / AVS: rockchip-io: make io-domains a child of the GRF

IO-domain handling is part of the general register files, so should live
under the grf directly. This change allows the grf to be a simple-mfd and
the io-domains fetching the syscon regmap from that parent-node.

The old binding is of course preserved, though deprecated.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 Documentation/devicetree/bindings/power/rockchip-io-domain.txt |  4 +++-
 drivers/power/avs/rockchip-io-domain.c                         | 10 +++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/rockchip-io-domain.txt b/Documentation/devicetree/bindings/power/rockchip-io-domain.txt
index b8627e7..7ed5518 100644
--- a/Documentation/devicetree/bindings/power/rockchip-io-domain.txt
+++ b/Documentation/devicetree/bindings/power/rockchip-io-domain.txt
@@ -35,8 +35,10 @@ Required properties:
   - "rockchip,rk3288-io-voltage-domain" for rk3288
   - "rockchip,rk3368-io-voltage-domain" for rk3368
   - "rockchip,rk3368-pmu-io-voltage-domain" for rk3368 pmu-domains
-- rockchip,grf: phandle to the syscon managing the "general register files"
 
+Deprecated properties:
+- rockchip,grf: phandle to the syscon managing the "general register files"
+    Systems should move the io-domains to a sub-node of the grf simple-mfd.
 
 You specify supplies using the standard regulator bindings by including
 a phandle the relevant regulator.  All specified supplies must be able
diff --git a/drivers/power/avs/rockchip-io-domain.c b/drivers/power/avs/rockchip-io-domain.c
index 8099456..3923e1f7 100644
--- a/drivers/power/avs/rockchip-io-domain.c
+++ b/drivers/power/avs/rockchip-io-domain.c
@@ -278,6 +278,7 @@ static int rockchip_iodomain_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *match;
 	struct rockchip_iodomain *iod;
+	struct device *parent;
 	int i, ret = 0;
 
 	if (!np)
@@ -293,7 +294,14 @@ static int rockchip_iodomain_probe(struct platform_device *pdev)
 	match = of_match_node(rockchip_iodomain_match, np);
 	iod->soc_data = (struct rockchip_iodomain_soc_data *)match->data;
 
-	iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+	parent = pdev->dev.parent;
+	if (parent) {
+		iod->grf = syscon_node_to_regmap(parent->of_node);
+	} else {
+		dev_dbg(&pdev->dev, "falling back to old binding\n");
+		iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+	}
+
 	if (IS_ERR(iod->grf)) {
 		dev_err(&pdev->dev, "couldn't find grf regmap\n");
 		return PTR_ERR(iod->grf);
-- 
2.6.4


[-- Attachment #3: 0003-arm64-dts-rockchip-add-rk3368-iodomains.patch --]
[-- Type: text/x-patch, Size: 1174 bytes --]

>From d2883f273e51b8cb2f06d587ecf56e46b91ef12f Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Wed, 29 Jul 2015 16:13:35 +0200
Subject: [PATCH 3/3] arm64: dts: rockchip: add rk3368 iodomains

probably unfinished
---
 arch/arm64/boot/dts/rockchip/rk3368-r88.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3368-r88.dts b/arch/arm64/boot/dts/rockchip/rk3368-r88.dts
index 104cbee..e6e2a1f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368-r88.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3368-r88.dts
@@ -211,6 +211,18 @@
 	status = "ok";
 };
 
+&grf {
+	io-domains {
+		compatible = "rockchip,rk3368-io-voltage-domain";
+
+		audio-supply = <&vcc_io>;
+		gpio30-supply = <&vcc_io>;
+		gpio1830-supply = <&vcc_io>;
+		sdcard-supply = <&vccio_sd>;
+		wifi-supply = <&vccio_wl>;
+	};
+};
+
 &i2c0 {
 	status = "okay";
 
@@ -331,6 +343,16 @@
 	};
 };
 
+&pmugrf {
+	io-domains {
+		compatible = "rockchip,rk3368-pmu-io-voltage-domain";
+		rockchip,grf = <&pmugrf>;
+
+		pmu-supply = <&vcc_io>;
+		vop-supply = <&vcc_io>;
+	};
+};
+
 &saradc {
 	vref-supply = <&vcc_18>;
 	status = "okay";
-- 
2.6.4


[-- Attachment #4: 0002-arm64-dts-rockchip-make-grf-syscons-simple-mfds.patch --]
[-- Type: text/x-patch, Size: 1316 bytes --]

>From 116217957ec3910b9dc05420441848f3343e450e Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Mon, 1 Feb 2016 22:09:03 +0100
Subject: [PATCH 2/3] arm64: dts: rockchip: make grf syscons simple-mfds

The general register files do contain a lot of separate functions and
while some really are only registers with a lot of different 1-bit
settings, there are also a lot of them containing some bigger function
blocks. To be able to define these as sub-devices, make them simple-mfds.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm64/boot/dts/rockchip/rk3368.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 122777b..4fc28e9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -554,7 +554,7 @@
 	};
 
 	pmugrf: syscon@ff738000 {
-		compatible = "rockchip,rk3368-pmugrf", "syscon";
+		compatible = "rockchip,rk3368-pmugrf", "syscon", "simple-mfd";
 		reg = <0x0 0xff738000 0x0 0x1000>;
 	};
 
@@ -567,7 +567,7 @@
 	};
 
 	grf: syscon@ff770000 {
-		compatible = "rockchip,rk3368-grf", "syscon";
+		compatible = "rockchip,rk3368-grf", "syscon", "simple-mfd";
 		reg = <0x0 0xff770000 0x0 0x1000>;
 	};
 
-- 
2.6.4

Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish

[David.Wu]

Hi Heiko,

在 2016/2/2 5:17, Heiko Stübner 写道:
> Hi David,
>
> Am Montag, 1. Februar 2016, 16:54:38 schrieb David.Wu:
>> 在 2016/1/30 20:39, Heiko Stuebner 写道:
>>> Am Samstag, 30. Januar 2016, 20:01:45 schrieb David Wu:
>>>> As rk3368 contained two separated iodomain areas, this was
>>>> determined to use which regmap base address.
>>>>
>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>> I don't think we need to specify this on a driver level. Both GRF areas
>>> are
>>> "General register files" only located in two separate power-domains.
>>> So the rockchip,grf property should work for both. Especially as nothing
>>> keeps designers from introducing yet another GRF-area somewhere else ;-)
>>>
>>> >From when I started working on the rk3368, I still have a preliminary
>>>
>>> patches for that sitting here, so I've attached on how I envisoned that to
>>> work.
>> Okay, i agree to you, but it make someone a little confused just from
>> the drive code,
>> not DT file,  about  pmugrf regmap base address.:-)
>>
>> How do you feel about intergating GRF and PMU drivers in one driver?
>> Thanks!
> I will very strongly disagree here ;-) .
> Similar to the power-domains being part of the pmu, the io-domains are
> part of their individual GRFs. So if you want it really clean and tidy the way
> to go foward will be the attached patches. Compile-tested only.

Thanks for your reply, the patchs look better than mine.
I have tested them on sdk board and i found something may be wrong.
"parent->of_node" instead of "parent", as the parent is not null if 
parent-node not used.
         if (parent->of_node) {
             iod->grf = syscon_node_to_regmap(parent->of_node);
         } else {
             dev_dbg(&pdev->dev, "falling back to old binding\n");
             iod->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
         }

> Other things like the usbphy control should move there as well in the long
> run. But that's not immediate necessary.
>
>
> Heiko