Re: [Gta04-owner] [PATCH 0/3] tty slave device support - version 3.

Dr. H. Nikolaus Schaller
2015-03-20T08:54:38Z

Hi,

Am 20.03.2015 um 09:43 schrieb NeilBrown <neilb@suse.de>:

> On Fri, 20 Mar 2015 08:54:35 +0100 "Dr. H. Nikolaus Schaller"
> <hns@goldelico.com> wrote:
>
>> Hi Neil,
>>
>> Am 18.03.2015 um 06:58 schrieb NeilBrown <neil@brown.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.
>
> Don’t read too much into the word "bus".

Then we should find a different word.

> It is really just a mechanism for
> grouping drivers together - with maybe a hint of a suggestions that it helps
> communicate with the devices which the drivers drive.
>
> And I'm not particularly interested in solving wide-spread problems, just in
> solving my problem in a suitably idiomatic way.
>
>
>>
>>> 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.
>
> But (nearly) every node has a "compatible" entry.
> Each node describes a device, and the "compatible" string tells us what sort
> of device.

Yes, it should - if necessary.

> Surely you agree that there is a particular device that needs to be
> controlled?

No, my opinion is that a specific regulator should be controlled.

> In that case there needs to be a node representing it and a
> "compatible" string describing it. That is how “devicetree" works.

For this there is the vdd-regulator = <&regulator> idiom and that is
how “devicetree” works as well.

>
>
>>
>>>
>>> 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.
>
> You can prefer that if you wish. But given that the w2sg0004 is not in fact
> a regulator,

therefore our proposal is to make it a drivers/misc and not a drivers/regulator.
But it has a regulator inside that can obviously be turned on or off. So I want to
keep close to the logical energy flow.

> but is in fact a GPS device, I doubt you will find a lot of
> support for your approach (as indeed I didn’t when I tried it...)

We got a lot of support. The main opposition was about using a “virtual”
gpio controller instead of a regulator API to turn the chip on and off as
directed by the tty driver.

And we do not write a driver for the w2sg0004 but the regulator inside that
chip. You also won’t expect that the omap3 driver hides everything. You
expect that the twl4030 provides some regulators that can be enabled
by subsystems inside the omap3. And if I remember correctly there are
regulators inside the omap3 which have explicit regulator nodes in the DT.

The w2sg0004-regulator approach just follows this principle. Anyone willing
to control the power of the w2sg0004 can use this regulator interface.

BR,
Nikolaus