Add dphy pmu to turn on/off the dphy power switch.
Signed-off-by: Changhuang Liang <[email protected]>
---
MAINTAINERS | 1 +
drivers/soc/starfive/jh71xx_pmu.c | 65 +++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0b2170e1e4ff..4d958f02403e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19944,6 +19944,7 @@ F: include/dt-bindings/reset/starfive?jh71*.h
STARFIVE JH71XX PMU CONTROLLER DRIVER
M: Walker Chen <[email protected]>
+M: Changhuang Liang <[email protected]>
S: Supported
F: Documentation/devicetree/bindings/power/starfive*
F: drivers/soc/starfive/jh71xx_pmu.c
diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
index 990db6735c48..d4092ca4dccf 100644
--- a/drivers/soc/starfive/jh71xx_pmu.c
+++ b/drivers/soc/starfive/jh71xx_pmu.c
@@ -24,6 +24,9 @@
#define JH71XX_PMU_EVENT_STATUS 0x88
#define JH71XX_PMU_INT_STATUS 0x8C
+/* DPHY pmu register offset */
+#define JH71XX_PMU_DPHY_SWITCH 0x00
+
/* sw encourage cfg */
#define JH71XX_PMU_SW_ENCOURAGE_EN_LO 0x05
#define JH71XX_PMU_SW_ENCOURAGE_EN_HI 0x50
@@ -94,6 +97,8 @@ static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_o
if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
offset = JH71XX_PMU_CURR_POWER_MODE;
+ else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
+ offset = JH71XX_PMU_DPHY_SWITCH;
regmap_read(pmu->base, offset, &val);
@@ -170,6 +175,23 @@ static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bo
return 0;
}
+static int jh71xx_pmu_dphy_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
+{
+ struct jh71xx_pmu *pmu = pmd->pmu;
+ unsigned long flags;
+
+ spin_lock_irqsave(&pmu->lock, flags);
+
+ if (on)
+ regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, mask);
+ else
+ regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, 0);
+
+ spin_unlock_irqrestore(&pmu->lock, flags);
+
+ return 0;
+}
+
static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
{
struct jh71xx_pmu *pmu = pmd->pmu;
@@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
ret = jh71xx_pmu_general_set_state(pmd, mask, on);
+ else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
+ ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
return ret;
}
@@ -280,6 +304,25 @@ static int jh7110_pmu_general_parse_dt(struct platform_device *pdev,
return 0;
}
+static int jh7110_pmu_dphy_parse_dt(struct platform_device *pdev,
+ struct jh71xx_pmu *pmu)
+{
+ struct device *parent;
+ struct device *dev = &pdev->dev;
+
+ parent = pdev->dev.parent;
+ if (!parent) {
+ dev_err(dev, "No parent for syscon pmu\n");
+ return -ENODEV;
+ }
+
+ pmu->base = syscon_node_to_regmap(parent->of_node);
+ if (IS_ERR(pmu->base))
+ return PTR_ERR(pmu->base);
+
+ return 0;
+}
+
static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
{
struct jh71xx_pmu_dev *pmd;
@@ -409,10 +452,31 @@ static const struct jh71xx_pmu_match_data jh7110_pmu = {
.pmu_parse_dt = jh7110_pmu_general_parse_dt,
};
+static const struct jh71xx_domain_info jh7110_dphy_power_domains[] = {
+ [JH7110_PD_DPHY_TX] = {
+ .name = "DPHY-TX",
+ .bit = 30,
+ },
+ [JH7110_PD_DPHY_RX] = {
+ .name = "DPHY-RX",
+ .bit = 31,
+ },
+};
+
+static const struct jh71xx_pmu_match_data jh7110_pmu_dphy = {
+ .num_domains = ARRAY_SIZE(jh7110_dphy_power_domains),
+ .domain_info = jh7110_dphy_power_domains,
+ .pmu_type = JH71XX_PMU_DPHY,
+ .pmu_parse_dt = jh7110_pmu_dphy_parse_dt,
+};
+
static const struct of_device_id jh71xx_pmu_of_match[] = {
{
.compatible = "starfive,jh7110-pmu",
.data = (void *)&jh7110_pmu,
+ }, {
+ .compatible = "starfive,jh7110-pmu-dphy",
+ .data = (void *)&jh7110_pmu_dphy,
}, {
/* sentinel */
}
@@ -429,5 +493,6 @@ static struct platform_driver jh71xx_pmu_driver = {
builtin_platform_driver(jh71xx_pmu_driver);
MODULE_AUTHOR("Walker Chen <[email protected]>");
+MODULE_AUTHOR("Changhuang Liang <[email protected]>");
MODULE_DESCRIPTION("StarFive JH71XX PMU Driver");
MODULE_LICENSE("GPL");
--
2.25.1
On Mon, Apr 10, 2023 at 11:47:42PM -0700, Changhuang Liang wrote:
> Add dphy pmu to turn on/off the dphy power switch.
>
> Signed-off-by: Changhuang Liang <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/soc/starfive/jh71xx_pmu.c | 65 +++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0b2170e1e4ff..4d958f02403e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19944,6 +19944,7 @@ F: include/dt-bindings/reset/starfive?jh71*.h
>
> STARFIVE JH71XX PMU CONTROLLER DRIVER
> M: Walker Chen <[email protected]>
> +M: Changhuang Liang <[email protected]>
Unmentioned in the commit message, plus I would like an R-b or an Ack
from Walker.
> S: Supported
> F: Documentation/devicetree/bindings/power/starfive*
> F: drivers/soc/starfive/jh71xx_pmu.c
> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
> index 990db6735c48..d4092ca4dccf 100644
> --- a/drivers/soc/starfive/jh71xx_pmu.c
> +++ b/drivers/soc/starfive/jh71xx_pmu.c
> @@ -24,6 +24,9 @@
> #define JH71XX_PMU_EVENT_STATUS 0x88
> #define JH71XX_PMU_INT_STATUS 0x8C
>
> +/* DPHY pmu register offset */
> +#define JH71XX_PMU_DPHY_SWITCH 0x00
> +
> /* sw encourage cfg */
> #define JH71XX_PMU_SW_ENCOURAGE_EN_LO 0x05
> #define JH71XX_PMU_SW_ENCOURAGE_EN_HI 0x50
> @@ -94,6 +97,8 @@ static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_o
>
> if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
> offset = JH71XX_PMU_CURR_POWER_MODE;
> + else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
There are only two options for this "enum", so why `else if`?
> + offset = JH71XX_PMU_DPHY_SWITCH;
>
> regmap_read(pmu->base, offset, &val);
>
> @@ -170,6 +175,23 @@ static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bo
> return 0;
> }
>
> +static int jh71xx_pmu_dphy_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
> +{
> + struct jh71xx_pmu *pmu = pmd->pmu;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&pmu->lock, flags);
> +
> + if (on)
> + regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, mask);
> + else
> + regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, 0);
> +
> + spin_unlock_irqrestore(&pmu->lock, flags);
> +
> + return 0;
> +}
> +
> static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
> {
> struct jh71xx_pmu *pmu = pmd->pmu;
> @@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>
> if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
> ret = jh71xx_pmu_general_set_state(pmd, mask, on);
> + else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
> + ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
Perhaps I am verging on over-complication, but I dislike this carry on.
Is this the only time we'll see a power domain provider coming out of
a syscon, or are there likely to be more?
Either way, I think having an ops struct w/ both parse_dt() and the
set_state() implementations would be neater than what you have here.
Very much open to dissenting opinions there though. Emil? Walker?
Cheers,
Conor.
>
> return ret;
> }
> @@ -280,6 +304,25 @@ static int jh7110_pmu_general_parse_dt(struct platform_device *pdev,
> return 0;
> }
>
> +static int jh7110_pmu_dphy_parse_dt(struct platform_device *pdev,
> + struct jh71xx_pmu *pmu)
> +{
> + struct device *parent;
> + struct device *dev = &pdev->dev;
> +
> + parent = pdev->dev.parent;
> + if (!parent) {
> + dev_err(dev, "No parent for syscon pmu\n");
> + return -ENODEV;
> + }
> +
> + pmu->base = syscon_node_to_regmap(parent->of_node);
> + if (IS_ERR(pmu->base))
> + return PTR_ERR(pmu->base);
> +
> + return 0;
> +}
> +
> static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
> {
> struct jh71xx_pmu_dev *pmd;
> @@ -409,10 +452,31 @@ static const struct jh71xx_pmu_match_data jh7110_pmu = {
> .pmu_parse_dt = jh7110_pmu_general_parse_dt,
> };
>
> +static const struct jh71xx_domain_info jh7110_dphy_power_domains[] = {
> + [JH7110_PD_DPHY_TX] = {
> + .name = "DPHY-TX",
> + .bit = 30,
> + },
> + [JH7110_PD_DPHY_RX] = {
> + .name = "DPHY-RX",
> + .bit = 31,
> + },
> +};
> +
> +static const struct jh71xx_pmu_match_data jh7110_pmu_dphy = {
> + .num_domains = ARRAY_SIZE(jh7110_dphy_power_domains),
> + .domain_info = jh7110_dphy_power_domains,
> + .pmu_type = JH71XX_PMU_DPHY,
> + .pmu_parse_dt = jh7110_pmu_dphy_parse_dt,
> +};
> +
> static const struct of_device_id jh71xx_pmu_of_match[] = {
> {
> .compatible = "starfive,jh7110-pmu",
> .data = (void *)&jh7110_pmu,
> + }, {
> + .compatible = "starfive,jh7110-pmu-dphy",
> + .data = (void *)&jh7110_pmu_dphy,
> }, {
> /* sentinel */
> }
> @@ -429,5 +493,6 @@ static struct platform_driver jh71xx_pmu_driver = {
> builtin_platform_driver(jh71xx_pmu_driver);
>
> MODULE_AUTHOR("Walker Chen <[email protected]>");
> +MODULE_AUTHOR("Changhuang Liang <[email protected]>");
> MODULE_DESCRIPTION("StarFive JH71XX PMU Driver");
> MODULE_LICENSE("GPL");
> --
> 2.25.1
>
On 2023/4/12 5:15, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:42PM -0700, Changhuang Liang wrote:
[...]
>> +++ b/MAINTAINERS
>> @@ -19944,6 +19944,7 @@ F: include/dt-bindings/reset/starfive?jh71*.h
>>
>> STARFIVE JH71XX PMU CONTROLLER DRIVER
>> M: Walker Chen <[email protected]>
>> +M: Changhuang Liang <[email protected]>
>
> Unmentioned in the commit message, plus I would like an R-b or an Ack
> from Walker.
>
OK, I will make a discuss with Walker.
>> S: Supported
>> F: Documentation/devicetree/bindings/power/starfive*
>> F: drivers/soc/starfive/jh71xx_pmu.c
>> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
>> index 990db6735c48..d4092ca4dccf 100644
[...]
>> @@ -94,6 +97,8 @@ static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_o
>>
>> if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>> offset = JH71XX_PMU_CURR_POWER_MODE;
>> + else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
>
> There are only two options for this "enum", so why `else if`?
>
OK, will change to else.
>> + offset = JH71XX_PMU_DPHY_SWITCH;
>>
>> regmap_read(pmu->base, offset, &val);
>>
>> @@ -170,6 +175,23 @@ static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bo
>> return 0;
>> }
>>
[...]
>> static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>> {
>> struct jh71xx_pmu *pmu = pmd->pmu;
>> @@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>
>> if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>> ret = jh71xx_pmu_general_set_state(pmd, mask, on);
>> + else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
>> + ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
>
> Perhaps I am verging on over-complication, but I dislike this carry on.
> Is this the only time we'll see a power domain provider coming out of
> a syscon, or are there likely to be more?
> Either way, I think having an ops struct w/ both parse_dt() and the
> set_state() implementations would be neater than what you have here.
>
> Very much open to dissenting opinions there though. Emil? Walker?
>
> Cheers,
> Conor.
>
"else if" will change to "else"
As far as I know, there are only two types power domain on the JH7110 SoC.
One is the original, another one is coming out of a syscon.
>>
>> return ret;
>> }
[...]
>> 2.25.1
>>
On 2023/4/12 5:15, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:42PM -0700, Changhuang Liang wrote:
>> Add dphy pmu to turn on/off the dphy power switch.
[...]
>> +
>> static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>> {
>> struct jh71xx_pmu *pmu = pmd->pmu;
>> @@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>
>> if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>> ret = jh71xx_pmu_general_set_state(pmd, mask, on);
>> + else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
>> + ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
>
> Perhaps I am verging on over-complication, but I dislike this carry on.
> Is this the only time we'll see a power domain provider coming out of
> a syscon, or are there likely to be more?
> Either way, I think having an ops struct w/ both parse_dt() and the
> set_state() implementations would be neater than what you have here.
>
OK, I will use call back make here neater.
> Very much open to dissenting opinions there though. Emil? Walker?
>
> Cheers,
> Conor.
>
>>
>> return ret;
>> }
[...]
>> --
>> 2.25.1
>>