On Wed, 2012-09-26 at 20:31 +0530, Kishon Vijay Abraham I wrote:

The PHY framework provides a set of API's for the PHY drivers to
create/destroy a PHY and API's 

Just some trivial notes.

[]

[]

+static void devm_phy_release(struct device *dev, void *res)
+{
+	struct phy *phy = *(struct phy **)res;

That's a bit twisted isn't it?
Perhaps
	struct phy *phy = res;

[]

+static int devm_phy_match(struct device *dev, void *res, void *match_data)
+{
+	return res == match_data;

static bool devm_phy_match(etc...)

[]

+struct phy *devm_of_phy_get(struct device *dev, const char *phandle, u8 index)
+{
+	struct phy *phy = NULL, **ptr;
+	struct device_node *node;
+
+	if (!dev->of_node) {
+		dev_dbg(dev, "device does not have a device node entry\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	node = of_parse_phandle(dev->of_node, phandle, index);
+	if (!node) {
+		dev_dbg(dev, "failed to get %s phandle in %s node\n", phandle,
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);

Is this the right size?

Because ptr is **, perhaps sizeof(struct phy) is clearer.

+	if (!ptr) {
+		dev_dbg(dev, "failed to allocate memory for devres\n");

alloc failures generally don't need specific OOM messages
as the general alloc OOM dumps stack.

+		return ERR_PTR(-ENOMEM);
+	}
+
+	mutex_lock(&phy_list_mutex);
+
+	phy = of_phy_lookup(dev, node);
+	if (IS_ERR(phy) || !phy->dev.class ||
+			!try_module_get(phy->dev.class->owner)) {

I think it's tasteful coding style to align like:

	if (IS_ERR(phy) || !phy->dev.class ||
	    !try_module_get(phy->dev.class->owner)) {
[]

+struct phy *phy_create(struct device *dev, struct phy_descriptor *desc)
+{
+	int ret;
+	struct phy *phy;
+	struct phy_bind *phy_bind;
+	const char *devname = NULL;
+
+	if (!dev || !desc) {
+		dev_err(dev, "no descriptor/device provided for PHY\n");
+		ret = -EINVAL;
+		goto err0;
+	}
+
+	if (!desc->ops) {
+		dev_err(dev, "no PHY ops provided\n");
+		ret = -EINVAL;
+		goto err0;
+	}
+
+	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+	if (!phy) {
+		dev_err(dev, "no memory for PHY\n");

No OOM message required...

[]

+static int __init phy_core_init(void)
+{
+	phy_class = class_create(THIS_MODULE, "phy");
+	if (IS_ERR(phy_class)) {
+		pr_err("failed to create phy class --> %ld\n",
+			PTR_ERR(phy_class));

You might add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
before any include so that pr_<levels> are prefixed.

cheers, Joe