Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1202898

Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding.

Show all headers | View raw

On 08/07/2015 05:42 AM, Tomasz Nowicki wrote:
> On 07.08.2015 13:56, Robert Richter wrote:
>> On 07.08.15 12:52:41, Tomasz Nowicki wrote:
[...]

>>>>
>>>> I would not pollute bgx_probe() with acpi and dt specifics, and instead
>>>> keep bgx_init_phy(). The typical design pattern for this is:
>>>>
>>>> static int bgx_init_phy(struct bgx *bgx)
>>>> {
>>>> #ifdef CONFIG_ACPI
>>>>          if (!acpi_disabled)
>>>>                  return bgx_init_acpi_phy(bgx);
>>>> #endif
>>>>          return bgx_init_of_phy(bgx);
>>>> }
>>>>
>>>> This adds acpi runtime detection (acpi=no), does not call dt code in
>>>> case of acpi, and saves the #else for bgx_init_acpi_phy().
>>>>
>>>
>>> I am fine with keeping it in bgx_init_phy(), however we can drop there
>>> #ifdefs since both of bgx_init_{acpi,of}_phy calls have empty stub
>>> for !ACPI
>>> and !OF case. Like that:
>>>
>>> static int bgx_init_phy(struct bgx *bgx)
>>> {
>>>
>>>          if (!acpi_disabled)
>>>                  return bgx_init_acpi_phy(bgx);
>>>     else
>>>              return bgx_init_of_phy(bgx);
>>> }
>>
>> As said, keeping it in #ifdefs makes the empty stub function for !acpi
>> obsolete, which makes the code smaller and better readable. This style
>> is common practice in the kernel. Apart from that, the 'else' should
>> be dropped as it is useless.
>>
>
> I would't say the code is better readable (bgx_init_of_phy has still two
> stubs) but this is just cosmetic, my point was to use run time detection
> using acpi_disabled.
>

Thanks Tomasz and Robert for the input.  Because of this, it seems that 
another version of the patch will be necessary.  In the interests of 
code clarity and asthetics, we will go with the code without the 
#ifdefs, and rely on the compiler to optimize away any dead code.

David Daney

> Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. David Daney <ddaney.cavm@gmail.com> - 2015-08-07 02:40 +0200
  Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Tomasz Nowicki <tomasz.nowicki@linaro.org> - 2015-08-07 10:10 +0200
    Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Robert Richter <robert.richter@caviumnetworks.com> - 2015-08-07 12:50 +0200
      Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Tomasz Nowicki <tomasz.nowicki@linaro.org> - 2015-08-07 13:10 +0200
        Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Tomasz Nowicki <tomasz.nowicki@linaro.org> - 2015-08-07 14:50 +0200
          Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. David Daney <ddaney@caviumnetworks.com> - 2015-08-07 18:50 +0200
      Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Arnd Bergmann <arnd@arndb.de> - 2015-08-08 13:30 +0200
  Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Mark Rutland <mark.rutland@arm.com> - 2015-08-07 16:10 +0200
    Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Mark Rutland <mark.rutland@arm.com> - 2015-08-07 20:00 +0200
      Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. "Rafael J. Wysocki" <rafael@kernel.org> - 2015-08-08 02:10 +0200
        Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. "Rafael J. Wysocki" <rafael@kernel.org> - 2015-08-08 02:30 +0200
    Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Mark Rutland <mark.rutland@arm.com> - 2015-08-07 20:00 +0200
  Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. Graeme Gregory <gg@slimlogic.co.uk> - 2015-08-07 17:00 +0200
    Re: [PATCH 2/2] net, thunder, bgx: Add support for ACPI binding. "Rafael J. Wysocki" <rafael@kernel.org> - 2015-08-08 02:40 +0200

csiph-web