devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hal Feng <hal.feng@starfivetech.com>
To: Tommaso Merciai <tomm.merciai@gmail.com>
Cc: <linux-clk@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-riscv@lists.infradead.org>,
	Stephen Boyd <sboyd@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Ben Dooks <ben.dooks@sifive.com>,
	"Daniel Lezcano" <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>,
	Emil Renner Berthing <emil.renner.berthing@canonical.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 04/21] clk: starfive: Rename "jh7100" to "jh71x0" for the common code
Date: Sat, 18 Mar 2023 12:19:57 +0800	[thread overview]
Message-ID: <069c0483-d536-7e66-659f-c6816fc65453@starfivetech.com> (raw)
In-Reply-To: <ZBNoaGd9l0HjFv2l@tom-HP-ZBook-Fury-15-G7-Mobile-Workstation>

On Thu, 16 Mar 2023 20:05:12 +0100, Tommaso Merciai wrote:
> Hello Hal,
> Patcht itself looks good to me, btw I have some style issue applying
> this:
> 
> On Sat, Mar 11, 2023 at 05:07:16PM +0800, Hal Feng wrote:
>> From: Emil Renner Berthing <kernel@esmil.dk>
>> 
>> Rename some variables from "jh7100" or "JH7100" to "jh71x0"
>> or "JH71X0".
>> 
>> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com>
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>  .../clk/starfive/clk-starfive-jh7100-audio.c  |  74 ++--
>>  drivers/clk/starfive/clk-starfive-jh7100.c    | 388 +++++++++---------
>>  drivers/clk/starfive/clk-starfive-jh71x0.c    | 284 ++++++-------
>>  drivers/clk/starfive/clk-starfive-jh71x0.h    |  72 ++--
>>  4 files changed, 409 insertions(+), 409 deletions(-)
>> 
[...]
>> diff --git a/drivers/clk/starfive/clk-starfive-jh71x0.h b/drivers/clk/starfive/clk-starfive-jh71x0.h
>> index a8ba6e25b5ce..baf4b5cb4b8a 100644
>> --- a/drivers/clk/starfive/clk-starfive-jh71x0.h
>> +++ b/drivers/clk/starfive/clk-starfive-jh71x0.h
>> @@ -1,6 +1,6 @@
>>  /* SPDX-License-Identifier: GPL-2.0 */
>> -#ifndef __CLK_STARFIVE_JH7100_H
>> -#define __CLK_STARFIVE_JH7100_H
>> +#ifndef __CLK_STARFIVE_JH71X0_H
>> +#define __CLK_STARFIVE_JH71X0_H
>>  
>>  #include <linux/bits.h>
>>  #include <linux/clk-provider.h>
>> @@ -8,107 +8,107 @@
>>  #include <linux/spinlock.h>
>>  
>>  /* register fields */
>> -#define JH7100_CLK_ENABLE	BIT(31)
>> -#define JH7100_CLK_INVERT	BIT(30)
>> -#define JH7100_CLK_MUX_MASK	GENMASK(27, 24)
>> -#define JH7100_CLK_MUX_SHIFT	24
>> -#define JH7100_CLK_DIV_MASK	GENMASK(23, 0)
>> -#define JH7100_CLK_FRAC_MASK	GENMASK(15, 8)
>> -#define JH7100_CLK_FRAC_SHIFT	8
>> -#define JH7100_CLK_INT_MASK	GENMASK(7, 0)
>> +#define JH71X0_CLK_ENABLE	BIT(31)
>> +#define JH71X0_CLK_INVERT	BIT(30)
>> +#define JH71X0_CLK_MUX_MASK	GENMASK(27, 24)
>> +#define JH71X0_CLK_MUX_SHIFT	24
>> +#define JH71X0_CLK_DIV_MASK	GENMASK(23, 0)
>> +#define JH71X0_CLK_FRAC_MASK	GENMASK(15, 8)
>> +#define JH71X0_CLK_FRAC_SHIFT	8
>> +#define JH71X0_CLK_INT_MASK	GENMASK(7, 0)
>>  
>>  /* fractional divider min/max */
>> -#define JH7100_CLK_FRAC_MIN	100UL
>> -#define JH7100_CLK_FRAC_MAX	25599UL
>> +#define JH71X0_CLK_FRAC_MIN	100UL
>> +#define JH71X0_CLK_FRAC_MAX	25599UL
>>  
>>  /* clock data */
>> -struct jh7100_clk_data {
>> +struct jh71x0_clk_data {
>>  	const char *name;
>>  	unsigned long flags;
>>  	u32 max;
>>  	u8 parents[4];
>>  };
>>  
>> -#define JH7100_GATE(_idx, _name, _flags, _parent) [_idx] = {			\
>> +#define JH71X0_GATE(_idx, _name, _flags, _parent) [_idx] = {			\
>>  	.name = _name,								\
>>  	.flags = CLK_SET_RATE_PARENT | (_flags),				\
>> -	.max = JH7100_CLK_ENABLE,						\
>> +	.max = JH71X0_CLK_ENABLE,						\
>>  	.parents = { [0] = _parent },						\
>>  }
> 
> 
> ERROR: space prohibited before open square bracket '['
> #1155: FILE: drivers/clk/starfive/clk-starfive-jh71x0.h:32:
> +#define JH71X0_GATE(_idx, _name, _flags, _parent) [_idx] = {			\
> 
> Same for others define.
> I would suggest this style.
> Hope this can help you:
> 
> #define JH71X0_GATE(_idx, _name, _flags, _parent)		\
> 	[_idx] = {						\
> 		.name = _name,					\
> 		.flags = CLK_SET_RATE_PARENT | (_flags),	\
> 		.max = JH71X0_CLK_ENABLE,			\
> 		.parents = { [0] = _parent },			\
> }
> 
> tested using:
> 
> scripts/checkpatch.pl -f drivers/clk/starfive/clk-starfive-jh71x0.h

I will fix the errors reported by checkpatch.pl in v6. Thanks.

Best regards,
Hal

> 
> 
> Thanks for your work,
> Tommaso
> 
> 
>>  
>> -#define JH7100__DIV(_idx, _name, _max, _parent) [_idx] = {			\
>> +#define JH71X0__DIV(_idx, _name, _max, _parent) [_idx] = {			\
>>  	.name = _name,								\
>>  	.flags = 0,								\
>>  	.max = _max,								\
>>  	.parents = { [0] = _parent },						\
>>  }
>>  
>> -#define JH7100_GDIV(_idx, _name, _flags, _max, _parent) [_idx] = {		\
>> +#define JH71X0_GDIV(_idx, _name, _flags, _max, _parent) [_idx] = {		\
>>  	.name = _name,								\
>>  	.flags = _flags,							\
>> -	.max = JH7100_CLK_ENABLE | (_max),					\
>> +	.max = JH71X0_CLK_ENABLE | (_max),					\
>>  	.parents = { [0] = _parent },						\
>>  }
>>  
>> -#define JH7100_FDIV(_idx, _name, _parent) [_idx] = {				\
>> +#define JH71X0_FDIV(_idx, _name, _parent) [_idx] = {				\
>>  	.name = _name,								\
>>  	.flags = 0,								\
>> -	.max = JH7100_CLK_FRAC_MAX,						\
>> +	.max = JH71X0_CLK_FRAC_MAX,						\
>>  	.parents = { [0] = _parent },						\
>>  }
>>  
>> -#define JH7100__MUX(_idx, _name, _nparents, ...) [_idx] = {			\
>> +#define JH71X0__MUX(_idx, _name, _nparents, ...) [_idx] = {			\
>>  	.name = _name,								\
>>  	.flags = 0,								\
>> -	.max = ((_nparents) - 1) << JH7100_CLK_MUX_SHIFT,			\
>> +	.max = ((_nparents) - 1) << JH71X0_CLK_MUX_SHIFT,			\
>>  	.parents = { __VA_ARGS__ },						\
>>  }
>>  
>> -#define JH7100_GMUX(_idx, _name, _flags, _nparents, ...) [_idx] = {		\
>> +#define JH71X0_GMUX(_idx, _name, _flags, _nparents, ...) [_idx] = {		\
>>  	.name = _name,								\
>>  	.flags = _flags,							\
>> -	.max = JH7100_CLK_ENABLE |						\
>> -		(((_nparents) - 1) << JH7100_CLK_MUX_SHIFT),			\
>> +	.max = JH71X0_CLK_ENABLE |						\
>> +		(((_nparents) - 1) << JH71X0_CLK_MUX_SHIFT),			\
>>  	.parents = { __VA_ARGS__ },						\
>>  }
>>  
>> -#define JH7100_MDIV(_idx, _name, _max, _nparents, ...) [_idx] = {		\
>> +#define JH71X0_MDIV(_idx, _name, _max, _nparents, ...) [_idx] = {		\
>>  	.name = _name,								\
>>  	.flags = 0,								\
>> -	.max = (((_nparents) - 1) << JH7100_CLK_MUX_SHIFT) | (_max),		\
>> +	.max = (((_nparents) - 1) << JH71X0_CLK_MUX_SHIFT) | (_max),		\
>>  	.parents = { __VA_ARGS__ },						\
>>  }
>>  
>> -#define JH7100__GMD(_idx, _name, _flags, _max, _nparents, ...) [_idx] = {	\
>> +#define JH71X0__GMD(_idx, _name, _flags, _max, _nparents, ...) [_idx] = {	\
>>  	.name = _name,								\
>>  	.flags = _flags,							\
>> -	.max = JH7100_CLK_ENABLE |						\
>> -		(((_nparents) - 1) << JH7100_CLK_MUX_SHIFT) | (_max),		\
>> +	.max = JH71X0_CLK_ENABLE |						\
>> +		(((_nparents) - 1) << JH71X0_CLK_MUX_SHIFT) | (_max),		\
>>  	.parents = { __VA_ARGS__ },						\
>>  }
>>  
>> -#define JH7100__INV(_idx, _name, _parent) [_idx] = {				\
>> +#define JH71X0__INV(_idx, _name, _parent) [_idx] = {				\
>>  	.name = _name,								\
>>  	.flags = CLK_SET_RATE_PARENT,						\
>> -	.max = JH7100_CLK_INVERT,						\
>> +	.max = JH71X0_CLK_INVERT,						\
>>  	.parents = { [0] = _parent },						\
>>  }
>>  
>> -struct jh7100_clk {
>> +struct jh71x0_clk {
>>  	struct clk_hw hw;
>>  	unsigned int idx;
>>  	unsigned int max_div;
>>  };
>>  
>> -struct jh7100_clk_priv {
>> +struct jh71x0_clk_priv {
>>  	/* protect clk enable and set rate/parent from happening at the same time */
>>  	spinlock_t rmw_lock;
>>  	struct device *dev;
>>  	void __iomem *base;
>>  	struct clk_hw *pll[3];
>> -	struct jh7100_clk reg[];
>> +	struct jh71x0_clk reg[];
>>  };
>>  
>> -const struct clk_ops *starfive_jh7100_clk_ops(u32 max);
>> +const struct clk_ops *starfive_jh71x0_clk_ops(u32 max);
>>  
>>  #endif
>> -- 
>> 2.38.1
>> 


  reply	other threads:[~2023-03-18  4:20 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-11  9:07 [PATCH v5 00/21] Basic clock, reset & device tree support for StarFive JH7110 RISC-V SoC Hal Feng
2023-03-11  9:07 ` [PATCH v5 01/21] clk: starfive: Replace SOC_STARFIVE with ARCH_STARFIVE Hal Feng
2023-03-11 12:56   ` Conor Dooley
2023-03-11  9:07 ` [PATCH v5 02/21] clk: starfive: Factor out common JH7100 and JH7110 code Hal Feng
2023-03-11  9:07 ` [PATCH v5 03/21] clk: starfive: Rename clk-starfive-jh7100.h to clk-starfive-jh71x0.h Hal Feng
2023-03-12 14:00   ` Conor Dooley
2023-03-13  2:37     ` Hal Feng
2023-03-11  9:07 ` [PATCH v5 04/21] clk: starfive: Rename "jh7100" to "jh71x0" for the common code Hal Feng
2023-03-16 19:05   ` Tommaso Merciai
2023-03-18  4:19     ` Hal Feng [this message]
2023-03-11  9:07 ` [PATCH v5 05/21] reset: starfive: Replace SOC_STARFIVE with ARCH_STARFIVE Hal Feng
2023-03-11 12:56   ` Conor Dooley
2023-03-14 14:34   ` Philipp Zabel
2023-03-20 11:51   ` Emil Renner Berthing
2023-03-11  9:07 ` [PATCH v5 06/21] reset: Create subdirectory for StarFive drivers Hal Feng
2023-03-14 14:34   ` Philipp Zabel
2023-03-17  8:17     ` Hal Feng
2023-03-11  9:07 ` [PATCH v5 07/21] reset: starfive: Factor out common JH71X0 reset code Hal Feng
2023-03-11  9:07 ` [PATCH v5 08/21] reset: starfive: Extract the " Hal Feng
2023-03-11  9:07 ` [PATCH v5 09/21] reset: starfive: Rename "jh7100" to "jh71x0" for the common code Hal Feng
2023-03-11  9:07 ` [PATCH v5 10/21] reset: starfive: jh71x0: Use 32bit I/O on 32bit registers Hal Feng
2023-03-11  9:07 ` [PATCH v5 11/21] dt-bindings: clock: Add StarFive JH7110 system clock and reset generator Hal Feng
2023-03-11 13:11   ` Conor Dooley
2023-03-13  3:22     ` Hal Feng
2023-03-13  8:53       ` Emil Renner Berthing
2023-03-14 14:09         ` Hal Feng
2023-03-11 14:17   ` Rob Herring
2023-03-13  2:47     ` Hal Feng
2023-03-13  7:51       ` Krzysztof Kozlowski
2023-03-14 14:18         ` Hal Feng
2023-03-11  9:07 ` [PATCH v5 12/21] dt-bindings: clock: Add StarFive JH7110 always-on " Hal Feng
2023-03-11 13:14   ` Conor Dooley
2023-03-19 13:28     ` Hal Feng
2023-03-11 14:18   ` Rob Herring
2023-03-13  2:49     ` Hal Feng
2023-03-11  9:07 ` [PATCH v5 13/21] clk: starfive: Add StarFive JH7110 system clock driver Hal Feng
2023-03-11  9:07 ` [PATCH v5 14/21] clk: starfive: Add StarFive JH7110 always-on " Hal Feng
2023-03-11  9:07 ` [PATCH v5 15/21] reset: starfive: Add StarFive JH7110 reset driver Hal Feng
2023-03-11  9:07 ` [PATCH v5 16/21] dt-bindings: timer: Add StarFive JH7110 clint Hal Feng
2023-03-11  9:07 ` [PATCH v5 17/21] dt-bindings: interrupt-controller: Add StarFive JH7110 plic Hal Feng
2023-03-11  9:07 ` [PATCH v5 18/21] dt-bindings: riscv: Add SiFive S7 compatible Hal Feng
2023-03-11  9:07 ` [PATCH v5 19/21] riscv: dts: starfive: Add initial StarFive JH7110 device tree Hal Feng
2023-03-11  9:07 ` [PATCH v5 20/21] riscv: dts: starfive: Add StarFive JH7110 pin function definitions Hal Feng
2023-03-11  9:07 ` [PATCH v5 21/21] riscv: dts: starfive: Add StarFive JH7110 VisionFive 2 board device tree Hal Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=069c0483-d536-7e66-659f-c6816fc65453@starfivetech.com \
    --to=hal.feng@starfivetech.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=ben.dooks@sifive.com \
    --cc=conor@kernel.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=emil.renner.berthing@canonical.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=p.zabel@pengutronix.de \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tomm.merciai@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).