diff mbox

spi/bcm63xx-hspi: Fix checkpatch warnings

Message ID 1498579630-10109-1-git-send-email-aravind.thk@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aravind Thokala June 27, 2017, 4:07 p.m. UTC
This patch fixes the checkpatch.pl warnings on the driver
file.

Signed-off-by: Aravind Thokala <aravind.thk@gmail.com>
---
 drivers/spi/spi-bcm63xx-hsspi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Mark Brown June 28, 2017, 6:45 p.m. UTC | #1
On Tue, Jun 27, 2017 at 09:37:10PM +0530, Aravind Thokala wrote:
> This patch fixes the checkpatch.pl warnings on the driver
> file.

This isn't a good changelog, what are the warnings and why are we fixing
them?  The fact that checkpatch complains isn't meaningful in and of
itself - it's OK to say that checkpatch helped identify issues but it's
not perfect and we need to understand what is being changed and why.
Aravind Thokala June 30, 2017, 5:38 p.m. UTC | #2
Hello Mark,

Thank you for reviewing the patch. I've used the checkpatch tool and
fixed the warnings provided by the tool. The warnings are in the
function definitions:

1) static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned
 cs, bool active)

2) static void bcm63xx_hsspi_set_clk(struct bcm63xx_hsspi *bs,
                                  struct spi_device *spi, int hz)
{
        unsigned profile = spi->chip_select;

3) static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct
spi_transfer *t)
{
        struct bcm63xx_hsspi *bs = spi_master_get_devdata(spi->master);
        unsigned chip_select = spi->chip_select;

These warnings looked straightforward for me so I've fixed those and
sent the patch.

Kindly let me know if you want to record the warnings in the patch so
that I could send v2 patch as soon as possible.

Thank you,

Aravind.


On 29 June 2017 at 00:15, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 27, 2017 at 09:37:10PM +0530, Aravind Thokala wrote:
>> This patch fixes the checkpatch.pl warnings on the driver
>> file.
>
> This isn't a good changelog, what are the warnings and why are we fixing
> them?  The fact that checkpatch complains isn't meaningful in and of
> itself - it's OK to say that checkpatch helped identify issues but it's
> not perfect and we need to understand what is being changed and why.
Mark Brown July 10, 2017, 6:32 p.m. UTC | #3
On Fri, Jun 30, 2017 at 11:08:15PM +0530, Aravind Thokala wrote:
> Hello Mark,

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

> These warnings looked straightforward for me so I've fixed those and
> sent the patch.

> Kindly let me know if you want to record the warnings in the patch so
> that I could send v2 patch as soon as possible.

It's not just a case of quoting the warnings, it's a case of explaining
what your patch changes which usually requires more than just quoting
the warning.
diff mbox

Patch

diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 5514cd0..17306c1 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -108,7 +108,7 @@  struct bcm63xx_hsspi {
 	u8 cs_polarity;
 };
 
-static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned cs,
+static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned int cs,
 				 bool active)
 {
 	u32 reg;
@@ -127,7 +127,7 @@  static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned cs,
 static void bcm63xx_hsspi_set_clk(struct bcm63xx_hsspi *bs,
 				  struct spi_device *spi, int hz)
 {
-	unsigned profile = spi->chip_select;
+	unsigned int profile = spi->chip_select;
 	u32 reg;
 
 	reg = DIV_ROUND_UP(2048, DIV_ROUND_UP(bs->speed_hz, hz));
@@ -154,7 +154,7 @@  static void bcm63xx_hsspi_set_clk(struct bcm63xx_hsspi *bs,
 static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
 {
 	struct bcm63xx_hsspi *bs = spi_master_get_devdata(spi->master);
-	unsigned chip_select = spi->chip_select;
+	unsigned int chip_select = spi->chip_select;
 	u16 opcode = 0;
 	int pending = t->len;
 	int step_size = HSSPI_BUFFER_LEN;