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

[Ed Tanous]

On Wed, Jul 8, 2020 at 2:46 PM Alex Qiu <xqiu at google.com> wrote:
>
> On Wed, Jul 8, 2020 at 11:06 AM Ed Tanous <ed at tanous.net> wrote:
> >
> > 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).
>
> We've talked with the maintainer of hwmon, and he doesn't think adding
> these to hwmon interface is a good idea, as hwmon is supposed to be a
> monitoring interface. Although we haven't yet met the need to set the
> voltage other than debugging.

Excellent.  I hadn't realized you'd done that.  You're right,
userspace is probably the right spot for this then.

>
> >
> > 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.
>
> First of all, I don't have the magic to turn downstream patches into
> an upstream fix in one day, one week or so, and we currently have the
> priority to fix sensor performance issues so that we can get our new
> platform out of the door this month. On the other hand, my intention
> is to get this conversation started and rolling before we eventually
> have the free time to deal with all the technical debt we accumulated
> downstream, so that we know the aim is as soon as we are at that
> stage.

To be clear, I wasn't expecting you to turn patches around in a day or
week.  Even patches that are public, but not upstreamable are a start,
as they show what you're trying to accomplish, and someone else might
pick it up from there.  Tech debt is a tough mistress, I don't envy
the position you're in.

>
> Clearly, I see the conversation we had so far is greatly valuable at
> least to us, and I believe we have an internal communication gap to
> fill between different teams first. For example, before the
> conversation, I never knew that we were supposed to upstream our
> configuration files. By contrast, I was told that these code names
> can't be publicized, and we've been keeping patches downstream, so
> there's definitely something to resolve internally first. Our team has
> been working underwater for a long time without knowing these
> intentions upstream, and I think it's the first time that we reach
> upstream in this level of detail.

Great!  I'm glad we could help out.  To be clear, for non-public
codename platforms, I wouldn't expect you to upstream the config
files, but you'll have continuing tech debt as the schemas for those
files evolve, so there is an argument to be made with your powers that
be that they should be upstreamed anyway, even if the codename isn't
public, or you need a different codename.
With that said, the config files really aren't the interesting part,
and only serve to make upstream build and run out of the box.  The
real wins are the new sensor and config types, and fixes to the
existing daemons.  In general, those help everyone.

>
> I hope this explains that I can't put up patches for things that I've
> been aware of right away, since we might have been doing something in
> the wrong way for quite some time. Thanks.
>

Whether you upsteam them next week, or a year from now, so long as
that's the direction you're going, and you're willing to make
improvements, I'm happy.

Good luck getting your platform launched.

Cheers,

-Ed