Skip to contentSkip to conversation

ep93xx: clock.c: fix all checkpatch.pl issues

PATCH New H Hartley Sweeten 2 participants 4 messages Aug 9, 2011

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm/Cirrus logic EP93XX Arm architecture
lore
Share & tools
lore

H Hartley Sweeten

author
#0 lore raw

PATCH 1/1 ep93xx: clock.c: fix all checkpatch.pl issues

This fixes all the checkpatch.pl errors and warnings found in this file.

#201: ERROR: space required after that ',' (ctx:VxV) #201: ERROR: space required after that ',' (ctx:VxV) #250: WARNING: line over 80 characters #281: WARNING: line over 80 characters #361: WARNING: line over 80 characters #435: ERROR: trailing whitespace
#438: ERROR: trailing whitespace
#449: ERROR: trailing whitespace
#451: ERROR: trailing whitespace

total: 6 errors, 3 warnings, 562 lines checked

Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>

---

arch/arm/mach-ep93xx/clock.c +15 -16
index ca4de71..14dba95 100644
@@ -198,7 +198,7 @@ static struct clk clk_m2m1 = {
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
 };
 
-#define INIT_CK(dev,con,ck)					\
+#define INIT_CK(dev, con, ck)					\
 	{ .dev_id = dev, .con_id = con, .clk = ck }
 
 static struct clk_lookup clocks[] = {

@@ -247,7 +247,8 @@ static void __clk_enable(struct clk *clk)
 			v = __raw_readl(clk->enable_reg);
 			v |= clk->enable_mask;
 			if (clk->sw_locked)
-				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+				ep93xx_syscon_swlocked_write(v,
+						clk->enable_reg);
 			else
 				__raw_writel(v, clk->enable_reg);
 		}

@@ -278,7 +279,8 @@ static void __clk_disable(struct clk *clk)
 			v = __raw_readl(clk->enable_reg);
 			v &= ~clk->enable_mask;
 			if (clk->sw_locked)
-				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+				ep93xx_syscon_swlocked_write(v,
+						clk->enable_reg);
 			else
 				__raw_writel(v, clk->enable_reg);
 		}

@@ -358,7 +360,7 @@ static int calc_clk_div(struct clk *clk, unsigned long rate,
 	int i, found = 0, __div = 0, __pdiv = 0;
 
 	/* Don't exceed the maximum rate */
-	max_rate = max3(clk_pll1.rate / 4, clk_pll2.rate / 4, clk_xtali.rate / 4);
+	max_rate = max3(clk_pll1.rate/4, clk_pll2.rate/4, clk_xtali.rate/4);
 	rate = min(rate, max_rate);
 
 	/*

@@ -432,35 +434,32 @@ static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate)
 	unsigned val = __raw_readl(clk->enable_reg);
 
 	if (rate == clk_i2s_mclk.rate / 2)
-		ep93xx_syscon_swlocked_write(val & ~EP93XX_I2SCLKDIV_SDIV, 
-					     clk->enable_reg);
+		val &= ~EP93XX_I2SCLKDIV_SDIV;
 	else if (rate == clk_i2s_mclk.rate / 4)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_SDIV, 
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_SDIV;
 	else
 		return -EINVAL;
 
+	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
 	clk_i2s_sclk.rate = rate;
 	return 0;
 }
 
 static int set_i2s_lrclk_rate(struct clk *clk, unsigned long rate)
 {
-	unsigned val = __raw_readl(clk->enable_reg) & 
+	unsigned val = __raw_readl(clk->enable_reg) &
 		~EP93XX_I2SCLKDIV_LRDIV_MASK;
-	
+
 	if (rate == clk_i2s_sclk.rate / 32)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV32,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV32;
 	else if (rate == clk_i2s_sclk.rate / 64)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV64,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV64;
 	else if (rate == clk_i2s_sclk.rate / 128)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV128,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV128;
 	else
 		return -EINVAL;
 
+	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
 	clk_i2s_lrclk.rate = rate;
 	return 0;
 }
#1 lore raw

PATCH 1/1 ep93xx: clock.c: fix all checkpatch.pl issues

Quoted 24 lines · H Hartley Sweeten

This fixes all the checkpatch.pl errors and warnings found in this file.

#201: ERROR: space required after that ',' (ctx:VxV) #201: ERROR: space required after that ',' (ctx:VxV) #250: WARNING: line over 80 characters #281: WARNING: line over 80 characters #361: WARNING: line over 80 characters #435: ERROR: trailing whitespace
#438: ERROR: trailing whitespace
#449: ERROR: trailing whitespace
#451: ERROR: trailing whitespace

total: 6 errors, 3 warnings, 562 lines checked

Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>

---

arch/arm/mach-ep93xx/clock.c +0 -0
index ca4de71..14dba95 100644

arch/arm/mach-ep93xx/clock.c static struct clk clk_m2m1 = { · line 202

Quoted 6 lines
arch/arm/mach-ep93xx/clock.c +1 -1
@@ -198,7 +198,7 @@ static struct clk clk_m2m1 = {
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
 };
 
-#define INIT_CK(dev,con,ck)					\
+#define INIT_CK(dev, con, ck)					\
 	{ .dev_id = dev, .con_id = con, .clk = ck }

Okay.

arch/arm/mach-ep93xx/clock.c static void __clk_enable(struct clk *clk) · line 251

Quoted 9 lines
arch/arm/mach-ep93xx/clock.c +2 -1
 static struct clk_lookup clocks[] = {

@@ -247,7 +247,8 @@ static void __clk_enable(struct clk *clk)
 			v = __raw_readl(clk->enable_reg);
 			v |= clk->enable_mask;
 			if (clk->sw_locked)
-				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+				ep93xx_syscon_swlocked_write(v,
+

I think this makes the code arguably less readable. The line only just extends over 80 characters.

arch/arm/mach-ep93xx/clock.c static void __clk_disable(struct clk *clk) · line 283

Quoted 11 lines
arch/arm/mach-ep93xx/clock.c +0 -0
 						clk->enable_reg);
 			else
 				__raw_writel(v, clk->enable_reg);
arch/arm/mach-ep93xx/clock.c +2 -1
 		}

@@ -278,7 +279,8 @@ static void __clk_disable(struct clk *clk)
 			v = __raw_readl(clk->enable_reg);
 			v &= ~clk->enable_mask;
 			if (clk->sw_locked)
-				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+				ep93xx_syscon_swlocked_write(v,
+						clk->enable_reg);

Same here.

arch/arm/mach-ep93xx/clock.c static int calc_clk_div(struct clk *clk, unsigned long rate, · line 363

Quoted 9 lines
arch/arm/mach-ep93xx/clock.c +1 -1
 			else
 				__raw_writel(v, clk->enable_reg);
 		}

@@ -358,7 +360,7 @@ static int calc_clk_div(struct clk *clk, unsigned long rate,
 	int i, found = 0, __div = 0, __pdiv = 0;
 
 	/* Don't exceed the maximum rate */
-	max_rate = max3(clk_pll1.rate / 4, clk_pll2.rate / 4, clk_xtali.rate / 4);
+	max_rate = max3(clk_pll1.rate/4, clk_pll2.rate/4, clk_xtali.rate/4);

Don't delete the spaces around the operators. Again, this line is only just over 80 characters. I think its okay as is. If you must fix it, break on one of the commas instead.

Quoted 10 lines
arch/arm/mach-ep93xx/clock.c +1 -2
 	rate = min(rate, max_rate);
 
 	/*

@@ -432,35 +434,32 @@ static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate)
 	unsigned val = __raw_readl(clk->enable_reg);
 
 	if (rate == clk_i2s_mclk.rate / 2)
-		ep93xx_syscon_swlocked_write(val & ~EP93XX_I2SCLKDIV_SDIV, 
-					     clk->enable_reg);
+		val &= ~EP93XX_I2SCLKDIV_SDIV;

arch/arm/mach-ep93xx/clock.c static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate) · line 445

Quoted 10 lines
arch/arm/mach-ep93xx/clock.c +2 -2
 	else if (rate == clk_i2s_mclk.rate / 4)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_SDIV, 
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_SDIV;
 	else
 		return -EINVAL;
 
+	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
 	clk_i2s_sclk.rate = rate;
 	return 0;

This change is good.

Quoted 18 lines
arch/arm/mach-ep93xx/clock.c +4 -6
 }
 
 static int set_i2s_lrclk_rate(struct clk *clk, unsigned long rate)
 {
-	unsigned val = __raw_readl(clk->enable_reg) & 
+	unsigned val = __raw_readl(clk->enable_reg) &
 		~EP93XX_I2SCLKDIV_LRDIV_MASK;
-	
+
 	if (rate == clk_i2s_sclk.rate / 32)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV32,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV32;
 	else if (rate == clk_i2s_sclk.rate / 64)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV64,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV64;
 	else if (rate == clk_i2s_sclk.rate / 128)

arch/arm/mach-ep93xx/clock.c static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate) · line 465

Quoted 10 lines
arch/arm/mach-ep93xx/clock.c +2 -2
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV128,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV128;
 	else
 		return -EINVAL;
 
+	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
 	clk_i2s_lrclk.rate = rate;
 	return 0;
 }

This is good too.

~Ryan

author
#2 lore raw

PATCH 1/1 ep93xx: clock.c: fix all checkpatch.pl issues

Quoted 28 lines · Ryan Mallon
Quoted 27 lines · H Hartley Sweeten

This fixes all the checkpatch.pl errors and warnings found in this file.

#201: ERROR: space required after that ',' (ctx:VxV) #201: ERROR: space required after that ',' (ctx:VxV) #250: WARNING: line over 80 characters #281: WARNING: line over 80 characters #361: WARNING: line over 80 characters #435: ERROR: trailing whitespace
#438: ERROR: trailing whitespace
#449: ERROR: trailing whitespace
#451: ERROR: trailing whitespace

total: 6 errors, 3 warnings, 562 lines checked

Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>

---

arch/arm/mach-ep93xx/clock.c +0 -0
index ca4de71..14dba95 100644
@@ -198,7 +198,7 @@ static struct clk clk_m2m1 = {
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
 };
Quoted 5 lines

arch/arm/mach-ep93xx/clock.c static struct clk clk_m2m1 = { · line 202

Quoted 3 lines
arch/arm/mach-ep93xx/clock.c +1 -1
-#define INIT_CK(dev,con,ck)					\
+#define INIT_CK(dev, con, ck)					\
 	{ .dev_id = dev, .con_id = con, .clk = ck }

Okay.

OK.

Quoted 13 lines

arch/arm/mach-ep93xx/clock.c static void __clk_enable(struct clk *clk) · line 251

Quoted 10 lines
arch/arm/mach-ep93xx/clock.c +2 -1
 static struct clk_lookup clocks[] = {

@@ -247,7 +247,8 @@ static void __clk_enable(struct clk *clk)
 			v = __raw_readl(clk->enable_reg);
 			v |= clk->enable_mask;
 			if (clk->sw_locked)
-				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+				ep93xx_syscon_swlocked_write(v,
+

I think this makes the code arguably less readable. The line only just extends over 80 characters.

See the comment below.

Quoted 5 lines
Quoted 5 lines
arch/arm/mach-ep93xx/clock.c +0 -0
 						clk->enable_reg);
 			else
 				__raw_writel(v, clk->enable_reg);
arch/arm/mach-ep93xx/clock.c +0 -0
 		}

@@ -278,7 +279,8 @@ static void __clk_disable(struct clk *clk)
Quoted 8 lines

arch/arm/mach-ep93xx/clock.c static void __clk_disable(struct clk *clk) · line 283

Quoted 6 lines
arch/arm/mach-ep93xx/clock.c +2 -1
 			v = __raw_readl(clk->enable_reg);
 			v &= ~clk->enable_mask;
 			if (clk->sw_locked)
-				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+				ep93xx_syscon_swlocked_write(v,
+						clk->enable_reg);

Same here.

The other way to fix both of these is to add a return on the clk->users check preceding the set/clear of the enable register.

Currently they are coded like this:

static void __clk_enable(struct clk *clk)

{
	if (!clk->users++) {
		...
	}
}

static void __clk_disable(struct clk *clk)

{
	if (!--clk->users) {
		...
	}
}

Both the over 80 character lines could be brought back a full tab by doing:

static void __clk_enable(struct clk *clk)

{
	if (clk->users++)
		return;

	...
}

static void __clk_disable(struct clk *clk)

{
	if (--clk->users)
		return;

	...
}

Are you ok with that change or should I just leave the over 80 char lines?

Quoted 6 lines
Quoted 6 lines
arch/arm/mach-ep93xx/clock.c +0 -0
 			else
 				__raw_writel(v, clk->enable_reg);
 		}

@@ -358,7 +360,7 @@ static int calc_clk_div(struct clk *clk, unsigned long rate,
 	int i, found = 0, __div = 0, __pdiv = 0;
Quoted 7 lines

arch/arm/mach-ep93xx/clock.c static int calc_clk_div(struct clk *clk, unsigned long rate, · line 363

Quoted 3 lines
arch/arm/mach-ep93xx/clock.c +1 -1
 	/* Don't exceed the maximum rate */
-	max_rate = max3(clk_pll1.rate / 4, clk_pll2.rate / 4, clk_xtali.rate / 4);
+	max_rate = max3(clk_pll1.rate/4, clk_pll2.rate/4, clk_xtali.rate/4);

Don't delete the spaces around the operators. Again, this line is only just over 80 characters. I think its okay as is. If you must fix it, break on one of the commas instead.

I'll break the line at a comma. I'm really trying to get ep93xx checkpatch clean.

Quoted 17 lines
Quoted 17 lines
arch/arm/mach-ep93xx/clock.c +2 -4
 	rate = min(rate, max_rate);
 
 	/*

@@ -432,35 +434,32 @@ static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate)
 	unsigned val = __raw_readl(clk->enable_reg);
 
 	if (rate == clk_i2s_mclk.rate / 2)
-		ep93xx_syscon_swlocked_write(val & ~EP93XX_I2SCLKDIV_SDIV, 
-					     clk->enable_reg);
+		val &= ~EP93XX_I2SCLKDIV_SDIV;
 	else if (rate == clk_i2s_mclk.rate / 4)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_SDIV, 
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_SDIV;
 	else
 		return -EINVAL;
Quoted 5 lines

arch/arm/mach-ep93xx/clock.c static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate) · line 445

Quoted 3 lines
arch/arm/mach-ep93xx/clock.c +1 -0
+	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
 	clk_i2s_sclk.rate = rate;
 	return 0;

This change is good.

OK.

Quoted 24 lines
Quoted 24 lines
arch/arm/mach-ep93xx/clock.c +5 -8
 }
 
 static int set_i2s_lrclk_rate(struct clk *clk, unsigned long rate)
 {
-	unsigned val = __raw_readl(clk->enable_reg) & 
+	unsigned val = __raw_readl(clk->enable_reg) &
 		~EP93XX_I2SCLKDIV_LRDIV_MASK;
-	
+
 	if (rate == clk_i2s_sclk.rate / 32)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV32,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV32;
 	else if (rate == clk_i2s_sclk.rate / 64)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV64,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV64;
 	else if (rate == clk_i2s_sclk.rate / 128)
-		ep93xx_syscon_swlocked_write(val | EP93XX_I2SCLKDIV_LRDIV128,
-					     clk->enable_reg);
+		val |= EP93XX_I2SCLKDIV_LRDIV128;
 	else
 		return -EINVAL;
Quoted 5 lines

arch/arm/mach-ep93xx/clock.c static int set_i2s_sclk_rate(struct clk *clk, unsigned long rate) · line 465

Quoted 4 lines
arch/arm/mach-ep93xx/clock.c +1 -0
+	ep93xx_syscon_swlocked_write(val, clk->enable_reg);
 	clk_i2s_lrclk.rate = rate;
 	return 0;
 }

This is good too.

OK.

Thanks,
Hartley

author
#3 lore raw

PATCH 1/1 ep93xx: clock.c: fix all checkpatch.pl issues

In reply to Ryan Mallon
Quoted 2 lines · Ryan Mallon
Quoted 1 line · H Hartley Sweeten

This fixes all the checkpatch.pl errors and warnings found in this file.

[snip]

Quoted 10 lines

arch/arm/mach-ep93xx/clock.c static int calc_clk_div(struct clk *clk, unsigned long rate, · line 363

Quoted 6 lines
arch/arm/mach-ep93xx/clock.c +1 -1
@@ -358,7 +360,7 @@ static int calc_clk_div(struct clk *clk, unsigned long rate,
 	int i, found = 0, __div = 0, __pdiv = 0;
 
 	/* Don't exceed the maximum rate */
-	max_rate = max3(clk_pll1.rate / 4, clk_pll2.rate / 4, clk_xtali.rate / 4);
+	max_rate = max3(clk_pll1.rate/4, clk_pll2.rate/4, clk_xtali.rate/4);

Don't delete the spaces around the operators. Again, this line is only just over 80 characters. I think its okay as is. If you must fix it, break on one of the commas instead.

Ryan,

How about fixing the one above like this:

- max_rate = max3(clk_pll1.rate / 4, clk_pll2.rate / 4, clk_xtali.rate / 4);
+ max_rate = max3(clk_pll1.rate, clk_pll2.rate, clk_xtali.rate);
+ max_rate /= 4;

Regards,
Hartley