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? > 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. 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. > > 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? > 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? -- Jean Delvare SUSE L3 Support
