Nouveau - Aug 2016 - [PATCH v2] drm/nouveau: add a LED driver for the NVIDIA logo

On 23 August 2016 at 00:42, Martin Peres <martin.peres at free.fr>
wrote:
> v2:
>  - guard LED framework calls with ifdef CONFIG_LEDS_CLASS
>
IIRC kernel has the tendency of using static inlines in the headers
when CONFIG_foo is not set. Worth using that and removing the ifdef
from the source file ?

-Emil

On 23/08/16 17:43, Emil Velikov wrote:
> On 23 August 2016 at 00:42, Martin Peres <martin.peres at free.fr>
wrote:
>> v2:
>>   - guard LED framework calls with ifdef CONFIG_LEDS_CLASS
>>
> IIRC kernel has the tendency of using static inlines in the headers
> when CONFIG_foo is not set. Worth using that and removing the ifdef
> from the source file ?
Oh, you mean, re-defining the functions I use but make them do nothing.

However, I should do it in the source file and not in the header since I do
not want to export these symbols outside of the object.

Do you have an example to share? Anyway, this seems like a good
candidate to reduce the number of ifdefs. Thanks!

Martin

On Wed, Aug 24, 2016 at 03:11:12PM +0300, Martin Peres
wrote:
> On 23/08/16 17:43, Emil Velikov wrote:
> > On 23 August 2016 at 00:42, Martin Peres <martin.peres at
free.fr> wrote:
> > > v2:
> > >   - guard LED framework calls with ifdef CONFIG_LEDS_CLASS
> > > 
> > IIRC kernel has the tendency of using static inlines in the headers
> > when CONFIG_foo is not set. Worth using that and removing the ifdef
> > from the source file ?
> 
> Oh, you mean, re-defining the functions I use but make them do nothing.
> 
> However, I should do it in the source file and not in the header since I do
> not want to export these symbols outside of the object.
> 
> Do you have an example to share? Anyway, this seems like a good
> candidate to reduce the number of ifdefs. Thanks!
See Documentation/CodingStyle, "Chapter 20: Conditional Compilation".
Plenty examples are in include/linux/.

Best regards,

Lukas

https://www.kernel.org/doc/Documentation/CodingStyle

"Chapter 20: Conditional Compilation" ;)

2016-08-24 14:11 GMT+02:00 Martin Peres <martin.peres at
free.fr>:
> On 23/08/16 17:43, Emil Velikov wrote:
>>
>> On 23 August 2016 at 00:42, Martin Peres <martin.peres at
free.fr> wrote:
>>>
>>> v2:
>>>   - guard LED framework calls with ifdef CONFIG_LEDS_CLASS
>>>
>> IIRC kernel has the tendency of using static inlines in the headers
>> when CONFIG_foo is not set. Worth using that and removing the ifdef
>> from the source file ?
>
>
> Oh, you mean, re-defining the functions I use but make them do nothing.
>
> However, I should do it in the source file and not in the header since I do
> not want to export these symbols outside of the object.
>
> Do you have an example to share? Anyway, this seems like a good
> candidate to reduce the number of ifdefs. Thanks!
>
> Martin
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

On 24 August 2016 at 13:11, Martin Peres <martin.peres at free.fr>
wrote:
> On 23/08/16 17:43, Emil Velikov wrote:
>>
>> On 23 August 2016 at 00:42, Martin Peres <martin.peres at
free.fr> wrote:
>>>
>>> v2:
>>>   - guard LED framework calls with ifdef CONFIG_LEDS_CLASS
>>>
>> IIRC kernel has the tendency of using static inlines in the headers
>> when CONFIG_foo is not set. Worth using that and removing the ifdef
>> from the source file ?
>
>
> Oh, you mean, re-defining the functions I use but make them do nothing.
>
> However, I should do it in the source file and not in the header since I do
> not want to export these symbols outside of the object.
>
Afaics they're used in nouveau_drm.c outside the object, which is why
I suggested it.
> Do you have an example to share? Anyway, this seems like a good
> candidate to reduce the number of ifdefs. Thanks!
>
Others have provided with a link, but here is an inline example:

cat nouveau_led.h
...

#ifdef CONFIG_LEDS_CLASS
int  nouveau_led_init(struct drm_device *dev);
void nouveau_led_suspend(struct drm_device *dev);
void nouveau_led_resume(struct drm_device *dev);
void nouveau_led_fini(struct drm_device *dev);
#else
static inline int  nouveau_led_init(struct drm_device *dev) { return 0; };
static inline ...
...
#endif /* CONFIG_LEDS_CLASS */


-Emil