Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1322343 > unrolled thread
| Started by | David Wu <david.wu@rock-chips.com> |
|---|---|
| First post | 2016-01-30 13:20 +0100 |
| Last post | 2016-02-02 10:40 +0100 |
| Articles | 5 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish David Wu <david.wu@rock-chips.com> - 2016-01-30 13:20 +0100
Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish Heiko Stuebner <heiko@sntech.de> - 2016-01-30 15:00 +0100
Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish "David.Wu" <wdc@rock-chips.com> - 2016-02-01 10:00 +0100
Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish Heiko Stübner <heiko@sntech.de> - 2016-02-01 22:20 +0100
Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish "David.Wu" <wdc@rock-chips.com> - 2016-02-02 10:40 +0100
| From | David Wu <david.wu@rock-chips.com> |
|---|---|
| Date | 2016-01-30 13:20 +0100 |
| Subject | [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish |
| Message-ID | <qWCFr-2wT-7@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Heiko Stuebner <heiko@sntech.de> |
|---|---|
| Date | 2016-01-30 15:00 +0100 |
| Message-ID | <qWEef-3Qn-11@gated-at.bofh.it> |
| In reply to | #1322343 |
[Multipart message — attachments visible in raw view] — view raw
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++) {
[toc] | [prev] | [next] | [standalone]
| From | "David.Wu" <wdc@rock-chips.com> |
|---|---|
| Date | 2016-02-01 10:00 +0100 |
| Subject | Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish |
| Message-ID | <qXiv0-4W-17@gated-at.bofh.it> |
| In reply to | #1322366 |
[Multipart message — attachments visible in raw view] — view raw
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++) {
[toc] | [prev] | [next] | [standalone]
| From | Heiko Stübner <heiko@sntech.de> |
|---|---|
| Date | 2016-02-01 22:20 +0100 |
| Message-ID | <qXu38-mK-13@gated-at.bofh.it> |
| In reply to | #1322916 |
[Multipart message — attachments visible in raw view] — view raw
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
[toc] | [prev] | [next] | [standalone]
| From | "David.Wu" <wdc@rock-chips.com> |
|---|---|
| Date | 2016-02-02 10:40 +0100 |
| Subject | Re: [PATCH] PM / AVS: rockchip-io: add GRF and PMUGRF types to distinguish |
| Message-ID | <qXFBg-xF-5@gated-at.bofh.it> |
| In reply to | #1323514 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web