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
Attachment:
signature.asc
Description: Digital signature
