On 2016-11-29 16:12, Tin Huynh wrote: > On Tue, Nov 29, 2016 at 5:15 PM, Peter Rosin <peda@xxxxxxxxxx> wrote: >> On 2016-11-29 11:12, Tin Huynh wrote: >>> This patch enables ACPI support for mux-pca954x driver. >> >> Looks good, thanks! >> >>> Signed-off-by: Tin Huynh <tnhuynh@xxxxxxx> >> >> Acked-by: Peter Rosin <peda@xxxxxxxxxx> >> >> Cheers, >> Peter >> > Added Mika > However , in http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpio-pca953x.c?id=f32517bf1ae0a2de72b3f27200233bd3ad65bfeb > log , i didn't see the CONFIG_ACPI and CONFIG_PTR. > I have been acked with the same patch to support for i2c-leds ( > without CONFIG_ACPI) > About logic , if we don't enable CONFIG_ACPI , acpi_match_table will > be NULL . So , the probe functions will be not called > and nothing happens. > I though we should use V3 (https://lkml.org/lkml/2016/11/28/893) > V3 will work, but you waste resources (not much, but still) for the ACPI table even though it is never going to be used, so it's best to #ifdef it out. (At least I think so, but maybe GCC sees that the variable isn't really used for anything useful and manages to kill it off? Anyway, I'm assuming that there really are resources wasted if you don't do the #ifdef...) But the thing is that you do not want a bunch of #ifdefs *in* the code. It's ok to #ifdef out a variable, but not *in* the code. That way lies madness. So the code is going to have a call to acpi_match_device(). If CONFIG_ACPI isn't set, that function compiles to an inline function simply returning NULL, but the compiler still evaluates its args and complains if the above variable is not found, which it isn't since it's #ifdef out. So, we need to "hide" the variable if it doesn't exist, using the ACPI_PTR macro. For gpio-pca953x, the same reasoning can be used, but the resource waste is even less in that case. Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
