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]
Message-ID: <069c0483-d536-7e66-659f-c6816fc65453@starfivetech.com>
Date:   Sat, 18 Mar 2023 12:19:57 +0800
From:   Hal Feng <hal.feng@...rfivetech.com>
To:     Tommaso Merciai <tomm.merciai@...il.com>
CC:     <linux-clk@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-riscv@...ts.infradead.org>, Stephen Boyd <sboyd@...nel.org>,
        "Michael Turquette" <mturquette@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor@...nel.org>,
        "Palmer Dabbelt" <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Ben Dooks <ben.dooks@...ive.com>,
        "Daniel Lezcano" <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>,
        Emil Renner Berthing <emil.renner.berthing@...onical.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 04/21] clk: starfive: Rename "jh7100" to "jh71x0" for
 the common code

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@...il.dk>
>> 
>> Rename some variables from "jh7100" or "JH7100" to "jh71x0"
>> or "JH71X0".
>> 
>> Tested-by: Tommaso Merciai <tomm.merciai@...il.com>
>> Reviewed-by: Conor Dooley <conor.dooley@...rochip.com>
>> Signed-off-by: Emil Renner Berthing <kernel@...il.dk>
>> Signed-off-by: Hal Feng <hal.feng@...rfivetech.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
>> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ