Message-ID: <CAGm1_kvSEcXRGoQrUoJcmFq6gTHkjupzDqesw3cTmL=V74yPOw@mail.gmail.com>
Date:	Fri, 25 Apr 2014 11:53:21 +0200
From:	Yegor Yefremov <yegorslists@...glemail.com>
To:	NeilBrown <neilb@...e.de>
Cc:	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>, balbi@...com,
	Nishanth Menon <nm@...com>,
	Greg KH <gregkh@...uxfoundation.org>,
	Marcel Holtmann <marcel@...tmann.org>, gustavo@...ovan.org,
	johan.hedberg@...il.com, jslaby@...e.cz,
	Grant Likely <grant.likely@...aro.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-bluetooth@...r.kernel.org, linux-serial@...r.kernel.org,
	devicetree@...r.kernel.org,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	Tony Lindgren <tony@...mide.com>
Subject: Re: [PATCH 10/13] tty: serial: omap: remove some dead code

On Fri, Apr 25, 2014 at 11:34 AM, NeilBrown <neilb@...e.de> wrote:
> On Thu, 24 Apr 2014 15:19:14 +0100 One Thousand Gnomes
> <gnomes@...rguk.ukuu.org.uk> wrote:
>
>> > > 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.
>>
>> Unless you are using it as a "real' DTR line then I think this is the
>> wrong approach. This problem actually is turning up in both PC class and
>> ARM boxes now all over the place for three reasons I am seeing.
>>
>> 1. We are getting a lot of hardware moving to serial attached
>> bluetooth/gps/etc because of the power win. In addition ACPI can describe
>> power relationships and what is on the other end of a UART embedded in
>> the device
>>
>> 2. We've got cheap hardware with modem lines being "retrofitted" via gpio
>>
>> 3. There are a subset of devices that have extra control lines beyond the
>> usual serial port ones. These range from additional control lines for
>> things like smartcard programmers to port muxing.
>>
>> > 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.
>>
>> serial-core provides power hooks. If the goal is that this comes on when
>> you power up the uart and goes away on the last close then the hooks are
>> there already.
>
> Could you be a bit more explicit, or point to an example user of these hooks?
>
> I had a quick look and I guess that uart_change_pm() is the most likely
> candidate for what you are referring to.
> I can see how that interfaces to the specific piece of uart hardware, such as
> omap-serial.c
> But I cannot see how you would plumb that though to the device that was
> plugged in to the serial port.  I guess if that device could be registered as
> a child of the omap_serial device, then power management inheritance might
> come in to play, but I cannot see any way to tell a serial port that it has
> some arbitrary child device.
>
> So maybe I'm missing something.
>
>>                  If its ldisc specific then the tty layer also calls the
>> device on ldisc changes precisely so it can make hardware specific
>> twiddles in such cases.
>>
>> A set of gpios on the tty_port object would not go amiss and would also
>> address the PC case.
>>
>> > 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.
>>
>> ldiscs are hardware independent. Nothing about hardware belongs in an
>> ldisc. Any ldisc should within reason work on any port.
>>
>> What I propsed when it came up ages ago was to expose some gpio settings
>> in the tty, to provide ways for them to be set by default and also ioctls
>> to configure them. I still think this (but moved into the tty_port as its
>> a persistent hardware property) is a good idea now that we are starting
>> to see more use cases than weird dongles and muxes on pre-production
>> reference boards.
>>
>> With my tty and serial hat on I think a power gpio is a no-brainer even
>> without doing the other work and therefore it should probably be described
>> generically for a serial port in the DT as well as in the ACPI data. If
>> you should also be able to give it a regulator to use as well that also
>> seems to make complete sense.
>
> In one case the "power on" sequence is substantially more complex that just a
> gpio or regulator.  I would need to write a device driver for the (GPS) chip
> which could receive a poweron/poweroff signal from the uart and do the
> required magic.
>
> Having serial-core know about gpios and regulators and random other stuff
> seems wrong.
> I would really like to see the "runtime interpreted power sequences" become a
> real thing.  Then serial-core could activate a "RIPS", and that would have
> the flexibility to do whatever is needed without adding complexity to
> serial-core.
> Using a virtual GPIO was my poor-mans RIPS.  Using gpiolib, and driver can
> pretend to be a gpio so it is a simple "pipe" to send a power-on/power-off
> signal over.
>
> So ... with your "serial" hat on, if I were to write/test a patch to allow
> any serial port to have a "power-gpio" described in DT and the gpio would be
> driven in uart_change_pm(), would you consider accepting that patch, or did I
> misunderstand?

As soon as this patch
(http://www.spinics.net/lists/arm-kernel/msg325197.html) will be
applied, we don't really need this DTR GPIO any more.

DTR_active is the only stuff, that is missing.

Yegor
--
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