| Message ID | ud4be3ads.wl%morimoto.kuninori@renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Morimoto-san, More feedback from me. =) On Wed, Apr 15, 2009 at 11:42 AM, Kuninori Morimoto <morimoto.kuninori@renesas.com> wrote: > > Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com> > --- >  arch/sh/kernel/cpu/sh4a/setup-sh7724.c |  45 ++++++++++++++++++++++++++++++++ >  drivers/usb/host/Kconfig        |   2 +- >  2 files changed, 46 insertions(+), 1 deletions(-) > > diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c > index 71ccb25..4257713 100644 > --- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c > +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c > @@ -99,15 +99,60 @@ static struct platform_device rtc_device = { >     .resource    = rtc_resources, >  }; > > +/* USB0 */ > +static struct resource usb0_host_resources[] = { > +    [0] = { > +        .name  = "r8a66597_hcd", Please use "USB0" as name for the hardware block. > +        .start  = 0xa4d80000, > +        .end   = 0xa4d800ff, Is D1FIFO included in this area? Shouldn't end be ..0123? >  static struct platform_device *sh7724_devices[] __initdata = { >     &sci_device, >     &rtc_device, > +    &usb0_device, >  }; How about usb1? > +#define UPONCR0         0xa40501d4 > +#define UPONCR1         0xa4050192 > +#define USBPOWERON   0x0600 > +static void __init sh7724_usb_setup(void) > +{ > +    /* > +     * USB initial settings > +     * > +     * The following settings are necessary > +     * for using the USB modules. > +     * > +     * see "USB Inital Settings" for detail > +     */ > +    __raw_writew(USBPOWERON , UPONCR0); > +    __raw_writew(USBPOWERON , UPONCR1); > +} Hm... In the future we want to control the above dynamically for better power savings. I guess it's ok as-is for now. > +++ b/drivers/usb/host/Kconfig > @@ -317,7 +317,7 @@ config USB_R8A66597_HCD > >  config SUPERH_ON_CHIP_R8A66597 >     boolean "Enable SuperH on-chip R8A66597 USB" > -    depends on USB_R8A66597_HCD && (CPU_SUBTYPE_SH7366 || CPU_SUBTYPE_SH7723) > +    depends on USB_R8A66597_HCD && (CPU_SUBTYPE_SH7366 || CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7724) >     help >      This driver enables support for the on-chip R8A66597 in the >      SH7366 and SH7723 processors. Please update the comment as well. Thanks for your help! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 15, 2009 at 09:16:10PM +0900, Magnus Damm wrote: > On Wed, Apr 15, 2009 at 8:49 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > > On Wed, Apr 15, 2009 at 11:42 AM, Kuninori Morimoto > > <morimoto.kuninori@renesas.com> wrote: > >> +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c > > >> +#define UPONCR0 ? ? ? ? ? ? ? ?0xa40501d4 > >> +#define UPONCR1 ? ? ? ? ? ? ? ?0xa4050192 > >> +#define USBPOWERON ? ? 0x0600 > >> +static void __init sh7724_usb_setup(void) > >> +{ > >> + ? ? ? /* > >> + ? ? ? ?* USB initial settings > >> + ? ? ? ?* > >> + ? ? ? ?* The following settings are necessary > >> + ? ? ? ?* for using the USB modules. > >> + ? ? ? ?* > >> + ? ? ? ?* see "USB Inital Settings" for detail > >> + ? ? ? ?*/ > >> + ? ? ? __raw_writew(USBPOWERON , UPONCR0); > >> + ? ? ? __raw_writew(USBPOWERON , UPONCR1); > >> +} > > > > Hm... In the future we want to control the above dynamically for > > better power savings. I guess it's ok as-is for now. > > Looking a bit in r8a66597.h, I wonder if USBPOWERON is similar to > VBOUT that is present in DVSTCR on external devices? > Also note that the same issue exists on SH7786, so if this is fixed a bit more intelligently, the same thing should carry over there. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi again Morimoto-san, [CC Shimoda-san] On Wed, Apr 15, 2009 at 8:49 PM, Magnus Damm <magnus.damm@gmail.com> wrote: > On Wed, Apr 15, 2009 at 11:42 AM, Kuninori Morimoto > <morimoto.kuninori@renesas.com> wrote: >> +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c >> +#define UPONCR0 Â Â Â Â Â Â Â Â 0xa40501d4 >> +#define UPONCR1 Â Â Â Â Â Â Â Â 0xa4050192 >> +#define USBPOWERON Â Â 0x0600 >> +static void __init sh7724_usb_setup(void) >> +{ >> + Â Â Â /* >> + Â Â Â Â * USB initial settings >> + Â Â Â Â * >> + Â Â Â Â * The following settings are necessary >> + Â Â Â Â * for using the USB modules. >> + Â Â Â Â * >> + Â Â Â Â * see "USB Inital Settings" for detail >> + Â Â Â Â */ >> + Â Â Â __raw_writew(USBPOWERON , UPONCR0); >> + Â Â Â __raw_writew(USBPOWERON , UPONCR1); >> +} > > Hm... In the future we want to control the above dynamically for > better power savings. I guess it's ok as-is for now. Looking a bit in r8a66597.h, I wonder if USBPOWERON is similar to VBOUT that is present in DVSTCR on external devices? / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus Thank you for your comment > > +/* USB0 */ > > +static struct resource usb0_host_resources[] = { > > +    [0] = { > > +        .name  = "r8a66597_hcd", > > Please use "USB0" as name for the hardware block. I will fix it. Thank you. > > +        .start  = 0xa4d80000, > > +        .end   = 0xa4d800ff, > > Is D1FIFO included in this area? Shouldn't end be ..0123? wow !! Sorry it is my miss. Thank you. > >  static struct platform_device *sh7724_devices[] __initdata = { > >     &sci_device, > >     &rtc_device, > > +    &usb0_device, > >  }; > > How about usb1? Now, we can not support usb1. Because latest manual (Rev 0.11) doesn't have explain for usb1 interrupt line number. sorry, I didn't explain about it. > >     boolean "Enable SuperH on-chip R8A66597 USB" > > -    depends on USB_R8A66597_HCD && (CPU_SUBTYPE_SH7366 || CPU_SUBTYPE_SH7723) > > +    depends on USB_R8A66597_HCD && (CPU_SUBTYPE_SH7366 || CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7724) > >     help > >      This driver enables support for the on-chip R8A66597 in the > >      SH7366 and SH7723 processors. > > Please update the comment as well. Thank you I will fix it. Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 16, 2009 at 09:41:55AM +0900, Kuninori Morimoto wrote: > > > ?static struct platform_device *sh7724_devices[] __initdata = { > > > ? ? ? ?&sci_device, > > > ? ? ? ?&rtc_device, > > > + ? ? ? &usb0_device, > > > ?}; > > > > How about usb1? > > Now, we can not support usb1. > Because latest manual (Rev 0.11) doesn't have > explain for usb1 interrupt line number. > sorry, I didn't explain about it. > This is going to be a pretty common problem, we should make sure that each of the registered platform devices actually has a working IRQ. The entire INTC section ought to be thrown out and burned for all the good it does us today. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus-san, Magnus Damm wrote: > Hi again Morimoto-san, > > [CC Shimoda-san] > > On Wed, Apr 15, 2009 at 8:49 PM, Magnus Damm <magnus.damm@gmail.com> wrote: >> On Wed, Apr 15, 2009 at 11:42 AM, Kuninori Morimoto >> <morimoto.kuninori@renesas.com> wrote: >>> +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c > >>> +#define UPONCR0 0xa40501d4 >>> +#define UPONCR1 0xa4050192 >>> +#define USBPOWERON 0x0600 >>> +static void __init sh7724_usb_setup(void) >>> +{ >>> + /* >>> + * USB initial settings >>> + * >>> + * The following settings are necessary >>> + * for using the USB modules. >>> + * >>> + * see "USB Inital Settings" for detail >>> + */ >>> + __raw_writew(USBPOWERON , UPONCR0); >>> + __raw_writew(USBPOWERON , UPONCR1); >>> +} >> Hm... In the future we want to control the above dynamically for >> better power savings. I guess it's ok as-is for now. > > Looking a bit in r8a66597.h, I wonder if USBPOWERON is similar to > VBOUT that is present in DVSTCR on external devices? I also think UPONCR register control port power. I think this register differ from SH7786. I will modify the r8a66597 driver and be able to control those registers with r8a66597's ops. It's like a sh_mobile_lcdcfb's display_on function. :) Thanks, Yoshihiro Shimoda -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear all > >> Hm... In the future we want to control the above dynamically for > >> better power savings. I guess it's ok as-is for now. > > > > Looking a bit in r8a66597.h, I wonder if USBPOWERON is similar to > > VBOUT that is present in DVSTCR on external devices? > > I also think UPONCR register control port power. I think this register > differ from SH7786. > I will modify the r8a66597 driver and be able to control those registers > with r8a66597's ops. It's like a sh_mobile_lcdcfb's display_on function. :) Thank you shimoda-san. When your modification was applied, I will send patch to use it for SH7786/SH7724. In my opinion, SH7724 needs .power_on function, and SH7786 needs .init function to r8a66597's ops. Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear all > >> Hm... In the future we want to control the above dynamically for > >> better power savings. I guess it's ok as-is for now. > > > > Looking a bit in r8a66597.h, I wonder if USBPOWERON is similar to > > VBOUT that is present in DVSTCR on external devices? > > I also think UPONCR register control port power. I think this register > differ from SH7786. > I will modify the r8a66597 driver and be able to control those registers > with r8a66597's ops. It's like a sh_mobile_lcdcfb's display_on function. :) Thank you shimoda-san. When your modification was applied, I will send patch to use it for SH7786/SH7724. In my opinion, SH7724 needs .power_on function, and SH7786 needs .init function to r8a66597's ops. Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Paul >> +        .start  = 0xa4d80000, >> +        .end   = 0xa4d800ff, > > Is D1FIFO included in this area? Shouldn't end be ..0123? Now I noticed that SH7724's USB (r8a66597) register mapping is different from other r8a66597. It mean r8a66597 driver need fix for SH7724. I will consult with shimoda-san about it Therefore, please ignore this patch this time. Sorry. Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c index 71ccb25..4257713 100644 --- a/arch/sh/kernel/cpu/sh4a/setup-sh7724.c +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7724.c @@ -99,15 +99,60 @@ static struct platform_device rtc_device = { .resource = rtc_resources, }; +/* USB0 */ +static struct resource usb0_host_resources[] = { + [0] = { + .name = "r8a66597_hcd", + .start = 0xa4d80000, + .end = 0xa4d800ff, + .flags = IORESOURCE_MEM, + }, + [1] = { + .start = 65, + .end = 65, + .flags = IORESOURCE_IRQ, + }, +}; + +static struct platform_device usb0_device = { + .name = "r8a66597_hcd", + .id = 0, + .dev = { + .dma_mask = NULL, /* not use dma */ + .coherent_dma_mask = 0xffffffff, + }, + .num_resources = ARRAY_SIZE(usb0_host_resources), + .resource = usb0_host_resources, +}; + static struct platform_device *sh7724_devices[] __initdata = { &sci_device, &rtc_device, + &usb0_device, }; +#define UPONCR0 0xa40501d4 +#define UPONCR1 0xa4050192 +#define USBPOWERON 0x0600 +static void __init sh7724_usb_setup(void) +{ + /* + * USB initial settings + * + * The following settings are necessary + * for using the USB modules. + * + * see "USB Inital Settings" for detail + */ + __raw_writew(USBPOWERON , UPONCR0); + __raw_writew(USBPOWERON , UPONCR1); +} + static int __init sh7724_devices_setup(void) { clk_always_enable("rtc0"); /* RTC */ + sh7724_usb_setup(); return platform_add_devices(sh7724_devices, ARRAY_SIZE(sh7724_devices)); } diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 845479f..e2b7393 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -317,7 +317,7 @@ config USB_R8A66597_HCD config SUPERH_ON_CHIP_R8A66597 boolean "Enable SuperH on-chip R8A66597 USB" - depends on USB_R8A66597_HCD && (CPU_SUBTYPE_SH7366 || CPU_SUBTYPE_SH7723) + depends on USB_R8A66597_HCD && (CPU_SUBTYPE_SH7366 || CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7724) help This driver enables support for the on-chip R8A66597 in the SH7366 and SH7723 processors.
Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com> --- arch/sh/kernel/cpu/sh4a/setup-sh7724.c | 45 ++++++++++++++++++++++++++++++++ drivers/usb/host/Kconfig | 2 +- 2 files changed, 46 insertions(+), 1 deletions(-)