linux-kernel - Re: [PATCH v2 3/7] pwm: rockchip: Remove the dumplicate rockchip_pwm

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]

Date:   Wed, 2 Aug 2017 10:59:02 +0200
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     David Wu <david.wu@...k-chips.com>
Cc:     thierry.reding@...il.com, heiko@...ech.de, robh+dt@...nel.org,
        catalin.marinas@....com, briannorris@...omium.org,
        dianders@...omium.org, mark.rutland@....com,
        huangtao@...k-chips.com, linux-pwm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/7] pwm: rockchip: Remove the dumplicate
 rockchip_pwm_ops ops

On Sat,  8 Jul 2017 12:03:45 +0800
David Wu <david.wu@...k-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@...k-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.