Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752750AbaA2HAt (ORCPT ); Wed, 29 Jan 2014 02:00:49 -0500 Received: from mail-ob0-f171.google.com ([209.85.214.171]:58402 "EHLO mail-ob0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750838AbaA2HAr (ORCPT ); Wed, 29 Jan 2014 02:00:47 -0500 Message-ID: <52E8A74F.9060603@gmail.com> Date: Tue, 28 Jan 2014 23:01:35 -0800 From: Florian Fainelli User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Max Filippov , linux-xtensa@linux-xtensa.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org CC: Chris Zankel , Marc Gauthier , "David S. Miller" , Ben Hutchings Subject: Re: [PATCH v2 3/4] net: ethoc: set up MII management bus clock References: <1390975218-13863-1-git-send-email-jcmvbkbc@gmail.com> <1390975218-13863-4-git-send-email-jcmvbkbc@gmail.com> In-Reply-To: <1390975218-13863-4-git-send-email-jcmvbkbc@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 28/01/2014 22:00, Max Filippov a ?crit : > MII management bus clock is derived from the MAC clock by dividing it by > MIIMODER register CLKDIV field value. This value may need to be set up > in case it is undefined or its default value is too high (and > communication with PHY is too slow) or too low (and communication with > PHY is impossible). The value of CLKDIV is not specified directly, but > is derived from the MAC clock for the default MII management bus frequency > of 2.5MHz. The MAC clock may be specified in the platform data, or as > either 'clock-frequency' or 'clocks' device tree attribute. > > Signed-off-by: Max Filippov > --- > Changes v1->v2: > - drop MDIO bus frequency configurability, always configure for standard > 2.5MHz; > - allow using common clock framework to provide ethoc clock. > > drivers/net/ethernet/ethoc.c | 37 +++++++++++++++++++++++++++++++++++-- > include/net/ethoc.h | 1 + > 2 files changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c > index 5643b2d..5854d41 100644 > --- a/drivers/net/ethernet/ethoc.c > +++ b/drivers/net/ethernet/ethoc.c > @@ -13,6 +13,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -216,6 +217,7 @@ struct ethoc { > > struct phy_device *phy; > struct mii_bus *mdio; > + struct clk *clk; > s8 phy_id; > }; > > @@ -925,6 +927,8 @@ static int ethoc_probe(struct platform_device *pdev) > int num_bd; > int ret = 0; > bool random_mac = false; > + u32 eth_clkfreq = 0; > + struct ethoc_platform_data *pdata = dev_get_platdata(&pdev->dev); > > /* allocate networking device */ > netdev = alloc_etherdev(sizeof(struct ethoc)); > @@ -1038,8 +1042,7 @@ static int ethoc_probe(struct platform_device *pdev) > } > > /* Allow the platform setup code to pass in a MAC address. */ > - if (dev_get_platdata(&pdev->dev)) { > - struct ethoc_platform_data *pdata = dev_get_platdata(&pdev->dev); > + if (pdata) { > memcpy(netdev->dev_addr, pdata->hwaddr, IFHWADDRLEN); > priv->phy_id = pdata->phy_id; > } else { > @@ -1077,6 +1080,32 @@ static int ethoc_probe(struct platform_device *pdev) > if (random_mac) > netdev->addr_assign_type = NET_ADDR_RANDOM; > > + /* Allow the platform setup code to adjust MII management bus clock. */ > + if (pdata) > + eth_clkfreq = pdata->eth_clkfreq; Since this is a new member, why not make it a struct clk pointer directly so you could simplify the code path? > + else > + of_property_read_u32(pdev->dev.of_node, > + "clock-frequency", ð_clkfreq); > + if (!eth_clkfreq) { > + struct clk *clk = clk_get(&pdev->dev, NULL); > + > + if (!IS_ERR(clk)) { > + priv->clk = clk; > + clk_prepare_enable(clk); > + eth_clkfreq = clk_get_rate(clk); > + } > + } > + if (eth_clkfreq) { > + u32 clkdiv = MIIMODER_CLKDIV(eth_clkfreq / 2500000 + 1); > + > + if (!clkdiv) > + clkdiv = 2; > + dev_dbg(&pdev->dev, "setting MII clkdiv to %u\n", clkdiv); > + ethoc_write(priv, MIIMODER, > + (ethoc_read(priv, MIIMODER) & MIIMODER_NOPRE) | > + clkdiv); > + } This does look a bit convoluted, and it looks like the clk_get() or getting the clock-frequency property should boil down to being the same thing with of_clk_get() as it should resolve all clocks phandles and fetch their frequencies appropriately. > + > /* register MII bus */ > priv->mdio = mdiobus_alloc(); > if (!priv->mdio) { > @@ -1141,6 +1170,8 @@ free_mdio: > kfree(priv->mdio->irq); > mdiobus_free(priv->mdio); > free: > + if (priv->clk) > + clk_disable_unprepare(priv->clk); This should probbaly be if (!IS_ERR(priv-clk)). > free_netdev(netdev); > out: > return ret; > @@ -1165,6 +1196,8 @@ static int ethoc_remove(struct platform_device *pdev) > kfree(priv->mdio->irq); > mdiobus_free(priv->mdio); > } > + if (priv->clk) > + clk_disable_unprepare(priv->clk); Same here > unregister_netdev(netdev); > free_netdev(netdev); > } > diff --git a/include/net/ethoc.h b/include/net/ethoc.h > index 96f3789..2a2d6bb 100644 > --- a/include/net/ethoc.h > +++ b/include/net/ethoc.h > @@ -16,6 +16,7 @@ > struct ethoc_platform_data { > u8 hwaddr[IFHWADDRLEN]; > s8 phy_id; > + u32 eth_clkfreq; > }; > > #endif /* !LINUX_NET_ETHOC_H */ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/