[PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Chen Gang]

Under blackfin, only bf527, bf548 and bf609 may use musb. The related
error with allmodconfig:

    CC [M]  drivers/usb/misc/trancevibrator.o
  In file included from drivers/usb/musb/musb_core.h:70:0,
                   from drivers/usb/musb/musb_core.c:103:
  drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
  drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
   #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
                                  ^

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 drivers/usb/musb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 39db8b6..5fca898 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -6,7 +6,7 @@
 # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
 config USB_MUSB_HDRC
 	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
-	depends on (USB || USB_GADGET)
+	depends on (USB || USB_GADGET) && (!BLACKFIN || (BLACKFIN && (BF527 || BF548 || BF609)))
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on the Mentor Graphics silicon IP.  Then
-- 
1.9.3

Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Richard Weinberger]

On Fri, Apr 3, 2015 at 11:51 PM, Chen Gang <xili_gchen_5257@hotmail.com> wrote:
> Under blackfin, only bf527, bf548 and bf609 may use musb. The related
> error with allmodconfig:
>
>     CC [M]  drivers/usb/misc/trancevibrator.o
>   In file included from drivers/usb/musb/musb_core.h:70:0,
>                    from drivers/usb/musb/musb_core.c:103:
>   drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
>   drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
>    #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
>                                   ^
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  drivers/usb/musb/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index 39db8b6..5fca898 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -6,7 +6,7 @@
>  # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
>  config USB_MUSB_HDRC
>         tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
> -       depends on (USB || USB_GADGET)
> +       depends on (USB || USB_GADGET) && (!BLACKFIN || (BLACKFIN && (BF527 || BF548 || BF609)))

This is ugly.
Can't you add a new Kconfig symbol in arch/blackfin and change the
#ifndef CONFIG_BLACKFIN
in drivers/usb/musb/musb_regs.h to it?

-- 
Thanks,
//richard

Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Chen Gang]

On 4/4/15 06:03, Richard Weinberger wrote:
> On Fri, Apr 3, 2015 at 11:51 PM, Chen Gang <xili_gchen_5257@hotmail.com> wrote:
>> Under blackfin, only bf527, bf548 and bf609 may use musb. The related
>> error with allmodconfig:
>>
>>     CC [M]  drivers/usb/misc/trancevibrator.o
>>   In file included from drivers/usb/musb/musb_core.h:70:0,
>>                    from drivers/usb/musb/musb_core.c:103:
>>   drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
>>   drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
>>    #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
>>                                   ^
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>  drivers/usb/musb/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>> index 39db8b6..5fca898 100644
>> --- a/drivers/usb/musb/Kconfig
>> +++ b/drivers/usb/musb/Kconfig
>> @@ -6,7 +6,7 @@
>>  # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller
>>  config USB_MUSB_HDRC
>>         tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
>> -       depends on (USB || USB_GADGET)
>> +       depends on (USB || USB_GADGET) && (!BLACKFIN || (BLACKFIN && (BF527 || BF548 || BF609)))
> 
> This is ugly.
> Can't you add a new Kconfig symbol in arch/blackfin and change the
> #ifndef CONFIG_BLACKFIN
> in drivers/usb/musb/musb_regs.h to it?
> 

For me, for configuration, we need to try to finish them in Kconfig and
keep the .c or .h source code no touch.

But it is really not quite well to let machine details (e.g. BF527,
BF548, BF609) in the outside of "arch/blackfin".

So, I guess, we need to add new config value HAVE_MUSB for it in patch
v2.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Greg Kroah-Hartman]

On Sat, Apr 04, 2015 at 05:51:21AM +0800, Chen Gang wrote:
> Under blackfin, only bf527, bf548 and bf609 may use musb. The related
> error with allmodconfig:
> 
>     CC [M]  drivers/usb/misc/trancevibrator.o
>   In file included from drivers/usb/musb/musb_core.h:70:0,
>                    from drivers/usb/musb/musb_core.c:103:
>   drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
>   drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
>    #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
>                                   ^
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

Why not fix the root cause and provide this symbol on blackfin?  There's
no specific reason why it shouldn't be able to be built on this
platform.

It's better to make platforms properly build everything, patches like
this just make things messier and harder to maintain.

thanks,

greg k-h

Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Chen Gang]

On 4/4/15 17:54, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2015 at 05:51:21AM +0800, Chen Gang wrote:
>> Under blackfin, only bf527, bf548 and bf609 may use musb. The related
>> error with allmodconfig:
>>
>>     CC [M]  drivers/usb/misc/trancevibrator.o
>>   In file included from drivers/usb/musb/musb_core.h:70:0,
>>                    from drivers/usb/musb/musb_core.c:103:
>>   drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
>>   drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
>>    #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
>>                                   ^
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> Why not fix the root cause and provide this symbol on blackfin?  There's
> no specific reason why it shouldn't be able to be built on this
> platform.
> 
> It's better to make platforms properly build everything, patches like
> this just make things messier and harder to maintain.
> 

I am not quite sure all machines of blackfin can support musb (according
to current code, at present, we can only sure bf527, bf548 and bf609 can
support it).

So, I suggest to add new macro HAVE_MUSB for BLACKFIN in patch v2. And
welcome any other members' idea.

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Greg Kroah-Hartman]

On Sun, Apr 05, 2015 at 06:33:44AM +0800, Chen Gang wrote:
> On 4/4/15 17:54, Greg Kroah-Hartman wrote:
> > On Sat, Apr 04, 2015 at 05:51:21AM +0800, Chen Gang wrote:
> >> Under blackfin, only bf527, bf548 and bf609 may use musb. The related
> >> error with allmodconfig:
> >>
> >>     CC [M]  drivers/usb/misc/trancevibrator.o
> >>   In file included from drivers/usb/musb/musb_core.h:70:0,
> >>                    from drivers/usb/musb/musb_core.c:103:
> >>   drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
> >>   drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
> >>    #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
> >>                                   ^
> >>
> >> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > 
> > Why not fix the root cause and provide this symbol on blackfin?  There's
> > no specific reason why it shouldn't be able to be built on this
> > platform.
> > 
> > It's better to make platforms properly build everything, patches like
> > this just make things messier and harder to maintain.
> > 
> 
> I am not quite sure all machines of blackfin can support musb (according
> to current code, at present, we can only sure bf527, bf548 and bf609 can
> support it).

What do you mean by "can support"?  What is missing?  Why doesn't the
code build there?

> So, I suggest to add new macro HAVE_MUSB for BLACKFIN in patch v2. And
> welcome any other members' idea.

I recommend fixing blackfin, what makes this one specific platform so
broken compared to the 20+ other platforms that don't need this special
treatment?

thanks,

greg k-h

Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

[Chen Gang]

On 4/5/15 16:29, Greg Kroah-Hartman wrote:
> On Sun, Apr 05, 2015 at 06:33:44AM +0800, Chen Gang wrote:
>> On 4/4/15 17:54, Greg Kroah-Hartman wrote:
>>> On Sat, Apr 04, 2015 at 05:51:21AM +0800, Chen Gang wrote:
>>>> Under blackfin, only bf527, bf548 and bf609 may use musb. The related
>>>> error with allmodconfig:
>>>>
>>>>     CC [M]  drivers/usb/misc/trancevibrator.o
>>>>   In file included from drivers/usb/musb/musb_core.h:70:0,
>>>>                    from drivers/usb/musb/musb_core.c:103:
>>>>   drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select':
>>>>   drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function)
>>>>    #define MUSB_INDEX  USB_OFFSET(USB_INDEX) /* 8 bit */
>>>>                                   ^
>>>>
>>>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>>>
>>> Why not fix the root cause and provide this symbol on blackfin?  There's
>>> no specific reason why it shouldn't be able to be built on this
>>> platform.
>>>
>>> It's better to make platforms properly build everything, patches like
>>> this just make things messier and harder to maintain.
>>>
>>
>> I am not quite sure all machines of blackfin can support musb (according
>> to current code, at present, we can only sure bf527, bf548 and bf609 can
>> support it).
> 
> What do you mean by "can support"?  What is missing?  Why doesn't the
> code build there?
> 

Only some of machines of blackfin define USB_INDEX, but machine bf533 (
which is in current building configuration) does not define USB_INDEX,
so cause building break. The related information:

  [root@localhost arch]# grep -rn "\<USB_INDEX\>" *
  blackfin/kernel/debug-mmrs.c:1587:	D16(USB_INDEX);
  blackfin/mach-bf527/include/mach/cdefBF525.h:33:#define bfin_read_USB_INDEX()			bfin_read16(USB_INDEX)
  blackfin/mach-bf527/include/mach/cdefBF525.h:34:#define bfin_write_USB_INDEX(val)		bfin_write16(USB_INDEX, val)
  blackfin/mach-bf527/include/mach/defBF525.h:24:#define                        USB_INDEX  0xffc03824   /* Index register for selecting the indexed endpoint registers */
  blackfin/mach-bf527/include/mach/defBF525.h:394:/* Bit masks for USB_INDEX */
  blackfin/mach-bf548/include/mach/cdefBF542.h:153:#define bfin_read_USB_INDEX()		bfin_read16(USB_INDEX)
  blackfin/mach-bf548/include/mach/cdefBF542.h:154:#define bfin_write_USB_INDEX(val)	bfin_write16(USB_INDEX, val)
  blackfin/mach-bf548/include/mach/cdefBF547.h:320:#define bfin_read_USB_INDEX()		bfin_read16(USB_INDEX)
  blackfin/mach-bf548/include/mach/cdefBF547.h:321:#define bfin_write_USB_INDEX(val)	bfin_write16(USB_INDEX, val)
  blackfin/mach-bf548/include/mach/defBF542.h:88:#define                        USB_INDEX  0xffc03c24   /* Index register for selecting the indexed endpoint registers */
  blackfin/mach-bf548/include/mach/defBF542.h:571:/* Bit masks for USB_INDEX */
  blackfin/mach-bf548/include/mach/defBF547.h:202:#define                        USB_INDEX  0xffc03c24   /* Index register for selecting the indexed endpoint registers */
  blackfin/mach-bf548/include/mach/defBF547.h:848:/* Bit masks for USB_INDEX */
  blackfin/mach-bf609/include/mach/defBF60x_base.h:3262:#define USB_INDEX                  0xFFCC100E         /* USB Index */

USB_INDEX is one of core macro for musb, so I guess, bf533 and other
blackfin machines which do not define USB_INDEX can not support musb.


>> So, I suggest to add new macro HAVE_MUSB for BLACKFIN in patch v2. And
>> welcome any other members' idea.
> 
> I recommend fixing blackfin, what makes this one specific platform so
> broken compared to the 20+ other platforms that don't need this special
> treatment?
> 

Originally, I try to remove CONFIG_BLACKFIN in source code, but it seems
not quite easy, and the original related git commit:

  commit c6cf8b003e5a37f8193c2883876c5942adcd7284
  Author: Bryan Wu <cooloney@kernel.org>
  Date:   Tue Dec 2 21:33:48 2008 +0200
  
      USB: musb: add Blackfin specific configuration to MUSB
      
      Some config registers are not avaiable in Blackfin, we have to comment them out.
      
      v1-v2:
       - remove Blackfin specific header file
       - add Blackfin register version to musb_regs.h header file
      
      Signed-off-by: Bryan Wu <cooloney@kernel.org>
      Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
      Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

So for me, only CONFIG_BLACKFIN can not be sure it must support musb, we
have to define a new config macro HAVE_MUSB (or SUPPORT_MUSB) for it (at
least, within blackfin).

Welcome other members ideas (especially blackfin related members).

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed