On Mon, Sep 23, 2024 at 06:53:07PM +0200, Jerome Brunet wrote: > On Mon 23 Sep 2024 at 14:21, Mark Brown <broonie@xxxxxxxxxx> wrote: > > So, I know we talked about this a bit on IRC but I didn't register the > > specific use case. Now I see that it's coming down to the fact that the > > chip is simply write protected I'm wondering if it might not be simpler > > to handle this at the ops level rather than the constraints level. When > > registering the driver could check for write protection and then instead > > of registering the normal operations register an alternative set with > > the relevant write operations removed. That would have the same effect > > that you're going for AFAICT. Sorry for not thinking of it earlier. > Actually I thaught about it, that was my first idea. > There is tiny difference between the 2 approach: > * A regulator that does not provide enable()/disable() is considered > always-on, so it is not really checked if it is enabled or not > * A regulator that does provide enable()/disable() but disallow status > change will be checked with is_enabled() > In the second case, we will pick up on regulator that is disabled and we > can't enable. In the first case, I don't think we do. I don't know if it > is a bug of not but that makes the 2nd case more correct for what we do > with pmbus regs I think I think for that we should just always use the is_enabled() operation if it's available. This is simply an oversight caused by not imagining a case where a regulator could have an enable control which is locked out like this, the normal case is that either you can control enable or the regulator is always on. > The other thing, although is more a pmbus implemantion consideration, > is that the type regulator is opaque in > pmbus_regulator_register. Different types could be registered so > manipulating the ops is tricky. That's where a callback is helpful > If building the ops dynamically is the preferred way, I'll find a way to > make it happen. I'll need to way to identify which one need it, loose > the 'const' qualifier in a lot of place, etc ... that will be a bit > hackish I'm afraid. With only a limited set of options it might be better to just have a set of static structs and pick one to register (some other drivers do this based on hardware options), but the number of combinations with this is getting a bit big for that.
Attachment:
signature.asc
Description: PGP signature
