| Message ID | 20230411064743.273388-7-changhuang.liang@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add JH7110 DPHY PMU support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
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 <changhuang.liang@starfivetech.com> > --- > 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 <walker.chen@starfivetech.com> > +M: Changhuang Liang <changhuang.liang@starfivetech.com> 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 <walker.chen@starfivetech.com>"); > +MODULE_AUTHOR("Changhuang Liang <changhuang.liang@starfivetech.com>"); > 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 <walker.chen@starfivetech.com> >> +M: Changhuang Liang <changhuang.liang@starfivetech.com> > > 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 >>
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 <walker.chen@starfivetech.com> +M: Changhuang Liang <changhuang.liang@starfivetech.com> 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 <walker.chen@starfivetech.com>"); +MODULE_AUTHOR("Changhuang Liang <changhuang.liang@starfivetech.com>"); MODULE_DESCRIPTION("StarFive JH71XX PMU Driver"); MODULE_LICENSE("GPL");
Add dphy pmu to turn on/off the dphy power switch. Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com> --- MAINTAINERS | 1 + drivers/soc/starfive/jh71xx_pmu.c | 65 +++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+)