On Thu, May 6, 2010 at 4:14 PM, Nishanth Menon <nm@xxxxxx> 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@xxxxxx> wrote: >>> >>> Hi Kevin, >>> >>> My Comments as below. >>> >>> Regards >>> Deepak >>> >>> -----Original Message----- >>> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>> 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@xxxxxx> writes: >>> >>>> From: Deepak K <deepak.k@xxxxxx> >>>> >>>> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
