| Message ID | 1498579630-10109-1-git-send-email-aravind.thk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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.
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 --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;
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(-)