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