[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <79f3beef-1afe-4b97-8f6e-435c0419b513@kernel.org>
Date: Thu, 16 Nov 2023 14:00:46 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Théo Lebrun <theo.lebrun@...tlin.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Peter Chen <peter.chen@...nel.org>,
Pawel Laszczak <pawell@...ence.com>,
Nishanth Menon <nm@...com>,
Vignesh Raghavendra <vigneshr@...com>,
Tero Kristo <kristo@...nel.org>
Cc: linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/6] usb: cdns3-ti: move reg writes from probe into an
init_hw helper
On 15/11/2023 16:23, Théo Lebrun wrote:
> Hello,
>
> On Wed Nov 15, 2023 at 12:33 PM CET, Roger Quadros wrote:
>>> --- a/drivers/usb/cdns3/cdns3-ti.c
>>> +++ b/drivers/usb/cdns3/cdns3-ti.c
>
> [...]
>
>>> static int cdns_ti_probe(struct platform_device *pdev)
>>> {
>>> struct device *dev = &pdev->dev;
>>> struct device_node *node = pdev->dev.of_node;
>>> struct cdns_ti *data;
>>> - int error;
>>> - u32 reg;
>>> - int rate_code, i;
>>> unsigned long rate;
>>> + int error, i;
>>
>> Should we leave rate_code and get rid of i?
>
> I see your point about i being usually a temp variable. Using rate_code
> instead of i means the for-loop becomes rather long (column 87) &
> should ideally be split.
>
> How about moving the data->usb2_refclk_rate_code assignment up, closer
> to the computation of i? That way we don't reference i three blocks
> down the road, seemingly out of nowhere.
That is much better. Thanks!
--
cheers,
-roger
Powered by blists - more mailing lists