On Thu, Apr 05, 2012 at 16:23:22, Alan Cox wrote:
> On Thu, 5 Apr 2012 10:40:55 +0000
> "Shankarmurthy, Akshay" <aksha...@ti.com> wrote:
>
> > Hi,
> >
> > On Tue, Apr 03, 2012 at 15:25:30, Alan Cox wrote:
> > > > This patch was submitted 2 years ago but didn't make it to the
> > > > mainline. Now i am reposting it.
> > >
> > > Can you fix it instead of just reposting it ?
> > >
> > >
> > > > +#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;
> > >
> > > What is your locking model ?
> > >
> >
> > I will have a look at this and add the lock if necessary.
>
> At the very least you need reference counts held on the tty struct and to
> allow for the tty having vanished under you.
>
I am planning to add "tty->termios_mutex" lock. Any comments ?
> > >
> > > > +static inline void serial8250_cpufreq_deregister(struct
> > > > +uart_8250_port *p)
> > >
> > > unregister
> >
> > Ok. I will change it.
> >
> > >
> > > > + ret = serial8250_cpufreq_register(uart);
> > > > + if (ret < 0)
> > > > + printk(KERN_ERR "Failed to add cpufreq
> > > > notifier\n");
> > >
> > > Why do this for devices that don't care.
> >
> > This is taken care in the code. If the device's frequency doesn't
> > change after the change in the cpu frequency then it doesn't go for
> > execution of the function "serial8250_set_termios".
>
> But you still call each function and walk the list for each port on each CPU
> transition. There are people with 128 ports in their systems and the cpu
> frequency change is latency critical.
>
> Nothing should even be getting registered in such cases.
>
I understand this and planning to avoid unnecessary registration like these.
As a solution, UART port's cpu freq status will be sent from the board file.
Adding a member to the structure "plat_serial8250_port" which says whether
a port requires cpu freq registration or not. cpu freq register function is
called in the driver depending upon the status of this structure member.
I can update the patch accordingly and send out a version 2 to the list.
Regards,
Akshay
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
