Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Wed, 18 Oct 2023 13:42:23 +0300
From:   Dan Carpenter <dan.carpenter@linaro.org>
To:     Karolina Stolarek <karolina.stolarek@intel.com>
Cc:     Julia Lawall <julia.lawall@inria.fr>,
        kenechukwu maduechesi <maduechesik@gmail.com>,
        outreachy@lists.linux.dev, shreeya.patel23498@gmail.com,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rts5208: Replace delay function.
Message-ID: <e5c482ab-0342-43f5-a62f-c0c176e2e094@kadam.mountain>
References: <20231018004300.GA3189@ubuntu>
 <7bc719c4-459f-3d8-7ed-b1e1adf158@inria.fr>
 <1118ec6d-e1e7-79f3-08e4-9a79c996eed1@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1118ec6d-e1e7-79f3-08e4-9a79c996eed1@intel.com>
Precedence: bulk

On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
> On 18.10.2023 09:03, Julia Lawall wrote:
> > 
> > 
> > On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
> > 
> > > Replace udelay() with usleep_range() for more precise delay handling.
> > > 
> > > Reported by checkpatch:
> > > 
> > > CHECK: usleep_range is preferred over udelay
> > 
> > This message is typically not a good candidate for outreachy patches,
> > because you need access to the device to be sure that any change is
> > correct.
> 
> Could we add a paragraph on how to pick good checkpatch.pl error to fix to
> the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
> section, for example.

I think you should put a note about usleep_range() and the extra
parentheses one.

Also say something about looking up stuff on lore.
https://lore.kernel.org/all/?q=sd_change_phase%20usleep_range
In this case someone tried to update this before.  The patch wasn't
merged but it wasn't clearly explained to the developer why the patch
wasn't merged.  Ah well.

Generally fresh warnings are better than old warnings because we fix the
simple stuff where checkpatch is obviously correct.

The other thing is that people really need to look at the wider context.
Look at the surrounding code.  Look at when the checkpatch warning was
introduced and try to put yourself in the developer's head to figure out
what they were thinking.  Are there other opportunities for cleanups
nearby.

regards,
dan carpenter