* [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up @ 2011-01-21 15:56 Guennadi Liakhovetski 2011-01-21 17:02 ` Grant Likely 0 siblings, 1 reply; 7+ messages in thread From: Guennadi Liakhovetski @ 2011-01-21 15:56 UTC (permalink / raw) To: spi-devel-general; +Cc: linux-sh 1. sort headers alphabetically 2. use fixed-size types u8, u16, u32 for register values and transferred data 3. simplify some arithmetic operations 4. remove leading spaces in front of labels Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- drivers/spi/spi_sh_msiof.c | 78 ++++++++++++++++++++++---------------------- 1 files changed, 39 insertions(+), 39 deletions(-) diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c index 56f60c8..07b4ead 100644 --- a/drivers/spi/spi_sh_msiof.c +++ b/drivers/spi/spi_sh_msiof.c @@ -9,22 +9,22 @@ * */ -#include <linux/kernel.h> -#include <linux/init.h> +#include <linux/bitmap.h> +#include <linux/clk.h> +#include <linux/completion.h> #include <linux/delay.h> +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/init.h> #include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> #include <linux/platform_device.h> -#include <linux/completion.h> #include <linux/pm_runtime.h> -#include <linux/gpio.h> -#include <linux/bitmap.h> -#include <linux/clk.h> -#include <linux/io.h> -#include <linux/err.h> +#include <linux/spi/sh_msiof.h> #include <linux/spi/spi.h> #include <linux/spi/spi_bitbang.h> -#include <linux/spi/sh_msiof.h> #include <asm/unaligned.h> @@ -67,7 +67,7 @@ struct sh_msiof_spi_priv { #define STR_TEOF (1 << 23) #define STR_REOF (1 << 7) -static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) +static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) { switch (reg_offs) { case TSCR: @@ -79,7 +79,7 @@ static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) } static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, - unsigned long value) + u32 value) { switch (reg_offs) { case TSCR: @@ -93,10 +93,10 @@ static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, } static int sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p, - unsigned long clr, unsigned long set) + u32 clr, u32 set) { - unsigned long mask = clr | set; - unsigned long data; + u32 mask = clr | set; + u32 data; int k; data = sh_msiof_read(p, CTR); @@ -166,10 +166,10 @@ static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p, } static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, - int cpol, int cpha, - int tx_hi_z, int lsb_first) + u32 cpol, u32 cpha, + u32 tx_hi_z, u32 lsb_first) { - unsigned long tmp; + u32 tmp; int edge; /* @@ -187,7 +187,7 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, tmp |= cpol << 30; /* TSCKIZ */ tmp |= cpol << 28; /* RSCKIZ */ - edge = cpol ? cpha : !cpha; + edge = cpol ^ !cpha; tmp |= edge << 27; /* TEDG */ tmp |= edge << 26; /* REDG */ @@ -197,11 +197,9 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p, const void *tx_buf, void *rx_buf, - int bits, int words) + u32 bits, u32 words) { - unsigned long dr2; - - dr2 = ((bits - 1) << 24) | ((words - 1) << 16); + u32 dr2 = ((bits - 1) << 24) | ((words - 1) << 16); if (tx_buf) sh_msiof_write(p, TMDR2, dr2); @@ -222,7 +220,7 @@ static void sh_msiof_reset_str(struct sh_msiof_spi_priv *p) static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, const void *tx_buf, int words, int fs) { - const unsigned char *buf_8 = tx_buf; + const u8 *buf_8 = tx_buf; int k; for (k = 0; k < words; k++) @@ -232,7 +230,7 @@ static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, const void *tx_buf, int words, int fs) { - const unsigned short *buf_16 = tx_buf; + const u16 *buf_16 = tx_buf; int k; for (k = 0; k < words; k++) @@ -242,7 +240,7 @@ static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, const void *tx_buf, int words, int fs) { - const unsigned short *buf_16 = tx_buf; + const u16 *buf_16 = tx_buf; int k; for (k = 0; k < words; k++) @@ -252,7 +250,7 @@ static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, const void *tx_buf, int words, int fs) { - const unsigned int *buf_32 = tx_buf; + const u32 *buf_32 = tx_buf; int k; for (k = 0; k < words; k++) @@ -262,7 +260,7 @@ static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, const void *tx_buf, int words, int fs) { - const unsigned int *buf_32 = tx_buf; + const u32 *buf_32 = tx_buf; int k; for (k = 0; k < words; k++) @@ -272,7 +270,7 @@ static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, void *rx_buf, int words, int fs) { - unsigned char *buf_8 = rx_buf; + u8 *buf_8 = rx_buf; int k; for (k = 0; k < words; k++) @@ -282,7 +280,7 @@ static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, void *rx_buf, int words, int fs) { - unsigned short *buf_16 = rx_buf; + u16 *buf_16 = rx_buf; int k; for (k = 0; k < words; k++) @@ -292,7 +290,7 @@ static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, void *rx_buf, int words, int fs) { - unsigned short *buf_16 = rx_buf; + u16 *buf_16 = rx_buf; int k; for (k = 0; k < words; k++) @@ -302,7 +300,7 @@ static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, void *rx_buf, int words, int fs) { - unsigned int *buf_32 = rx_buf; + u32 *buf_32 = rx_buf; int k; for (k = 0; k < words; k++) @@ -312,7 +310,7 @@ static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, static void sh_msiof_spi_read_fifo_32u(struct sh_msiof_spi_priv *p, void *rx_buf, int words, int fs) { - unsigned int *buf_32 = rx_buf; + u32 *buf_32 = rx_buf; int k; for (k = 0; k < words; k++) @@ -324,7 +322,8 @@ static int sh_msiof_spi_bits(struct spi_device *spi, struct spi_transfer *t) int bits; bits = t ? t->bits_per_word : 0; - bits = bits ? bits : spi->bits_per_word; + if (!bits) + bits = spi->bits_per_word; return bits; } @@ -334,7 +333,8 @@ static unsigned long sh_msiof_spi_hz(struct spi_device *spi, unsigned long hz; hz = t ? t->speed_hz : 0; - hz = hz ? hz : spi->max_speed_hz; + if (!hz) + hz = spi->max_speed_hz; return hz; } @@ -453,7 +453,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p, return words; - err: +err: sh_msiof_write(p, IER, 0); return ret; } @@ -617,13 +617,13 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) return 0; pm_runtime_disable(&pdev->dev); - err3: +err3: iounmap(p->mapbase); - err2: +err2: clk_put(p->clk); - err1: +err1: spi_master_put(master); - err0: +err0: return ret; } -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up 2011-01-21 15:56 [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up Guennadi Liakhovetski @ 2011-01-21 17:02 ` Grant Likely 2011-01-21 17:10 ` Guennadi Liakhovetski 2011-01-25 5:02 ` Simon Horman 0 siblings, 2 replies; 7+ messages in thread From: Grant Likely @ 2011-01-21 17:02 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: spi-devel-general, linux-sh On Fri, Jan 21, 2011 at 04:56:37PM +0100, Guennadi Liakhovetski wrote: > 1. sort headers alphabetically > 2. use fixed-size types u8, u16, u32 for register values and transferred data > 3. simplify some arithmetic operations > 4. remove leading spaces in front of labels There's actually good reason to leave these labels alone. Labels in column zero confuse diff and it gives the wrong function name. For example: diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c index 658bd05..840164b 100644 --- a/drivers/spi/spi_sh_msiof.c +++ b/drivers/spi/spi_sh_msiof.c @@ -682,7 +682,7 @@ err2: clk_put(p->clk); err1: spi_master_put(master); -err0: + err0: return ret; } See how the function name in the @@ line is 'err2' instead of sh_msiof_spi_probe() in this example? Indenting the labels by one space is an easy fix and is intentional. I've picked up the patch, but I've dropped the last two hunks. g. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > drivers/spi/spi_sh_msiof.c | 78 ++++++++++++++++++++++---------------------- > 1 files changed, 39 insertions(+), 39 deletions(-) > > diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c > index 56f60c8..07b4ead 100644 > --- a/drivers/spi/spi_sh_msiof.c > +++ b/drivers/spi/spi_sh_msiof.c > @@ -9,22 +9,22 @@ > * > */ > > -#include <linux/kernel.h> > -#include <linux/init.h> > +#include <linux/bitmap.h> > +#include <linux/clk.h> > +#include <linux/completion.h> > #include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/gpio.h> > +#include <linux/init.h> > #include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > #include <linux/platform_device.h> > -#include <linux/completion.h> > #include <linux/pm_runtime.h> > -#include <linux/gpio.h> > -#include <linux/bitmap.h> > -#include <linux/clk.h> > -#include <linux/io.h> > -#include <linux/err.h> > > +#include <linux/spi/sh_msiof.h> > #include <linux/spi/spi.h> > #include <linux/spi/spi_bitbang.h> > -#include <linux/spi/sh_msiof.h> > > #include <asm/unaligned.h> > > @@ -67,7 +67,7 @@ struct sh_msiof_spi_priv { > #define STR_TEOF (1 << 23) > #define STR_REOF (1 << 7) > > -static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > +static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > { > switch (reg_offs) { > case TSCR: > @@ -79,7 +79,7 @@ static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > } > > static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, > - unsigned long value) > + u32 value) > { > switch (reg_offs) { > case TSCR: > @@ -93,10 +93,10 @@ static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, > } > > static int sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p, > - unsigned long clr, unsigned long set) > + u32 clr, u32 set) > { > - unsigned long mask = clr | set; > - unsigned long data; > + u32 mask = clr | set; > + u32 data; > int k; > > data = sh_msiof_read(p, CTR); > @@ -166,10 +166,10 @@ static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p, > } > > static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > - int cpol, int cpha, > - int tx_hi_z, int lsb_first) > + u32 cpol, u32 cpha, > + u32 tx_hi_z, u32 lsb_first) > { > - unsigned long tmp; > + u32 tmp; > int edge; > > /* > @@ -187,7 +187,7 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > tmp |= cpol << 30; /* TSCKIZ */ > tmp |= cpol << 28; /* RSCKIZ */ > > - edge = cpol ? cpha : !cpha; > + edge = cpol ^ !cpha; > > tmp |= edge << 27; /* TEDG */ > tmp |= edge << 26; /* REDG */ > @@ -197,11 +197,9 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p, > const void *tx_buf, void *rx_buf, > - int bits, int words) > + u32 bits, u32 words) > { > - unsigned long dr2; > - > - dr2 = ((bits - 1) << 24) | ((words - 1) << 16); > + u32 dr2 = ((bits - 1) << 24) | ((words - 1) << 16); > > if (tx_buf) > sh_msiof_write(p, TMDR2, dr2); > @@ -222,7 +220,7 @@ static void sh_msiof_reset_str(struct sh_msiof_spi_priv *p) > static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, > const void *tx_buf, int words, int fs) > { > - const unsigned char *buf_8 = tx_buf; > + const u8 *buf_8 = tx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -232,7 +230,7 @@ static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, > const void *tx_buf, int words, int fs) > { > - const unsigned short *buf_16 = tx_buf; > + const u16 *buf_16 = tx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -242,7 +240,7 @@ static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, > const void *tx_buf, int words, int fs) > { > - const unsigned short *buf_16 = tx_buf; > + const u16 *buf_16 = tx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -252,7 +250,7 @@ static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, > const void *tx_buf, int words, int fs) > { > - const unsigned int *buf_32 = tx_buf; > + const u32 *buf_32 = tx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -262,7 +260,7 @@ static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, > const void *tx_buf, int words, int fs) > { > - const unsigned int *buf_32 = tx_buf; > + const u32 *buf_32 = tx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -272,7 +270,7 @@ static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, > void *rx_buf, int words, int fs) > { > - unsigned char *buf_8 = rx_buf; > + u8 *buf_8 = rx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -282,7 +280,7 @@ static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, > void *rx_buf, int words, int fs) > { > - unsigned short *buf_16 = rx_buf; > + u16 *buf_16 = rx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -292,7 +290,7 @@ static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, > void *rx_buf, int words, int fs) > { > - unsigned short *buf_16 = rx_buf; > + u16 *buf_16 = rx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -302,7 +300,7 @@ static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, > void *rx_buf, int words, int fs) > { > - unsigned int *buf_32 = rx_buf; > + u32 *buf_32 = rx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -312,7 +310,7 @@ static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, > static void sh_msiof_spi_read_fifo_32u(struct sh_msiof_spi_priv *p, > void *rx_buf, int words, int fs) > { > - unsigned int *buf_32 = rx_buf; > + u32 *buf_32 = rx_buf; > int k; > > for (k = 0; k < words; k++) > @@ -324,7 +322,8 @@ static int sh_msiof_spi_bits(struct spi_device *spi, struct spi_transfer *t) > int bits; > > bits = t ? t->bits_per_word : 0; > - bits = bits ? bits : spi->bits_per_word; > + if (!bits) > + bits = spi->bits_per_word; > return bits; > } > > @@ -334,7 +333,8 @@ static unsigned long sh_msiof_spi_hz(struct spi_device *spi, > unsigned long hz; > > hz = t ? t->speed_hz : 0; > - hz = hz ? hz : spi->max_speed_hz; > + if (!hz) > + hz = spi->max_speed_hz; > return hz; > } > > @@ -453,7 +453,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p, > > return words; > > - err: > +err: > sh_msiof_write(p, IER, 0); > return ret; > } > @@ -617,13 +617,13 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) > return 0; > > pm_runtime_disable(&pdev->dev); > - err3: > +err3: > iounmap(p->mapbase); > - err2: > +err2: > clk_put(p->clk); > - err1: > +err1: > spi_master_put(master); > - err0: > +err0: > return ret; > } > > -- > 1.7.2.3 > > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better price-free! > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > spi-devel-general mailing list > spi-devel-general@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/spi-devel-general ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up 2011-01-21 17:02 ` Grant Likely @ 2011-01-21 17:10 ` Guennadi Liakhovetski 2011-01-21 19:12 ` Grant Likely 2011-01-25 5:02 ` Simon Horman 1 sibling, 1 reply; 7+ messages in thread From: Guennadi Liakhovetski @ 2011-01-21 17:10 UTC (permalink / raw) To: Grant Likely; +Cc: spi-devel-general, linux-sh On Fri, 21 Jan 2011, Grant Likely wrote: > On Fri, Jan 21, 2011 at 04:56:37PM +0100, Guennadi Liakhovetski wrote: > > 1. sort headers alphabetically > > 2. use fixed-size types u8, u16, u32 for register values and transferred data > > 3. simplify some arithmetic operations > > 4. remove leading spaces in front of labels > > There's actually good reason to leave these labels alone. Labels in > column zero confuse diff and it gives the wrong function name. For > example: > > diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c > index 658bd05..840164b 100644 > --- a/drivers/spi/spi_sh_msiof.c > +++ b/drivers/spi/spi_sh_msiof.c > @@ -682,7 +682,7 @@ err2: > clk_put(p->clk); > err1: > spi_master_put(master); > -err0: > + err0: > return ret; > } > > See how the function name in the @@ line is 'err2' instead of > sh_msiof_spi_probe() in this example? Indenting the labels by one > space is an easy fix and is intentional. > > I've picked up the patch, but I've dropped the last two hunks. Strange, my "git diff" doesn't get confused - as in the patch, that I've sent to you... Time to update your git?;) So, personally, I wouldn't hold _that_ for a reason. Well, CodingStyle doesn't directly say anything about those labels, but an example in it puts one at offset 0... Anyway, that's not critical, of course. Thanks Guennadi > > g. > > > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > --- > > drivers/spi/spi_sh_msiof.c | 78 ++++++++++++++++++++++---------------------- > > 1 files changed, 39 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c > > index 56f60c8..07b4ead 100644 > > --- a/drivers/spi/spi_sh_msiof.c > > +++ b/drivers/spi/spi_sh_msiof.c > > @@ -9,22 +9,22 @@ > > * > > */ > > > > -#include <linux/kernel.h> > > -#include <linux/init.h> > > +#include <linux/bitmap.h> > > +#include <linux/clk.h> > > +#include <linux/completion.h> > > #include <linux/delay.h> > > +#include <linux/err.h> > > +#include <linux/gpio.h> > > +#include <linux/init.h> > > #include <linux/interrupt.h> > > +#include <linux/io.h> > > +#include <linux/kernel.h> > > #include <linux/platform_device.h> > > -#include <linux/completion.h> > > #include <linux/pm_runtime.h> > > -#include <linux/gpio.h> > > -#include <linux/bitmap.h> > > -#include <linux/clk.h> > > -#include <linux/io.h> > > -#include <linux/err.h> > > > > +#include <linux/spi/sh_msiof.h> > > #include <linux/spi/spi.h> > > #include <linux/spi/spi_bitbang.h> > > -#include <linux/spi/sh_msiof.h> > > > > #include <asm/unaligned.h> > > > > @@ -67,7 +67,7 @@ struct sh_msiof_spi_priv { > > #define STR_TEOF (1 << 23) > > #define STR_REOF (1 << 7) > > > > -static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > > +static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > > { > > switch (reg_offs) { > > case TSCR: > > @@ -79,7 +79,7 @@ static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > > } > > > > static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, > > - unsigned long value) > > + u32 value) > > { > > switch (reg_offs) { > > case TSCR: > > @@ -93,10 +93,10 @@ static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, > > } > > > > static int sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p, > > - unsigned long clr, unsigned long set) > > + u32 clr, u32 set) > > { > > - unsigned long mask = clr | set; > > - unsigned long data; > > + u32 mask = clr | set; > > + u32 data; > > int k; > > > > data = sh_msiof_read(p, CTR); > > @@ -166,10 +166,10 @@ static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p, > > } > > > > static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > - int cpol, int cpha, > > - int tx_hi_z, int lsb_first) > > + u32 cpol, u32 cpha, > > + u32 tx_hi_z, u32 lsb_first) > > { > > - unsigned long tmp; > > + u32 tmp; > > int edge; > > > > /* > > @@ -187,7 +187,7 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > tmp |= cpol << 30; /* TSCKIZ */ > > tmp |= cpol << 28; /* RSCKIZ */ > > > > - edge = cpol ? cpha : !cpha; > > + edge = cpol ^ !cpha; > > > > tmp |= edge << 27; /* TEDG */ > > tmp |= edge << 26; /* REDG */ > > @@ -197,11 +197,9 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > > > static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p, > > const void *tx_buf, void *rx_buf, > > - int bits, int words) > > + u32 bits, u32 words) > > { > > - unsigned long dr2; > > - > > - dr2 = ((bits - 1) << 24) | ((words - 1) << 16); > > + u32 dr2 = ((bits - 1) << 24) | ((words - 1) << 16); > > > > if (tx_buf) > > sh_msiof_write(p, TMDR2, dr2); > > @@ -222,7 +220,7 @@ static void sh_msiof_reset_str(struct sh_msiof_spi_priv *p) > > static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, > > const void *tx_buf, int words, int fs) > > { > > - const unsigned char *buf_8 = tx_buf; > > + const u8 *buf_8 = tx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -232,7 +230,7 @@ static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, > > const void *tx_buf, int words, int fs) > > { > > - const unsigned short *buf_16 = tx_buf; > > + const u16 *buf_16 = tx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -242,7 +240,7 @@ static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, > > const void *tx_buf, int words, int fs) > > { > > - const unsigned short *buf_16 = tx_buf; > > + const u16 *buf_16 = tx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -252,7 +250,7 @@ static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, > > const void *tx_buf, int words, int fs) > > { > > - const unsigned int *buf_32 = tx_buf; > > + const u32 *buf_32 = tx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -262,7 +260,7 @@ static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, > > const void *tx_buf, int words, int fs) > > { > > - const unsigned int *buf_32 = tx_buf; > > + const u32 *buf_32 = tx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -272,7 +270,7 @@ static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, > > void *rx_buf, int words, int fs) > > { > > - unsigned char *buf_8 = rx_buf; > > + u8 *buf_8 = rx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -282,7 +280,7 @@ static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, > > void *rx_buf, int words, int fs) > > { > > - unsigned short *buf_16 = rx_buf; > > + u16 *buf_16 = rx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -292,7 +290,7 @@ static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, > > void *rx_buf, int words, int fs) > > { > > - unsigned short *buf_16 = rx_buf; > > + u16 *buf_16 = rx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -302,7 +300,7 @@ static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, > > void *rx_buf, int words, int fs) > > { > > - unsigned int *buf_32 = rx_buf; > > + u32 *buf_32 = rx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -312,7 +310,7 @@ static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, > > static void sh_msiof_spi_read_fifo_32u(struct sh_msiof_spi_priv *p, > > void *rx_buf, int words, int fs) > > { > > - unsigned int *buf_32 = rx_buf; > > + u32 *buf_32 = rx_buf; > > int k; > > > > for (k = 0; k < words; k++) > > @@ -324,7 +322,8 @@ static int sh_msiof_spi_bits(struct spi_device *spi, struct spi_transfer *t) > > int bits; > > > > bits = t ? t->bits_per_word : 0; > > - bits = bits ? bits : spi->bits_per_word; > > + if (!bits) > > + bits = spi->bits_per_word; > > return bits; > > } > > > > @@ -334,7 +333,8 @@ static unsigned long sh_msiof_spi_hz(struct spi_device *spi, > > unsigned long hz; > > > > hz = t ? t->speed_hz : 0; > > - hz = hz ? hz : spi->max_speed_hz; > > + if (!hz) > > + hz = spi->max_speed_hz; > > return hz; > > } > > > > @@ -453,7 +453,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p, > > > > return words; > > > > - err: > > +err: > > sh_msiof_write(p, IER, 0); > > return ret; > > } > > @@ -617,13 +617,13 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) > > return 0; > > > > pm_runtime_disable(&pdev->dev); > > - err3: > > +err3: > > iounmap(p->mapbase); > > - err2: > > +err2: > > clk_put(p->clk); > > - err1: > > +err1: > > spi_master_put(master); > > - err0: > > +err0: > > return ret; > > } > > > > -- > > 1.7.2.3 > > > > > > ------------------------------------------------------------------------------ > > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > > Finally, a world-class log management solution at an even better price-free! > > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > > February 28th, so secure your free ArcSight Logger TODAY! > > http://p.sf.net/sfu/arcsight-sfd2d > > _______________________________________________ > > spi-devel-general mailing list > > spi-devel-general@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/spi-devel-general > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up 2011-01-21 17:10 ` Guennadi Liakhovetski @ 2011-01-21 19:12 ` Grant Likely 0 siblings, 0 replies; 7+ messages in thread From: Grant Likely @ 2011-01-21 19:12 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: spi-devel-general, linux-sh On Fri, Jan 21, 2011 at 06:10:50PM +0100, Guennadi Liakhovetski wrote: > On Fri, 21 Jan 2011, Grant Likely wrote: > > > On Fri, Jan 21, 2011 at 04:56:37PM +0100, Guennadi Liakhovetski wrote: > > > 1. sort headers alphabetically > > > 2. use fixed-size types u8, u16, u32 for register values and transferred data > > > 3. simplify some arithmetic operations > > > 4. remove leading spaces in front of labels > > > > There's actually good reason to leave these labels alone. Labels in > > column zero confuse diff and it gives the wrong function name. For > > example: > > > > diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c > > index 658bd05..840164b 100644 > > --- a/drivers/spi/spi_sh_msiof.c > > +++ b/drivers/spi/spi_sh_msiof.c > > @@ -682,7 +682,7 @@ err2: > > clk_put(p->clk); > > err1: > > spi_master_put(master); > > -err0: > > + err0: > > return ret; > > } > > > > See how the function name in the @@ line is 'err2' instead of > > sh_msiof_spi_probe() in this example? Indenting the labels by one > > space is an easy fix and is intentional. > > > > I've picked up the patch, but I've dropped the last two hunks. > > Strange, my "git diff" doesn't get confused - as in the patch, that > I've sent to you... Time to update your git?;) So, personally, I wouldn't > hold _that_ for a reason. Well, CodingStyle doesn't directly say anything > about those labels, but an example in it puts one at offset 0... Anyway, > that's not critical, of course. Of course it didn't. The diff hunk started before a label. My machine does the same with your patch, here: diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c index 56f60c8..a81a836 100644 --- a/drivers/spi/spi_sh_msiof.c +++ b/drivers/spi/spi_sh_msiof.c @@ -617,13 +617,13 @@ static int sh_msiof_spi_probe(struct platform_device *pdev return 0; pm_runtime_disable(&pdev->dev); - err3: +err3: iounmap(p->mapbase); - err2: +err2: clk_put(p->clk); - err1: +err1: spi_master_put(master); - err0: +err0: return ret; } However, try it with only changing the last label so that diff hunk begins later. Actually, what you need to try is apply your original patch so that all the labels are in column zero, and then indent only the last label so it looks like the example I sent you. I'll bet you a beer that it gets confused. :-) g. > > Thanks > Guennadi > > > > > g. > > > > > > > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > > --- > > > drivers/spi/spi_sh_msiof.c | 78 ++++++++++++++++++++++---------------------- > > > 1 files changed, 39 insertions(+), 39 deletions(-) > > > > > > diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c > > > index 56f60c8..07b4ead 100644 > > > --- a/drivers/spi/spi_sh_msiof.c > > > +++ b/drivers/spi/spi_sh_msiof.c > > > @@ -9,22 +9,22 @@ > > > * > > > */ > > > > > > -#include <linux/kernel.h> > > > -#include <linux/init.h> > > > +#include <linux/bitmap.h> > > > +#include <linux/clk.h> > > > +#include <linux/completion.h> > > > #include <linux/delay.h> > > > +#include <linux/err.h> > > > +#include <linux/gpio.h> > > > +#include <linux/init.h> > > > #include <linux/interrupt.h> > > > +#include <linux/io.h> > > > +#include <linux/kernel.h> > > > #include <linux/platform_device.h> > > > -#include <linux/completion.h> > > > #include <linux/pm_runtime.h> > > > -#include <linux/gpio.h> > > > -#include <linux/bitmap.h> > > > -#include <linux/clk.h> > > > -#include <linux/io.h> > > > -#include <linux/err.h> > > > > > > +#include <linux/spi/sh_msiof.h> > > > #include <linux/spi/spi.h> > > > #include <linux/spi/spi_bitbang.h> > > > -#include <linux/spi/sh_msiof.h> > > > > > > #include <asm/unaligned.h> > > > > > > @@ -67,7 +67,7 @@ struct sh_msiof_spi_priv { > > > #define STR_TEOF (1 << 23) > > > #define STR_REOF (1 << 7) > > > > > > -static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > > > +static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > > > { > > > switch (reg_offs) { > > > case TSCR: > > > @@ -79,7 +79,7 @@ static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs) > > > } > > > > > > static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, > > > - unsigned long value) > > > + u32 value) > > > { > > > switch (reg_offs) { > > > case TSCR: > > > @@ -93,10 +93,10 @@ static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs, > > > } > > > > > > static int sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p, > > > - unsigned long clr, unsigned long set) > > > + u32 clr, u32 set) > > > { > > > - unsigned long mask = clr | set; > > > - unsigned long data; > > > + u32 mask = clr | set; > > > + u32 data; > > > int k; > > > > > > data = sh_msiof_read(p, CTR); > > > @@ -166,10 +166,10 @@ static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p, > > > } > > > > > > static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > > - int cpol, int cpha, > > > - int tx_hi_z, int lsb_first) > > > + u32 cpol, u32 cpha, > > > + u32 tx_hi_z, u32 lsb_first) > > > { > > > - unsigned long tmp; > > > + u32 tmp; > > > int edge; > > > > > > /* > > > @@ -187,7 +187,7 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > > tmp |= cpol << 30; /* TSCKIZ */ > > > tmp |= cpol << 28; /* RSCKIZ */ > > > > > > - edge = cpol ? cpha : !cpha; > > > + edge = cpol ^ !cpha; > > > > > > tmp |= edge << 27; /* TEDG */ > > > tmp |= edge << 26; /* REDG */ > > > @@ -197,11 +197,9 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, > > > > > > static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p, > > > const void *tx_buf, void *rx_buf, > > > - int bits, int words) > > > + u32 bits, u32 words) > > > { > > > - unsigned long dr2; > > > - > > > - dr2 = ((bits - 1) << 24) | ((words - 1) << 16); > > > + u32 dr2 = ((bits - 1) << 24) | ((words - 1) << 16); > > > > > > if (tx_buf) > > > sh_msiof_write(p, TMDR2, dr2); > > > @@ -222,7 +220,7 @@ static void sh_msiof_reset_str(struct sh_msiof_spi_priv *p) > > > static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, > > > const void *tx_buf, int words, int fs) > > > { > > > - const unsigned char *buf_8 = tx_buf; > > > + const u8 *buf_8 = tx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -232,7 +230,7 @@ static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, > > > const void *tx_buf, int words, int fs) > > > { > > > - const unsigned short *buf_16 = tx_buf; > > > + const u16 *buf_16 = tx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -242,7 +240,7 @@ static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, > > > const void *tx_buf, int words, int fs) > > > { > > > - const unsigned short *buf_16 = tx_buf; > > > + const u16 *buf_16 = tx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -252,7 +250,7 @@ static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, > > > const void *tx_buf, int words, int fs) > > > { > > > - const unsigned int *buf_32 = tx_buf; > > > + const u32 *buf_32 = tx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -262,7 +260,7 @@ static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, > > > const void *tx_buf, int words, int fs) > > > { > > > - const unsigned int *buf_32 = tx_buf; > > > + const u32 *buf_32 = tx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -272,7 +270,7 @@ static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, > > > void *rx_buf, int words, int fs) > > > { > > > - unsigned char *buf_8 = rx_buf; > > > + u8 *buf_8 = rx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -282,7 +280,7 @@ static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, > > > void *rx_buf, int words, int fs) > > > { > > > - unsigned short *buf_16 = rx_buf; > > > + u16 *buf_16 = rx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -292,7 +290,7 @@ static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, > > > void *rx_buf, int words, int fs) > > > { > > > - unsigned short *buf_16 = rx_buf; > > > + u16 *buf_16 = rx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -302,7 +300,7 @@ static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, > > > void *rx_buf, int words, int fs) > > > { > > > - unsigned int *buf_32 = rx_buf; > > > + u32 *buf_32 = rx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -312,7 +310,7 @@ static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p, > > > static void sh_msiof_spi_read_fifo_32u(struct sh_msiof_spi_priv *p, > > > void *rx_buf, int words, int fs) > > > { > > > - unsigned int *buf_32 = rx_buf; > > > + u32 *buf_32 = rx_buf; > > > int k; > > > > > > for (k = 0; k < words; k++) > > > @@ -324,7 +322,8 @@ static int sh_msiof_spi_bits(struct spi_device *spi, struct spi_transfer *t) > > > int bits; > > > > > > bits = t ? t->bits_per_word : 0; > > > - bits = bits ? bits : spi->bits_per_word; > > > + if (!bits) > > > + bits = spi->bits_per_word; > > > return bits; > > > } > > > > > > @@ -334,7 +333,8 @@ static unsigned long sh_msiof_spi_hz(struct spi_device *spi, > > > unsigned long hz; > > > > > > hz = t ? t->speed_hz : 0; > > > - hz = hz ? hz : spi->max_speed_hz; > > > + if (!hz) > > > + hz = spi->max_speed_hz; > > > return hz; > > > } > > > > > > @@ -453,7 +453,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p, > > > > > > return words; > > > > > > - err: > > > +err: > > > sh_msiof_write(p, IER, 0); > > > return ret; > > > } > > > @@ -617,13 +617,13 @@ static int sh_msiof_spi_probe(struct platform_device *pdev) > > > return 0; > > > > > > pm_runtime_disable(&pdev->dev); > > > - err3: > > > +err3: > > > iounmap(p->mapbase); > > > - err2: > > > +err2: > > > clk_put(p->clk); > > > - err1: > > > +err1: > > > spi_master_put(master); > > > - err0: > > > +err0: > > > return ret; > > > } > > > > > > -- > > > 1.7.2.3 > > > > > > > > > ------------------------------------------------------------------------------ > > > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > > > Finally, a world-class log management solution at an even better price-free! > > > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > > > February 28th, so secure your free ArcSight Logger TODAY! > > > http://p.sf.net/sfu/arcsight-sfd2d > > > _______________________________________________ > > > spi-devel-general mailing list > > > spi-devel-general@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/spi-devel-general > > > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up 2011-01-21 17:02 ` Grant Likely 2011-01-21 17:10 ` Guennadi Liakhovetski @ 2011-01-25 5:02 ` Simon Horman 2011-01-25 5:26 ` Grant Likely 1 sibling, 1 reply; 7+ messages in thread From: Simon Horman @ 2011-01-25 5:02 UTC (permalink / raw) To: Grant Likely; +Cc: Guennadi Liakhovetski, spi-devel-general, linux-sh On Fri, Jan 21, 2011 at 10:02:47AM -0700, Grant Likely wrote: > On Fri, Jan 21, 2011 at 04:56:37PM +0100, Guennadi Liakhovetski wrote: > > 1. sort headers alphabetically > > 2. use fixed-size types u8, u16, u32 for register values and transferred data > > 3. simplify some arithmetic operations > > 4. remove leading spaces in front of labels > > There's actually good reason to leave these labels alone. Labels in > column zero confuse diff and it gives the wrong function name. For > example: > > diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c > index 658bd05..840164b 100644 > --- a/drivers/spi/spi_sh_msiof.c > +++ b/drivers/spi/spi_sh_msiof.c > @@ -682,7 +682,7 @@ err2: > clk_put(p->clk); > err1: > spi_master_put(master); > -err0: > + err0: > return ret; > } > > See how the function name in the @@ line is 'err2' instead of > sh_msiof_spi_probe() in this example? Indenting the labels by one > space is an easy fix and is intentional. > > I've picked up the patch, but I've dropped the last two hunks. Surely this is a bug in diff. Or perhaps put more politely, diff isn't smart enough to realise a) this is C and b) xxx: isn't a valid function name in C. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up 2011-01-25 5:02 ` Simon Horman @ 2011-01-25 5:26 ` Grant Likely 2011-01-25 5:47 ` Paul Mundt 0 siblings, 1 reply; 7+ messages in thread From: Grant Likely @ 2011-01-25 5:26 UTC (permalink / raw) To: Simon Horman; +Cc: Guennadi Liakhovetski, spi-devel-general, linux-sh On Mon, Jan 24, 2011 at 10:02 PM, Simon Horman <horms@verge.net.au> wrote: > On Fri, Jan 21, 2011 at 10:02:47AM -0700, Grant Likely wrote: >> On Fri, Jan 21, 2011 at 04:56:37PM +0100, Guennadi Liakhovetski wrote: >> > 1. sort headers alphabetically >> > 2. use fixed-size types u8, u16, u32 for register values and transferred data >> > 3. simplify some arithmetic operations >> > 4. remove leading spaces in front of labels >> >> There's actually good reason to leave these labels alone. Labels in >> column zero confuse diff and it gives the wrong function name. For >> example: >> >> diff --git a/drivers/spi/spi_sh_msiof.c b/drivers/spi/spi_sh_msiof.c >> index 658bd05..840164b 100644 >> --- a/drivers/spi/spi_sh_msiof.c >> +++ b/drivers/spi/spi_sh_msiof.c >> @@ -682,7 +682,7 @@ err2: >> clk_put(p->clk); >> err1: >> spi_master_put(master); >> -err0: >> + err0: >> return ret; >> } >> >> See how the function name in the @@ line is 'err2' instead of >> sh_msiof_spi_probe() in this example? Indenting the labels by one >> space is an easy fix and is intentional. >> >> I've picked up the patch, but I've dropped the last two hunks. > > Surely this is a bug in diff. Or perhaps put more politely, > diff isn't smart enough to realise a) this is C and b) xxx: isn't > a valid function name in C. Oh, perhaps. But regardless, it is long-standing behaviour and the single space convention is widely used. Nobody seems offended enough by it to go and change diff. To see, just do: git grep '^ [a-zA-Z0-9_]*:' If it were particularly ugly, or actively dangerous, then I'd be against this pattern, but it is neither and it noticeably improves diff readability. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up 2011-01-25 5:26 ` Grant Likely @ 2011-01-25 5:47 ` Paul Mundt 0 siblings, 0 replies; 7+ messages in thread From: Paul Mundt @ 2011-01-25 5:47 UTC (permalink / raw) To: Grant Likely Cc: Simon Horman, Guennadi Liakhovetski, spi-devel-general, linux-sh On Mon, Jan 24, 2011 at 10:26:55PM -0700, Grant Likely wrote: > On Mon, Jan 24, 2011 at 10:02 PM, Simon Horman <horms@verge.net.au> wrote: > > Surely this is a bug in diff. Or perhaps put more politely, > > diff isn't smart enough to realise a) this is C and b) xxx: isn't > > a valid function name in C. > > Oh, perhaps. But regardless, it is long-standing behaviour and the > single space convention is widely used. Nobody seems offended enough > by it to go and change diff. To see, just do: > > git grep '^ [a-zA-Z0-9_]*:' > > If it were particularly ugly, or actively dangerous, then I'd be > against this pattern, but it is neither and it noticeably improves > diff readability. > It's largely a personal preference thing. I personally don't care for the leading spaces, and remove them whenever I encounter them. I know that Magnus and some others like to use them however, so generally don't care one way or the other. I assume general apathy has a lot to do with diff not being fixed (note that this topic does come up from time to time, where the general conclusion is that someone should probably do something about it, at some point in time). ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-01-25 5:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-21 15:56 [PATCH 1/3] SPI: spi_sh_msiof: cosmetic clean-up Guennadi Liakhovetski 2011-01-21 17:02 ` Grant Likely 2011-01-21 17:10 ` Guennadi Liakhovetski 2011-01-21 19:12 ` Grant Likely 2011-01-25 5:02 ` Simon Horman 2011-01-25 5:26 ` Grant Likely 2011-01-25 5:47 ` Paul Mundt
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).