Re: [PATCH v2 3/7] pwm: rockchip: Remove the dumplicate rockchip_pwm_ops ops

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Sat,  8 Jul 2017 12:03:45 +0800
David Wu <david.wu@rock-chips.com> wrote:

> The rockchip_pwm_ops_v1 and rockchip_pwm_ops_v2 ops are the same
> struct members, remove one of them.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> ---
>  drivers/pwm/pwm-rockchip.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
> index cd45f17..85f9515 100644
> --- a/drivers/pwm/pwm-rockchip.c
> +++ b/drivers/pwm/pwm-rockchip.c
> @@ -255,13 +255,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	return ret;
>  }
>  
> -static const struct pwm_ops rockchip_pwm_ops_v1 = {
> -	.get_state = rockchip_pwm_get_state,
> -	.apply = rockchip_pwm_apply,
> -	.owner = THIS_MODULE,
> -};
> -
> -static const struct pwm_ops rockchip_pwm_ops_v2 = {
> +static const struct pwm_ops rockchip_pwm_ops = {
>  	.get_state = rockchip_pwm_get_state,
>  	.apply = rockchip_pwm_apply,
>  	.owner = THIS_MODULE,
> @@ -275,7 +269,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  		.ctrl = 0x0c,
>  	},
>  	.prescaler = 2,
> -	.ops = &rockchip_pwm_ops_v1,
> +	.ops = &rockchip_pwm_ops,
>  	.set_enable = rockchip_pwm_set_enable_v1,
>  	.get_state = rockchip_pwm_get_state_v1,
>  };
> @@ -289,7 +283,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	},
>  	.prescaler = 1,
>  	.supports_polarity = true,
> -	.ops = &rockchip_pwm_ops_v2,
> +	.ops = &rockchip_pwm_ops,
>  	.set_enable = rockchip_pwm_set_enable_v2,
>  	.get_state = rockchip_pwm_get_state_v2,
>  };
> @@ -303,7 +297,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	},
>  	.prescaler = 1,
>  	.supports_polarity = true,
> -	.ops = &rockchip_pwm_ops_v2,
> +	.ops = &rockchip_pwm_ops,
>  	.set_enable = rockchip_pwm_set_enable_v2,
>  	.get_state = rockchip_pwm_get_state_v2,
>  };

Actually, when I suggested to just implement ->apply_state() and be
done with all other fields I was thinking that you could get rid of
this rockchip_pwm_data struct entirely and just have 3 different
pwm_ops. You seem to take the other direction here: you're removing
rockchip_pwm_ops_v1 and renaming rockchip_pwm_ops_v2 into
rockchip_pwm_ops.