| Message ID | 51DE780C.2070701@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/11/2013 12:17 PM, Rajendra Nayak wrote: > On Wednesday 10 July 2013 09:37 PM, Felipe Balbi wrote: >> how about something like below ? It makes omap_device/hwmod and >> pm_runtime agree on the initial state of the device and will prevent >> ->runtime_resume() from being called on first pm_runtime_get*() done >> during probe. >> >> This is similar to what PCI bus does (if you look at pci_pm_init()). > > I tried something similar [1] but what I found is that the serial > runtime resume was called despite it being marked as active using > pm_runtime_set_active(). > > That seems to be because of the pm_runtime_set_autosuspend_delay() > because we have the autosuspend_delay = -1 > > ----- > static void update_autosuspend(struct device *dev, int old_delay, int old_use) > { > int delay = dev->power.autosuspend_delay; > > /* Should runtime suspend be prevented now? */ > if (dev->power.use_autosuspend && delay < 0) { > > /* If it used to be allowed then prevent it. */ > if (!old_use || old_delay >= 0) { > atomic_inc(&dev->power.usage_count); > rpm_resume(dev, 0); <------------------------------- calls serial runtime resume. It's strange - the check for status is inside rpm_resume() if (dev->power.runtime_status == RPM_ACTIVE) { retval = 1; goto out; } So, if status is set to RPM_ACTIVE before first call to: pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, omap_up_info->autosuspend_timeout); -- or -- pm_runtime_get*() everything should be ok. May be the problem is inside serial_omap_set_termios() where pm_runtime_get_sync is called before context is initialized. probe: get - status is ACTIVE and serial resume is not called put - status is IDLE set_termios get - status is IDLE and serial resume is called > } > } > ----- > > So we end up with the same issue with serial resume being called before set_termios() > > [1] > > diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c > index 5cc9287..c71d47d 100644 > --- a/arch/arm/mach-omap2/omap_device.c > +++ b/arch/arm/mach-omap2/omap_device.c > @@ -129,6 +129,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev) > struct device_node *node = pdev->dev.of_node; > const char *oh_name; > int oh_cnt, i, ret = 0; > + bool device_active = false; > > oh_cnt = of_property_count_strings(node, "ti,hwmods"); > if (oh_cnt <= 0) { > @@ -152,6 +153,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev) > goto odbfd_exit1; > } > hwmods[i] = oh; > + if (oh->flags & HWMOD_INIT_NO_IDLE) > + device_active = true; > + > } > > od = omap_device_alloc(pdev, hwmods, oh_cnt); > @@ -172,6 +176,11 @@ static int omap_device_build_from_dt(struct platform_device *pdev) > > pdev->dev.pm_domain = &omap_device_pm_domain; > > + if (device_active) { > + omap_device_enable(pdev); > + pm_runtime_set_active(&pdev->dev); > + } > + > odbfd_exit1: > kfree(hwmods); > odbfd_exit: > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" 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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rajendra, On 07/11/2013 12:17 PM, Rajendra Nayak wrote: > On Wednesday 10 July 2013 09:37 PM, Felipe Balbi wrote: >> how about something like below ? It makes omap_device/hwmod and >> pm_runtime agree on the initial state of the device and will prevent >> ->runtime_resume() from being called on first pm_runtime_get*() done >> during probe. >> >> This is similar to what PCI bus does (if you look at pci_pm_init()). > > I tried something similar [1] but what I found is that the serial > runtime resume was called despite it being marked as active using > pm_runtime_set_active(). > > That seems to be because of the pm_runtime_set_autosuspend_delay() > because we have the autosuspend_delay = -1 > > ----- > static void update_autosuspend(struct device *dev, int old_delay, int old_use) > { > int delay = dev->power.autosuspend_delay; > > /* Should runtime suspend be prevented now? */ > if (dev->power.use_autosuspend && delay < 0) { > > /* If it used to be allowed then prevent it. */ > if (!old_use || old_delay >= 0) { > atomic_inc(&dev->power.usage_count); > rpm_resume(dev, 0); <------------------------------- calls serial runtime resume. > } > } > ----- > > So we end up with the same issue with serial resume being called before set_termios() > > [1] > > diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c > index 5cc9287..c71d47d 100644 > --- a/arch/arm/mach-omap2/omap_device.c > +++ b/arch/arm/mach-omap2/omap_device.c > @@ -129,6 +129,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev) > struct device_node *node = pdev->dev.of_node; > const char *oh_name; > int oh_cnt, i, ret = 0; > + bool device_active = false; > > oh_cnt = of_property_count_strings(node, "ti,hwmods"); > if (oh_cnt <= 0) { > @@ -152,6 +153,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev) > goto odbfd_exit1; > } > hwmods[i] = oh; > + if (oh->flags & HWMOD_INIT_NO_IDLE) > + device_active = true; > + > } > > od = omap_device_alloc(pdev, hwmods, oh_cnt); > @@ -172,6 +176,11 @@ static int omap_device_build_from_dt(struct platform_device *pdev) > > pdev->dev.pm_domain = &omap_device_pm_domain; > > + if (device_active) { > + omap_device_enable(pdev); > + pm_runtime_set_active(&pdev->dev); > + } > + > odbfd_exit1: > kfree(hwmods); > odbfd_exit: This solution works good for me in combination with "serial: omap: enable PM runtime only when its fully configured" http://www.spinics.net/lists/linux-serial/msg10317.html (earlyprintk use case) And I think the best way would be to move forward with yours solution. - it will affect only on needed IPs drivers - not on all, so no issues with iommu and etc. But issue with *non console" UARTs will still be here and, I think, there are no way ti solve it from OMAP device/hwmod frameworks side, because only driver can know when his context is ready. To test do: #echo 0xDEAD > dev/ttyO3 Regards, -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 16 July 2013 03:57 PM, Grygorii Strashko wrote: > Hi Rajendra, > > On 07/11/2013 12:17 PM, Rajendra Nayak wrote: >> On Wednesday 10 July 2013 09:37 PM, Felipe Balbi wrote: >>> how about something like below ? It makes omap_device/hwmod and >>> pm_runtime agree on the initial state of the device and will prevent >>> ->runtime_resume() from being called on first pm_runtime_get*() done >>> during probe. >>> >>> This is similar to what PCI bus does (if you look at pci_pm_init()). >> >> I tried something similar [1] but what I found is that the serial >> runtime resume was called despite it being marked as active using >> pm_runtime_set_active(). >> >> That seems to be because of the pm_runtime_set_autosuspend_delay() >> because we have the autosuspend_delay = -1 >> >> ----- >> static void update_autosuspend(struct device *dev, int old_delay, int old_use) >> { >> int delay = dev->power.autosuspend_delay; >> >> /* Should runtime suspend be prevented now? */ >> if (dev->power.use_autosuspend && delay < 0) { >> >> /* If it used to be allowed then prevent it. */ >> if (!old_use || old_delay >= 0) { >> atomic_inc(&dev->power.usage_count); >> rpm_resume(dev, 0); <------------------------------- calls serial runtime resume. >> } >> } >> ----- >> >> So we end up with the same issue with serial resume being called before set_termios() >> >> [1] >> >> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c >> index 5cc9287..c71d47d 100644 >> --- a/arch/arm/mach-omap2/omap_device.c >> +++ b/arch/arm/mach-omap2/omap_device.c >> @@ -129,6 +129,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev) >> struct device_node *node = pdev->dev.of_node; >> const char *oh_name; >> int oh_cnt, i, ret = 0; >> + bool device_active = false; >> >> oh_cnt = of_property_count_strings(node, "ti,hwmods"); >> if (oh_cnt <= 0) { >> @@ -152,6 +153,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev) >> goto odbfd_exit1; >> } >> hwmods[i] = oh; >> + if (oh->flags & HWMOD_INIT_NO_IDLE) >> + device_active = true; >> + >> } >> >> od = omap_device_alloc(pdev, hwmods, oh_cnt); >> @@ -172,6 +176,11 @@ static int omap_device_build_from_dt(struct platform_device *pdev) >> >> pdev->dev.pm_domain = &omap_device_pm_domain; >> >> + if (device_active) { >> + omap_device_enable(pdev); >> + pm_runtime_set_active(&pdev->dev); >> + } >> + >> odbfd_exit1: >> kfree(hwmods); >> odbfd_exit: > > This solution works good for me in combination with > "serial: omap: enable PM runtime only when its fully configured" > http://www.spinics.net/lists/linux-serial/msg10317.html > (earlyprintk use case) > > And I think the best way would be to move forward with yours solution. > - it will affect only on needed IPs drivers - not on all, so no issues with iommu and etc. Ok, good to know that this solution works. Anyone has any other thoughts on whats the best way to fix this for the -rcs? (Note that we have bootup broken on all OMAPs with DEBUG_LL for now, atleast for all DT only platforms) The other option of course is to use the patch posted by Felipe [1], thought that had issues handling processor IPs as pointed out by Grygorii and Suman [2], the fix I posted [3] to make omap_device aware of hwmod bypassing these IPs might fix some of those, but this solution in general would need more testing as it affects all modules on the bus. [1] http://www.spinics.net/lists/arm-kernel/msg258062.html [2] http://www.spinics.net/lists/arm-kernel/msg258061.html [3] http://www.spinics.net/lists/arm-kernel/msg258465.html > > But issue with *non console" UARTs will still be here and, I think, > there are no way ti solve it from OMAP device/hwmod frameworks side, > because only driver can know when his context is ready. > To test do: #echo 0xDEAD > dev/ttyO3 > > Regards, > -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c index 5cc9287..c71d47d 100644 --- a/arch/arm/mach-omap2/omap_device.c +++ b/arch/arm/mach-omap2/omap_device.c @@ -129,6 +129,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev) struct device_node *node = pdev->dev.of_node; const char *oh_name; int oh_cnt, i, ret = 0; + bool device_active = false; oh_cnt = of_property_count_strings(node, "ti,hwmods"); if (oh_cnt <= 0) { @@ -152,6 +153,9 @@ static int omap_device_build_from_dt(struct platform_device *pdev) goto odbfd_exit1; } hwmods[i] = oh; + if (oh->flags & HWMOD_INIT_NO_IDLE) + device_active = true; + } od = omap_device_alloc(pdev, hwmods, oh_cnt); @@ -172,6 +176,11 @@ static int omap_device_build_from_dt(struct platform_device *pdev) pdev->dev.pm_domain = &omap_device_pm_domain; + if (device_active) { + omap_device_enable(pdev); + pm_runtime_set_active(&pdev->dev); + } + odbfd_exit1: kfree(hwmods); odbfd_exit: