Re: [PM][PATCH v3 2/4] OMAP3: Serial: Errata i202: fix for MDR1 access

[Govindraj <govindraj.ti@gmail.com>]

On Thu, May 6, 2010 at 4:14 PM, Nishanth Menon <nm@ti.com> wrote:
> Govindraj had written, on 05/06/2010 04:54 AM, the following:
>>
>> On Thu, May 6, 2010 at 5:49 AM, Kattungal, Deepak <deepak.k@ti.com> wrote:
>>>
>>> Hi Kevin,
>>>
>>> My Comments as below.
>>>
>>> Regards
>>> Deepak
>>>
>>> -----Original Message-----
>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>> Sent: Wednesday, May 05, 2010 6:55 PM
>>> To: Menon, Nishanth
>>> Cc: linux-omap; Kattungal, Deepak; Raja, Govindraj; Tero Kristo
>>> Subject: Re: [PM][PATCH v3 2/4] OMAP3: Serial: Errata i202: fix for MDR1
>>> access
>>>
>>> Nishanth Menon <nm@ti.com> writes:
>>>
>>>> From: Deepak K <deepak.k@ti.com>
>>>>
>>>> Original patch:
>>>>
>>>> http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=42d4a342c009bd9727c100abc8a4bc3063c22f0c
>>>>
>>>> Errata i202 (OMAP3430 - 1.12, OMAP3630 - 1.6):
>>>
>>> This workaround is currently done for all OMAPs.
>>>
>>> Presumably, this errata will eventually be fixed.  So, as with other
>>> errata fixes, we need some sort of SoC-rev based flag and the errata
>>> workaround done based on that flag.
>>>
>>> Deepak : This would be a good fix, thanks for the suggestion.
>>>
>>> Also, shouldn't there be a fix for this in the 8250 and omap-serial
>>> drivers too?
>>>
>>>
>>> Deepak : The 8250 is not using the MDR Register. This would be needed
>>> only by the omap-serial. The 8250 being a general driver we may not require
>>> the access to MDR1 register. Hence the fix not required for 8250.
>>>
>>
>> Why do you say this fix is not necessary for 8250?
>
> drivers/serial/8250.c does not use mdr register. it does not make sense for
> a generic driver like this to have OMAP specific register access.
>
>>
>> Isn't the omap_uart_reset configuring uart for 16x mode?
>>
>> Context save and restore is an common for whether it is 8250 or
>> omap-serial.c
>
> ack.
>
>>
>> The only difference here wrt to MDR and 8250 is
>> -> MDR though not accesed by 8250 currently serial.c sets it to uart16x
>> mode.
>
>>
>> In omap-serial.c we configure MDR based on baudrate as we have an uart13x
>> mode
>> which is ignored in 8250 and thus 8250 may not support 13x baud's
>> correctly
>> which is taken care in omap-serial.
>
> given that MDR register deals with mostly IRDA settings, i am curious to see
> how omap-serial hits on a shared register here :).

MDR is not for only IrDA,

MDR ---> Mode Definition register

There are two MDR register:

MDR1 [2:0]

MODE_SELECT: UART-IrDA-CIR mode selection

MDR2 ---> IR-IrDA and IR-CIR modes only


>
>>
>> I think this patch is fine and only concern is to make it applicable
>> to appropriate socs as kevin said.
>
> Sure. trivial to do. Could you comment on the proposal i posted earlier?

As said in previous mail I have no issue with the function added.
Except if taken care to handle the comment from kevin.


-- 
---
Regards,
Govindraj.R
--
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