Skip to contentSkip to conversation

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

DISCUSSION Antti Palosaari 3 participants 8 messages Jul 12, 2012 – Jul 30, 2012 1 message not in the archive Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

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

Signature · 1 line
http://palosaari.fi/

A message that is not in the archive (sent off-list or not yet received): CALzAhNVwN3TJhn-3i9SDhKfk=tvZZ49RTKkUzWC8RZ_m=v=A+w@mail.gmail.com

#2 lore raw

Resend in plaintext, it got bounced from vger.

Quoted 49 lines · Steven Toth
Quoted 4 lines

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

#3 lore raw
Quoted 17 lines · Steven Toth

Resend in plaintext, it got bounced from vger.

Quoted 14 lines · Steven Toth
Quoted 4 lines

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.

Quoted 11 lines
Quoted 11 lines

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.

Quoted 7 lines
Quoted 7 lines

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.

Quoted 3 lines
Quoted 3 lines

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.

Quoted 3 lines
Quoted 3 lines

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

Signature · 1 line
http://palosaari.fi/
#4 lore raw
Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 7 lines
Quoted 4 lines
Quoted 4 lines

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.

Signature · 3 lines
Steven Toth - Kernel Labs
http://www.kernellabs.com
+1.646.355.8490
#5 lore raw
Quoted 9 lines · Steven Toth
Quoted 8 lines

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.

Quoted 6 lines

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 <[email protected]>
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.

Quoted 2 lines

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

That is what I see cache could be needed.

Quoted 2 lines

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.

Quoted 6 lines
Quoted 5 lines

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.

Quoted 8 lines

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.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

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

Signature · 1 line
http://palosaari.fi/
#6 lore raw
In reply to Steven Toth
Quoted 9 lines · Steven Toth
Quoted 8 lines · Steven Toth

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.

Quoted 6 lines
Quoted 6 lines

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

Signature · 1 line
http://palosaari.fi/
#7 lore raw
Quoted 29 lines · Antti Palosaari
Quoted 12 lines · Steven Toth
Quoted 10 lines · Steven Toth

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.

Quoted 7 lines
Quoted 7 lines

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

Quoted 7 lines

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

#8 lore raw
Quoted 34 lines · poma
Quoted 33 lines · Antti Palosaari
Quoted 12 lines · Steven Toth
Quoted 10 lines · Steven Toth

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.

Quoted 7 lines
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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

Signature · 1 line
http://palosaari.fi/