[...]
> >> +
> >> +struct pwrseq *of_pwrseq_xlate_onecell(void *data, struct of_phandle_args *args)
> >> +{
> >> + struct pwrseq_onecell_data *pwrseq_data = data;
> >> + unsigned int idx;
> >> +
> >> + if (args->args_count != 1)
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + idx = args->args[0];
> >> + if (idx >= pwrseq_data->num) {
> >> + pr_err("%s: invalid index %u\n", __func__, idx);
> >> + return ERR_PTR(-EINVAL);
> >> + }
> >
> > In many cases it's reasonable to leave room for future extensions, so
> > that a provider could serve with more than one power-sequencer. I
> > guess that is what you intend to do here, right?
> >
> > In my opinion, I don't think what would happen, especially since a
> > power-sequence is something that should be specific to one particular
> > device (a Qcom WiFi/Blutooth chip, for example).
> >
> > That said, I suggest limiting this to a 1:1 mapping between the device
> > node and power-sequencer. I think that should simplify the code a bit.
>
> In fact the WiFi/BT example itself provides a non 1:1 mapping. In my
> current design the power sequencer provides two instances (one for WiFi,
> one for BT). This allows us to move the knowledge about "enable" pins to
> the pwrseq. Once the QCA BT driver acquires and powers up the pwrseq,
> the BT part is ready. No need to toggle any additional pins. Once the
> WiFi pwrseq is powered up, the WiFi part is present on the bus and
> ready, without any additional pin toggling.
Aha, that seems reasonable.
>
> I can move onecell support to the separate patch if you think this might
> simplify the code review.
It doesn't matter, both options work for me.
[...]
Kind regards
Uffe
