linux-kernel - Re: [PATCH v3 7/8] net: phy: Add support to configure clock in Broadcom iProc mdio mux

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]

Message-ID: <9af7e7a3-54d4-8802-f64a-fc4f9c49c8b9@broadcom.com>
Date:   Wed, 1 Aug 2018 12:55:56 -0700
From:   Arun Parameswaran <arun.parameswaran@...adcom.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Florian Fainelli <f.fainelli@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>, netdev@...r.kernel.org,
        bcm-kernel-feedback-list@...adcom.com,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v3 7/8] net: phy: Add support to configure clock in
 Broadcom iProc mdio mux

Hi Russell, Florian,

On 18-08-01 11:46 AM, Russell King - ARM Linux wrote:
> On Wed, Aug 01, 2018 at 11:40:33AM -0700, Florian Fainelli wrote:
>> On 08/01/2018 10:56 AM, Arun Parameswaran wrote:
>>>  static int iproc_mdio_wait_for_idle(void __iomem *base, bool result)
>>> @@ -204,6 +225,20 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
>>>  		return -ENOMEM;
>>>  	}
>>>  
>>> +	md->core_clk = devm_clk_get(&pdev->dev, NULL);
>>> +	if (IS_ERR(md->core_clk)) {
>>> +		if (PTR_ERR(md->core_clk) == -EPROBE_DEFER)
>>> +			return -EPROBE_DEFER;
>>> +
>>> +		md->core_clk = NULL;
>>
>> I would simplify this a bit:
>>
>> 	if (IS_ERR(md->core_clk) && PTR_ERR(md->core_clk) == -EPROBE_DEFER)
> 
> Even better is:
> 
> 	if (md->core_clk == ERR_PTR(-EPROBE_DEFER))
> 

The clock is an optional parameter. But I realize, that I did not handle all
the error cases.

My intention was to proceed with probe if the clock is not found in the DT.
If the clock is specified in the DT, make use of it.

So, should I modify to something like:

md->core_clk = devm_clk_get(&pdev->dev, NULL);
if (md->core_clk == ERR_PTR(-ENOENT)) {
	/* clock is optional, without it the default
	 * rate divider register values will be used
	 */
	md->core_clk = NULL;
} else if (IS_ERR(md->core_clk)) {
	return PTR_ERR(md->core_clk);
} else {
	rc = clk_prepare_enable(md->core_clk);
	...
}

Thanks
Arun