linux-kernel - Re: [PATCH v2] pwm: Fix compilation error when CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120911144814.GB3824@avionic-0098.mockup.avionic-design.de>
Date:	Tue, 11 Sep 2012 16:48:14 +0200
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Tushar Behera <tushar.behera@...aro.org>
Cc:	linux-kernel@...r.kernel.org, sachin.kamat@...aro.org,
	patches@...aro.org
Subject: Re: [PATCH v2] pwm: Fix compilation error when CONFIG_PWM is not
 defined

On Tue, Sep 11, 2012 at 03:14:15PM +0530, Tushar Behera wrote:
> Add dummy implemention of public symbols for compilation-safe inclusion
> of include/linux/pwm.h file when CONFIG_PWM is not defined.
> 
> While at it, also reorganize the file.
> 
> Reported-by: Sachin Kamat <sachin.kamat@...aro.org>
> Signed-off-by: Tushar Behera <tushar.behera@...aro.org>
> ---
> Changes since v1:
> * Incorporated Thierry's suggestions regarding adding dummy function
> implemention for all global functions
> * Reorganized header file to have structure definitions first and then the
> function definitions.
> 
>  include/linux/pwm.h |  135 ++++++++++++++++++++++++++++++++++++--------------
>  1 files changed, 97 insertions(+), 38 deletions(-)
> 
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 21d076c..f1e685b 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -6,32 +6,6 @@
>  struct pwm_device;
>  struct seq_file;
>  
> -/*
> - * pwm_request - request a PWM device
> - */
> -struct pwm_device *pwm_request(int pwm_id, const char *label);
> -
> -/*
> - * pwm_free - free a PWM device
> - */
> -void pwm_free(struct pwm_device *pwm);
> -
> -/*
> - * pwm_config - change a PWM device configuration
> - */
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> -
> -/*
> - * pwm_enable - start a PWM output toggling
> - */
> -int pwm_enable(struct pwm_device *pwm);
> -
> -/*
> - * pwm_disable - stop a PWM output toggling
> - */
> -void pwm_disable(struct pwm_device *pwm);
> -
> -#ifdef CONFIG_PWM
>  struct pwm_chip;
>  
>  enum {
> @@ -113,18 +87,6 @@ struct pwm_chip {
>  	unsigned int		of_pwm_n_cells;
>  };
>  
> -int pwm_set_chip_data(struct pwm_device *pwm, void *data);
> -void *pwm_get_chip_data(struct pwm_device *pwm);
> -
> -int pwmchip_add(struct pwm_chip *chip);
> -int pwmchip_remove(struct pwm_chip *chip);
> -struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
> -					 unsigned int index,
> -					 const char *label);
> -
> -struct pwm_device *pwm_get(struct device *dev, const char *consumer);
> -void pwm_put(struct pwm_device *pwm);
> -
>  struct pwm_lookup {
>  	struct list_head list;
>  	const char *provider;
> @@ -141,8 +103,105 @@ struct pwm_lookup {
>  		.con_id = _con_id,			\
>  	}
>  
> +#ifdef CONFIG_PWM
> +/*
> + * pwm_request - request a PWM device
> + */
> +struct pwm_device *pwm_request(int pwm_id, const char *label);
> +
> +/*
> + * pwm_free - free a PWM device
> + */
> +void pwm_free(struct pwm_device *pwm);
> +
> +/*
> + * pwm_config - change a PWM device configuration
> + */
> +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> +
> +/*
> + * pwm_enable - start a PWM output toggling
> + */
> +int pwm_enable(struct pwm_device *pwm);
> +
> +/*
> + * pwm_disable - stop a PWM output toggling
> + */
> +void pwm_disable(struct pwm_device *pwm);

The legacy functions probably need to be declared unconditionally
because they are also available if HAVE_PWM is defined. Or rather than
unconditionally they should probably be protected by something like:

#if IS_ENABLED(CONFIG_PWM) || IS_ENABLED(CONFIG_HAVE_PWM)
  ...
#else
  dummies go here
#endif

In that case it may be worth splitting this into two #if blocks, one for
the legacy API and one for the new stuff, maybe even keeping the file
layout to reduce the patch size.

Alternatively we could postpone this patch a bit until HAVE_PWM can be
removed. I've posted patches that convert all remaining legacy
implementations and except for Unicore32 it looks like we may be able to
get all of them into 3.7.

In the meantime you could solve the problem on your end, as I mentioned,
by selecting PWM from the board's Kconfig. If enough people think this
needs to be done now I may just be persuaded to accept a patch like this
and remove the extra check for HAVE_PWM along with HAVE_PWM when that
happens.

Thierry

Content of type "application/pgp-signature" skipped