Santosh Shilimkar <santosh.shilimkar@xxxxxx> writes:
> On Friday 19 April 2013 06:19 AM, Jon Hunter wrote:
>>
>> On 04/18/2013 07:34 PM, Jon Hunter wrote:
>>>
>>> On 04/18/2013 06:10 PM, Jon Hunter wrote:
>>>> On 04/18/2013 04:34 PM, Kevin Hilman wrote:
>>>
>>> ...
>>>
>>>>> Why not just init context right here if bank->loses_context &&
>>>>> !bank->context_valid?
>>>
>>> I really like this idea a lot. It can really clean-up the code
>>> and really make it much more readable. Before we were playing
>>> some tricks with when we init'ed the get_context_loss_count()
>>> function pointer. How about the below?
>>>
>>> Tony, care to re-test?
>>>
>>> Cheers
>>> Jon
>>>
>>> From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001
>>> From: Jon Hunter <jon-hunter@xxxxxx>
>>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>>>
>>> Commit a2797be (gpio/omap: force restore if context loss is not
>>> detectable) broke gpio support for OMAP when booting with device-tree
>>> because a restore of the gpio context being performed without ever
>>> initialising the gpio context. In other words, the context restored was
>>> bad.
>>>
>>> This problem could also occur in the non device-tree case, however, it
>>> is much less likely because when booting without device-tree we can
>>> detect context loss via a platform specific API and so context restore
>>> is performed less often.
>>>
>>> Nevertheless we should ensure that the gpio context is initialised
>>> on the first pm-runtime resume for gpio banks that could lose their
>>> state regardless of whether we are booting with device-tree or not.
>>>
>>> The context loss count was being initialised on the first pm-runtime
>>> suspend following a resume, by populating the get_count_loss_count()
>>> function pointer after the first pm-runtime resume. To make the code
>>> more readable and logical, initialise the context loss count on the
>>> first pm-runtime resume if the context is not yet valid.
>>>
>>> Reported-by: Tony Lindgren <tony@xxxxxxxxxxx>
>>> Signed-off-by: Jon Hunter <jon-hunter@xxxxxx>
>>> ---
>>> drivers/gpio/gpio-omap.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 42 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>>> index 0557529..db3c732 100644
>>> --- a/drivers/gpio/gpio-omap.c
>>> +++ b/drivers/gpio/gpio-omap.c
>>> @@ -70,6 +70,7 @@ struct gpio_bank {
>>> bool is_mpuio;
>>> bool dbck_flag;
>>> bool loses_context;
>>> + bool context_valid;
>>> int stride;
>>> u32 width;
>>> int context_loss_count;
>>> @@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
>>> bank->loses_context = true;
>>> } else {
>>> bank->loses_context = pdata->loses_context;
>>> + bank->get_context_loss_count = pdata->get_context_loss_count;
>>
>> Still need to check loses_context for populating
>> get_context_loss_count here. Updated patch below.
>>
>> Jon
>>
>> From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001
>> From: Jon Hunter <jon-hunter@xxxxxx>
>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>>
>> Commit a2797be (gpio/omap: force restore if context loss is not
>> detectable) broke gpio support for OMAP when booting with device-tree
>> because a restore of the gpio context being performed without ever
>> initialising the gpio context. In other words, the context restored was
>> bad.
>>
>> This problem could also occur in the non device-tree case, however, it
>> is much less likely because when booting without device-tree we can
>> detect context loss via a platform specific API and so context restore
>> is performed less often.
>>
>> Nevertheless we should ensure that the gpio context is initialised
>> on the first pm-runtime resume for gpio banks that could lose their
>> state regardless of whether we are booting with device-tree or not.
>>
>> The context loss count was being initialised on the first pm-runtime
>> suspend following a resume, by populating the get_count_loss_count()
>> function pointer after the first pm-runtime resume. To make the code
>> more readable and logical, initialise the context loss count on the
>> first pm-runtime resume if the context is not yet valid.
>>
>> Reported-by: Tony Lindgren <tony@xxxxxxxxxxx>
>> Signed-off-by: Jon Hunter <jon-hunter@xxxxxx>
>> ---
> This version looks better than the first one for sure. I am still not
> happy with per bank "context_valid" flag whose job just ends after
> the probe.
Assuming this driver could become a module someday (not terribly likely,
I know), but context_valid would have meaning for each module reload.
> But then I do agree with you about the global flag might
> be fragile and less maintainable.
>
> So, FWIW,
> Acked-by: Santosh Shilimkar<santosh.shilimkar@xxxxxx>
Yes, this version is more readable. Thanks for the update.
Reviewed-by: Kevin Hilman <khilman@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
