Re: [PATCH v3] sh_eth: add device tree support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hello.

On 02/11/2014 07:08 PM, Laurent Pinchart wrote:

Thanks a lot for the patch. DT support for sh-eth was number one on my most
wanted list :-)

   It's been available for several months already (though just for BOCK-W).
Since the clock DT support turned to be the requirement, I had to change to Lager/Koelsch where CCF is already available.

Please see below for two minor comments.

   Thanks.
Sigh, I had to scroll thru quite a lot of uncommented text to find them. Please trim such text in the future to save the readers' time.

Add support of the device tree probing for the Renesas SH-Mobile SoCs
documenting the device tree binding as necessary.

This work is loosely based on the original patch by Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@xxxxxxxxxxx>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>

[...]

Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net-next/drivers/net/ethernet/renesas/sh_eth.c
[...]
@@ -2710,6 +2714,56 @@ static const struct net_device_ops sh_et
  	.ndo_change_mtu		= eth_change_mtu,
  };

+#ifdef CONFIG_OF
+static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct sh_eth_plat_data *pdata;
+	struct device_node *phy;
+	const char *mac_addr;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->phy_interface = of_get_phy_mode(np);
+
+	phy = of_parse_phandle(np, "phy-handle", 0);
+	if (of_property_read_u32(phy, "reg", &pdata->phy)) {
+		devm_kfree(dev, pdata);

No need to free memory here, this will be handled by the core after the
probe() function fails.

   OK, we should fail the probe when we return NULL from here.

+		return NULL;
+	}
[...]
@@ -2778,7 +2834,19 @@ static int sh_eth_drv_probe(struct platf
  	mdp->ether_link_active_low = pd->ether_link_active_low;

  	/* set cpu data */
-	mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
+	if (id) {
+		mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
+	} else	{
+		const struct of_device_id *match;
+
+		match = of_match_device(of_match_ptr(sh_eth_match_table),
+					&pdev->dev);
+		if (!match) {
+			ret = -EINVAL;
+			goto out_release;
+		}

You can probably remove error checking, if no match is found the probe
function wouldn't have been called in the first place.

   Yes, you seem to be correct here as well.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux