* [PATCH 1/1] designware: add clock support
@ 2012-10-07 13:03 Jean-Christophe PLAGNIOL-VILLARD
2012-10-08 9:03 ` Johannes Stezenbach
2012-10-10 9:03 ` Sascha Hauer
0 siblings, 2 replies; 6+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-07 13:03 UTC (permalink / raw)
To: barebox
allow the driver to request it's clock and enable it
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
drivers/net/designware.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index ec51825..1cbcd5a 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -32,8 +32,10 @@
#include <asm/mmu.h>
#include <net/designware.h>
#include <linux/phy.h>
-#include "designware.h"
+#include <linux/clk.h>
+#include <linux/err.h>
+#include "designware.h"
struct dw_eth_dev {
struct eth_device netdev;
@@ -390,6 +392,8 @@ static int dwc_ether_probe(struct device_d *dev)
struct mii_bus *miibus;
void __iomem *base;
struct dwc_ether_platform_data *pdata = dev->platform_data;
+ struct clk *clk;
+ int ret;
if (!pdata) {
printf("dwc_ether: no platform_data\n");
@@ -427,6 +431,20 @@ static int dwc_ether_probe(struct device_d *dev)
miibus->write = dwc_ether_mii_write;
miibus->priv = priv;
+ clk = clk_get(dev, NULL);
+ if (IS_ERR(clk)) {
+ ret = PTR_ERR(clk);
+ dev_err(dev, "no clk ret = %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_enable(clk);
+ if (ret) {
+ dev_err(dev, "can not enable clk ret = %d\n", ret);
+ clk_put(clk);
+ return ret;
+ }
+
mdiobus_register(miibus);
eth_register(edev);
return 0;
--
1.7.10.4
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] designware: add clock support
2012-10-07 13:03 [PATCH 1/1] designware: add clock support Jean-Christophe PLAGNIOL-VILLARD
@ 2012-10-08 9:03 ` Johannes Stezenbach
2012-10-08 9:27 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-10 9:03 ` Sascha Hauer
1 sibling, 1 reply; 6+ messages in thread
From: Johannes Stezenbach @ 2012-10-08 9:03 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox
On Sun, Oct 07, 2012 at 03:03:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> allow the driver to request it's clock and enable it
...
> + clk = clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + dev_err(dev, "no clk ret = %d\n", ret);
> + return ret;
> + }
Just a minor nit: s/allow/require/. Users of the driver may
now have to define a dummy clock. Since the actual clock freqeuency
is not used by the driver you could also make it optional?
And while at it, it's "its", not "it's". :-)
Johannes
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] designware: add clock support
2012-10-08 9:03 ` Johannes Stezenbach
@ 2012-10-08 9:27 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-08 10:00 ` Johannes Stezenbach
0 siblings, 1 reply; 6+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-08 9:27 UTC (permalink / raw)
To: Johannes Stezenbach; +Cc: barebox
On 11:03 Mon 08 Oct , Johannes Stezenbach wrote:
> On Sun, Oct 07, 2012 at 03:03:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > allow the driver to request it's clock and enable it
> ...
> > + clk = clk_get(dev, NULL);
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + dev_err(dev, "no clk ret = %d\n", ret);
> > + return ret;
> > + }
>
> Just a minor nit: s/allow/require/. Users of the driver may
> now have to define a dummy clock. Since the actual clock freqeuency
> is not used by the driver you could also make it optional?
>
> And while at it, it's "its", not "it's". :-)
srry but there is no user at hte mainline and I'm preparing the ST support
where I nned to enabld the clock and this IP require 2 clock one for the gmac
and one for phy
so no please provide a clock on generic drivers I do want to use the clk
framework
btw which SoC do you use because I see no code for it
Best Regards,
J.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] designware: add clock support
2012-10-08 9:27 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-10-08 10:00 ` Johannes Stezenbach
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Stezenbach @ 2012-10-08 10:00 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox
On Mon, Oct 08, 2012 at 11:27:06AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:03 Mon 08 Oct , Johannes Stezenbach wrote:
> > On Sun, Oct 07, 2012 at 03:03:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > allow the driver to request it's clock and enable it
> > ...
> > > + clk = clk_get(dev, NULL);
> > > + if (IS_ERR(clk)) {
> > > + ret = PTR_ERR(clk);
> > > + dev_err(dev, "no clk ret = %d\n", ret);
> > > + return ret;
> > > + }
> >
> > Just a minor nit: s/allow/require/. Users of the driver may
> > now have to define a dummy clock. Since the actual clock freqeuency
> > is not used by the driver you could also make it optional?
> >
> > And while at it, it's "its", not "it's". :-)
> srry but there is no user at hte mainline and I'm preparing the ST support
> where I nned to enabld the clock and this IP require 2 clock one for the gmac
> and one for phy
>
> so no please provide a clock on generic drivers I do want to use the clk
> framework
That is not the issue I tried to point out. The problem is that
your commit description is inaccurate.
> btw which SoC do you use because I see no code for it
SoC that doesn't exist yet (in development).
Johannes
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] designware: add clock support
2012-10-07 13:03 [PATCH 1/1] designware: add clock support Jean-Christophe PLAGNIOL-VILLARD
2012-10-08 9:03 ` Johannes Stezenbach
@ 2012-10-10 9:03 ` Sascha Hauer
2012-10-10 9:06 ` Jean-Christophe PLAGNIOL-VILLARD
1 sibling, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2012-10-10 9:03 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox
On Sun, Oct 07, 2012 at 03:03:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> allow the driver to request it's clock and enable it
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> drivers/net/designware.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index ec51825..1cbcd5a 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -32,8 +32,10 @@
> #include <asm/mmu.h>
> #include <net/designware.h>
> #include <linux/phy.h>
> -#include "designware.h"
> +#include <linux/clk.h>
> +#include <linux/err.h>
>
> +#include "designware.h"
>
> struct dw_eth_dev {
> struct eth_device netdev;
> @@ -390,6 +392,8 @@ static int dwc_ether_probe(struct device_d *dev)
> struct mii_bus *miibus;
> void __iomem *base;
> struct dwc_ether_platform_data *pdata = dev->platform_data;
> + struct clk *clk;
> + int ret;
>
> if (!pdata) {
> printf("dwc_ether: no platform_data\n");
> @@ -427,6 +431,20 @@ static int dwc_ether_probe(struct device_d *dev)
> miibus->write = dwc_ether_mii_write;
> miibus->priv = priv;
>
> + clk = clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + dev_err(dev, "no clk ret = %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_enable(clk);
> + if (ret) {
> + dev_err(dev, "can not enable clk ret = %d\n", ret);
> + clk_put(clk);
> + return ret;
> + }
I really doubt it makes sense to request clks in drivers just to enable
it in barebox. The clock ends up being enabled everytime anyway since
the clk enable is done in the probe function and is never disabled
again. This forces every user of this driver to provide the clk API for
absolutely no gain.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] designware: add clock support
2012-10-10 9:03 ` Sascha Hauer
@ 2012-10-10 9:06 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 6+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-10 9:06 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
On 11:03 Wed 10 Oct , Sascha Hauer wrote:
> On Sun, Oct 07, 2012 at 03:03:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > allow the driver to request it's clock and enable it
> >
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> > drivers/net/designware.c | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> > index ec51825..1cbcd5a 100644
> > --- a/drivers/net/designware.c
> > +++ b/drivers/net/designware.c
> > @@ -32,8 +32,10 @@
> > #include <asm/mmu.h>
> > #include <net/designware.h>
> > #include <linux/phy.h>
> > -#include "designware.h"
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> >
> > +#include "designware.h"
> >
> > struct dw_eth_dev {
> > struct eth_device netdev;
> > @@ -390,6 +392,8 @@ static int dwc_ether_probe(struct device_d *dev)
> > struct mii_bus *miibus;
> > void __iomem *base;
> > struct dwc_ether_platform_data *pdata = dev->platform_data;
> > + struct clk *clk;
> > + int ret;
> >
> > if (!pdata) {
> > printf("dwc_ether: no platform_data\n");
> > @@ -427,6 +431,20 @@ static int dwc_ether_probe(struct device_d *dev)
> > miibus->write = dwc_ether_mii_write;
> > miibus->priv = priv;
> >
> > + clk = clk_get(dev, NULL);
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + dev_err(dev, "no clk ret = %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = clk_enable(clk);
> > + if (ret) {
> > + dev_err(dev, "can not enable clk ret = %d\n", ret);
> > + clk_put(clk);
> > + return ret;
> > + }
>
> I really doubt it makes sense to request clks in drivers just to enable
> it in barebox. The clock ends up being enabled everytime anyway since
> the clk enable is done in the probe function and is never disabled
> again. This forces every user of this driver to provide the clk API for
> absolutely no gain.
I'm working on the phy and on specific part no ready yet but I net the rate
too as this IP support specific rate on SH4
Best Regards,
J.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-10 9:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-07 13:03 [PATCH 1/1] designware: add clock support Jean-Christophe PLAGNIOL-VILLARD
2012-10-08 9:03 ` Johannes Stezenbach
2012-10-08 9:27 ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-08 10:00 ` Johannes Stezenbach
2012-10-10 9:03 ` Sascha Hauer
2012-10-10 9:06 ` Jean-Christophe PLAGNIOL-VILLARD
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox