Feedback on Current OpenBMC and Proposing Some Improvements ---- "Improvements" Ideas

[Ed Tanous]

On Wed, Jul 1, 2020 at 10:06 AM Alex Qiu <xqiu at google.com> wrote:
>
> On Tue, Jun 30, 2020 at 7:00 PM Ed Tanous <ed at tanous.net> wrote:
> >
> > I'm not following that statement.  "find the bus number" would occur
> > whether or not you have the busses hardcoded.  Are you advocating for
> > not using hwmon sensors here?  Needing to do a calculation for the new
> > part you're adding would need to be done regardless.  If you turn it
> > into a hwmon sensor, you could have the kernel do the math for you,
> > and keep your debugability.
>
> Hardware engineers want to set the output voltage for voltage
> regulators for debugging, which is not covered by hwmon interface or
> drivers, so we need to send raw I2C commands.
Or add support to the drivers.....
I'm not against raw i2c commands for debugging, but long term, it's
really hard to maintain (as you seem to be finding).

> When a system is not
> fully populated, I believe the kernel always assigns the largest
> sequential numbers to newly created MUX channels, so that number will
> vary based on the debug system configurations. On the other hand, our
> current workaround to populate the MUXes in the device tree while they
> may not exist fixes the bus numbers which can be calculated from a
> formula, instead of tracing symlinks.
It sounds like we have different priorities.  You want to make it easy
to debug a single given hardware configuration, entity managers goal
was to make it straightforward to debug any hardware configuration on
any platform.  Different goals, neither are wrong, just different.

>
> For the concern of compatibility, we worry that other companies are
> also using these features downstream. FYI, we are heavily relying on
> it right now, even though we find out it's not following arithmetic
> order of operations.
>
With respect to those companies, their downstream is their problem.
That's why upstreaming is important.  I'm not saying we need to break
things unnecessarily, but it's a really poor excuse to say we can't
change things because of an unknown entity that didn't share their
code.  If they exist, they're using a feature that's relatively new to
entity manager, and so far as I know, is only used in a single case,
and in that case, mod operator is at the same or greater precedence
than + operator, so you could make the change with zero impact to a
anyone that I'm aware of.


We've gone several rounds on this email, with a lot of places where
you could make improvements, including many that wouldn't break
anything, but you always seem to come back to being too busy for it,
or it not being "upstreamable".  Is there anything that the project
could do to help convince you to at least share some changes that
you've suggested?  The feedback is really great, but I was hoping with
the level of interest you have in this, you'd be interested in putting
together some patchsets to do some of these things, even if they're
minor, like adding support for your new chip.