gmane.linux.kernel.input

John Linn | 3 Jul 18:40

[PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2 driver

Added a new driver for Xilinx XPS PS2 IP. This driver is
a flat driver to better match the Linux driver pattern.

Signed-off-by: Sadanand <sadanan <at> xilinx.com>
Signed-off-by: John Linn <john.linn <at> xilinx.com>
---
V2
	Updated the driver based on feedback from Dmitry, Peter, and Grant.
	We believe Montavista copyright is still valid.

 drivers/input/serio/Kconfig      |    5 +
 drivers/input/serio/Makefile     |    1 +
 drivers/input/serio/xilinx_ps2.c |  448 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 454 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/serio/xilinx_ps2.c

diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index ec4b661..0e62b39 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -190,4 +190,9 @@ config SERIO_RAW
 	  To compile this driver as a module, choose M here: the
 	  module will be called serio_raw.

+config SERIO_XILINX_XPS_PS2
+	tristate "Xilinx XPS PS/2 Controller Support"
+	help
+	  This driver supports XPS PS/2 IP from Xilinx EDK.
+
 endif

(Continue reading)

headers

Dmitry Torokhov | 3 Jul 19:27

Re: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2 driver

Hi John,

On Thu, Jul 03, 2008 at 09:42:31AM -0700, John Linn wrote:
> +
> +	/* Initialize the PS/2 interface */
> +	mutex_lock(&drvdata->cfg_mutex);
> +	if (xps2_initialize(drvdata)) {
> +		mutex_unlock(&drvdata->cfg_mutex);
> +		dev_err(dev, "Could not initialize device\n");
> +		retval = -ENODEV;
> +		goto failed3;
> +	}
> +	mutex_unlock(&drvdata->cfg_mutex);

The drvdata is allocated per-port and so both (there are 2 PS/2 ports,
right?) ports get their own copy of cfg_mutex. Since you are trying to
serialze access to resource shared by both ports it will not work.
The original driver-global mutex was appropriate (the only thing I
objected there was use of a counting semaphore instead of a mutex).

--

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

headers

Grant Likely | 3 Jul 19:42

Re: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2 driver

On Thu, Jul 03, 2008 at 01:27:00PM -0400, Dmitry Torokhov wrote:
> Hi John,
> 
> On Thu, Jul 03, 2008 at 09:42:31AM -0700, John Linn wrote:
> > +
> > +	/* Initialize the PS/2 interface */
> > +	mutex_lock(&drvdata->cfg_mutex);
> > +	if (xps2_initialize(drvdata)) {
> > +		mutex_unlock(&drvdata->cfg_mutex);
> > +		dev_err(dev, "Could not initialize device\n");
> > +		retval = -ENODEV;
> > +		goto failed3;
> > +	}
> > +	mutex_unlock(&drvdata->cfg_mutex);
> 
> The drvdata is allocated per-port and so both (there are 2 PS/2 ports,
> right?) ports get their own copy of cfg_mutex. Since you are trying to
> serialze access to resource shared by both ports it will not work.
> The original driver-global mutex was appropriate (the only thing I
> objected there was use of a counting semaphore instead of a mutex).

John, correct me if I'm wrong, but I don't think there are any shared
resources being accessed here and I believe the mutex is entirely
unnecessary.

Cheers,
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo <at> vger.kernel.org

(Continue reading)

headers

John Linn | 3 Jul 19:59

RE: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2 driver

In reviewing the data sheet for the device, I don't see any shared
information between the 2 channels of the device.

The mutex looks like it's not needed to me also.  

I'll review with some others here to make sure I'm not overlooking
something. 

-- John

> -----Original Message-----
> From: Grant Likely [mailto:glikely <at> secretlab.ca] On Behalf Of Grant
Likely
> Sent: Thursday, July 03, 2008 11:42 AM
> To: Dmitry Torokhov
> Cc: John Linn; linuxppc-dev <at> ozlabs.org; linux-input <at> vger.kernel.org;
jwboyer <at> linux.vnet.ibm.com;
> jacmet <at> sunsite.dk; Sadanand Mutyala
> Subject: Re: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2
driver
> 
> On Thu, Jul 03, 2008 at 01:27:00PM -0400, Dmitry Torokhov wrote:
> > Hi John,
> >
> > On Thu, Jul 03, 2008 at 09:42:31AM -0700, John Linn wrote:
> > > +
> > > +	/* Initialize the PS/2 interface */
> > > +	mutex_lock(&drvdata->cfg_mutex);
> > > +	if (xps2_initialize(drvdata)) {
> > > +		mutex_unlock(&drvdata->cfg_mutex);

(Continue reading)

headers

Grant Likely | 3 Jul 19:37

Re: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2 driver

On Thu, Jul 03, 2008 at 09:42:31AM -0700, John Linn wrote:
> Added a new driver for Xilinx XPS PS2 IP. This driver is
> a flat driver to better match the Linux driver pattern.
> 
> Signed-off-by: Sadanand <sadanan <at> xilinx.com>
> Signed-off-by: John Linn <john.linn <at> xilinx.com>
> ---
> V2
> 	Updated the driver based on feedback from Dmitry, Peter, and Grant.
> 	We believe Montavista copyright is still valid.
> 
>  drivers/input/serio/Kconfig      |    5 +
>  drivers/input/serio/Makefile     |    1 +
>  drivers/input/serio/xilinx_ps2.c |  448 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 454 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/serio/xilinx_ps2.c
> 
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index ec4b661..0e62b39 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -190,4 +190,9 @@ config SERIO_RAW
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called serio_raw.
>  
> +config SERIO_XILINX_XPS_PS2
> +	tristate "Xilinx XPS PS/2 Controller Support"
> +	help
> +	  This driver supports XPS PS/2 IP from Xilinx EDK.
> +

(Continue reading)

headers

John Linn | 3 Jul 21:31

RE: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2 driver

Hi Grant,

Good comments.

With regard to the port-number, I don't see the value of using it unless
I'm just missing something.  The address of the device seems just as
useful.

We'll spin the patch again taking these into account and Dmitry's
comment also.

Thanks,
John

> -----Original Message-----
> From: Grant Likely [mailto:glikely <at> secretlab.ca] On Behalf Of Grant
Likely
> Sent: Thursday, July 03, 2008 11:37 AM
> To: John Linn
> Cc: linuxppc-dev <at> ozlabs.org; linux-input <at> vger.kernel.org;
jwboyer <at> linux.vnet.ibm.com;
> dmitry.torokhov <at> gmail.com; jacmet <at> sunsite.dk; Sadanand Mutyala
> Subject: Re: [PATCH] [V2] powerpc: Xilinx: PS2: Added new XPS PS2
driver
> 
> On Thu, Jul 03, 2008 at 09:42:31AM -0700, John Linn wrote:
> > Added a new driver for Xilinx XPS PS2 IP. This driver is
> > a flat driver to better match the Linux driver pattern.
> >
> > Signed-off-by: Sadanand <sadanan <at> xilinx.com>

(Continue reading)