Re: [PATCH] drivers/regulator: Notify sysfs about status changes

[Naresh Solanki]

Hi Mark,

On Thu, 2 Nov 2023 at 22:20, Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> On Thu, Nov 02, 2023 at 09:03:40PM +0530, Naresh Solanki wrote:
> > On Thu, 2 Nov 2023 at 20:31, Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> > > That's the opposite sense to what I was thinking of - we're reporting
> > > voltage changes and enables to userspace rather than just errors.  My
> > > concern here is that this could generate an awful lot of notificaitons
> > > for normal operation on systems that don't use the uevents, I was
> > > expecting this to be used for errors.  Could you remind me what the use
> > > case is here, I think I might've got myself confused sorry?
>
> > Sorry for confusion caused because I should first described my application
> > requirements.
> > Currently my application is interested in know regulator status i.e.,
> > ENABLE, DISABLE or ERROR.
> > Also events are needed specifically to get them logged like
> > UNDER_VOLTAGE, OVER_CURRENT, REGULATION_OUT,
> > OVER_TEMP.
>
> Ah, right.  Everything except for the enable and disable there looks
> like it should be OK since they should normally just not happen but the
> enables and disables might get a bit frequent with runtime PM - not
> *super* frequent like voltage scaling but enough that people could have
> an issue with it.
I aim for a straightforward implementation.
Using attributes such as status or state seems ideal for receiving
notifications.
In my case, the application focuses on status changes—whether it's on, off,
or encountering an error.
These changes are driven by events originating from the regulator.
So below change is what I see fit well.

        status_events = REGULATOR_EVENT_DISABLE;
        status_events |= REGULATOR_EVENT_ENABLE;
        status_events |= REGULATOR_EVENT_FAIL;
        status_events |= REGULATOR_EVENT_FORCE_DISABLE;
        status_events |= REGULATOR_EVENT_ABORT_DISABLE;

        if (event & status_events) {
                name = dev_attr_status.attr.name;
                sysfs_notify(&rdev->dev.kobj, NULL, name);
        }

Let me know if this can be further tuned to be better..

Regards,
Naresh

>
> Netlink feels like it might be a better fit?  Not really looked at the
> kernel side of implementing that and how sensible that ends up looking.