All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	sachi@mistralsolutions.com,
	davinci-linux-open-source@linux.davincidsp.com,
	Subhasish Ghosh <subhasish@mistralsolutions.com>,
	"Greg Kroah-Hartman" <gregkh@suse.de>,
	nsekhar@ti.com, open list <linux-kernel@vger.kernel.org>,
	m-watkins@ti.com
Subject: Re: [PATCH v2 13/13] tty: pruss SUART driver
Date: Fri, 18 Feb 2011 19:51:32 +0100	[thread overview]
Message-ID: <201102181951.32631.arnd@arndb.de> (raw)
In-Reply-To: <alpine.LFD.2.00.1102181917520.2701@localhost6.localdomain6>

On Friday 18 February 2011 19:23:49 Thomas Gleixner wrote:
> On Fri, 18 Feb 2011, Alan Cox wrote:
> 
> > On Fri, 18 Feb 2011 19:17:38 +0530
> > "Subhasish Ghosh" <subhasish@mistralsolutions.com> wrote:
> > 
> > > Hello,
> > > 
> > > Regarding the semaphore to mutex migration.
> > > We are using down_trylock in interrupt context,
> > > mutex_trylock cannot be used in interrupt context, so we cannot use mutex in 
> > > our driver.
> > 
> > Then you probably need to rework your locking. Best bet might be to fix
> > all the other stuff and report the driver, and people can think about the
> > locking problem.
> 
> That semaphore is utterly useless to begin with. There are more
> serious locking problems than this one. Non serialized calls to
> suart_intr_clrmask/suart_intr_setmask are the most obvious ones.
> 
> Aside of that the code is complete unreadable.

I think it mostly suffers from the same problem as the CAN driver
I commented on earlier: One of the files (pruss_suart_api.c) was
clearly not written with Linux as the target, and the other files
try to work around this by wrapping a Linux driver around it.

The suart_api HAL stuff clearly needs to go away, so that the rest
can be rewritten into a proper device driver.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 13/13] tty: pruss SUART driver
Date: Fri, 18 Feb 2011 19:51:32 +0100	[thread overview]
Message-ID: <201102181951.32631.arnd@arndb.de> (raw)
In-Reply-To: <alpine.LFD.2.00.1102181917520.2701@localhost6.localdomain6>

On Friday 18 February 2011 19:23:49 Thomas Gleixner wrote:
> On Fri, 18 Feb 2011, Alan Cox wrote:
> 
> > On Fri, 18 Feb 2011 19:17:38 +0530
> > "Subhasish Ghosh" <subhasish@mistralsolutions.com> wrote:
> > 
> > > Hello,
> > > 
> > > Regarding the semaphore to mutex migration.
> > > We are using down_trylock in interrupt context,
> > > mutex_trylock cannot be used in interrupt context, so we cannot use mutex in 
> > > our driver.
> > 
> > Then you probably need to rework your locking. Best bet might be to fix
> > all the other stuff and report the driver, and people can think about the
> > locking problem.
> 
> That semaphore is utterly useless to begin with. There are more
> serious locking problems than this one. Non serialized calls to
> suart_intr_clrmask/suart_intr_setmask are the most obvious ones.
> 
> Aside of that the code is complete unreadable.

I think it mostly suffers from the same problem as the CAN driver
I commented on earlier: One of the files (pruss_suart_api.c) was
clearly not written with Linux as the target, and the other files
try to work around this by wrapping a Linux driver around it.

The suart_api HAL stuff clearly needs to go away, so that the rest
can be rewritten into a proper device driver.

	Arnd

  reply	other threads:[~2011-02-18 18:51 UTC|newest]

Thread overview: 161+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-11 14:51 [PATCH v2 00/13] pruss mfd drivers Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 01/13] mfd: pruss mfd driver Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-21 16:30   ` Samuel Ortiz
2011-02-21 16:30     ` Samuel Ortiz
2011-02-22  5:43     ` Subhasish Ghosh
2011-02-22  5:43       ` Subhasish Ghosh
2011-02-22 10:31       ` Samuel Ortiz
2011-02-22 10:31         ` Samuel Ortiz
2011-02-22 10:48         ` Wolfgang Grandegger
2011-02-22 10:48           ` Wolfgang Grandegger
2011-02-22 11:33           ` Samuel Ortiz
2011-02-22 11:33             ` Samuel Ortiz
2011-02-22 12:49             ` Subhasish Ghosh
2011-02-22 12:49               ` Subhasish Ghosh
2011-02-22 16:27               ` Wolfgang Grandegger
2011-02-22 16:27                 ` Wolfgang Grandegger
2011-02-23 12:25         ` Subhasish Ghosh
2011-02-23 12:25           ` Subhasish Ghosh
2011-02-23 13:09         ` Russell King - ARM Linux
2011-02-23 13:09           ` Russell King - ARM Linux
2011-02-11 14:51 ` [PATCH v2 02/13] da850: pruss platform specific additions Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 18:41   ` Sergei Shtylyov
2011-02-11 18:41     ` Sergei Shtylyov
2011-02-18  7:18     ` Subhasish Ghosh
2011-02-18  7:18       ` Subhasish Ghosh
2011-02-28 13:04   ` TK, Pratheesh Gangadhar
2011-02-28 13:04     ` TK, Pratheesh Gangadhar
2011-03-01  6:59     ` Subhasish Ghosh
2011-03-01  6:59       ` Subhasish Ghosh
2011-03-03 11:12       ` TK, Pratheesh Gangadhar
2011-03-03 11:12         ` TK, Pratheesh Gangadhar
2011-02-11 14:51 ` [PATCH v2 03/13] da850: pruss board " Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 18:43   ` Sergei Shtylyov
2011-02-11 18:43     ` Sergei Shtylyov
2011-02-18  7:18     ` Subhasish Ghosh
2011-02-18  7:18       ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 04/13] mfd: pruss CAN private data Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 05/13] da850: pruss CAN platform specific additions Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 06/13] da850: pruss CAN board " Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 18:45   ` Sergei Shtylyov
2011-02-11 18:45     ` Sergei Shtylyov
2011-02-18  7:19     ` Subhasish Ghosh
2011-02-18  7:19       ` Subhasish Ghosh
2011-02-18  7:19     ` Subhasish Ghosh
2011-02-18  7:19       ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 07/13] da850: pruss CAN platform specific changes for gpios Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 18:47   ` Sergei Shtylyov
2011-02-11 18:47     ` Sergei Shtylyov
2011-02-18  7:20     ` Subhasish Ghosh
2011-02-18  7:20       ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 08/13] da850: pruss CAN board " Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 09/13] can: pruss CAN driver Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 15:06   ` Kurt Van Dijck
2011-02-11 15:06     ` Kurt Van Dijck
2011-02-11 15:06     ` Kurt Van Dijck
2011-02-14  4:54     ` Subhasish Ghosh
2011-02-14  4:54       ` Subhasish Ghosh
2011-02-14  7:23       ` Wolfgang Grandegger
     [not found]         ` <4D58D854.5090503-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-02-14  7:42           ` Kurt Van Dijck
2011-02-14  7:42             ` Kurt Van Dijck
2011-02-14  8:45         ` Subhasish Ghosh
2011-02-14  8:45           ` Subhasish Ghosh
2011-02-14  9:28           ` Wolfgang Grandegger
2011-02-14  9:35           ` Marc Kleine-Budde
2011-02-14 13:15             ` Subhasish Ghosh
2011-02-14 13:15               ` Subhasish Ghosh
2011-02-14 13:15               ` Subhasish Ghosh
2011-02-14 13:33               ` Marc Kleine-Budde
2011-02-14 13:42               ` Wolfgang Grandegger
2011-02-11 15:20   ` Kurt Van Dijck
2011-02-11 15:20     ` Kurt Van Dijck
2011-02-11 15:20     ` Kurt Van Dijck
2011-02-18  7:07     ` Subhasish Ghosh
2011-02-18  7:07       ` Subhasish Ghosh
2011-02-18  7:07       ` Subhasish Ghosh
2011-02-18  7:53       ` Wolfgang Grandegger
2011-02-18  8:15         ` Subhasish Ghosh
2011-02-18  8:15           ` Subhasish Ghosh
2011-02-18  8:15           ` Subhasish Ghosh
2011-02-18  8:36           ` Marc Kleine-Budde
2011-02-18  8:36             ` Marc Kleine-Budde
2011-02-18  8:36             ` Marc Kleine-Budde
2011-02-18  9:09             ` Subhasish Ghosh
2011-02-18  9:09               ` Subhasish Ghosh
2011-02-18  9:09               ` Subhasish Ghosh
     [not found]   ` <1297435892-28278-10-git-send-email-subhasish-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
2011-02-11 20:33     ` Wolfgang Grandegger
2011-02-11 21:33     ` Marc Kleine-Budde
2011-02-18 15:07   ` Arnd Bergmann
2011-02-18 15:07     ` Arnd Bergmann
2011-03-22  7:30     ` Subhasish Ghosh
2011-03-22  7:30       ` Subhasish Ghosh
2011-03-22  7:30       ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 10/13] mfd: pruss SUART private data Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 11/13] da850: pruss SUART board specific additions Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 15:26   ` Michael Williamson
2011-02-11 15:26     ` Michael Williamson
2011-02-18  7:13     ` Subhasish Ghosh
2011-02-18  7:13       ` Subhasish Ghosh
2011-02-11 18:50   ` Sergei Shtylyov
2011-02-11 18:50     ` Sergei Shtylyov
2011-02-22  6:22     ` Subhasish Ghosh
2011-02-22  6:22       ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 12/13] da850: pruss SUART platform " Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 18:55   ` Sergei Shtylyov
2011-02-11 18:55     ` Sergei Shtylyov
2011-02-22  9:18     ` Subhasish Ghosh
2011-02-22  9:18       ` Subhasish Ghosh
2011-02-22 11:20       ` Sergei Shtylyov
2011-02-22 11:20         ` Sergei Shtylyov
2011-02-22 13:24         ` Subhasish Ghosh
2011-02-22 13:24           ` Subhasish Ghosh
2011-02-11 14:51 ` [PATCH v2 13/13] tty: pruss SUART driver Subhasish Ghosh
2011-02-11 14:51   ` Subhasish Ghosh
2011-02-11 16:28   ` Alan Cox
2011-02-11 16:28     ` Alan Cox
2011-02-18 13:47     ` Subhasish Ghosh
2011-02-18 13:47       ` Subhasish Ghosh
2011-02-18 14:35       ` Alan Cox
2011-02-18 14:35         ` Alan Cox
2011-02-18 18:23         ` Thomas Gleixner
2011-02-18 18:23           ` Thomas Gleixner
2011-02-18 18:51           ` Arnd Bergmann [this message]
2011-02-18 18:51             ` Arnd Bergmann
2011-02-22  8:42             ` Subhasish Ghosh
2011-02-22  8:42               ` Subhasish Ghosh
2011-02-22 14:37               ` Greg KH
2011-02-22 14:37                 ` Greg KH
2011-02-23  5:30                 ` Subhasish Ghosh
2011-02-23  5:30                   ` Subhasish Ghosh
2011-02-23 18:20                   ` Greg KH
2011-02-23 18:20                     ` Greg KH
2011-02-22  8:43             ` Subhasish Ghosh
2011-02-22  8:43               ` Subhasish Ghosh
2011-02-22 16:34               ` Arnd Bergmann
2011-02-22 16:34                 ` Arnd Bergmann
2011-02-24 10:31                 ` Subhasish Ghosh
2011-02-24 10:31                   ` Subhasish Ghosh
2011-02-22 10:26     ` Subhasish
2011-02-22 10:26       ` Subhasish
2011-02-22 11:11       ` Alan Cox
2011-02-22 11:11         ` Alan Cox
2011-03-01 13:37         ` Subhasish Ghosh
2011-03-01 13:37           ` Subhasish Ghosh
2011-03-01 14:07           ` Alan Cox
2011-03-01 14:07             ` Alan Cox
2011-02-23 13:35 Subhasish Ghosh
2011-02-23 13:34 ` Subhasish Ghosh
2011-02-23 18:21 ` Greg KH
2011-02-23 18:21   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201102181951.32631.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=davinci-linux-open-source@linux.davincidsp.com \
    --cc=gregkh@suse.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m-watkins@ti.com \
    --cc=nsekhar@ti.com \
    --cc=sachi@mistralsolutions.com \
    --cc=subhasish@mistralsolutions.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.