Re: [PATCH 5/5] arm: msm: smd: fix SMD modem processor sync condition

[Dima Zavin <dima@android.com>]

On Mon, Apr 19, 2010 at 12:11 PM, Daniel Walker <dwalker@codeaurora.org> wrote:
> On Mon, 2010-04-19 at 12:06 -0700, Dima Zavin wrote:
>> On Mon, Apr 19, 2010 at 12:01 PM, Daniel Walker <dwalker@codeaurora.org> wrote:
>> > On Mon, 2010-04-19 at 11:34 -0700, Dima Zavin wrote:
>> >> Do we really need a formalized blocking point here? The apps processor
>> >> can do other useful initialization work while the modem is booting.
>> >> The first time you do a proc_comm call, it checks the PCOM_READY
>> >> state, and will block anyway. Preventing the apps processor from
>> >> continuing until then is suboptimal. If there are bugs in the modem
>> >> code where it incorrectly stomps on shared resources, then those
>> >> should be fixed. This patch looks like a hack to me.
>> >
>> >
>> > Yes, we need to formalize a blocking point .. The apps processor waits
>> > in this way no matter what you do .. Like your saying above "The first
>> > time you do a proc_comm call, it checks the PCOM_READY state, and will
>> > block anyway" that's a hack .. What your saying is _maybe_ there exists
>> > a proc_comm call early enough to prevent a crash, or maybe not .. That's
>> > not formal enough.
>>
>> That's not at all what I am saying. There's no maybe. If I don't need
>> anything from the modem, I won't make a proc_comm call. If there is a
>> crash because the modem is modifying shared resources that affect the
>> apps processor without an appropriate synchronization point, then it's
>> a bug on the modem side. Making this change will only mask modem bugs.
>
> If you don't make a proc_call call SMD won't initialize properly early
> on, since the modem may or may not be booted far enough to accept input
> over SMD.. Then you can basically have a failed SMD init, which means
> you crash when you actually need stuff through SMD.

But smd_core_init is already waiting for smd to be up. It waits for
SMEM_SMSM_SHARED_STATE to be published. What does proc_comm have to do
with it except for the fact that PCOM_READY gets set relatively late
in the modem boot? If the shared state infrastructure is not yet
initialized, it shouldn't be publishing it. Otherwise, you are just
overloading PCOM_READY and again masking issues. Especially when we
move to chips that don't need proc_comm for most things, and 7x30 is
one of them where we get a lot of local clock control, it really seems
wrong to wait for proc_comm to be up.

You mentioned that this change will prevent some random crashes. Have
you traced them down to what exactly is failing?

Thanks.

--Dima

>
> Daniel
>
>