Date:   Wed, 29 Jan 2020 09:31:00 +0000
From:   Peter Rosin <peda@...ntia.se>
To:     Joe Perches <joe@...ches.com>,
        Colin Ian King <colin.king@...onical.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Michal Simek <michal.simek@...inx.com>,
        Johan Hovold <johan@...nel.org>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH][next][V2] i2c: xiic: fix indentation issue

On 2020-01-29 00:48, Joe Perches wrote:
> On Tue, 2020-01-28 at 23:10 +0000, Colin Ian King wrote:
>> On 28/01/2020 21:45, Peter Rosin wrote:
>>> On 2020-01-27 13:08, Colin Ian King wrote:
>>>> On 27/01/2020 12:05, Dan Carpenter wrote:
>>>>> On Mon, Jan 27, 2020 at 12:03:02PM +0100, Michal Simek wrote:
>>>>>> On 27. 01. 20 11:23, Colin King wrote:
>>>>>>> From: Colin Ian King <colin.king@...onical.com>
>>>>>>>
>>>>>>> There is a statement that is indented one level too deeply, remove
>>>>>>> the extraneous tab.
>>>>>>>
>>>>>>> Signed-off-by: Colin Ian King <colin.king@...onical.com>
>>>>>>> ---
>>>>>>> V2: fix type in commit message
>>>>>>> ---
>>>>>>>  drivers/i2c/busses/i2c-xiic.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
>>>>>>> index b17d30c9ab40..90c1c362394d 100644
>>>>>>> --- a/drivers/i2c/busses/i2c-xiic.c
>>>>>>> +++ b/drivers/i2c/busses/i2c-xiic.c
>>>>>>> @@ -261,7 +261,7 @@ static int xiic_clear_rx_fifo(struct xiic_i2c *i2c)
>>>>>>>  		xiic_getreg8(i2c, XIIC_DRR_REG_OFFSET);
>>>>>>>  		if (time_after(jiffies, timeout)) {
>>>>>>>  			dev_err(i2c->dev, "Failed to clear rx fifo\n");
>>>>>>> -				return -ETIMEDOUT;
>>>>>>> +			return -ETIMEDOUT;
>>>>>>>  		}
>>>>>>>  	}
>>>>>>>  
>>>>>>>
>>>>>>
>>>>>> As was suggested by Peter you should also add Fixes: <sha1> ("patch
>>>>>> subject")
>>>>>>
>>>>>
>>>>> It's not really a bugfix, it's just a cleanup.
>>>>
>>>> I'm surprised i wasn't asked for a bug number too.
>>>
>>> Very funny.
>>
>> Apologies for being flippant. I didn't mean to offend. My bad #1.
>>
>>> I realize that you, the three complainers (Johan, Dan and Colin), together
>>> have almost 10000 commits. So, I feel a bit outranked.
>>>
>>> However, this ridicule is unfair.

And the 4th heavy-weight steps in, and the grand total is now well past
10000 commits.

> It's not unfair.

Are you saying that it's fair???

> In _no_ sense is a whitespace only change a valid reason
> to use a "Fixes:" tag.
> 
> A whitespace only coding-style issue is _not_ a bug.

Right. That's one opinion. And it seems that many long time contributors agree
that this is the case. So it must be the only acceptable opinion. Or?

In my book coding-style issues is about taste, i.e. if you use spaces or tabs
to indent, and if and when you add a new-line before {. Etc. This fix is about
directly misleading code that noone writes on purpose. Clearly a bug in my
book, and not some meager "style" issue.

> From submitting-patches:
> 
> If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> the SHA-1 ID, and the one line summary.

You need to read that carefully. Do you see that "e.g." part? This quote states
*one* example of how you can find a bug. It's not some exhaustive list and thus
"proves" nothing what-so-ever.

Besides, reading the git biscet manual reveals that

"In fact, git bisect can be used to find the commit that changed any property
 of your project"

So, if we are being ridiculous, thanks for the support! Below is the proof that
I found a really horrible *bisectable* bug! Looks like a fixes-tag was warranted
after all!!!1!

On a more serious note I challenge you to find anything in Documentation/ that
tells me, without ambiguity, that the patch in question is *not* fixing a bug,
because your quote is clearly not it.

I'm *only* saying that it is not always so damn easy to categorize issues, and
that a mis-step does not warrant ridicule.

Cheers,
Peter


$ cat finder.sh
#! /bin/sh

if grep 'return -ETIMEDOUT;' drivers/i2c/busses/i2c-xiic.c; then
    exit 1
fi
$ git bisect start
$ git bisect bad
$ git bisect good v5.5-rc1
Bisecting: 814 revisions left to test after this (roughly 10 steps)
[0dd1e3773ae8afc4bfdce782bdeffc10f9cae6ec] pipe: fix empty pipe check in pipe_write()
$ git bisect run ./finder.sh
running ./finder.sh
Bisecting: 406 revisions left to test after this (roughly 9 steps)
[ca78fdeb00fa656f09afee977750e85da929d259] Merge tag 'drm-fixes-2020-01-03' of git://anongit.freedesktop.org/drm/drm
running ./finder.sh
Bisecting: 210 revisions left to test after this (roughly 8 steps)
[a5f48c7878d2365f6ff7008e9317abbc16f68847] Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
running ./finder.sh
Bisecting: 102 revisions left to test after this (roughly 7 steps)
[7da37cd0520e71707a1190022377941b9cec3b0b] Merge tag 'staging-5.5-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
running ./finder.sh
Bisecting: 52 revisions left to test after this (roughly 6 steps)
[040a3c33623ba4bd11588ab0820281b854a3ffaf] Merge tag 'iommu-fixes-v5.5-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu
running ./finder.sh
Bisecting: 26 revisions left to test after this (roughly 5 steps)
[b3ec946975737b949137fbb1a2db9e7cc5b9ae82] i2c: tegra: Check DMA completion status in addition to left time
running ./finder.sh
Bisecting: 13 revisions left to test after this (roughly 4 steps)
[e8d51e962936bf4527f41db318d53a80006f2bf7] docs: i2c: use the new API in 'writing-clients'
running ./finder.sh
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[4a2d5f663dab6614772d8e28ca190b127ba46d9d] i2c: Enable compile testing for more drivers
running ./finder.sh
                                return -ETIMEDOUT;
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[40b2ec1251c370bc5557568e259d4058c651d405] i2c: highlander: Use proper printk format for size_t
running ./finder.sh
Bisecting: 1 revision left to test after this (roughly 1 step)
[c9d059681b846f5d0a280950ea4dc58495b9b7a9] i2c: xiic: defer the probe if clock is not found
running ./finder.sh
                                return -ETIMEDOUT;
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[b4c119dbc300c7a6ee2da70d5c7ba14747b35142] i2c: xiic: Add timeout to the rx fifo wait loop
running ./finder.sh
                                return -ETIMEDOUT;
b4c119dbc300c7a6ee2da70d5c7ba14747b35142 is the first bad commit
commit b4c119dbc300c7a6ee2da70d5c7ba14747b35142
Author: Shubhrajyoti Datta <shubhrajyoti.datta@...inx.com>
Date:   Thu Jan 9 17:07:58 2020 +0530

    i2c: xiic: Add timeout to the rx fifo wait loop

    Add timeout to the rx fifo empty wait loop. Also check for the return
    value for xiic_reinit and pass it to xiic_start_xfer.

    Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@...inx.com>
    Signed-off-by: Michal Simek <michal.simek@...inx.com>
    Signed-off-by: Wolfram Sang <wsa@...-dreams.de>

:040000 040000 ca301fc2df019bb2acd07bddabe4afa492ad8f65 22d719ef351bf4897b465d10630e30e7c5cf665f M      drivers
bisect run success

Powered by blists - more mailing lists