Message-Id: <80D7E742-4633-4CCD-B754-221387D82922@goldelico.com>
Date:	Fri, 20 Mar 2015 08:54:35 +0100
From:	"Dr. H. Nikolaus Schaller" <hns@...delico.com>
To:	NeilBrown <neilb@...e.de>
Cc:	Mark Rutland <mark.rutland@....com>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Peter Hurley <peter@...leysoftware.com>,
	Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sebastian Reichel <sre@...nel.org>,
	Pavel Machek <pavel@....cz>,
	Grant Likely <grant.likely@...aro.org>,
	Jiri Slaby <jslaby@...e.cz>, devicetree@...r.kernel.org,
	lkml <linux-kernel@...r.kernel.org>,
	Belisko Marek <marek.belisko@...il.com>,
	List for communicating with real GTA04 owners 
	<gta04-owner@...delico.com>
Subject: Re: [Gta04-owner] [PATCH 0/3] tty slave device support - version 3.

Hi Neil,

Am 18.03.2015 um 06:58 schrieb NeilBrown <neil@...wn.name>:

> Hi again,
> here is version 3 of support for tty-slaves.
> 
> This version introduces a new bus-type for tty-slaves,

Hm. I am still not convinced that a tty is a „bus“ and 
that this is a solution for a wide-spread problem.

> and causes
> a tty-slave device to appear in /sys/devices between the uart and the
> tty.
> It effectively intercepts and calls from the tty to the uart (i.e. any
> tty_operations) and applies extra functionality at that point.

> 
> Currently the only driver intercepts open and close.
> It powers on the device on open, and powers off at last-close.

That is what the missing piece in Linux is to make the w2sg0004
chip work.

> 
> Power can be controlled by a regulator or by toggling a GPIO.

I think such a GPIO logic has nothing to do with serial and
should be left over to the regulator logic, i.e. we need a special
regulator-w2sg0004 driver.

So I suggest to remove the GPIO logic from your 

drivers/tty/slave/serial-power-manager.c

And then you can even get rid of adding a chip specific „compatible“
entry for the subnodes.

> 
> I think I've incorporated most of the feed back I received from
> previous versions, but if I missed something - I apologize.  If
> this approach is structurally acceptable then I can fix up all the
> smaller issues.

As said I would prefer that the w2sg0004 driver is just a separate
„regulator“ driver as we had proposed before.

Nikolaus



> 
> Thanks for your review,
> NeilBrown
> 
> 
> ---
> 
> NeilBrown (3):
>      TTY: use class_find_device to find port in uart_suspend/resume.
>      TTY: add support for tty_slave devices.
>      tty/slaves: add a driver to power on/off UART attached devices.
> 
> 
> .../bindings/tty_slave/wi2wi,w2cbw003.txt          |   19 +
> .../bindings/tty_slave/wi2wi,w2sg0004.txt          |   37 +
> .../devicetree/bindings/vendor-prefixes.txt        |    1 
> drivers/tty/Kconfig                                |    1 
> drivers/tty/Makefile                               |    1 
> drivers/tty/serial/serial_core.c                   |   21 -
> drivers/tty/slave/Kconfig                          |   21 +
> drivers/tty/slave/Makefile                         |    4 
> drivers/tty/slave/serial-power-manager.c           |  510 ++++++++++++++++++++
> drivers/tty/slave/tty_slave_core.c                 |  136 +++++
> drivers/tty/tty_io.c                               |   60 ++
> include/linux/tty.h                                |    2 
> include/linux/tty_slave.h                          |   26 +
> 13 files changed, 813 insertions(+), 26 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/tty_slave/wi2wi,w2cbw003.txt
> create mode 100644 Documentation/devicetree/bindings/tty_slave/wi2wi,w2sg0004.txt
> create mode 100644 drivers/tty/slave/Kconfig
> create mode 100644 drivers/tty/slave/Makefile
> create mode 100644 drivers/tty/slave/serial-power-manager.c
> create mode 100644 drivers/tty/slave/tty_slave_core.c
> create mode 100644 include/linux/tty_slave.h
> 
> --
> Signature
> 
> _______________________________________________
> Gta04-owner mailing list
> Gta04-owner@...delico.com
> http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists