<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-im" style="color:rgb(80,0,80)">> >       What is it that makes you want to write your code using low-level<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        i2c and PMBus APIs directly in userspace instead of writing or<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        updating drivers and using the various high-level user APIs<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        provided by kernel subsystems?<br></span><span class="gmail-im" style="color:rgb(80,0,80)">><br></span><span class="gmail-im" style="color:rgb(80,0,80)">> I speak in the context of hwmon/pmbus but these patches were simply<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> rejected. A lot of times the device you want to upgrade is also the device<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> you're gathering telemetry from.</span><span class="gmail-im" style="color:rgb(80,0,80)"><br></span>I think the "is also" is the part that gives me concern.  Generally this<br>means binding/unbinding the kernel side of it, which isn't pleasant.</blockquote><div><br></div><div>Yup, this definitely isn't pleasant but it's doable. Have you had experiences where unbinding/binding caused lots of pain? The only pain that I've seen is that telemetry daemons generally don't take well to having hwmon sysfs attributes yanked from underneath them.</div><div>Just spitballing.. but for devices that need to be upgradeable <b>and</b> need to report telemetry, that such things should be done in a single service and perhaps it makes the most sense to do it all in userspace (to avoid ugly unbinding/binding). </div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-im" style="color:rgb(80,0,80)">><br></span><span class="gmail-im" style="color:rgb(80,0,80)">>        I see you mentioned "pmbus device upgrades" and I know the PMBus<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        subsystem doesn't currently support firmware upgrade paths.  But,<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        there has been LKML threads about it and what I recollect was<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        that it wasn't "not allowed" but just "not implemented".<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        Shouldn't we be focusing our efforts on enhancing features for<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        the whole OSS community rather than building our own different<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> >        library?<br></span><span class="gmail-im" style="color:rgb(80,0,80)">><br></span><span class="gmail-im" style="color:rgb(80,0,80)">> Fair point but I don't see them as mutually exclusive, use hwmon/pmbus<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> drivers where they make sense and work for you. Switch to userspace for<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> stuff that gets strong pushback from hwmon/pmbus maintainer or stuff that<br></span><span class="gmail-im" style="color:rgb(80,0,80)">> just doesn't make sense to put into a driver.</span><span class="gmail-im" style="color:rgb(80,0,80)"><br></span>My feeling is that we need more definition on what that boundary is.  As<br>long as we are really only doing stuff from userspace when there is no<br>other path forward, I don't have much concern.  But, I've seen too many<br>cases where someone has tried to write an i2c-driver-in-userspace<br>because they "didn't like working with the kernel" or some similar<br>excuse.<br>What is something that doesn't make sense to put into a driver and why?</blockquote><div><br></div><div>Firmware/config upgrades and the reason is that your patch will get rejected.</div><div>The feeling is that "dangerous" i2c things that could brick the system or damage it shouldn't be put into the kernel and that the preference is to have this done in userspace via i2c-dev. This statement was made about sequencer config and firmware upgrades. </div><div>I suspect it would extend to arbitrarily adjusting voltages, putting devices into special states, sending control commands to a device's non pmbus standard i2c interface (vendor specific stuff, like indirect register accesses).<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Apr 30, 2021 at 8:45 AM Jason Ling <<a href="mailto:jasonling@google.com">jasonling@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><span style="color:rgb(80,0,80)">On Fri, Apr 30, 2021 at 07:52:36AM -0700, Jason Ling wrote:<br>><br>>     2. How does the availability (and potential recommendation of usage<br>> >        by our community) affect the effort to ensure that all i2c and<br>> >        pmbus drivers are upstreamed?<br>><br>> Well, this library is meant for userspace usage. So i2c (hwmon?) and pmbus<br>> drivers would continue to be upstreamed as per usual.<br>> Motivating use case for userspace i2c transactions are pmbus firmware<br>> updates. With adm1266 we tried to upstream sequencer configuration update<br>> via the hwmon/pmbus driver, it failed spectacularly. So we have to do it<br>> userspace.<br><br></span>Do you have pointers to the discussion?<div><br></div><div>Whew...took a lot longer to find the thread but here is the explicit rejection of firmware and configuration upgrade from within the kernel</div><div><br></div><div><a href="https://lkml.org/lkml/2020/8/7/565" target="_blank">https://lkml.org/lkml/2020/8/7/565</a><br><div><br></div><div>other things like don't put in ioctls</div><div><br></div><div><a href="https://lkml.org/lkml/2020/6/24/830" target="_blank">https://lkml.org/lkml/2020/6/24/830</a><br></div></div><div><br></div><div>This is the strongest use-case as it's been explicitly rejected by the subsystem maintainer.</div><div><br></div><div>I suspect that things like adjusting voltages would similarly be rejected but honestly we haven't gone down that path yet.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Apr 30, 2021 at 8:10 AM Patrick Williams <<a href="mailto:patrick@stwcx.xyz" target="_blank">patrick@stwcx.xyz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Apr 30, 2021 at 07:52:36AM -0700, Jason Ling wrote:<br>
> <br>
>     2. How does the availability (and potential recommendation of usage<br>
> >        by our community) affect the effort to ensure that all i2c and<br>
> >        pmbus drivers are upstreamed?<br>
> <br>
> Well, this library is meant for userspace usage. So i2c (hwmon?) and pmbus<br>
> drivers would continue to be upstreamed as per usual.<br>
> Motivating use case for userspace i2c transactions are pmbus firmware<br>
> updates. With adm1266 we tried to upstream sequencer configuration update<br>
> via the hwmon/pmbus driver, it failed spectacularly. So we have to do it<br>
> userspace.<br>
<br>
Do you have pointers to the discussion?<br>
<br>
> >       What is it that makes you want to write your code using low-level<br>
> >        i2c and PMBus APIs directly in userspace instead of writing or<br>
> >        updating drivers and using the various high-level user APIs<br>
> >        provided by kernel subsystems?<br>
> <br>
> I speak in the context of hwmon/pmbus but these patches were simply<br>
> rejected. A lot of times the device you want to upgrade is also the device<br>
> you're gathering telemetry from.<br>
<br>
I think the "is also" is the part that gives me concern.  Generally this<br>
means binding/unbinding the kernel side of it, which isn't pleasant.<br>
<br>
> <br>
>        I see you mentioned "pmbus device upgrades" and I know the PMBus<br>
> >        subsystem doesn't currently support firmware upgrade paths.  But,<br>
> >        there has been LKML threads about it and what I recollect was<br>
> >        that it wasn't "not allowed" but just "not implemented".<br>
> >        Shouldn't we be focusing our efforts on enhancing features for<br>
> >        the whole OSS community rather than building our own different<br>
> >        library?<br>
> <br>
> Fair point but I don't see them as mutually exclusive, use hwmon/pmbus<br>
> drivers where they make sense and work for you. Switch to userspace for<br>
> stuff that gets strong pushback from hwmon/pmbus maintainer or stuff that<br>
> just doesn't make sense to put into a driver.<br>
<br>
My feeling is that we need more definition on what that boundary is.  As<br>
long as we are really only doing stuff from userspace when there is no<br>
other path forward, I don't have much concern.  But, I've seen too many<br>
cases where someone has tried to write an i2c-driver-in-userspace<br>
because they "didn't like working with the kernel" or some similar<br>
excuse.<br>
<br>
What is something that doesn't make sense to put into a driver and why?<br>
<br>
-- <br>
Patrick Williams<br>
</blockquote></div>
</blockquote></div>