Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
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.5...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.5...@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 coolo...@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 coolo...@kernel.org Signed-off-by: Felipe Balbi felipe.ba...@nokia.com Signed-off-by: Greg Kroah-Hartman gre...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
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.5...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
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.5...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
On 4/4/15 06:03, Richard Weinberger wrote: On Fri, Apr 3, 2015 at 11:51 PM, Chen Gang xili_gchen_5...@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.5...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
On Fri, Apr 3, 2015 at 11:51 PM, Chen Gang xili_gchen_5...@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.5...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
