linux-kernel - Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

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: <20130618093829.GP5461@arwen.pp.htv.fi>
Date:	Tue, 18 Jun 2013 12:38:29 +0300
From:	Felipe Balbi <balbi@...com>
To:	Kishon Vijay Abraham I <kishon@...com>
CC:	<grant.likely@...aro.org>, <tony@...mide.com>, <balbi@...com>,
	<arnd@...db.de>, <swarren@...dia.com>,
	<sylvester.nawrocki@...il.com>, <linux-kernel@...r.kernel.org>,
	<linux-omap@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-usb@...r.kernel.org>, <gregkh@...uxfoundation.org>,
	<akpm@...ux-foundation.org>, <rob.herring@...xeda.com>,
	<rob@...dley.net>, <b-cousson@...com>, <linux@....linux.org.uk>,
	<benoit.cousson@...aro.org>, <mchehab@...hat.com>,
	<cesarb@...arb.net>, <davem@...emloft.net>, <rnayak@...com>,
	<shawn.guo@...aro.org>, <santosh.shilimkar@...com>,
	<devicetree-discuss@...ts.ozlabs.org>, <linux-doc@...r.kernel.org>,
	<nsekhar@...com>
Subject: Re: [PATCH v7 1/9] drivers: phy: add generic PHY framework

Hi,

On Thu, Jun 13, 2013 at 02:13:51PM +0530, Kishon Vijay Abraham I wrote:
> +struct phy_provider *of_phy_provider_register(struct device *dev,
> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> +	struct of_phandle_args *args))

I would rename this to __of_phy_provider_register() and in a header
have:

#define of_phy_provider_register(dev, xlate)	\
	__of_phy_provider_register((dev), THIS_MODULE, (xlate))

then your users don't need to remember always passing THIS_MODULE
argument.

> +struct phy_provider *devm_of_phy_provider_register(struct device *dev,
> +	struct module *owner, struct phy * (*of_xlate)(struct device *dev,
> +	struct of_phandle_args *args))

likewise.

> +/**
> + * phy_release() - release the phy
> + * @dev: the dev member within phy
> + *
> + * When the last reference to the device is removed, it is called
> + * from the embedded kobject as release method.
> + */
> +static void phy_release(struct device *dev)
> +{
> +	struct phy *phy;
> +
> +	phy = container_of(dev, struct phy, dev);

I would have a:

#define to_phy(dev)	(container_of((dev), struct phy dev))

somewhere in a header, just for the sake of brevity :-p

> +	dev_dbg(dev, "releasing '%s'\n", dev_name(dev));

make it dev_vdbg() instead.

> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> new file mode 100644
> index 0000000..df0c98a
> --- /dev/null
> +++ b/include/linux/phy/phy.h

[ ... ]

> +static inline int phy_init(struct phy *phy)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	if (!pm_runtime_enabled(&phy->dev))
> +		goto no_pm_runtime;
> +
> +	ret = pm_runtime_get_sync(&phy->dev);
> +	if (ret < 0) {
> +		dev_err(&phy->dev, "get sync failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +no_pm_runtime:

you can avoid this goto if you code as below, note that I'm also solving
a possible unbalanced PM runtime call which would prevent actual
*runtime* suspend of the PHY:

	ret = phy_pm_runtime_get_sync(phy);
	if (ret < 0 && ret != -ENOTSUPP)
		return ret;

	if (phy->ops->init) {
		ret = phy->ops->init(phy);
		if (ret < 0) {
			dev_err(&phy->dev, "failed to initialize --> %d\n", ret);
			goto out;
		}
	}

out:
	/*
	 * In order to avoid unbalanced PM runtime calls, let's make
	 * sure to disable what we might have enabled when entering this
	 * function.
	 */
	 phy_pm_runtime_put(phy);

	return ret;


> +}
> +
> +static inline int phy_exit(struct phy *phy)
> +{
> +	int ret = -ENOTSUPP;
> +
> +	if (phy->ops->exit)
> +		ret = phy->ops->exit(phy);
> +
> +	if (!pm_runtime_enabled(&phy->dev))
> +		goto no_pm_runtime;
> +
> +	ret = pm_runtime_put_sync(&phy->dev);

move your pm runtime wrappers before these calls and you can use them.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)