Re: [PATCH 01/12] power: sequencing: Introduce an API to check whether the pwrseq is fixed or controllable

[Manivannan Sadhasivam]

On Thu, Apr 23, 2026 at 12:24:02PM -0400, Bartosz Golaszewski wrote:
> On Wed, 22 Apr 2026 13:24:42 +0200, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.oss.qualcomm.com@xxxxxxxxxx> said:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> >
> > Introduce an API pwrseq_is_fixed() so that the consumers can check whether
> > the given power sequencer is fixed or controllable. This will come handy
> > in situations where the consumers need to know whether the specific power
> > sequencer like 'Bluetooth' can be controllable using properties like BT_EN.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > ---
> >
> 
> I have several concerns about this new function: its name is very vague and
> doesn't really indicate its function (how is it "fixed" if you still control
> it?), it can be used to expose all kinds of things because of this lack of
> precision. I feel like it goes against the goal of pwrseq which is to abstract
> these things away from consumers.
> 
> The problem we have here is for now a HW quirk affecting a single driver. I'm
> thinking that we can live with this driver just checking the relevant property
> of the provider device.
> 
> Many subsystems provide functions that allow accessing the struct device
> associated with the provider. Could we introduce something like:
> 
> struct device *pwrseq_to_device(struct pwrseq_desc);
> 
> that would return the address of struct device associated with the provider of
> the descriptor? It wouldn't even have to return a new reference as holding a
> descriptor already implies also holding a reference to the pwrseq device
> backing it.
> 
> Then in the bluetooth driver you could do:
> 
> 	struct pwrseq_desc *pwrseq = pwrseq_get(dev, "bluetooth");
> 	struct device *dev = pwrseq_to_device(pwrseq);
> 
> 	// Big fat comment stating why you do this
> 	if (!device_property_present(dev, "enable-gpios")) {
> 		// do whatever quirk is required
> 	}
> 
> Would that make sense?
> 

Yeah, sounds good to me. I'll incorporate this in v2, thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்