Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hello. Felipe Balbi wrote: /* * Find the correct struct clk for the device and connection ID. * We do slightly fuzzy matching here: * An entry with a NULL ID is assumed to be a wildcard. * If an entry has a device ID, it must match * If an entry has a connection ID, it must match * Then we take the most specific entry - with the following * order of precidence: dev+con > dev only > con only. */ and then we have found our clock :-) Hm, you're right, I haven't completely followed the logic there... in any case, specifying con_id wasn't really needed. good we've sorted things out. Want me to send a patch for the device id to davinci tree or can you take care of that ? I have a for-kevin branch with a few patches I'll send to davinci tree so I could add the device id patch there. Go ahead, I'm actually ill now. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hi, On Wed, 2009-12-30 at 16:43 +0300, Sergei Shtylyov wrote: > > /* > > * Find the correct struct clk for the device and connection ID. > > * We do slightly fuzzy matching here: > > * An entry with a NULL ID is assumed to be a wildcard. > > * If an entry has a device ID, it must match > > * If an entry has a connection ID, it must match > > * Then we take the most specific entry - with the following > > * order of precidence: dev+con > dev only > con only. > > */ > > > > and then we have found our clock :-) > >Hm, you're right, I haven't completely followed the logic there... in > any case, specifying con_id wasn't really needed. good we've sorted things out. Want me to send a patch for the device id to davinci tree or can you take care of that ? I have a for-kevin branch with a few patches I'll send to davinci tree so I could add the device id patch there. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hello. Felipe Balbi wrote: Gotcha. This will match in clk_find() by both device and clock name, so clk_get() will fail with your patch. then, when it doesn't find, it'll try device id only: /* * Find the correct struct clk for the device and connection ID. * We do slightly fuzzy matching here: * An entry with a NULL ID is assumed to be a wildcard. * If an entry has a device ID, it must match * If an entry has a connection ID, it must match * Then we take the most specific entry - with the following * order of precidence: dev+con > dev only > con only. */ and then we have found our clock :-) Hm, you're right, I haven't completely followed the logic there... in any case, specifying con_id wasn't really needed. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
On Wed, 2009-12-30 at 16:25 +0300, Sergei Shtylyov wrote: >Gotcha. This will match in clk_find() by both device and clock name, > so clk_get() will fail with your patch. then, when it doesn't find, it'll try device id only: /* * Find the correct struct clk for the device and connection ID. * We do slightly fuzzy matching here: * An entry with a NULL ID is assumed to be a wildcard. * If an entry has a device ID, it must match * If an entry has a connection ID, it must match * Then we take the most specific entry - with the following * order of precidence: dev+con > dev only > con only. */ and then we have found our clock :-) -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hello.
Felipe Balbi wrote:
Why always "ick"?! Do you think OMAPs are the only boards using MUSB
out there?
err, I'm afraid you have to read more about clock framework. The clock
name should not matter when you issue clk_get().
It depends on the matching type determined from a clkdev matching
table -- see dm644x_clks[] in arch/mach-davinci/dm644x.c as an example:
'struct davinci_clk' incorporates 'struct clk_lookup', and where its 1st
member is initialized, the matching is done by device, where its second
member is initialized, the matching is done by clock name -- see
clk_find() in arch/arm/common/clkdev.c for the logic.
diff --git a/arch/arm/mach-davinci/dm644x.c
b/arch/arm/mach-davinci/dm644x.c
index 2cd0081..a0ad7b6 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -306,7 +306,7 @@ struct davinci_clk dm644x_clks[] = {
CLK("davinci_mmc.0", NULL, &mmcsd_clk),
CLK(NULL, "spi", &spi_clk),
CLK(NULL, "gpio", &gpio_clk),
- CLK(NULL, "usb", &usb_clk),
+ CLK("musb_hdrc", "usb", &usb_clk),
CLK(NULL, "vlynq", &vlynq_clk),
CLK(NULL, "aemif", &aemif_clk),
CLK(NULL, "pwm0", &pwm0_clk),
there you are...
Gotcha. This will match in clk_find() by both device and clock name,
so clk_get() will fail with your patch.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hi,
adding Russell to Cc list as he might have comments.
On Wed, 2009-12-30 at 15:14 +0200, Felipe Balbi wrote:
> Hi,
>
> On Wed, 2009-12-30 at 16:06 +0300, Sergei Shtylyov wrote:
> > Hello.
> >
> > Felipe Balbi wrote:
> >
> > >>Why always "ick"?! Do you think OMAPs are the only boards using MUSB
> > >> out there?
> > >>
> > >
> > > err, I'm afraid you have to read more about clock framework. The clock
> > > name should not matter when you issue clk_get().
> >
> >It depends on the matching type determined from a clkdev matching
> > table -- see dm644x_clks[] in arch/mach-davinci/dm644x.c as an example:
> > 'struct davinci_clk' incorporates 'struct clk_lookup', and where its 1st
> > member is initialized, the matching is done by device, where its second
> > member is initialized, the matching is done by clock name -- see
> > clk_find() in arch/arm/common/clkdev.c for the logic.
>
> diff --git a/arch/arm/mach-davinci/dm644x.c
> b/arch/arm/mach-davinci/dm644x.c
> index 2cd0081..a0ad7b6 100644
> --- a/arch/arm/mach-davinci/dm644x.c
> +++ b/arch/arm/mach-davinci/dm644x.c
> @@ -306,7 +306,7 @@ struct davinci_clk dm644x_clks[] = {
> CLK("davinci_mmc.0", NULL, &mmcsd_clk),
> CLK(NULL, "spi", &spi_clk),
> CLK(NULL, "gpio", &gpio_clk),
> - CLK(NULL, "usb", &usb_clk),
> + CLK("musb_hdrc", "usb", &usb_clk),
> CLK(NULL, "vlynq", &vlynq_clk),
> CLK(NULL, "aemif", &aemif_clk),
> CLK(NULL, "pwm0", &pwm0_clk),
>
>
> there you are...
and the point is that we want to stop passing clock names down to the
driver so either we agree on using a generic name like "ick" or "usb",
or the clockdev implementation for that particular clock will match
device id. We sure need to think about clocks used by several devices,
we want to keep device id NULL, for sure.
Russell, how should we tackle this ?
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hi,
On Wed, 2009-12-30 at 16:06 +0300, Sergei Shtylyov wrote:
> Hello.
>
> Felipe Balbi wrote:
>
> >>Why always "ick"?! Do you think OMAPs are the only boards using MUSB
> >> out there?
> >>
> >
> > err, I'm afraid you have to read more about clock framework. The clock
> > name should not matter when you issue clk_get().
>
>It depends on the matching type determined from a clkdev matching
> table -- see dm644x_clks[] in arch/mach-davinci/dm644x.c as an example:
> 'struct davinci_clk' incorporates 'struct clk_lookup', and where its 1st
> member is initialized, the matching is done by device, where its second
> member is initialized, the matching is done by clock name -- see
> clk_find() in arch/arm/common/clkdev.c for the logic.
diff --git a/arch/arm/mach-davinci/dm644x.c
b/arch/arm/mach-davinci/dm644x.c
index 2cd0081..a0ad7b6 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -306,7 +306,7 @@ struct davinci_clk dm644x_clks[] = {
CLK("davinci_mmc.0", NULL, &mmcsd_clk),
CLK(NULL, "spi", &spi_clk),
CLK(NULL, "gpio", &gpio_clk),
- CLK(NULL, "usb", &usb_clk),
+ CLK("musb_hdrc", "usb", &usb_clk),
CLK(NULL, "vlynq", &vlynq_clk),
CLK(NULL, "aemif", &aemif_clk),
CLK(NULL, "pwm0", &pwm0_clk),
there you are...
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hello. Felipe Balbi wrote: Why always "ick"?! Do you think OMAPs are the only boards using MUSB out there? err, I'm afraid you have to read more about clock framework. The clock name should not matter when you issue clk_get(). It depends on the matching type determined from a clkdev matching table -- see dm644x_clks[] in arch/mach-davinci/dm644x.c as an example: 'struct davinci_clk' incorporates 'struct clk_lookup', and where its 1st member is initialized, the matching is done by device, where its second member is initialized, the matching is done by clock name -- see clk_find() in arch/arm/common/clkdev.c for the logic. I used the shortest name that came to my mind. could've been "usb", "usb_clock", "musb_otg_default_clock". It doesn't matter. I know for sure that it still *can* matter with clkdev, and I do use matching by clock name with DA8xx/OMAP-L1x glue layer. So I' strongly against this patch. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hi, On Wed, 2009-12-30 at 14:18 +0300, Sergei Shtylyov wrote: >Why always "ick"?! Do you think OMAPs are the only boards using MUSB > out there? err, I'm afraid you have to read more about clock framework. The clock name should not matter when you issue clk_get(). I used the shortest name that came to my mind. could've been "usb", "usb_clock", "musb_otg_default_clock". It doesn't matter. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
Hello.
Felipe Balbi wrote:
get rid of the set_clock hackery.
Cc: linux-...@vger.kernel.org
Signed-off-by: Felipe Balbi
---
Greg, I'll send this patch to you shortly, it's
only here so that Tony can have it in linux-omap-2.6
to avoid breaking any of his boards.
Er, I'm afraid I have to NAK this patch...
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 5eb9318..37ca87d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -977,10 +977,8 @@ static void musb_shutdown(struct platform_device *pdev)
spin_lock_irqsave(&musb->lock, flags);
musb_platform_disable(musb);
musb_generic_disable(musb);
- if (musb->clock) {
- clk_put(musb->clock);
- musb->clock = NULL;
- }
+ clk_put(musb->clock);
+ musb->clock = NULL;
spin_unlock_irqrestore(&musb->lock, flags);
/* FIXME power down */
@@ -1864,10 +1862,8 @@ static void musb_free(struct musb *musb)
musb_platform_exit(musb);
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
- if (musb->clock) {
- clk_disable(musb->clock);
- clk_put(musb->clock);
- }
+ clk_disable(musb->clock);
+ clk_put(musb->clock);
#ifdef CONFIG_USB_MUSB_HDRC_HCD
usb_put_hcd(musb_to_hcd(musb));
@@ -1930,7 +1926,6 @@ bad_config:
spin_lock_init(&musb->lock);
musb->board_mode = plat->mode;
musb->board_set_power = plat->set_power;
- musb->set_clock = plat->set_clock;
musb->min_power = plat->min_power;
/* Clock usage is chip-specific ... functional clock (DaVinci,
@@ -1938,15 +1933,17 @@ bad_config:
* code does is make sure a clock handle is available; platform
* code manages it during start/stop and suspend/resume.
*/
- if (plat->clock) {
- musb->clock = clk_get(dev, plat->clock);
- if (IS_ERR(musb->clock)) {
- status = PTR_ERR(musb->clock);
- musb->clock = NULL;
- goto fail;
- }
+ musb->clock = clk_get(dev, "ick");
Why always "ick"?! Do you think OMAPs are the only boards using MUSB
out there?
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch-v2.6.34 7/9] usb: musb: use only clk framework for clk handling
get rid of the set_clock hackery.
Cc: linux-...@vger.kernel.org
Signed-off-by: Felipe Balbi
---
Greg, I'll send this patch to you shortly, it's
only here so that Tony can have it in linux-omap-2.6
to avoid breaking any of his boards.
drivers/usb/musb/musb_core.c | 48 ++
drivers/usb/musb/musb_core.h |2 -
drivers/usb/musb/omap2430.c | 10 +---
drivers/usb/musb/tusb6010.c | 16 +
include/linux/usb/musb.h |4 +-
5 files changed, 26 insertions(+), 54 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 5eb9318..37ca87d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -977,10 +977,8 @@ static void musb_shutdown(struct platform_device *pdev)
spin_lock_irqsave(&musb->lock, flags);
musb_platform_disable(musb);
musb_generic_disable(musb);
- if (musb->clock) {
- clk_put(musb->clock);
- musb->clock = NULL;
- }
+ clk_put(musb->clock);
+ musb->clock = NULL;
spin_unlock_irqrestore(&musb->lock, flags);
/* FIXME power down */
@@ -1864,10 +1862,8 @@ static void musb_free(struct musb *musb)
musb_platform_exit(musb);
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
- if (musb->clock) {
- clk_disable(musb->clock);
- clk_put(musb->clock);
- }
+ clk_disable(musb->clock);
+ clk_put(musb->clock);
#ifdef CONFIG_USB_MUSB_HDRC_HCD
usb_put_hcd(musb_to_hcd(musb));
@@ -1930,7 +1926,6 @@ bad_config:
spin_lock_init(&musb->lock);
musb->board_mode = plat->mode;
musb->board_set_power = plat->set_power;
- musb->set_clock = plat->set_clock;
musb->min_power = plat->min_power;
/* Clock usage is chip-specific ... functional clock (DaVinci,
@@ -1938,15 +1933,17 @@ bad_config:
* code does is make sure a clock handle is available; platform
* code manages it during start/stop and suspend/resume.
*/
- if (plat->clock) {
- musb->clock = clk_get(dev, plat->clock);
- if (IS_ERR(musb->clock)) {
- status = PTR_ERR(musb->clock);
- musb->clock = NULL;
- goto fail;
- }
+ musb->clock = clk_get(dev, "ick");
+ if (IS_ERR(musb->clock)) {
+ status = PTR_ERR(musb->clock);
+ musb->clock = NULL;
+ goto fail;
}
+ status = clk_enable(musb->clock);
+ if (status < 0)
+ goto fail;
+
/* The musb_platform_init() call:
* - adjusts musb->mregs and musb->isr if needed,
* - may initialize an integrated tranceiver
@@ -2104,8 +2101,7 @@ fail:
dev_err(musb->controller,
"musb_init_controller failed with status %d\n", status);
- if (musb->clock)
- clk_put(musb->clock);
+ clk_put(musb->clock);
device_init_wakeup(dev, 0);
musb_free(musb);
@@ -2179,9 +2175,6 @@ static int musb_suspend(struct device *dev)
unsigned long flags;
struct musb *musb = dev_to_musb(&pdev->dev);
- if (!musb->clock)
- return 0;
-
spin_lock_irqsave(&musb->lock, flags);
if (is_peripheral_active(musb)) {
@@ -2194,10 +2187,7 @@ static int musb_suspend(struct device *dev)
*/
}
- if (musb->set_clock)
- musb->set_clock(musb->clock, 0);
- else
- clk_disable(musb->clock);
+ clk_disable(musb->clock);
spin_unlock_irqrestore(&musb->lock, flags);
return 0;
}
@@ -2207,13 +2197,7 @@ static int musb_resume_noirq(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct musb *musb = dev_to_musb(&pdev->dev);
- if (!musb->clock)
- return 0;
-
- if (musb->set_clock)
- musb->set_clock(musb->clock, 1);
- else
- clk_enable(musb->clock);
+ clk_enable(musb->clock);
/* for static cmos like DaVinci, register values were preserved
* unless for some reason the whole soc powered down or the USB
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 03d5090..3816d79 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -394,8 +394,6 @@ struct musb {
u8 board_mode; /* enum musb_mode */
int (*board_set_power)(int state);
- int (*set_clock)(struct clk *clk, int is_active);
-
u8 min_power; /* vbus for periph, in mA/2 */
boolis_host;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 83beeac..b8e2451 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
