2023-09-20 13:09:07

by Zev Weiss

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: ABI: sysfs-driver-regulator-output

On Wed, Sep 20, 2023 at 02:29:15AM PDT, Greg Kroah-Hartman wrote:
>On Wed, Sep 20, 2023 at 02:02:49AM -0700, Zev Weiss wrote:
>> static int regulator_userspace_notify(struct notifier_block *nb,
>> unsigned long event,
>> void *ignored)
>> {
>> struct userspace_consumer_data *data =
>> container_of(nb, struct userspace_consumer_data, nb);
>> - static const char * const *envp[] = { "NAME=events", NULL };
>
>You removed this user/kernel api value, what will break if you do that?
>

Sorry, I don't follow -- what removal are you referring to? The envp
array still has a NAME entry -- I changed its value from "events" to
"event", but I wrote that part before I realized that the 'event'
parameter of the function was actually a bitmask that might convey
multiple events and just forgot to change it back, so keeping it
pluralized is probably more appropriate.

And FWIW, I didn't intend for the exact format of the EVENT parameter
that I sketched there to be something that had to be kept; given that
there might be multiple entries perhaps it'd be better to use separate
parameters more like NUMEVENTS, EVENT0, EVENT1, etc? (Or omit NUMEVENTS
and just let the consumer count upward until it doesn't find a match.)


Zev


2023-09-21 00:38:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: ABI: sysfs-driver-regulator-output

On Wed, Sep 20, 2023 at 03:44:29AM -0700, Zev Weiss wrote:
> On Wed, Sep 20, 2023 at 02:29:15AM PDT, Greg Kroah-Hartman wrote:
> > On Wed, Sep 20, 2023 at 02:02:49AM -0700, Zev Weiss wrote:
> > > static int regulator_userspace_notify(struct notifier_block *nb,
> > > unsigned long event,
> > > void *ignored)
> > > {
> > > struct userspace_consumer_data *data =
> > > container_of(nb, struct userspace_consumer_data, nb);
> > > - static const char * const *envp[] = { "NAME=events", NULL };
> >
> > You removed this user/kernel api value, what will break if you do that?
> >
>
> Sorry, I don't follow -- what removal are you referring to? The envp array
> still has a NAME entry -- I changed its value from "events" to "event",

Yes, that value.

> but
> I wrote that part before I realized that the 'event' parameter of the
> function was actually a bitmask that might convey multiple events and just
> forgot to change it back, so keeping it pluralized is probably more
> appropriate.
>
> And FWIW, I didn't intend for the exact format of the EVENT parameter that I
> sketched there to be something that had to be kept; given that there might
> be multiple entries perhaps it'd be better to use separate parameters more
> like NUMEVENTS, EVENT0, EVENT1, etc? (Or omit NUMEVENTS and just let the
> consumer count upward until it doesn't find a match.)

I don't know, what does userspace do with this value today? If you
change it, what will break?

thanks,

greg k-h