linux-kernel - Re: [PATCH 2/6] usb: cdns3-ti: move reg writes from probe into an init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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