Re: [PATCH 10/13] tty: serial: omap: remove some dead code

[Felipe Balbi]

Hi,

On Thu, Apr 24, 2014 at 10:13:29AM +1000, NeilBrown wrote:
> On Wed, 23 Apr 2014 18:01:21 -0500 Felipe Balbi <balbi@xxxxxx> wrote:
> 
> > Hi,
> > 
> > On Thu, Apr 24, 2014 at 08:43:05AM +1000, NeilBrown wrote:
> > > On Wed, 23 Apr 2014 10:35:04 -0500 Nishanth Menon <nm@xxxxxx> wrote:
> > > 
> > > > On 04/23/2014 09:58 AM, Felipe Balbi wrote:
> > > > > nobody passes a DTR_gpio to this driver, so
> > > > > this code is not necessary.
> > > > > 
> > > > > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > > > > ---
> > > > 
> > > > Niel,
> > > > this seems to revert the functionality introduced in
> > > > commit 9574f36fb801035f6ab0fbb1b53ce2c12c17d100
> > > > (OMAP/serial: Add support for driving a GPIO as DTR.)
> > > > 
> > > > would you like to Ack this change?
> > > 
> > > I have a couple of out-of-tree drivers that use this support.
> > > 
> > > I hope to get back to working on that code one day and even get those drivers
> > > upstream.  So I would really prefer this code to remain if it isn't causing
> > > any actual problems.
> > 
> > it causes problem with DT (not really). That suport is only available on
> > legacy platform_data-based boot, it's not available on DT. I hear Tony
> > is pretty close to turning OMAP3 DT-only.
> > 
> > > Of course, I can always re-submit it when I need it again, but that it just
> > > extra work all around.
> > 
> > I wonder how you will pass those attributes through DT considering they
> > are *really* SW configuration. Why can't you use the real DTR pin, btw ?
> > 
> 
> This myth that DT is only about hardware is probably one of the
> reasons that I haven't got these out-of-tree drivers upstream yet.

take that up with DT maintainers

> There is no "real" DTR pin.

heh, my bad... confused with RTS which is supported in this HW.

> When I open /dev/ttyWHATEVER, I need the driver for the device that is
> permanently connected to that serial port to be told that the serial-device
> has been opened so that it can "power on" or "wake up" the device.
> 
> I don't much care how that happens, or how I tell DT that it has to happen.
> I just need it to happen.
> 
> In discrete hardware devices, the DTR line is what you would use.  The RS232
> port would raise (or lower or whatever) DTR when /dev/ttyWHATEVER was open,
> and  the device that was plugged in would detect the level change and do
> stuff.
> 
> But I don't have discrete hardware.  I have a bunch of stuff soldered onto a
> board with ad-hoc connections chosen to make the life of the hardware builder
> easy rather than chosen to make the life of the software developer easy
> (which I think is the correct choice).
> 
> So I need to tell DT "This device is plugged into this UART, and there is no
> DTR line, but when the UARTs DTR line would be asserted (if it had one), then
> I need that regulator of there turned on". or maybe "I need to toggle this
> GPIO  with exactly this pattern, while watching that GPIO to see if it is
> working".
> 
> So I thought:
> 
>  1/ give the UART a "virtual" DTR so it could drive any GPIO
>  2/ create a "gpio-to-regulator" driver which presented as a (virtual) gpio
>     and responded to state changes on that GPIO by turning on or off the
>     regulator
>  3/ create a dedicated driver which knows how to toggle the magic GPIO while
>     watching the other GPIO to convince the silly device to wakeup, or go to
>     sleep, as required, and have this appear as a (virtual) GPIO.
> 
> Then I can just tell DT that these (virtual) GPIOs are connected together,
> and it will all just work.  And it does.
> 
> But given the whole "no no, DT is for describing hardware, and you are
> describing software" attitude,  it seems that I lost motivation for a while
> (that wasn't the only reason, but it didn't help).
> 
> I have a patch which converts the OMAP serial driver to use DT to configure
> these virtual DTR lines.  I'm very happy to submit that if there is some
> chance it might be accepted and will keep the current DTR code in place.

I have no problem either way, just that unused code doesn't have to be
sitting in the tree and I'm not entirely sure this GPIO should be
handled by omap-serial.c, perhaps something more generic inside
serial-core so other UART drivers can benefit from it.

> On the other hand, if you can point out to me what I'm missing, and how I can
> solve my problem with any virtual GPIOs, I'm all ears.
> 
> To make my problem simple and explicit:  I have a device attached to a UART
> which has a separate regulator.  The regulator should be powered on if and

So you're using DTR to power the GPIO and hoping that the regulator
stabilizes quickly enough so that by the time your open() finishes you
don't have to add nonsensical msleep() calls before writing to the
device. Sounds a bit fragile to me.

> only if the /dev/ttyXX interface to the UART is open.  The device is a
> bluetooth transceiver.

considering this is a BTUART device, why didn't you do this at the ldisc
level ? hci_uart_open() sounds like a good choice from a quick thinking.

-- 
balbi

Attachment: signature.asc
Description: Digital signature