Re: [PATCHv3 1/3] pwm: make the PWM_POLARITY flag in DTB optional

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Mar 28, 2014 at 09:48:58AM +0100, Lothar Waßmann wrote:
> Change the pwm chip driver registration, so that a chip driver that
> supports polarity inversion can still be used with DTBs that don't
> provide the 'PWM_POLARITY' flag.
> 
> This is done to provide polarity inversion support for the pwm-imx
> driver without having to modify all existing DTS files.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  drivers/pwm/core.c |   46 ++++++++++++++++------------------------------
>  1 file changed, 16 insertions(+), 30 deletions(-)
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index a804713..dc15543 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -131,12 +131,12 @@ static int pwm_device_request(struct pwm_device *pwm, const char *label)
>  	return 0;
>  }
>  
> -struct pwm_device *
> -of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
> +struct pwm_device *of_pwm_xlate(struct pwm_chip *pc,
> +				const struct of_phandle_args *args)
>  {
>  	struct pwm_device *pwm;
>  
> -	if (pc->of_pwm_n_cells < 3)
> +	if (pc->of_pwm_n_cells < 2)
>  		return ERR_PTR(-EINVAL);
>  
>  	if (args->args[0] >= pc->npwm)
> @@ -148,6 +148,9 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
>  
>  	pwm_set_period(pwm, args->args[1]);
>  
> +	if (pc->of_pwm_n_cells < 3)
> +		return pwm;
> +
>  	if (args->args[2] & PWM_POLARITY_INVERTED)
>  		pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
>  	else
> @@ -155,27 +158,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
>  
>  	return pwm;
>  }
> -EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags);
>  
> -static struct pwm_device *
> -of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> +struct pwm_device *
> +of_pwm_xlate_with_flags(struct pwm_chip *pc,
> +			const struct of_phandle_args *args)
>  {
> -	struct pwm_device *pwm;
> -
> -	if (pc->of_pwm_n_cells < 2)
> -		return ERR_PTR(-EINVAL);
> -
> -	if (args->args[0] >= pc->npwm)
> -		return ERR_PTR(-EINVAL);
> -
> -	pwm = pwm_request_from_chip(pc, args->args[0], NULL);
> -	if (IS_ERR(pwm))
> -		return pwm;
> -
> -	pwm_set_period(pwm, args->args[1]);
> -
> -	return pwm;
> +	return of_pwm_xlate(pc, args);
>  }
> +EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags);
>  
>  static void of_pwmchip_add(struct pwm_chip *chip)
>  {
> @@ -183,8 +173,11 @@ static void of_pwmchip_add(struct pwm_chip *chip)
>  		return;
>  
>  	if (!chip->of_xlate) {
> -		chip->of_xlate = of_pwm_simple_xlate;
> -		chip->of_pwm_n_cells = 2;
> +		chip->of_xlate = of_pwm_xlate;
> +		if (chip->ops->set_polarity)
> +			chip->of_pwm_n_cells = 3;
> +		else
> +			chip->of_pwm_n_cells = 2;

I think the presence of the set_polarity callback shouldn't influence
the number of cells the parser expects. As commented on 2/2 this doesn't
actually mean the device actually support polarity inversion. Also,
polarity inversion could still be done in software for hardware that
doesn't support it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |