Hi,
On 3/25/20 3:10 PM, Bastien Nocera wrote:
On Wed, 2020-03-25 at 15:05 +0100, Hans de Goede wrote:
Hi,
On 3/25/20 3:02 PM, Bastien Nocera wrote:
On Wed, 2020-03-25 at 14:55 +0100, Hans de Goede wrote:
We could do something like that, but TBH I'm not a fan of that
adding extra wrappers makes it harder to see what the code is
actually doing.
I understand your dislike for the extra braces I added and
I'm fine with fixing that by adding __maybe_unused to the
variable declarations at the top. I don't really see what
the problem with the #ifdef-s is given how clean they are,
with the braces thing fixed by using __maybe_unused things
would look like e.g. this:
It's not only the fact that there's extra #ifdef's, it's that the
ifdef's need to be just "that". It's not "#ifdef FOO", it's "#if
defined CONFIG_X86 && defined CONFIG_ACPI".
If that is the problem I would prefer adding:
/* Our special handling for GPIO accesses through ACPI is x86
specific */
#if defined CONFIG_X86 && defined CONFIG_ACPI
#define ACPI_GPIO_SUPPORT
#endif
And use:
#ifdef ACPI_GPIO_SUPPORT
Elsewhere.
Would that work for you?
That's slightly better, but I would still have preferred stubbing out
those ACPI calls directly. Right now, the fact that we expect half of
the commands to be stubbed out and the other half to not be called is
just weird.
I agree that ideally we would have stubs for this in the include/linux/...
headers, but unfortunately that is not the case here; and fixing this
is tricky for the acpi case as explained before.
I'm not a fan of adding local stubs for this, so I'm going to go with
adding:
#if defined CONFIG_X86 && defined CONFIG_ACPI
#define ACPI_GPIO_SUPPORT
#endif
+ using __maybe_unused to avoid the extra braces for v2, and then lets
see if Dmitry has anything to add to this discussion.
Regards,
Hans
