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 >>