On Sun, 23 Aug 2020, Dmitry Osipenko wrote: > 23.08.2020 21:16, Lubomir Rintel пишет: > > Hello, > ... > >> +config MFD_ACER_A500_EC > >> + tristate "Embedded Controller driver for Acer Iconia Tab A500" > >> + depends on (I2C_TEGRA && ARCH_TEGRA_2x_SOC) || COMPILE_TEST > > > > This seems to also depend on I2C and OF. Perhaps I2C_TEGRA and > > ARCH_TEGRA_2x_SOC imply that, but it could lead to build failures with > > COMPILE_TEST=y. > > Hello, Lubomir! You're right about the I2C because it could be compiled > as a loadable module, good catch! The OF seems should fine as-is. > > ... > >> +static struct a500_ec *a500_ec_scratch; > > > > If this is only used for power_off, please rename it. I've been told to > > do so in my driver: https://lore.kernel.org/lkml/20200519104933.GX271301@dell/ > > I don't mind to rename the variable, but not sure whether it will be a > worthwhile change since _scratch is also a common naming scheme among > MFD drivers. Please see max77620_scratch for example, which I added > about a year ago. If something is used once, it does not make it 'common'. Not sure how this slipped my notice before, but I don't like it. Ensure any global struct used for power_off only includes items required for this purpose. It's unfortunate this API requires a global variable at all. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog