GPIO interface between DVB sub-drivers (bridge, demod, tuner)

GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Antti Palosaari]

I am looking kinda standardized interface for setting GPIOs between the 
DVB drivers chip drivers. For most typical example there is LNA which is 
controlled by RF-tuner GPIO. That kind of LNA controlling logic belongs 
to interface driver thus I will need to call demod driver from bridge in 
order to set LNA.

Simply solution is to add own own callbacks, set_gpio() and get_gpio() 
for both tuner and demod ops but I will not like to that of there is 
existing solutions. Grepping Documentation reveals quite much documents, 
most interesting seems to be:
Documentation/gpio.txt
Documentation/devicetree/bindings/gpio/gpio.txt
Documentation/pinctrl.txt

Is there anyone who could say straight now what is best approach and 
where to look example?


regards
Antti

-- 
http://palosaari.fi/

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Steven Toth]

Resend in plaintext, it got bounced from vger.

On Thu, Jul 12, 2012 at 4:49 PM, Steven Toth <stoth@kernellabs.com> wrote:
>>
>> Is there anyone who could say straight now what is best approach and
>> where to look example?
>>
>
> I don't have an answer but the topic does interest me. :)
>
> Nobody understands the relationship between the bridge and the
> sub-component as well as the bridge driver. The current interfaces are
> limiting in many ways. We solve that today with rather ugly 'attach'
> structures that are inflexible, for example to set gpios to a default state.
> Then, once that interface is attached, the bridge effectively loses most of
> the control to the tuner and/or demod. The result is a large disconnect
> between the bridge and subcomponents.
>
> Why limit any interface extension to GPIOs? Why not make something a
> little more flexible so we can pass custom messages around?
>
> get(int parm, *value) and set(int parm, value)
>
> Bridges and demods can define whatever parmid's they like in their attach
> headers. (Like we do for callbacks currently).
>
> For example, some tuners have temperature sensors, I have no ability to
> read that today from the bridge, other than via I2C - then I'm pulling i2c
> device specific logic into the bridge. :(
>
> It would be nice to be able to demod/tunerptr->get(XC5000_PARAM_TEMP,
> &value), for example.
>
> Get/Set I/F is a bit of a classic, between tuners and video decoders. This
> (at least a while ago) was poorly handled, or not at all. Only the bridge
> really knows how to deal with odd component configurations like this, yet it
> has little or no control.
>
> I'd want to see a list of 4 or 5 good get/set use cases though, with some
> unusual parms, before I'd really believe a generic get/set is more
> appropriate than a strongly typed set of additional function pointers.
>
> What did you ever decide about the enable/disable of the LNA? And, how
> would the bridge do that in your proposed solution? Via the proposed GPIO
> interface?
>
> Regards,
>
> --
> Steven Toth - Kernel Labs
> http://www.kernellabs.com
> +1.646.355.8490

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Antti Palosaari]

On 07/13/2012 12:07 AM, Steven Toth wrote:
> Resend in plaintext, it got bounced from vger.
>
> On Thu, Jul 12, 2012 at 4:49 PM, Steven Toth <stoth@kernellabs.com> wrote:
>>>
>>> Is there anyone who could say straight now what is best approach and
>>> where to look example?
>>>
>>
>> I don't have an answer but the topic does interest me. :)
>>
>> Nobody understands the relationship between the bridge and the
>> sub-component as well as the bridge driver. The current interfaces are
>> limiting in many ways. We solve that today with rather ugly 'attach'
>> structures that are inflexible, for example to set gpios to a default state.
>> Then, once that interface is attached, the bridge effectively loses most of
>> the control to the tuner and/or demod. The result is a large disconnect
>> between the bridge and subcomponents.

I agree that mostly. For example that GPIO. It fits very poorly for 
interfaces that are highly given hardware design dependent like GPIOs. 
You can code logic like GPIO0 is LED, GPIO0 is tuner reset, GPIO0 is LNA 
and then set that logic in attach(). Due to that I am looking for better 
alternative.

>> Why limit any interface extension to GPIOs? Why not make something a
>> little more flexible so we can pass custom messages around?
>>
>> get(int parm, *value) and set(int parm, value)
>>
>> Bridges and demods can define whatever parmid's they like in their attach
>> headers. (Like we do for callbacks currently).
>>
>> For example, some tuners have temperature sensors, I have no ability to
>> read that today from the bridge, other than via I2C - then I'm pulling i2c
>> device specific logic into the bridge. :(

Yes! indeed, it is only possibly just like you said. Fortunately this 
kind of properties are not very common. Temperature is offered by many 
tuners, but there is no much need for that info in bridge. Maybe force 
sleep or switch powers totally off by using GPIO to prevent overheat.

>> It would be nice to be able to demod/tunerptr->get(XC5000_PARAM_TEMP,
>> &value), for example.
>>
>> Get/Set I/F is a bit of a classic, between tuners and video decoders. This
>> (at least a while ago) was poorly handled, or not at all. Only the bridge
>> really knows how to deal with odd component configurations like this, yet it
>> has little or no control.

IF information is now set tuner on attach() and then tuner delivers that 
info to demodulator via .get_if_frequency() which is member of tuner 
ops. Technically that solution works. If hardware design based IFs are 
not given as parameter on tuner attach() tuner should use tuner default 
IFs which are likely quite correct.

>> I'd want to see a list of 4 or 5 good get/set use cases though, with some
>> unusual parms, before I'd really believe a generic get/set is more
>> appropriate than a strongly typed set of additional function pointers.

Generally speaking for that set/get, I sent mail about ~same issue 
yesterday.
http://www.spinics.net/lists/linux-media/msg50202.html

There is set_property() and get_property() callbacks defined for FE 
already. But those are not used. My opinion is that those callbacks 
should be changed to deliver data for demodulator and for tuner too 
instead of current method - which is common huge properties cache 
structure shared between all sub-drivers. I don't like it all as it is 
stupid to add stuff that common structure for every standard we has. 
Lets take example device that supports DVB-C and other device supports 
ISDB-T. How many common parameters we has? I think only one - frequency. 
All the others are just waste.

What I think I am going to make new RFC which changes that so every 
parameter from userspace is passed to the demodulator driver (using 
set_property() - change it current functionality) and not cached to the 
common cache at all. Shortly: change current common cache based 
parameter transmission to happen set parameter to demodulator one by one.

>> What did you ever decide about the enable/disable of the LNA? And, how
>> would the bridge do that in your proposed solution? Via the proposed GPIO
>> interface?

I sent PATCH RFC for DVB API to add LNA support yesterday. It is new API 
parameter. User could select ON/OFF/AUTO and on AUTO mode driver should 
make decision, likely taking signal measurements etc.
New callback is added for frontend. If bridge likes to handle LNA it 
sets that callback in order to get user requests. When bridge gets that 
set_lna() callback it examines what user request and likely sets GPIOs.

regards
Antti



-- 
http://palosaari.fi/

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Steven Toth]

> There is set_property() and get_property() callbacks defined for FE already.
> But those are not used. My opinion is that those callbacks should be changed
> to deliver data for demodulator and for tuner too instead of current method
> - which is common huge properties cache structure shared between all
> sub-drivers. I don't like it all as it is stupid to add stuff that common
> structure for every standard we has. Lets take example device that supports
> DVB-C and other device supports ISDB-T. How many common parameters we has? I
> think only one - frequency. All the others are just waste.

When we did DVB V5 for S2 we added set/get property for the
demodulators, from memory I had some cx24116 S2 specifics that I was
passing, and I expected other demod drivers to adopt the same
mechanism. It was fairly obvious at the time that we needed a much
more generic way to pass internel controls around from the core to the
demods.

The cache was to support backwards compatible V3 interfaces and
demods, amongst other things.

No reason why a new demod today could not support get/set only for
configuration.

>
> What I think I am going to make new RFC which changes that so every
> parameter from userspace is passed to the demodulator driver (using
> set_property() - change it current functionality) and not cached to the
> common cache at all. Shortly: change current common cache based parameter
> transmission to happen set parameter to demodulator one by one.

The other reason for the common cache was to allow sets of parameters
to be pushed into the kernel from apps then, at the most appropriate
time, tuned. The order of operations becomes irrelevant, so the cache
is highly useful, else you end up with demods caching all of their own
parameters anyway, many drivers duplicating a frequency field in their
provate contexts, for example.

It's hard to imaging how not using the cache today.

>
>>> What did you ever decide about the enable/disable of the LNA? And, how
>>> would the bridge do that in your proposed solution? Via the proposed GPIO
>>> interface?
>
>
> I sent PATCH RFC for DVB API to add LNA support yesterday. It is new API

Understood, thanks for the note.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com
+1.646.355.8490

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Antti Palosaari]

On 07/13/2012 04:16 PM, Steven Toth wrote:
>> There is set_property() and get_property() callbacks defined for FE already.
>> But those are not used. My opinion is that those callbacks should be changed
>> to deliver data for demodulator and for tuner too instead of current method
>> - which is common huge properties cache structure shared between all
>> sub-drivers. I don't like it all as it is stupid to add stuff that common
>> structure for every standard we has. Lets take example device that supports
>> DVB-C and other device supports ISDB-T. How many common parameters we has? I
>> think only one - frequency. All the others are just waste.
>
> When we did DVB V5 for S2 we added set/get property for the
> demodulators, from memory I had some cx24116 S2 specifics that I was
> passing, and I expected other demod drivers to adopt the same
> mechanism. It was fairly obvious at the time that we needed a much
> more generic way to pass internel controls around from the core to the
> demods.

cx24116 driver does not define set_property() or get_property() 
currently. I looked the history and yes, there has been those calls. But 
what I saw - only stub implementation. It still may be possible that 
there has been real implementation in some time as I didn't looked 
through all commits - was too many commits to check manually.

And later Mauro has removed totally those unused calls with mention it 
uses DVB v5 *only*.

commit 1ac6a854ad444680bffbacd9e340e40c75adc367
Author: Mauro Carvalho Chehab <mchehab@redhat.com>
Date:   Thu Dec 22 17:28:11 2011 -0300

     [media] cx24116: report delivery system and cleanups

     This is one of the first drivers using DVBv5. It relies only
     on DVBv5 way, but still it contains some stub for unused
     methods. Remove them, add the delivery system and do some
     trivial cleanups.


So I suspect you remember wrong. At least there is now common 
misunderstood what is aimed and what is really done about those callbacks.

> The cache was to support backwards compatible V3 interfaces and
> demods, amongst other things.

That is what I see cache could be needed.

> No reason why a new demod today could not support get/set only for
> configuration.

That patch adds set_property() and get_property() handling for dvb-frontend.
http://kerneltrap.org/mailarchive/git-commits-head/2008/10/13/3643454/thread

For me it looks like meaning is to use those for valid parameters. For 
example demod driver supports DVB-S but set_property() is called with 
unsupported delivery system DVB-S2. Driver could nack it and it is never 
even added to the cache. If success is returned then dvb-frontend adds 
given parameter to cache and finally calls set_frontend() in order to 
make demod make tuning request using cached values.


But yes, it looks like those calls are possible to use for setting 
parameters to demod as every parameter is passed for demod using 
set_frontend() too.

>> What I think I am going to make new RFC which changes that so every
>> parameter from userspace is passed to the demodulator driver (using
>> set_property() - change it current functionality) and not cached to the
>> common cache at all. Shortly: change current common cache based parameter
>> transmission to happen set parameter to demodulator one by one.
>
> The other reason for the common cache was to allow sets of parameters
> to be pushed into the kernel from apps then, at the most appropriate
> time, tuned. The order of operations becomes irrelevant, so the cache
> is highly useful, else you end up with demods caching all of their own
> parameters anyway, many drivers duplicating a frequency field in their
> provate contexts, for example.
>
> It's hard to imaging how not using the cache today.

Maybe I should make an example. For demod it should be trivial, but for 
tuner you must still pass frequency and bandwidth using cache as struct 
dvb_frontend_parameters was removed some time ago.

>>>> What did you ever decide about the enable/disable of the LNA? And, how
>>>> would the bridge do that in your proposed solution? Via the proposed GPIO
>>>> interface?
>>
>>
>> I sent PATCH RFC for DVB API to add LNA support yesterday. It is new API
>
> Understood, thanks for the note.

All in all, I would like to see kind of solution where every property is 
passed to the every driver:
* bridge - set_property(FREQUENCY, 666555000)
* tuner - set_property(FREQUENCY, 666555000)
* demod - set_property(FREQUENCY, 666555000)

and each driver then picks needed parameters. Likely it is still too 
much work without enough benefits to implement...

regards
Antti


-- 
http://palosaari.fi/

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Antti Palosaari]

On 07/13/2012 12:07 AM, Steven Toth wrote:
> On Thu, Jul 12, 2012 at 4:49 PM, Steven Toth <stoth@kernellabs.com> wrote:
>> Nobody understands the relationship between the bridge and the
>> sub-component as well as the bridge driver. The current interfaces are
>> limiting in many ways. We solve that today with rather ugly 'attach'
>> structures that are inflexible, for example to set gpios to a default state.
>> Then, once that interface is attached, the bridge effectively loses most of
>> the control to the tuner and/or demod. The result is a large disconnect
>> between the bridge and subcomponents.
>>
>> Why limit any interface extension to GPIOs? Why not make something a
>> little more flexible so we can pass custom messages around?

>> What did you ever decide about the enable/disable of the LNA? And, how
>> would the bridge do that in your proposed solution? Via the proposed GPIO
>> interface?

GPIO / LNA is ready, see following patches:
add LNA support for DVB API
cxd2820r: use Kernel GPIO for GPIO access
em28xx: implement FE set_lna() callback

from:
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/dvb_core

Kernel GPIOs were quite easy to implement and use - when needed 
knowledge was gathered after all the testing and study. I wonder why 
none was done that earlier for DVB...

It also offer nice debug/devel feature as you can mount those GPIOs via 
sysfs and use directly.


Next-step: DVB power management.

regards
Antti
-- 
http://palosaari.fi/

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[poma]

On 07/20/2012 03:43 AM, Antti Palosaari wrote:
> On 07/13/2012 12:07 AM, Steven Toth wrote:
>> On Thu, Jul 12, 2012 at 4:49 PM, Steven Toth <stoth@kernellabs.com>
>> wrote:
>>> Nobody understands the relationship between the bridge and the
>>> sub-component as well as the bridge driver. The current interfaces are
>>> limiting in many ways. We solve that today with rather ugly 'attach'
>>> structures that are inflexible, for example to set gpios to a default
>>> state.
>>> Then, once that interface is attached, the bridge effectively loses
>>> most of
>>> the control to the tuner and/or demod. The result is a large disconnect
>>> between the bridge and subcomponents.
>>>
>>> Why limit any interface extension to GPIOs? Why not make something a
>>> little more flexible so we can pass custom messages around?
> 
>>> What did you ever decide about the enable/disable of the LNA? And, how
>>> would the bridge do that in your proposed solution? Via the proposed
>>> GPIO
>>> interface?
> 
> GPIO / LNA is ready, see following patches:
> add LNA support for DVB API
> cxd2820r: use Kernel GPIO for GPIO access
> em28xx: implement FE set_lna() callback
> 
> from:
> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/dvb_core
> 
> Kernel GPIOs were quite easy to implement and use - when needed
> knowledge was gathered after all the testing and study. I wonder why
> none was done that earlier for DVB...
> 
> It also offer nice debug/devel feature as you can mount those GPIOs via
> sysfs and use directly.
> 

Above mentioned GPIO functionality must be implemented in driver itself
to use /sys/class/gpio/… sysfs interface, right?
It is not enough to build kernel with CONFIG_GENERIC_GPIO=y,
CONFIG_GPIOLIB=y, CONFIG_GPIO_SYSFS, right?

Cheers,
poma

Re: GPIO interface between DVB sub-drivers (bridge, demod, tuner)

[Antti Palosaari]

On 07/30/2012 12:21 AM, poma wrote:
> On 07/20/2012 03:43 AM, Antti Palosaari wrote:
>> On 07/13/2012 12:07 AM, Steven Toth wrote:
>>> On Thu, Jul 12, 2012 at 4:49 PM, Steven Toth <stoth@kernellabs.com>
>>> wrote:
>>>> Nobody understands the relationship between the bridge and the
>>>> sub-component as well as the bridge driver. The current interfaces are
>>>> limiting in many ways. We solve that today with rather ugly 'attach'
>>>> structures that are inflexible, for example to set gpios to a default
>>>> state.
>>>> Then, once that interface is attached, the bridge effectively loses
>>>> most of
>>>> the control to the tuner and/or demod. The result is a large disconnect
>>>> between the bridge and subcomponents.
>>>>
>>>> Why limit any interface extension to GPIOs? Why not make something a
>>>> little more flexible so we can pass custom messages around?
>>
>>>> What did you ever decide about the enable/disable of the LNA? And, how
>>>> would the bridge do that in your proposed solution? Via the proposed
>>>> GPIO
>>>> interface?
>>
>> GPIO / LNA is ready, see following patches:
>> add LNA support for DVB API
>> cxd2820r: use Kernel GPIO for GPIO access
>> em28xx: implement FE set_lna() callback
>>
>> from:
>> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/dvb_core
>>
>> Kernel GPIOs were quite easy to implement and use - when needed
>> knowledge was gathered after all the testing and study. I wonder why
>> none was done that earlier for DVB...
>>
>> It also offer nice debug/devel feature as you can mount those GPIOs via
>> sysfs and use directly.
>>
>
> Above mentioned GPIO functionality must be implemented in driver itself
> to use /sys/class/gpio/… sysfs interface, right?
> It is not enough to build kernel with CONFIG_GENERIC_GPIO=y,
> CONFIG_GPIOLIB=y, CONFIG_GPIO_SYSFS, right?

You will need to implement callbacks for gpiolib. sysfs interface is 
then get for free.

regards
Antti

-- 
http://palosaari.fi/