Re: [PATCH v2 3/7] pwm: rockchip: Remove the dumplicate rockchip_pwm_ops ops
From: Boris Brezillon
Date: Wed Aug 02 2017 - 04:59:10 EST
On Sat, 8 Jul 2017 12:03:45 +0800
David Wu <david.wu@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
> 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.