| Message ID | uzleql7ud.wl%morimoto.kuninori@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi, Kuninori Morimoto wrote: > Timeout counter "i" can reach -1 if USB setup isn't done. > And "i" can be 0 if USB setup is done when last possible moment. > Then it is judged to fail though setup is done. > This patch modify this problem. > Special thanks to Mr. Juha Leppanen for nice advice. > > Reported-by: "Juha Leppanen" <juha_motorsportcom@luukku.com> > Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com> > --- > >> Paul >> > Can you test this patch on Urquell board please ? > > arch/sh/kernel/cpu/sh4a/setup-sh7786.c | 10 +++++++--- > 1 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/arch/sh/kernel/cpu/sh4a/setup-sh7786.c b/arch/sh/kernel/cpu/sh4a/setup-sh7786.c > index 5a47e1c..ee30e68 100644 > --- a/arch/sh/kernel/cpu/sh4a/setup-sh7786.c > +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7786.c > @@ -127,6 +127,7 @@ void __init sh7786_usb_use_exclock(void) > static void __init sh7786_usb_setup(void) > { > int i = 1000000; > + u32 val; > > /* > * USB initial settings > @@ -143,11 +144,14 @@ static void __init sh7786_usb_setup(void) > * Set the PHY and PLL enable bit > */ > __raw_writel(PHY_ENB | PLL_ENB, USBPCTL1); > - while (i-- && > - ((__raw_readl(USBST) & ACT_PLL_STATUS) != ACT_PLL_STATUS)) > + while (i--) { > + val = __raw_readl(USBST) & ACT_PLL_STATUS; > + if (ACT_PLL_STATUS == val) > + break; > cpu_relax(); > + } > > - if (i) { > + if (ACT_PLL_STATUS == val) { > /* Set the PHY RST bit */ > __raw_writel(PHY_ENB | PLL_ENB | PHY_RST, USBPCTL1); > printk(KERN_INFO "sh7786 usb setup done\n"); > I think that this initialization function maybe pass to the platform data of the usb driver. Give a change to go ahead and deferrable the inizialization of the system. So during the bootup we can avoid the "while loop". What is exaclty 100000? Michael -- 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 Michael Thank you for advice. > I think that this initialization function maybe pass to the platform > data of the > usb driver. Give a change to go ahead and deferrable the inizialization > of the system. Hmm. I can agree with you. This usb setup register have not been mapped as USB module. And, (I think) "current" usb driver (sh_ohci/r8a66597) can not call peculiar init function. I think following step is best. 1) Change usb driver to be able to call peculiar init function. 2) setup-sh7786 set peculiar init funtion to usb driver. 3) usb driver call it when start How about this idea ? 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
Hi, morimoto.kuninori@renesas.com wrote: > Dear Michael > > Thank you for advice. > > >> I think that this initialization function maybe pass to the platform >> data of the >> usb driver. Give a change to go ahead and deferrable the inizialization >> of the system. >> > > Hmm. I can agree with you. > > This usb setup register have not been mapped as USB module. > And, (I think) "current" usb driver (sh_ohci/r8a66597) > can not call peculiar init function. > > I think following step is best. > > 1) Change usb driver to be able to call peculiar init function. > 2) setup-sh7786 set peculiar init funtion to usb driver. > 3) usb driver call it when start > > How about this idea ? > I think that is a good approch. If it is possible, before the setup, the best thing is to test if the bootloader has just enable it and skip the setup part in that case. Michael > 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 Michael Thank you for comment > > 1) Change usb driver to be able to call peculiar init function. > > 2) setup-sh7786 set peculiar init funtion to usb driver. > > 3) usb driver call it when start > > > > How about this idea ? > > > I think that is a good approch. If it is possible, before the setup, the > best thing is to test if the bootloader has just > enable it and skip the setup part in that case. wow !! how cool idea !! > shimoda san Can you change use driver to be able to call peculiar init function ? Or Can I do that ? 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
Hi, morimoto.kuninori@renesas.com wrote: > Dear Michael > > Thank you for comment > > >>> 1) Change usb driver to be able to call peculiar init function. >>> 2) setup-sh7786 set peculiar init funtion to usb driver. >>> 3) usb driver call it when start >>> >>> How about this idea ? >>> >>> >> I think that is a good approch. If it is possible, before the setup, the >> best thing is to test if the bootloader has just >> enable it and skip the setup part in that case. >> > > wow !! > how cool idea !! > > >> shimoda san >> > > Can you change use driver to be able to call peculiar init function ? > Or Can I do that ? > I will try to provide a patch if you want on 14 of April, or if you prefer just start and send a patch to the reviewer :) Michael -- 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 Michael > >> shimoda san > > > > Can you change use driver to be able to call peculiar init function ? > > Or Can I do that ? > > > I will try to provide a patch if you want on 14 of April, or if you > prefer just start and > send a patch to the reviewer :) Ooops. sorry. My request is to "shimoda san" who created usb driver for SH :) 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
Hi Morimoto-san, morimoto.kuninori@renesas.com wrote: > Dear Michael > >>>> shimoda san >>> Can you change use driver to be able to call peculiar init function ? >>> Or Can I do that ? >>> >> I will try to provide a patch if you want on 14 of April, or if you >> prefer just start and >> send a patch to the reviewer :) > > Ooops. sorry. > My request is to "shimoda san" who created usb driver for SH :) > I think that it is after next month if I try it. Because linux-2.6.30 merge window was just closed recently :) 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
Hi, Yoshihiro Shimoda wrote: > Hi Morimoto-san, > > morimoto.kuninori@renesas.com wrote: >> Dear Michael >> >>>>> shimoda san >>>> Can you change use driver to be able to call peculiar init function ? >>>> Or Can I do that ? >>>> >>> I will try to provide a patch if you want on 14 of April, or if you >>> prefer just start and >>> send a patch to the reviewer :) >> Ooops. sorry. >> My request is to "shimoda san" who created usb driver for SH :) >> > > I think that it is after next month if I try it. > Because linux-2.6.30 merge window was just closed recently :) > I don't have all the data. Look at the code there only the init function to be deferrable. Introduce a platform data it can be usefull but I would like to know if there are other info that can be passed with the platform data. Is that the only cpu that needs a specific initialization? Before a patch maybe we can ask how can we use it? Just to know: what is this function sh7786_usb_use_exclock? for example the s3c2410 define and use a platform data, for port flags or power control or overun current situation. What can we put in a platform data? Michael - > 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 > -- 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 Michael, Michael Trimarchi wrote: > Hi, > > Yoshihiro Shimoda wrote: >> Hi Morimoto-san, >> >> morimoto.kuninori@renesas.com wrote: >>> Dear Michael >>> >>>>>> shimoda san >>>>> Can you change use driver to be able to call peculiar init function ? >>>>> Or Can I do that ? >>>>> >>>> I will try to provide a patch if you want on 14 of April, or if you >>>> prefer just start and >>>> send a patch to the reviewer :) >>> Ooops. sorry. >>> My request is to "shimoda san" who created usb driver for SH :) >>> >> I think that it is after next month if I try it. >> Because linux-2.6.30 merge window was just closed recently :) >> > I don't have all the data. Look at the code there only the init function > to be > deferrable. Introduce a platform data it can be usefull but I would like to > know if there are other info that can be passed with the platform data. Thank you for your comment! I understood it. > Is that the only cpu that needs a specific initialization? Before a patch > maybe we can ask how can we use it? > Just to know: > what is this function sh7786_usb_use_exclock? SH7763's OHCI need initialization too (but differ proccess: arch/sh/board/mach-sh7763rdp/setup.c: line 172). I'm sorry, I don't know detail of this function. Morimoto-san, please tell me this information? > for example the s3c2410 define and use a platform data, for port flags or > power control or overun current situation. What can we put in a platform > data? The Renesas usb host controllers are: - OHCI (drivers/usb/host/ohci-sh.c) - r8a66597 (drivers/usb/host/r8a66597-hcd.c) In OHCI, we can put extend initialize function in a platform data only at the moment. However, in r8a66597, we can put all module_param data. 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
On Thu, Apr 09, 2009 at 08:08:21PM +0900, Yoshihiro Shimoda wrote: > Michael Trimarchi wrote: > > Is that the only cpu that needs a specific initialization? Before a patch > > maybe we can ask how can we use it? > > Just to know: > > what is this function sh7786_usb_use_exclock? > > SH7763's OHCI need initialization too > (but differ proccess: arch/sh/board/mach-sh7763rdp/setup.c: line 172). > > I'm sorry, I don't know detail of this function. > Morimoto-san, please tell me this information? > > > for example the s3c2410 define and use a platform data, for port flags or > > power control or overun current situation. What can we put in a platform > > data? > > The Renesas usb host controllers are: > - OHCI (drivers/usb/host/ohci-sh.c) > - r8a66597 (drivers/usb/host/r8a66597-hcd.c) > > In OHCI, we can put extend initialize function in a platform data only at the moment. > However, in r8a66597, we can put all module_param data. > I don't see any reason why we can't accomodate the r7a66597 platform data change within -rc2, so if that is the way you want to fix this, then that should be fine. Of course if it is too invasive, we will just use the original fix in the board, but there is not much point in doing a stop-gap fix if we already have some plan on how to fix it properly. -- 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 > > SH7763's OHCI need initialization too > > (but differ proccess: arch/sh/board/mach-sh7763rdp/setup.c: line 172). > > > > I'm sorry, I don't know detail of this function. > > Morimoto-san, please tell me this information? Sorry, I don't know either... > deferrable. Introduce a platform data it can be usefull but I would like to > know if there are other info that can be passed with the platform data. > Is that the only cpu that needs a specific initialization? Before a patch > maybe we can ask how can we use it? SH7724 CPU (new CPU) need specific initialization too. It use r8a66597. > Just to know: > what is this function sh7786_usb_use_exclock? sh7786 can select usb clock mode. - crystal resonator (default) - external clock So, if board use external clock, we have to set it to register. This function for 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
Hi Paul, Paul Mundt wrote: > On Thu, Apr 09, 2009 at 08:08:21PM +0900, Yoshihiro Shimoda wrote: >> Michael Trimarchi wrote: >>> Is that the only cpu that needs a specific initialization? Before a patch >>> maybe we can ask how can we use it? >>> Just to know: >>> what is this function sh7786_usb_use_exclock? >> SH7763's OHCI need initialization too >> (but differ proccess: arch/sh/board/mach-sh7763rdp/setup.c: line 172). >> >> I'm sorry, I don't know detail of this function. >> Morimoto-san, please tell me this information? >> >>> for example the s3c2410 define and use a platform data, for port flags or >>> power control or overun current situation. What can we put in a platform >>> data? >> The Renesas usb host controllers are: >> - OHCI (drivers/usb/host/ohci-sh.c) >> - r8a66597 (drivers/usb/host/r8a66597-hcd.c) >> >> In OHCI, we can put extend initialize function in a platform data only at the moment. >> However, in r8a66597, we can put all module_param data. >> > I don't see any reason why we can't accomodate the r7a66597 platform data > change within -rc2, so if that is the way you want to fix this, then that > should be fine. Of course if it is too invasive, we will just use the > original fix in the board, but there is not much point in doing a > stop-gap fix if we already have some plan on how to fix it properly. Thank you for your comment! I will try this changing properly. But I may need much time. 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 Paul > > I don't see any reason why we can't accomodate the r7a66597 platform data > > change within -rc2, so if that is the way you want to fix this, then that > > should be fine. Of course if it is too invasive, we will just use the > > original fix in the board, but there is not much point in doing a > > stop-gap fix if we already have some plan on how to fix it properly. > > Thank you for your comment! > I will try this changing properly. But I may need much time. So, what is the best way about this problem ? Can you agree to use this (niche) patch until shimoda-san's work will be completed ? If you can agree, I would like to send v2 patch. Or should I wait for shimoda-san's work ? 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 Fri, Apr 10, 2009 at 01:56:09PM +0900, morimoto.kuninori@renesas.com wrote: > > > I don't see any reason why we can't accomodate the r7a66597 platform data > > > change within -rc2, so if that is the way you want to fix this, then that > > > should be fine. Of course if it is too invasive, we will just use the > > > original fix in the board, but there is not much point in doing a > > > stop-gap fix if we already have some plan on how to fix it properly. > > > > Thank you for your comment! > > I will try this changing properly. But I may need much time. > > So, what is the best way about this problem ? > > Can you agree to use this (niche) patch until > shimoda-san's work will be completed ? > If you can agree, I would like to send v2 patch. > > Or should I wait for shimoda-san's work ? > You can send a v2 patch. If Shimoda-san does not believe he will be able to submit a more complete patch within the -rc2 timeframe, then we will use your v2 patch instead. -- 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-sh7786.c b/arch/sh/kernel/cpu/sh4a/setup-sh7786.c index 5a47e1c..ee30e68 100644 --- a/arch/sh/kernel/cpu/sh4a/setup-sh7786.c +++ b/arch/sh/kernel/cpu/sh4a/setup-sh7786.c @@ -127,6 +127,7 @@ void __init sh7786_usb_use_exclock(void) static void __init sh7786_usb_setup(void) { int i = 1000000; + u32 val; /* * USB initial settings @@ -143,11 +144,14 @@ static void __init sh7786_usb_setup(void) * Set the PHY and PLL enable bit */ __raw_writel(PHY_ENB | PLL_ENB, USBPCTL1); - while (i-- && - ((__raw_readl(USBST) & ACT_PLL_STATUS) != ACT_PLL_STATUS)) + while (i--) { + val = __raw_readl(USBST) & ACT_PLL_STATUS; + if (ACT_PLL_STATUS == val) + break; cpu_relax(); + } - if (i) { + if (ACT_PLL_STATUS == val) { /* Set the PHY RST bit */ __raw_writel(PHY_ENB | PLL_ENB | PHY_RST, USBPCTL1); printk(KERN_INFO "sh7786 usb setup done\n");