"Chaithrika U S" <[email protected]> writes:

> On Mon, Jan 11, 2010 at 21:23:48, Kevin Hilman wrote:
>> Chaithrika U S <[email protected]> writes:
>> 
>> > On DA850/OMAP-L138 SoC, the PLL which supplies the clock to CPU also feeds 
>> > the
>> > UART and the UART input frequency can change when the CPU frequency is 
>> > scaled.
>> >
>> > This patch adds cpufreq support for 8250 serial driver. A clk structure 
>> > member
>> > has been added to the platform and port data structure. This member is 
>> > used by
>> > the cpufreq notifier callback to get the updated clock rate. The 
>> > implementation
>> > is based on the cpufreq implementation for Samsung serial driver.
>> >
>> > This implementation has been tested on TI DA850/OMAP-L138 EVM.
>> >
>> > Signed-off-by: Chaithrika U S <[email protected]>
>> > ---
>> > This patch is posted here for review and will be posted to LKML later.
>> 
>> Some minor comments/questions below.  Otherwise, should be posted to
>> linux-serial + LKML and Cc davinci list.
>> 
>> Hopefully somebody (hopefully Andrew) will pick this up since the 8250 
>> driver has
>> been orphaned for sometime.
>>      
>
> I will post this patch to linux-serial and LKML lists soon.
>
>> Kevin
>> 
>> 
>> >  drivers/serial/8250.c       |   81 
>> > +++++++++++++++++++++++++++++++++++++++++++
>> >  include/linux/serial_8250.h |    1 +
>> >  2 files changed, 82 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
>> > index c3e37c8..612e129 100644
>> > --- a/drivers/serial/8250.c
>> > +++ b/drivers/serial/8250.c
>> > @@ -38,6 +38,8 @@
>> >  #include <linux/serial_8250.h>
>> >  #include <linux/nmi.h>
>> >  #include <linux/mutex.h>
>> > +#include <linux/cpufreq.h>
>> > +#include <linux/clk.h>
>> >  
>> >  #include <asm/io.h>
>> >  #include <asm/irq.h>
>> > @@ -156,6 +158,10 @@ struct uart_8250_port {
>> >     */
>> >    void                    (*pm)(struct uart_port *port,
>> >                                  unsigned int state, unsigned int old);
>> > +  struct clk              *uart_clk;
>> > +#ifdef CONFIG_CPU_FREQ
>> > +  struct notifier_block   freq_transition;
>> > +#endif
>> >  };
>> >  
>> >  struct irq_info {
>> > @@ -2931,6 +2937,70 @@ void serial8250_resume_port(int line)
>> >    uart_resume_port(&serial8250_reg, &up->port);
>> >  }
>> >  
>> > +#ifdef CONFIG_CPU_FREQ
>> > +static int serial8250_cpufreq_transition(struct notifier_block *nb,
>> > +                                       unsigned long val, void *data)
>> > +{
>> > +  struct uart_8250_port *p;
>> > +  struct uart_port *uport;
>> > +
>> > +  p = container_of(nb, struct uart_8250_port, freq_transition);
>> > +  uport = &p->port;
>> > +
>> > +  if(!p->port.uartclk)
>> > +          goto cpu_freq_exit;
>> 
>> Isn't it OK to have a zero value here if you're going to use
>> clk_get_rate()?  or does uartclk == 0 have a special meaning here?  If
>> so, should be commented.
>> 
>
> This is a mistake here! The if condition should check 'uart_clk' - the
> clock struct pointer. I will correct this and post the updated patch.

OK, please be sure to use IS_ERR() to check clock pointer instead of
NULL pointer check.

Kevin
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to