lkml.org 
[lkml]   [2023]   [Apr]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 6/7] soc: starfive: Add dphy pmu support
    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
    >
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-04-11 23:16    [W:3.864 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site