HAVE_CLK (for <linux/clk.h> support)

HAVE_CLK (for <linux/clk.h> support)

[David Brownell]

On Tuesday 27 May 2008, Felipe Balbi wrote:
> This patch adds support for MUSB and TUSB controllers
> integrated into omap2430 and davinci. It also adds support
> for external tusb6010 controller.

The most significant issue I know of with this patch [0] is actually
an arch issue.  On ones like x86, with no support for the clock API,
this driver will build but not link:

ERROR: "clk_enable" [drivers/usb/musb/musb_hdrc.ko] undefined!
ERROR: "clk_disable" [drivers/usb/musb/musb_hdrc.ko] undefined!
ERROR: "clk_put" [drivers/usb/musb/musb_hdrc.ko] undefined!
ERROR: "clk_get" [drivers/usb/musb/musb_hdrc.ko] undefined!

I don't know of any other drivers with this particular issue,
but nonetheless ... I think the following is an appropriate
resolution.

Comments from anyone else?

- Dave

[0] Posted this AM on linux-usb but evidently too big for any
    of the archives I know about ... 450 KB, which includes
    host, gadget, and OTG functionality.  Previous versions
    are archived, in the one-file-added-per-patch style.


--- g26.orig/drivers/usb/musb/Kconfig	2008-05-27 16:02:41.000000000 -0700
+++ g26/drivers/usb/musb/Kconfig	2008-05-27 16:01:09.000000000 -0700
@@ -8,7 +8,7 @@ comment "Enable Host or Gadget support t
 
 # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
 config USB_MUSB_HDRC
-	depends on USB || USB_GADGET
+	depends on HAVE_CLK && (USB || USB_GADGET)
 	select TWL4030_USB if MACH_OMAP_3430SDP
 	tristate 'Inventra Highspeed Dual Role Controller (TI, ...)'
 	help
--- g26.orig/arch/Kconfig	2008-05-27 16:00:14.000000000 -0700
+++ g26/arch/Kconfig	2008-05-27 16:00:41.000000000 -0700
@@ -39,3 +39,6 @@ config HAVE_KRETPROBES
 
 config HAVE_DMA_ATTRS
 	def_bool n
+
+config HAVE_CLK
+	def_bool n
--- g26.orig/arch/arm/Kconfig	2008-05-27 16:01:15.000000000 -0700
+++ g26/arch/arm/Kconfig	2008-05-27 16:02:33.000000000 -0700
@@ -458,12 +458,14 @@ config ARCH_DAVINCI
 	select GENERIC_TIME
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_GPIO
+	select HAVE_CLK
 	help
 	  Support for TI's DaVinci platform.
 
 config ARCH_OMAP
 	bool "TI OMAP"
 	select GENERIC_GPIO
+	select HAVE_CLK
 	select HAVE_GPIO_LIB
 	select GENERIC_TIME
 	select GENERIC_CLOCKEVENTS

Re: HAVE_CLK (for <linux/clk.h> support)

[Felipe Balbi]

Hi,

On Wed, May 28, 2008 at 2:14 AM, David Brownell <david-b@pacbell.net> wrote:
> On Tuesday 27 May 2008, Felipe Balbi wrote:
>> This patch adds support for MUSB and TUSB controllers
>> integrated into omap2430 and davinci. It also adds support
>> for external tusb6010 controller.
>
> The most significant issue I know of with this patch [0] is actually
> an arch issue.  On ones like x86, with no support for the clock API,
> this driver will build but not link:
>
> ERROR: "clk_enable" [drivers/usb/musb/musb_hdrc.ko] undefined!
> ERROR: "clk_disable" [drivers/usb/musb/musb_hdrc.ko] undefined!
> ERROR: "clk_put" [drivers/usb/musb/musb_hdrc.ko] undefined!
> ERROR: "clk_get" [drivers/usb/musb/musb_hdrc.ko] undefined!
>
> I don't know of any other drivers with this particular issue,
> but nonetheless ... I think the following is an appropriate
> resolution.
>
> Comments from anyone else?

I actually like it. I didn't see this error cuz I was only building
musb for arm architecture.

But why only davinci and omap?
What about: mach-aaec2000, mach-at91, mach-ep93xx, mach-integrator,
mach-lh7a40x, mach-ns9xxx, mach-pnx4008, mach-pxa, mach-realview,
mach-sa1100, mach-versatile, mach-s3c24x.


-- 
Best Regards,

Felipe Balbi
felipebalbi@users.sourceforge.net

Re: HAVE_CLK (for <linux/clk.h> support)

[David Brownell]

On Tuesday 27 May 2008, Felipe Balbi wrote:
> >
> > I don't know of any other drivers with this particular issue,
> > but nonetheless ... I think the following is an appropriate
> > resolution.
> >
> > Comments from anyone else?
> 
> I actually like it. I didn't see this error cuz I was only building
> musb for arm architecture.
> 
> But why only davinci and omap?

They're the only one affected by the musb_hdrc patch...


> What about: mach-aaec2000, mach-at91, mach-ep93xx, mach-integrator,
> mach-lh7a40x, mach-ns9xxx, mach-pnx4008, mach-pxa, mach-realview,
> mach-sa1100, mach-versatile, mach-s3c24x.

If this approach goes forward, I'd expect those platforms
to "select HAVE_CLK" too.  Ditto some SH, PowerPC, etc.


> Instead of making the driver dependant on HAVE_CLK (which would be
> true for virtually any driver comming from linux-omap), you could make
> clk_enable a nop in case arch doesn't HAVE_CLK. How does that sound?

Best done by #ifdef HAVE_CLK in <linux/clk.h> support, and
putting the stubs there.  However, since those platforms
don't actually have clocks, I'd make them all report errors
of whichever kind.

That's kind of a second-order question to "should we create
a HAVE_CLK in Kconfig".  Since Certain People have objected
to patches touching <linux/clk.h> I avoided doing that the
first time around ... it'd be a better solution, yes.

- Dave

Re: HAVE_CLK (for <linux/clk.h> support)

[Bryan Wu]

Sorry for jumping in so later. I noticed Blackfin does not support
HAVE_CLK currently.
As the HAVE_CLK dependence is included in the kernel mainline, MUSB
parts do not show up in the Kconfig because of lacking of HAVE_CLK.

To implement clk infrastructure in Blackfin will be later than merging
MUSB Blackfin parts to upstream.

Any idea?

Thanks
-Bryan

On Wed, May 28, 2008 at 9:41 AM, David Brownell <david-b@pacbell.net> wrote:
> On Tuesday 27 May 2008, Felipe Balbi wrote:
>> >
>> > I don't know of any other drivers with this particular issue,
>> > but nonetheless ... I think the following is an appropriate
>> > resolution.
>> >
>> > Comments from anyone else?
>>
>> I actually like it. I didn't see this error cuz I was only building
>> musb for arm architecture.
>>
>> But why only davinci and omap?
>
> They're the only one affected by the musb_hdrc patch...
>
>
>> What about: mach-aaec2000, mach-at91, mach-ep93xx, mach-integrator,
>> mach-lh7a40x, mach-ns9xxx, mach-pnx4008, mach-pxa, mach-realview,
>> mach-sa1100, mach-versatile, mach-s3c24x.
>
> If this approach goes forward, I'd expect those platforms
> to "select HAVE_CLK" too.  Ditto some SH, PowerPC, etc.
>
>
>> Instead of making the driver dependant on HAVE_CLK (which would be
>> true for virtually any driver comming from linux-omap), you could make
>> clk_enable a nop in case arch doesn't HAVE_CLK. How does that sound?
>
> Best done by #ifdef HAVE_CLK in <linux/clk.h> support, and
> putting the stubs there.  However, since those platforms
> don't actually have clocks, I'd make them all report errors
> of whichever kind.
>
> That's kind of a second-order question to "should we create
> a HAVE_CLK in Kconfig".  Since Certain People have objected
> to patches touching <linux/clk.h> I avoided doing that the
> first time around ... it'd be a better solution, yes.
>
> - Dave
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: HAVE_CLK (for <linux/clk.h> support)

[Felipe Balbi]

On Mon, Sep 01, 2008 at 05:48:14PM +0800, ext Bryan Wu wrote:
> Sorry for jumping in so later. I noticed Blackfin does not support
> HAVE_CLK currently.
> As the HAVE_CLK dependence is included in the kernel mainline, MUSB
> parts do not show up in the Kconfig because of lacking of HAVE_CLK.
> 
> To implement clk infrastructure in Blackfin will be later than merging
> MUSB Blackfin parts to upstream.
> 
> Any idea?

dude, you're really late on this one. Now that musb depends on HAVE_CLK
and we have things working for that. What you can do is locally (on your
tree) remove the HAVE_CLK dependency so you can keep musb development
going. But don't push that patch upstream. Meanwhile, you'll have time
to implement clk framework support for blackfin.

-- 
balbi

Re: HAVE_CLK (for <linux/clk.h> support)

[Bryan Wu]

On Mon, Sep 1, 2008 at 6:00 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Mon, Sep 01, 2008 at 05:48:14PM +0800, ext Bryan Wu wrote:
>> Sorry for jumping in so later. I noticed Blackfin does not support
>> HAVE_CLK currently.
>> As the HAVE_CLK dependence is included in the kernel mainline, MUSB
>> parts do not show up in the Kconfig because of lacking of HAVE_CLK.
>>
>> To implement clk infrastructure in Blackfin will be later than merging
>> MUSB Blackfin parts to upstream.
>>
>> Any idea?
>
> dude, you're really late on this one. Now that musb depends on HAVE_CLK
> and we have things working for that. What you can do is locally (on your
> tree) remove the HAVE_CLK dependency so you can keep musb development
> going. But don't push that patch upstream. Meanwhile, you'll have time
> to implement clk framework support for blackfin.
>

Too bad, I just realize this when I try to port my local blackfin
patch for upstream merge.
You're right, currently I remove the dependency locally. So I need to
add a new task to implement clk framework for Blackfin.

OK, another question, do you know where is the latest usb development
git tree, buddy?

Thanks
-Bryan

Re: HAVE_CLK (for <linux/clk.h> support)

[Felipe Balbi]

On Mon, Sep 01, 2008 at 06:05:19PM +0800, ext Bryan Wu wrote:
> On Mon, Sep 1, 2008 at 6:00 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> > On Mon, Sep 01, 2008 at 05:48:14PM +0800, ext Bryan Wu wrote:
> >> Sorry for jumping in so later. I noticed Blackfin does not support
> >> HAVE_CLK currently.
> >> As the HAVE_CLK dependence is included in the kernel mainline, MUSB
> >> parts do not show up in the Kconfig because of lacking of HAVE_CLK.
> >>
> >> To implement clk infrastructure in Blackfin will be later than merging
> >> MUSB Blackfin parts to upstream.
> >>
> >> Any idea?
> >
> > dude, you're really late on this one. Now that musb depends on HAVE_CLK
> > and we have things working for that. What you can do is locally (on your
> > tree) remove the HAVE_CLK dependency so you can keep musb development
> > going. But don't push that patch upstream. Meanwhile, you'll have time
> > to implement clk framework support for blackfin.
> >
> 
> Too bad, I just realize this when I try to port my local blackfin
> patch for upstream merge.
> You're right, currently I remove the dependency locally. So I need to
> add a new task to implement clk framework for Blackfin.
> 
> OK, another question, do you know where is the latest usb development
> git tree, buddy?

I replied on the other mail :-)

It's a quilt tree on kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/

Just take a look at MAINTAINERS file ;-)

-- 
balbi

Re: HAVE_CLK (for <linux/clk.h> support)

[Felipe Balbi]

On Wed, May 28, 2008 at 2:14 AM, David Brownell <david-b@pacbell.net> wrote:

[snip]

> --- g26.orig/drivers/usb/musb/Kconfig   2008-05-27 16:02:41.000000000 -0700
> +++ g26/drivers/usb/musb/Kconfig        2008-05-27 16:01:09.000000000 -0700
> @@ -8,7 +8,7 @@ comment "Enable Host or Gadget support t
>
>  # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
>  config USB_MUSB_HDRC
> -       depends on USB || USB_GADGET
> +       depends on HAVE_CLK && (USB || USB_GADGET)
>        select TWL4030_USB if MACH_OMAP_3430SDP
>        tristate 'Inventra Highspeed Dual Role Controller (TI, ...)'
>        help

Sorry, another comment.

Instead of making the driver dependant on HAVE_CLK (which would be
true for virtually any driver comming from linux-omap), you could make
clk_enable a nop in case arch doesn't HAVE_CLK. How does that sound?

-- 
Best Regards,

Felipe Balbi
felipebalbi@users.sourceforge.net