Re: [PATCH] pwm: Fix compilation error when CONFIG_PWM is not defined

[Thierry Reding <thierry.reding@avionic-design.de>]

[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

On Tue, Sep 11, 2012 at 11:34:24AM +0530, Tushar Behera wrote:
> As per Documentation/pwm.txt, PWM_LOOKUP and pwm_add_table are used in
> board support files to add PWM chip entries. Currently these
> definitions are protected within CONFIG_PWM macro in
> include/linux/pwm.h.
> 
> Otherwise, we have to add ifdef's in machine file to fix following
> compilation error.
> 
> error: array type has incomplete element type
> error: implicit declaration of function ‘PWM_LOOKUP’ [-Werror=implicit-function-declaration]
> error: implicit declaration of function ‘pwm_add_table’ [-Werror=implicit-function-declaration]
> error: bit-field ‘<anonymous>’ width not an integer constant

I think it would make more sense to have the board's Kconfig option
select PWM instead. After all if you're defining a lookup table you
probably want to use it as well.

Eventually I was going to rework the pwm.h a bit to safely compile
out if the PWM symbol is not selected, similar to how the GPIO or
clock subsystems do this. Something like your patch will be part of
that but more needs to be done. If you feel up to it, maybe you can
take this patch further and provide dummies for all the remaining
functions as well.

If you do that, one additional comment below.

> Reported-by: Sachin Kamat <sachin.kamat@linaro.org>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  include/linux/pwm.h |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 21d076c..87e7f45 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -124,6 +124,7 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
>  
>  struct pwm_device *pwm_get(struct device *dev, const char *consumer);
>  void pwm_put(struct pwm_device *pwm);
> +#endif
>  
>  struct pwm_lookup {
>  	struct list_head list;
> @@ -141,8 +142,10 @@ struct pwm_lookup {
>  		.con_id = _con_id,			\
>  	}
>  
> +#ifdef CONFIG_PWM
>  void pwm_add_table(struct pwm_lookup *table, size_t num);
> -
> +#else
> +static void pwm_add_table(struct pwm_lookup *table, size_t num) {}
>  #endif

This should be "static inline" and I prefer to have the {} on separate
lines.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]