On 4/9/19 2:44 PM, Jean Delvare wrote:
Hi Eddie,
Thanks for the quick answer.
On Tue, 9 Apr 2019 10:20:22 -0500, Eddie James wrote:
On 4/9/19 7:45 AM, Jean Delvare wrote:
Don't propose PowerPC-only drivers on other architectures, unless
build-testing.
This driver does NOT only run on PowerPC; rather it runs on a BMC
processor connected to a PowerPC processor. BMC will most likely be ARM,
Thanks for clarifying. So, you have a server with one or more PowerPC
processors, running any operating system decided by the customer, and
in the same system, is a BMC, running a Linux operating system provided
by IBM? Do I understand it correctly?
Exactly, though the operating system running on the BMC (OpenBMC) is
used/developed by a number of other companies as well.
but shouldn't be restricted to that arch only.
Why not? Restricting drivers to the architectures or platforms where
they make sense significantly eases the work of the maintainers of
distribution kernels. Each new kernel version comes with several dozens
of new options. Without hints, they have no idea what is needed on
which architecture, and they either select everything, resulting in an
overweight kernel forever, or nothing, resulting in missing features
until someone complains.
I see, that makes sense to restrict it then. It is possible someone will
want to run OpenBMC managing a Power processor on something other than
an ARM chip, but I don't think that configuration is needed right now.
Regardless of any dependency at the Kconfig level, the help text of
these drivers should explain exactly what you wrote above. At the
moment, the reader has no way to guess that the drivers only make sense
for an embedded Linux running on a BMC. As I understand it now, general
purpose distributions do not need these drivers, right? But for example
SUSE kernels included them on all architectures. I just restricted them
to ppc64, but apparently I was wrong and it should be disabled there
too.
A good Kconfig help text should help the user decide whether they need
the driver or not.
Yep, the Kconfig text can probably be improved as well. I'll look into that.
Also drop configuration symbol SENSORS_OCC which serves no purpose
that I can see.
Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
Cc: Eddie James <eajames@xxxxxxxxxxxxx>
Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
---
SENSORS_OCC *would* serve a purpose if the common code between the
POWER8 driver and the POWER9 driver would go in a separate, shared
module, and occ-p8-hwmon and occ-p9-hwmon would only contain the
specific code. This would avoid packaging the same code twice in 2
separate modules, therefore saving some storage space for ppc
distributions.
Well you'd never have both P8 and P9 enabled at once, so space shouldn't
be an issue. I agree this could be cleaner but I think I was getting
Where "you" is IBM, provider of the embedded Linux running on the BMC?
Yes, though again could be any distributor of OpenBMC.
Thanks for looking into this!
Eddie
duplicate symbol errors for the compile test and so I did it this way.
If this doesn't lead to errors in the compile test, I'm fine with this
(without the change for PPC only though).
Dropping SENSORS_OCC can't result in duplicate symbols because it is a
no-op. But while this is the easiest solution, it it not the cleanest
in my opinion. OTOH, if you are certain that both modules will never be
shipped at the same time, then it indeed doesn't matter that much. I
can provide a patch going in either direction. Guenter, any preference?
