| Message ID | 1347343464-15417-1-git-send-email-tushar.behera@linaro.org |
|---|---|
| State | Accepted |
| Commit | 0bcf168b024871c64eb5df157739efd2ae9b0bdf |
| Headers | show |
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
On 09/11/2012 11:49 AM, Thierry Reding wrote: > 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. > Sure, I will extend the remaining APIs and resubmit. > 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. > Ok. > Thierry >
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 #endif /* __LINUX_PWM_H */
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 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(-)