| Message ID | 20220209064755.7287-1-zhaoxiao@uniontech.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pwm: vt8500: Rename variable pointing to driver private data | expand |
On Wed, Feb 09, 2022 at 02:47:55PM +0800, zhaoxiao wrote: > Status quo is that variables of type struct vt8500_chip * are named > "pwm", "chip" or "pc". The two formers are all not optimal because There are no variables named "pwm" or "pc". > usually only struct pwm_device * variables are named "pwm" and "chip" is > usually used for variabled of type struct pwm_chip *. > > So consistently use the same and non-conflicting name "pc". The intention is fine, but you missed a few instances that are named "vt8500". The statistic in mainline looks as follows: $ git grep -o -h -E 'struct vt8500_chip \*[a-zA-Z0-9_]*' linus/master drivers/pwm/pwm-vt8500.c | sort | uniq -c 2 struct vt8500_chip *chip 5 struct vt8500_chip *vt8500 (So there 2 variabled named "chip" (that you renamed to "pc") and 5 that are named "vt8500". I prefer to rename the "chip"s to "vt8500". Best regards Uwe
On Fri, 11 Feb 2022, 赵晓 wrote: > Thanks for you suggestion. I modified the patch following your instructions and commit the v2 version. > Would you be kind enough to turn HTML off in your browser please. Also, please refrain from top-posting. Replies should be inline (like this). > ------------------ Original ------------------ > From: "Uwe Kleine-König"<u.kleine-koenig@pengutronix.de>; > Date: Thu, Feb 10, 2022 03:40 PM > To: "zhaoxiao"<zhaoxiao@uniontech.com>; > Cc: "thierry.reding"<thierry.reding@gmail.com>; "lee.jones"<lee.jones@linaro.org>; "linux-arm-kernel"<linux-arm-kernel@lists.infradead.org>; "linux-pwm"<linux-pwm@vger.kernel.org>; "linux-kernel"<linux-kernel@vger.kernel.org>; > Subject: Re: [PATCH] pwm: vt8500: Rename variable pointing to driver private data Please configure your mailer to strip mail headers from the body. > > > On Wed, Feb 09, 2022 at 02:47:55PM +0800, zhaoxiao wrote: > > Status quo is that variables of type struct vt8500_chip * are named > > "pwm", "chip" or "pc". The two formers are all not optimal because > > There are no variables named "pwm" or "pc". > > > usually only struct pwm_device * variables are named "pwm" and "chip" is > > usually used for variabled of type struct pwm_chip *. > > > > So consistently use the same and non-conflicting name "pc". > > The intention is fine, but you missed a few instances that are named > "vt8500". The statistic in mainline looks as follows: > > $ git grep -o -h -E 'struct vt8500_chip \*[a-zA-Z0-9_]*' linus/master drivers/pwm/pwm-vt8500.c | sort | uniq -c > 2 struct vt8500_chip *chip > 5 struct vt8500_chip *vt8500 > > (So there 2 variabled named "chip" (that you renamed to "pc") and 5 that > are named "vt8500". I prefer to rename the "chip"s to "vt8500". > > Best regards > Uwe >
diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c index 7170a315535b..fc0a572f7d06 100644 --- a/drivers/pwm/pwm-vt8500.c +++ b/drivers/pwm/pwm-vt8500.c @@ -235,7 +235,7 @@ MODULE_DEVICE_TABLE(of, vt8500_pwm_dt_ids); static int vt8500_pwm_probe(struct platform_device *pdev) { - struct vt8500_chip *chip; + struct vt8500_chip *pc; struct device_node *np = pdev->dev.of_node; int ret; @@ -244,48 +244,48 @@ static int vt8500_pwm_probe(struct platform_device *pdev) return -EINVAL; } - chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL); - if (chip == NULL) + pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); + if (pc == NULL) return -ENOMEM; - chip->chip.dev = &pdev->dev; - chip->chip.ops = &vt8500_pwm_ops; - chip->chip.npwm = VT8500_NR_PWMS; + pc->chip.dev = &pdev->dev; + pc->chip.ops = &vt8500_pwm_ops; + pc->chip.npwm = VT8500_NR_PWMS; - chip->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(chip->clk)) { + pc->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(pc->clk)) { dev_err(&pdev->dev, "clock source not specified\n"); - return PTR_ERR(chip->clk); + return PTR_ERR(pc->clk); } - chip->base = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(chip->base)) - return PTR_ERR(chip->base); + pc->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(pc->base)) + return PTR_ERR(pc->base); - ret = clk_prepare(chip->clk); + ret = clk_prepare(pc->clk); if (ret < 0) { dev_err(&pdev->dev, "failed to prepare clock\n"); return ret; } - ret = pwmchip_add(&chip->chip); + ret = pwmchip_add(&pc->chip); if (ret < 0) { dev_err(&pdev->dev, "failed to add PWM chip\n"); - clk_unprepare(chip->clk); + clk_unprepare(pc->clk); return ret; } - platform_set_drvdata(pdev, chip); + platform_set_drvdata(pdev, pc); return ret; } static int vt8500_pwm_remove(struct platform_device *pdev) { - struct vt8500_chip *chip = platform_get_drvdata(pdev); + struct vt8500_chip *pc = platform_get_drvdata(pdev); - pwmchip_remove(&chip->chip); + pwmchip_remove(&pc->chip); - clk_unprepare(chip->clk); + clk_unprepare(pc->clk); return 0; }
Status quo is that variables of type struct vt8500_chip * are named "pwm", "chip" or "pc". The two formers are all not optimal because usually only struct pwm_device * variables are named "pwm" and "chip" is usually used for variabled of type struct pwm_chip *. So consistently use the same and non-conflicting name "pc". Signed-off-by: zhaoxiao <zhaoxiao@uniontech.com> --- drivers/pwm/pwm-vt8500.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)