Re: [PATCH] ARM: OMAP2+: omap-device: remove omap_device_late_init call completely

From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Keerthy <a0393675@ti.com>, Tony Lindgren <tony@atomide.com>
Cc: linux@arm.linux.org.uk, Kevin Hilman <khilman@kernel.org>,
	Keerthy <j-keerthy@ti.com>,
	nsekhar@ti.com, balbi@ti.com, t-kristo@ti.com,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: OMAP2+: omap-device: remove omap_device_late_init call completely
Date: Fri, 28 Aug 2015 15:04:52 +0300	[thread overview]
Message-ID: <55E04E64.2070204@ti.com> (raw)
In-Reply-To: <55E028C7.3060509@ti.com>

On 08/28/2015 12:24 PM, Keerthy wrote:
>
>
> On Thursday 27 August 2015 10:36 PM, Grygorii Strashko wrote:
>> On 08/27/2015 07:38 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [150827 06:42]:
>>>> Hi Tony,
>>>>
>>>> On 08/26/2015 09:10 PM, Tony Lindgren wrote:
>>>>> * Grygorii Strashko <grygorii.strashko@ti.com> [150826 11:01]:
>>>>>> Now Kernel fails to boot 50% of times (form build to build) with
>>>>>> RT-patchset applied due to the following race - on late boot
>>>>>> stages deferred_probe_work_func races with omap_device_late_ini
>>>>>>
>>>>>> late_initcall
>>>>>>     - deferred_probe_initcal() tries to re-probe all pending
>>>>>> driver's probe.
>>>>>>       [In general, It's NOT expected to probe any other built-in
>>>>>> drivers after
>>>>>>       deferred_probe_initcal() is finished, because most of
>>>>>>       late_initcall_sync/late_initcall functions expected that all
>>>>>> driver
>>>>>>       or probed or deferred already.]
>>>>>>
>>>>>> - later on, some driver is probing in this case It's could cpsw.c
>>>>>>      (but could be any other drivers)
>>>>>>      cpsw_init
>>>>>>      - platform_driver_register
>>>>>>        - really_probe
>>>>>>           - driver_bound
>>>>>>             - driver_deferred_probe_trigger
>>>>>>      and boot proceed.
>>>>>>      So, at this moment we have  deferred_probe_work_func scheduled.
>>>>>>
>>>>>> late_initcall_sync
>>>>>>      - omap_device_late_init
>>>>>>        - omap_device_idle
>>>>>>
>>>>>> CPU1                    CPU2
>>>>>>      - deferred_probe_work_func
>>>>>>        - really_probe
>>>>>>          - omap_hsmmc_probe
>>>>>>     - pm_runtime_get_sync
>>>>>>                     late_initcall_sync
>>>>>>                     - omap_device_late_init
>>>>>>                         if (od->_driver_status !=
>>>>>> BUS_NOTIFY_BOUND_DRIVER) {
>>>>>>                             if (od->_state ==
>>>>>> OMAP_DEVICE_STATE_ENABLED) {
>>>>>>                                 - omap_device_idle [ops - IP is
>>>>>> disabled, ]
>>>>>>     - [fail]
>>>>>>     - pm_runtime_put_sync
>>>>>>              - omap_hsmmc_runtime_suspend [ooops!]
>>>>>
>>>>> OK idling of unclaimed devices should not happen for deferred probe,
>>>>> it should only happen when there's no driver and no probing happening.
>>>>>
>>>>>> Lets remove just remove omap_device_late_init completely as suggested
>>>>>> by Tero Kristo:
>>>>>>
>>>>>> "How about remove omap_device_late_init call completely. I don't
>>>>>> think
>>>>>> it does anything useful at the moment; none of the omap devices get
>>>>>> enabled outside runtime_pm, so there should be no need to explicitly
>>>>>> disable the devices."
>>>>>
>>>>> I think this is still needed from PM point of view as otherwise we
>>>>> don't idle any devices that don't have a driver available. Or am I
>>>>> missing something?
>>>>>
>>>>> To me it seems the bug is relying on the BUS_NOTIFY_BOUND_DRIVER is
>>>>> not set in the deferred probe case.
>>>>>
>>>>
>>>>
>>>> What do you think about below alternative?
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_device.c
>>>> b/arch/arm/mach-omap2/omap_device.c
>>>> index 4cb8fd9..72ebc4c 100644
>>>> --- a/arch/arm/mach-omap2/omap_device.c
>>>> +++ b/arch/arm/mach-omap2/omap_device.c
>>>> @@ -901,7 +901,8 @@ static int __init omap_device_late_idle(struct
>>>> device *dev, void *data)
>>>>                   if (od->hwmods[i]->flags & HWMOD_INIT_NO_IDLE)
>>>>                           return 0;
>>>>
>>>> -       if (od->_driver_status != BUS_NOTIFY_BOUND_DRIVER) {
>>>> +       if (od->_driver_status != BUS_NOTIFY_BOUND_DRIVER &&
>>>> +           od->_driver_status != BUS_NOTIFY_BIND_DRIVER) {
>>>>                   if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
>>>>                           dev_warn(dev, "%s: enabled but no driver.
>>>> Idling\n",
>>>>                                    __func__);
>>>
>>> Seems better to me if it really fixes the issue.
>>>
>>
>> My dra7-evm failed to boot on "2b186e5 Add linux-next specific files
>> for 20150827"
>> and this change restores boot.
>>
>> Will wait for confirmation from Keerthy.
>
> I confirm that with this patch the boot crash is fixed.
>
> Tested-by: Keerthy <j-keerthy@ti.com>
>
>
> Without this patch i see this crash during boot:
>

Thanks, Keerthy.

I'll update and resend this new patch version.

-- 
regards,
-grygorii