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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <BLU436-SMTP245B50673120A4F72193AFCB9FF0@phx.gbl>
Date:	Sun, 5 Apr 2015 23:32:43 +0800
From:	Chen Gang <xili_gchen_5257@...mail.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:	Richard Weinberger <richard.weinberger@...il.com>, balbi@...com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-usb@...r.kernel.org
Subject: Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin

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@...il.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@...alhost 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@...nel.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@...nel.org>
      Signed-off-by: Felipe Balbi <felipe.balbi@...ia.com>
      Signed-off-by: Greg Kroah-Hartman <gregkh@...e.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/