[PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Srinivas KANDAGATLA]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch attempts to collate generic bindings which can be used by
the remote control hardwares. Currently the list is not long as there
are only 2 drivers which are device tree'd.

Mainly this patch tries to document few bindings used by ST IRB driver
which can be generic as well. This document also add fews common
bindings used by most of the drivers like, interrupts, regs, clocks and
pinctrls.

This document can also be holding place to describe generic bindings
used in remote controls devices.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
Hi All, 
Following Stephen Warren's suggestions at https://lkml.org/lkml/2013/9/24/452
this patch is an attempt to document such generic bindings in a common
document.

This document currently collates all the generic bindings used with
remote-controls and act as holding place to describe generic bindings for
remote controls.

Comments?

Thanks,
srini

 .../devicetree/bindings/media/remote-control.txt   |   31 ++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/remote-control.txt

diff --git a/Documentation/devicetree/bindings/media/remote-control.txt b/Documentation/devicetree/bindings/media/remote-control.txt
new file mode 100644
index 0000000..901ea56
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/remote-control.txt
@@ -0,0 +1,31 @@
+Generic device tree bindings for remote control.
+
+properties:
+	- compatible: Can contain any remote control driver compatible string.
+	  example: "st-comms-irb, "gpio-ir-receiver".
+	- reg: Base physical address of the controller and length of memory
+	  mapped region.
+	- interrupts: Interrupt-specifier for the sole interrupt generated by
+	  the device. The interrupt specifier format depends on the
+	  interrupt controller parent. Iff the device supports interrupts.
+	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
+	  the rx pins are wired up.
+	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
+	  the tx pins are wired up.
+
+Optional properties:
+	- linux,rc-map-name: Linux specific remote control map name. Refer to
+	  include/media/rc-map.h for full list of maps.
+	- pinctrl-names, pinctrl-0: The pincontrol settings to configure muxing
+	  properly for the device pins.
+	- clocks : phandle with clock-specifier pair for the device specified
+	  in compatible.
+
+example:
+
+	rc: rc@fe518000 {
+		compatible	= "st,comms-irb";
+		reg		= <0xfe518000 0x234>;
+		interrupts	= <0 203 0>;
+		rx-mode		= "infrared";
+	};
-- 
1.7.6.5

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mark Rutland]

On Fri, Sep 27, 2013 at 10:33:11AM +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> 
> This patch attempts to collate generic bindings which can be used by
> the remote control hardwares. Currently the list is not long as there
> are only 2 drivers which are device tree'd.
> 
> Mainly this patch tries to document few bindings used by ST IRB driver
> which can be generic as well. This document also add fews common
> bindings used by most of the drivers like, interrupts, regs, clocks and
> pinctrls.
> 
> This document can also be holding place to describe generic bindings
> used in remote controls devices.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> ---
> Hi All, 
> Following Stephen Warren's suggestions at https://lkml.org/lkml/2013/9/24/452
> this patch is an attempt to document such generic bindings in a common
> document.
> 
> This document currently collates all the generic bindings used with
> remote-controls and act as holding place to describe generic bindings for
> remote controls.
> 
> Comments?
> 
> Thanks,
> srini
> 
>  .../devicetree/bindings/media/remote-control.txt   |   31 ++++++++++++++++++++
>  1 files changed, 31 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/remote-control.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/remote-control.txt b/Documentation/devicetree/bindings/media/remote-control.txt
> new file mode 100644
> index 0000000..901ea56
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/remote-control.txt
> @@ -0,0 +1,31 @@
> +Generic device tree bindings for remote control.
> +
> +properties:
> +	- compatible: Can contain any remote control driver compatible string.
> +	  example: "st-comms-irb, "gpio-ir-receiver".

This is more generic than remote control, could this not just be left
for the specific binding to describe?

> +	- reg: Base physical address of the controller and length of memory
> +	  mapped region.

What if it's on a bus that isn't memory mapped (e.g. i2c, SPI)?

> +	- interrupts: Interrupt-specifier for the sole interrupt generated by
> +	  the device. The interrupt specifier format depends on the
> +	  interrupt controller parent. Iff the device supports interrupts.

What if it has multiple interrupts, and has interrupts-names?

It might be better to only describe the properties that relate
specifically to remote controls, rather than listing all of the generic
properties that device tree bidnings may have. That would match the
style of the clock bindings.

> +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> +	  the rx pins are wired up.

I'm unsure on this. What if the device has multiple receivers that can
be independently configured? What if it supports something other than
"infrared" or "uhf"? What if a device can only be wired up as
"infrared"? 

I'm not sure how generic these are, though we should certainly encourage
bindings that can be described this way to be described in the same way.

> +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> +	  the tx pins are wired up.

I have similar concerns here to those for the rx-mode property.

> +
> +Optional properties:
> +	- linux,rc-map-name: Linux specific remote control map name. Refer to
> +	  include/media/rc-map.h for full list of maps.

We shouldn't refer to Linux specifics (i.e. headers) in general in
bindings. While it's possible to relax that a bit for linux,*
properties, I'd prefer to explicitly list elements in the binding. That
prevents the ABI from changing under our feet by someone altering what
looks like a completely internal header file.

> +	- pinctrl-names, pinctrl-0: The pincontrol settings to configure muxing
> +	  properly for the device pins.
> +	- clocks : phandle with clock-specifier pair for the device specified
> +	  in compatible.

While devices may have these, they're also more general than remote
control devices. I'm not sure that they need to be listed here when they
need to be described fully in any binding that needs them anyway,
especially as this gives an impression that they are valid for bindings
that don't need them.

I think what we actually need to document is the process of creating a
binding in such a way as to encourage uniformity. Something like the
following steps:

1. Look to see if a binding already exists. If so, use it.

2. Is there a binding for a compatible device? If so, use/extend it.

3. Is there a binding for a similar (but incompatible) device? Use it as
   a template, possibly factor out portions into a class binding if
   those portions are truly general.

4. Is there a binding for the class of device? If so, build around that,
   possibly extending it.

5. If there's nothing relevant, create a binding aiming for as much
   commonality as possible with other devices of that class that may
   have bindings later.

Cheers,
Mark.

> +
> +example:
> +
> +	rc: rc@fe518000 {
> +		compatible	= "st,comms-irb";
> +		reg		= <0xfe518000 0x234>;
> +		interrupts	= <0 203 0>;
> +		rx-mode		= "infrared";
> +	};
> -- 
> 1.7.6.5
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Srinivas KANDAGATLA]

On 27/09/13 12:34, Mark Rutland wrote:

>> > +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
>> > +	  the rx pins are wired up.
> I'm unsure on this. What if the device has multiple receivers that can
> be independently configured? What if it supports something other than
> "infrared" or "uhf"? What if a device can only be wired up as
> "infrared"? 
> 
> I'm not sure how generic these are, though we should certainly encourage
> bindings that can be described this way to be described in the same way.
> 
>> > +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
>> > +	  the tx pins are wired up.
> I have similar concerns here to those for the rx-mode property.
> 
Initially rx-mode and tx-mode sounded like more generic properties
that's the reason I ended up in this route. But after this discussion it
looks like its not really generic enough to cater all the use cases.

It make sense for me to perfix "st," for these properties in the st-rc
driver rather than considering them as generic properties.

> I think what we actually need to document is the process of creating a
> binding in such a way as to encourage uniformity. Something like the
> following steps:
I agree, It will help.. :-)
> 
> 1. Look to see if a binding already exists. If so, use it.
> 
> 2. Is there a binding for a compatible device? If so, use/extend it.
> 
> 3. Is there a binding for a similar (but incompatible) device? Use it as
>    a template, possibly factor out portions into a class binding if
>    those portions are truly general.
> 
> 4. Is there a binding for the class of device? If so, build around that,
>    possibly extending it.
> 
> 5. If there's nothing relevant, create a binding aiming for as much
>    commonality as possible with other devices of that class that may
>    have bindings later.

Thanks for this little guide...

--srini

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mauro Carvalho Chehab]

Em Fri, 27 Sep 2013 14:26:12 +0100
Srinivas KANDAGATLA <srinivas.kandagatla@st.com> escreveu:

> On 27/09/13 12:34, Mark Rutland wrote:
> 
> >> > +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> >> > +	  the rx pins are wired up.
> > I'm unsure on this. What if the device has multiple receivers that can
> > be independently configured? What if it supports something other than
> > "infrared" or "uhf"? What if a device can only be wired up as
> > "infrared"? 
> > 
> > I'm not sure how generic these are, though we should certainly encourage
> > bindings that can be described this way to be described in the same way.
> > 
> >> > +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> >> > +	  the tx pins are wired up.
> > I have similar concerns here to those for the rx-mode property.
> > 
> Initially rx-mode and tx-mode sounded like more generic properties
> that's the reason I ended up in this route. But after this discussion it
> looks like its not really generic enough to cater all the use cases.
> 
> It make sense for me to perfix "st," for these properties in the st-rc
> driver rather than considering them as generic properties.

Well, for sure the direction (TX, RX, both) is a generic property.

I'd say that the level 1 protocol (IR, UHF, Bluetooth, ...) is also a
generic property. Most remotes are IR, but there are some that are
bluetooth, and your hardware is using UHF.

Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via
the RC subsystem. So, another L1 protocol would be "hdmi-cec".

Yet, it seems unlikely that the very same remote controller IP would use
a different protocol for RX and TX, while sharing the same registers.

So, for example, a hardware with "hdmi-cec" and "infrared" will actually
have two remote controller devices. Eventually, the "infrared" being
just RX, while "hdmi-cec" being bi-directional.

So, IMHO, this could be mapped as "l1_protocol" ("infrared", "uhf", ...)
and another one "direction" ("rx", "tx", "bi-directional").

> 
> > I think what we actually need to document is the process of creating a
> > binding in such a way as to encourage uniformity. Something like the
> > following steps:
> I agree, It will help.. :-)
> > 
> > 1. Look to see if a binding already exists. If so, use it.
> > 
> > 2. Is there a binding for a compatible device? If so, use/extend it.
> > 
> > 3. Is there a binding for a similar (but incompatible) device? Use it as
> >    a template, possibly factor out portions into a class binding if
> >    those portions are truly general.
> > 
> > 4. Is there a binding for the class of device? If so, build around that,
> >    possibly extending it.
> > 
> > 5. If there's nothing relevant, create a binding aiming for as much
> >    commonality as possible with other devices of that class that may
> >    have bindings later.
> 
> Thanks for this little guide...
> 
> --srini


-- 

Cheers,
Mauro

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Srinivas KANDAGATLA]

On 27/09/13 14:57, Mauro Carvalho Chehab wrote:
> Em Fri, 27 Sep 2013 14:26:12 +0100
> Srinivas KANDAGATLA <srinivas.kandagatla@st.com> escreveu:
> 
>> On 27/09/13 12:34, Mark Rutland wrote:
>>
>>>>> +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
>>>>> +	  the rx pins are wired up.
>>> I'm unsure on this. What if the device has multiple receivers that can
>>> be independently configured? What if it supports something other than
>>> "infrared" or "uhf"? What if a device can only be wired up as
>>> "infrared"? 
>>>
>>> I'm not sure how generic these are, though we should certainly encourage
>>> bindings that can be described this way to be described in the same way.
>>>
>>>>> +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
>>>>> +	  the tx pins are wired up.
>>> I have similar concerns here to those for the rx-mode property.
>>>
>> Initially rx-mode and tx-mode sounded like more generic properties
>> that's the reason I ended up in this route. But after this discussion it
>> looks like its not really generic enough to cater all the use cases.
>>
>> It make sense for me to perfix "st," for these properties in the st-rc
>> driver rather than considering them as generic properties.
> 
> Well, for sure the direction (TX, RX, both) is a generic property.
> 
> I'd say that the level 1 protocol (IR, UHF, Bluetooth, ...) is also a
> generic property. Most remotes are IR, but there are some that are
> bluetooth, and your hardware is using UHF.
Yes these are generic.

> 
> Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via
> the RC subsystem. So, another L1 protocol would be "hdmi-cec".
> 
Ok.
> Yet, it seems unlikely that the very same remote controller IP would use
> a different protocol for RX and TX, while sharing the same registers.

ST IRB block has one IR processor which has both TX and RX support and
one UHF Processor which has RX support only. However the register map
for all these support is in single IRB IP block.

So the driver can configure the IP as TX in "infrared" and RX in "uhf".
This is supported in ST IRB IP.

This case can not be represented in a single device tree node with
l1-protocol and direction properties.

IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol
properties will give more flexibility.

What do you think?

> 
> So, for example, a hardware with "hdmi-cec" and "infrared" will actually
> have two remote controller devices. Eventually, the "infrared" being
> just RX, while "hdmi-cec" being bi-directional.
> 
> So, IMHO, this could be mapped as "l1_protocol" ("infrared", "uhf", ...)
> and another one "direction" ("rx", "tx", "bi-directional").
> 

Thanks,
srini

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mauro Carvalho Chehab]

Em Mon, 30 Sep 2013 09:27:02 +0100
Srinivas KANDAGATLA <srinivas.kandagatla@st.com> escreveu:

> On 27/09/13 14:57, Mauro Carvalho Chehab wrote:
> > Em Fri, 27 Sep 2013 14:26:12 +0100
> > Srinivas KANDAGATLA <srinivas.kandagatla@st.com> escreveu:
> > 
> >> On 27/09/13 12:34, Mark Rutland wrote:
> >>
> >>>>> +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> >>>>> +	  the rx pins are wired up.
> >>> I'm unsure on this. What if the device has multiple receivers that can
> >>> be independently configured? What if it supports something other than
> >>> "infrared" or "uhf"? What if a device can only be wired up as
> >>> "infrared"? 
> >>>
> >>> I'm not sure how generic these are, though we should certainly encourage
> >>> bindings that can be described this way to be described in the same way.
> >>>
> >>>>> +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> >>>>> +	  the tx pins are wired up.
> >>> I have similar concerns here to those for the rx-mode property.
> >>>
> >> Initially rx-mode and tx-mode sounded like more generic properties
> >> that's the reason I ended up in this route. But after this discussion it
> >> looks like its not really generic enough to cater all the use cases.
> >>
> >> It make sense for me to perfix "st," for these properties in the st-rc
> >> driver rather than considering them as generic properties.
> > 
> > Well, for sure the direction (TX, RX, both) is a generic property.
> > 
> > I'd say that the level 1 protocol (IR, UHF, Bluetooth, ...) is also a
> > generic property. Most remotes are IR, but there are some that are
> > bluetooth, and your hardware is using UHF.
> Yes these are generic.
> 
> > 
> > Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via
> > the RC subsystem. So, another L1 protocol would be "hdmi-cec".
> > 
> Ok.
> > Yet, it seems unlikely that the very same remote controller IP would use
> > a different protocol for RX and TX, while sharing the same registers.
> 
> ST IRB block has one IR processor which has both TX and RX support and
> one UHF Processor which has RX support only. However the register map
> for all these support is in single IRB IP block.
> 
> So the driver can configure the IP as TX in "infrared" and RX in "uhf".
> This is supported in ST IRB IP.
> 
> This case can not be represented in a single device tree node with
> l1-protocol and direction properties.
> 
> IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol
> properties will give more flexibility.
> 
> What do you think?

Yeah, if they're using the same registers, then your proposal works
better.

I would prefer to not call it as just protocol, as IR has an
upper layer protocol that defines how the bits are encoded, e. g.
RC5, RC6, NEC, SONY, ..., with is what we generally call as protocol
on rc-core. 

A proper naming for it is hard to find. Well, for IR/UHF, it is actually
specifying the medium, but for Bluetooth, HDMI-CEC, it defines a
protocol stack to be used, with covers not only the physical layer of
the OSI model.

Perhaps the better would be to call it as: tx-proto-stack/rx-proto-stack.

> 
> > 
> > So, for example, a hardware with "hdmi-cec" and "infrared" will actually
> > have two remote controller devices. Eventually, the "infrared" being
> > just RX, while "hdmi-cec" being bi-directional.
> > 
> > So, IMHO, this could be mapped as "l1_protocol" ("infrared", "uhf", ...)
> > and another one "direction" ("rx", "tx", "bi-directional").
> > 
> 
> Thanks,
> srini

Regards,
Mauro

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Srinivas KANDAGATLA]

On 01/10/13 15:49, Mauro Carvalho Chehab wrote:
>>> > > 
>>> > > Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via
>>> > > the RC subsystem. So, another L1 protocol would be "hdmi-cec".
>>> > > 
>> > Ok.
>>> > > Yet, it seems unlikely that the very same remote controller IP would use
>>> > > a different protocol for RX and TX, while sharing the same registers.
>> > 
>> > ST IRB block has one IR processor which has both TX and RX support and
>> > one UHF Processor which has RX support only. However the register map
>> > for all these support is in single IRB IP block.
>> > 
>> > So the driver can configure the IP as TX in "infrared" and RX in "uhf".
>> > This is supported in ST IRB IP.
>> > 
>> > This case can not be represented in a single device tree node with
>> > l1-protocol and direction properties.
>> > 
>> > IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol
>> > properties will give more flexibility.
>> > 
>> > What do you think?
> Yeah, if they're using the same registers, then your proposal works
> better.
> 
> I would prefer to not call it as just protocol, as IR has an
> upper layer protocol that defines how the bits are encoded, e. g.
> RC5, RC6, NEC, SONY, ..., with is what we generally call as protocol
> on rc-core. 
> 
> A proper naming for it is hard to find. Well, for IR/UHF, it is actually

Yes I agree.

> specifying the medium, but for Bluetooth, HDMI-CEC, it defines a
> protocol stack to be used, with covers not only the physical layer of
> the OSI model.
> 
> Perhaps the better would be to call it as: tx-proto-stack/rx-proto-stack.
> 
How are we going to address use case highlighted by Mark R, like N
Connections on a single IP block?

This use-case can not be addressed with tx-mode and rx-mode or
tx-proto-stack/rx-proto-stack properties.

So the idea of generic properties for tx and rx sounds incorrect.

IMHO, Best thing would be to drop the idea of using tx-mode and rx-mode
as generic properties and use "st,tx-mode" and "st,rx-mode" instead for
st-rc driver.

What do you think?

Thanks,
srini

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mauro Carvalho Chehab]

Em Wed, 02 Oct 2013 17:22:06 +0100
Srinivas KANDAGATLA <srinivas.kandagatla@st.com> escreveu:

> On 01/10/13 15:49, Mauro Carvalho Chehab wrote:
> >>> > > 
> >>> > > Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via
> >>> > > the RC subsystem. So, another L1 protocol would be "hdmi-cec".
> >>> > > 
> >> > Ok.
> >>> > > Yet, it seems unlikely that the very same remote controller IP would use
> >>> > > a different protocol for RX and TX, while sharing the same registers.
> >> > 
> >> > ST IRB block has one IR processor which has both TX and RX support and
> >> > one UHF Processor which has RX support only. However the register map
> >> > for all these support is in single IRB IP block.
> >> > 
> >> > So the driver can configure the IP as TX in "infrared" and RX in "uhf".
> >> > This is supported in ST IRB IP.
> >> > 
> >> > This case can not be represented in a single device tree node with
> >> > l1-protocol and direction properties.
> >> > 
> >> > IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol
> >> > properties will give more flexibility.
> >> > 
> >> > What do you think?
> > Yeah, if they're using the same registers, then your proposal works
> > better.
> > 
> > I would prefer to not call it as just protocol, as IR has an
> > upper layer protocol that defines how the bits are encoded, e. g.
> > RC5, RC6, NEC, SONY, ..., with is what we generally call as protocol
> > on rc-core. 
> > 
> > A proper naming for it is hard to find. Well, for IR/UHF, it is actually
> 
> Yes I agree.
> 
> > specifying the medium, but for Bluetooth, HDMI-CEC, it defines a
> > protocol stack to be used, with covers not only the physical layer of
> > the OSI model.
> > 
> > Perhaps the better would be to call it as: tx-proto-stack/rx-proto-stack.
> > 
> How are we going to address use case highlighted by Mark R, like N
> Connections on a single IP block?
> 
> This use-case can not be addressed with tx-mode and rx-mode or
> tx-proto-stack/rx-proto-stack properties.
> 
> So the idea of generic properties for tx and rx sounds incorrect.
> 
> IMHO, Best thing would be to drop the idea of using tx-mode and rx-mode
> as generic properties and use "st,tx-mode" and "st,rx-mode" instead for
> st-rc driver.
> 
> What do you think?

Well, from userspace PoV, it should have just one devnode for each
TX/RX.

So, if the device has N TX and/or RX simultaneous connections, it should
be exposing N device nodes, and the DT should for it should have N entries,
one for each.

A completely independent issue is how the driver will prevent to have
two simultaneous access to the same resource.

As on any other type of resource, there are several alternatives:

	- block the reads/writes, if some I/O operation is pending;
	- return -EAGAIN where the API allows (non-block calls),
	  and the error is temporary;
	- return -EBUSY if the resource is more "permanently" allocated.

So, if the very same registers are used by more than one TX/RX unit,
then the driver should for example have a mutex/semaphore to lock such
I/O while another I/O operation is undergoing.

That solution is the same used by I2C devices: the I2C bus has a lock,
serializing the access to all devices on that bus.

There is another possible scenario: a device that have more than one
connection, and that userspace could setup what connection is active,
putting all the other ones inactive.

On such scenario, we would need to add more bits at RC API, in order
to allow userspace to enumerate the possible RX/TX connections,
and to change it at runtime.

If we had such scenario, then the DT representation for it could be
different. So, instead of having a single TX/RX mode/protocol-stack,
we would have a connections table. Also, each entry would likely need
a name, in order to allow userspace to distinguish between the diferent
entries that are wired on a given board.

Anyway, we don't have such scenario yet. So, let's not overdesign the
API, thinking on a possible scenario that may never happen.

> 
> Thanks,
> srini
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 

Cheers,
Mauro

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Stephen Warren]

On 10/02/2013 11:33 AM, Mauro Carvalho Chehab wrote:
...
> Well, from userspace PoV, it should have just one devnode for each
> TX/RX.

I'm fine with that.

> So, if the device has N TX and/or RX simultaneous connections, it should
> be exposing N device nodes, and the DT should for it should have N entries,
> one for each.

DT is based on the actual HW construction, not how a particular OS wants
to expose that HW through its APIs. If there is a single HW block, there
should be a single DT node, even if that HW block supports multiple
channels.

In some circumstances, it might make sense for the single top-level node
that represents the HW-block to have child nodes that represent the
channels, depending on what exactly the HW is doing and whether this
level of detail is useful in DT. I would qualify this as rare though.

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mauro Carvalho Chehab]

Em Fri, 27 Sep 2013 12:34:58 +0100
Mark Rutland <mark.rutland@arm.com> escreveu:

> On Fri, Sep 27, 2013 at 10:33:11AM +0100, Srinivas KANDAGATLA wrote:
> > From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> > 
> > This patch attempts to collate generic bindings which can be used by
> > the remote control hardwares. Currently the list is not long as there
> > are only 2 drivers which are device tree'd.
> > 
> > Mainly this patch tries to document few bindings used by ST IRB driver
> > which can be generic as well. This document also add fews common
> > bindings used by most of the drivers like, interrupts, regs, clocks and
> > pinctrls.
> > 
> > This document can also be holding place to describe generic bindings
> > used in remote controls devices.
> > 
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> > ---
> > Hi All, 
> > Following Stephen Warren's suggestions at https://lkml.org/lkml/2013/9/24/452
> > this patch is an attempt to document such generic bindings in a common
> > document.
> > 
> > This document currently collates all the generic bindings used with
> > remote-controls and act as holding place to describe generic bindings for
> > remote controls.
> > 
> > Comments?
> > 
> > Thanks,
> > srini
> > 
> >  .../devicetree/bindings/media/remote-control.txt   |   31 ++++++++++++++++++++
> >  1 files changed, 31 insertions(+), 0 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/media/remote-control.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/remote-control.txt b/Documentation/devicetree/bindings/media/remote-control.txt
> > new file mode 100644
> > index 0000000..901ea56
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/remote-control.txt
> > @@ -0,0 +1,31 @@
> > +Generic device tree bindings for remote control.
> > +
> > +properties:
> > +	- compatible: Can contain any remote control driver compatible string.
> > +	  example: "st-comms-irb, "gpio-ir-receiver".
> 
> This is more generic than remote control, could this not just be left
> for the specific binding to describe?
> 
> > +	- reg: Base physical address of the controller and length of memory
> > +	  mapped region.
> 
> What if it's on a bus that isn't memory mapped (e.g. i2c, SPI)?
> 
> > +	- interrupts: Interrupt-specifier for the sole interrupt generated by
> > +	  the device. The interrupt specifier format depends on the
> > +	  interrupt controller parent. Iff the device supports interrupts.
> 
> What if it has multiple interrupts, and has interrupts-names?
> 
> It might be better to only describe the properties that relate
> specifically to remote controls, rather than listing all of the generic
> properties that device tree bidnings may have. That would match the
> style of the clock bindings.
> 
> > +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> > +	  the rx pins are wired up.
> 
> I'm unsure on this. What if the device has multiple receivers that can
> be independently configured? 

Well, if a given remote controller hardware has more than one independent
receiver (or transmitter), each one should have its own devnode.
Likely, two entries at DT.

> What if it supports something other than
> "infrared" or "uhf"? What if a device can only be wired up as
> "infrared"? 

I would say that a hardware that has both infrared and uhf has actually
two different devices. So, it should be mapped as two separate devnodes.

> I'm not sure how generic these are, though we should certainly encourage
> bindings that can be described this way to be described in the same way.
> 
> > +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> > +	  the tx pins are wired up.
> 
> I have similar concerns here to those for the rx-mode property.
> 
> > +
> > +Optional properties:
> > +	- linux,rc-map-name: Linux specific remote control map name. Refer to
> > +	  include/media/rc-map.h for full list of maps.
> 
> We shouldn't refer to Linux specifics (i.e. headers) in general in
> bindings. While it's possible to relax that a bit for linux,*
> properties, I'd prefer to explicitly list elements in the binding. That
> prevents the ABI from changing under our feet by someone altering what
> looks like a completely internal header file.

Well, the remote controller keymaps at include/media/rc-map.h is just a
bunch of string names, defined as macro to avoid duplicating those names
everywhere, to avoid typos and to help some userspace parsing logic to get
all in just one single place. I don't see why the same names couldn't be 
used on any other OS using DT.

The logic behind include/media/rc-map.h, is that those names are used
by:

1) kernelspace: in order to locate a keytable with the same name, that
would be loaded when the device is initialized;

2) userspace: to seek for a keytable with that name, allowing to
dynamically load the keymap table on userspace, instead of hardwiring
them on Kernelspace (or replacing the kernel's one by an user-customized
one).

So, I would simply call it as "keymap-name", keep pointing it to 
include/media/rc-map.h.

That's said, this is actually a mandatory requirement, as without it,
the RC core will not be able to load a keytable, and the userspace tool
won't load the proper keymap, being confused on what to do.

It should be noticed that, from time to time, manufacturers change the
remote control unit, as those devices are generally manufactured by a
third part. So, they change, for example, when they get a new BID to
provide IRs for a cheaper cost, or when they need/want to provide a "deluxe"
remote, a simplified "thin" one and/or when they need to provide customized
remotes to some Cable company, for example.

So, it makes sense for it to be mandatory, as only with this information it
is possible to load the keymap that matches that specific IR unit model.

In other words, I would add this as a mandatory property, with a text
similar to:

	- keymap-name: Name of the keyboard scancode table used to match 
	  the remote controller unit model (or model set). Please add it at
	  include/media/rc-map.h for newer models.

It could be interesting to also warn there that, when the remotes are
made exclusively to a given hardware vendor, and the entries are not
incompatible, the same keymap name could be used by more than one remote
controller unit.

> > +	- pinctrl-names, pinctrl-0: The pincontrol settings to configure muxing
> > +	  properly for the device pins.
> > +	- clocks : phandle with clock-specifier pair for the device specified
> > +	  in compatible.
> 
> While devices may have these, they're also more general than remote
> control devices. I'm not sure that they need to be listed here when they
> need to be described fully in any binding that needs them anyway,
> especially as this gives an impression that they are valid for bindings
> that don't need them.
> 
> I think what we actually need to document is the process of creating a
> binding in such a way as to encourage uniformity. Something like the
> following steps:
> 
> 1. Look to see if a binding already exists. If so, use it.
> 
> 2. Is there a binding for a compatible device? If so, use/extend it.
> 
> 3. Is there a binding for a similar (but incompatible) device? Use it as
>    a template, possibly factor out portions into a class binding if
>    those portions are truly general.
> 
> 4. Is there a binding for the class of device? If so, build around that,
>    possibly extending it.
> 
> 5. If there's nothing relevant, create a binding aiming for as much
>    commonality as possible with other devices of that class that may
>    have bindings later.
> 
> Cheers,
> Mark.
> 
> > +
> > +example:
> > +
> > +	rc: rc@fe518000 {
> > +		compatible	= "st,comms-irb";
> > +		reg		= <0xfe518000 0x234>;
> > +		interrupts	= <0 203 0>;
> > +		rx-mode		= "infrared";
> > +	};
> > -- 
> > 1.7.6.5
> > 
> > 


-- 

Cheers,
Mauro

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mark Rutland]

On Fri, Sep 27, 2013 at 02:47:19PM +0100, Mauro Carvalho Chehab wrote:
> Em Fri, 27 Sep 2013 12:34:58 +0100
> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> escreveu:
> 
> > On Fri, Sep 27, 2013 at 10:33:11AM +0100, Srinivas KANDAGATLA wrote:
> > > From: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> > > 
> > > This patch attempts to collate generic bindings which can be used by
> > > the remote control hardwares. Currently the list is not long as there
> > > are only 2 drivers which are device tree'd.
> > > 
> > > Mainly this patch tries to document few bindings used by ST IRB driver
> > > which can be generic as well. This document also add fews common
> > > bindings used by most of the drivers like, interrupts, regs, clocks and
> > > pinctrls.
> > > 
> > > This document can also be holding place to describe generic bindings
> > > used in remote controls devices.
> > > 
> > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> > > ---
> > > Hi All, 
> > > Following Stephen Warren's suggestions at https://lkml.org/lkml/2013/9/24/452
> > > this patch is an attempt to document such generic bindings in a common
> > > document.
> > > 
> > > This document currently collates all the generic bindings used with
> > > remote-controls and act as holding place to describe generic bindings for
> > > remote controls.
> > > 
> > > Comments?
> > > 
> > > Thanks,
> > > srini
> > > 
> > >  .../devicetree/bindings/media/remote-control.txt   |   31 ++++++++++++++++++++
> > >  1 files changed, 31 insertions(+), 0 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/media/remote-control.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/remote-control.txt b/Documentation/devicetree/bindings/media/remote-control.txt
> > > new file mode 100644
> > > index 0000000..901ea56
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/remote-control.txt
> > > @@ -0,0 +1,31 @@
> > > +Generic device tree bindings for remote control.
> > > +
> > > +properties:
> > > +	- compatible: Can contain any remote control driver compatible string.
> > > +	  example: "st-comms-irb, "gpio-ir-receiver".
> > 
> > This is more generic than remote control, could this not just be left
> > for the specific binding to describe?
> > 
> > > +	- reg: Base physical address of the controller and length of memory
> > > +	  mapped region.
> > 
> > What if it's on a bus that isn't memory mapped (e.g. i2c, SPI)?
> > 
> > > +	- interrupts: Interrupt-specifier for the sole interrupt generated by
> > > +	  the device. The interrupt specifier format depends on the
> > > +	  interrupt controller parent. Iff the device supports interrupts.
> > 
> > What if it has multiple interrupts, and has interrupts-names?
> > 
> > It might be better to only describe the properties that relate
> > specifically to remote controls, rather than listing all of the generic
> > properties that device tree bidnings may have. That would match the
> > style of the clock bindings.
> > 
> > > +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> > > +	  the rx pins are wired up.
> > 
> > I'm unsure on this. What if the device has multiple receivers that can
> > be independently configured? 
> 
> Well, if a given remote controller hardware has more than one independent
> receiver (or transmitter), each one should have its own devnode.
> Likely, two entries at DT.

Why? If an IP block happens to have support for N connections, that
doesn't mean that each must be described individually. They likely share
a bank of registers, and depending on the device they might not even be
assigned consistently orgranised windows of that register bank.

> 
> > What if it supports something other than
> > "infrared" or "uhf"? What if a device can only be wired up as
> > "infrared"? 
> 
> I would say that a hardware that has both infrared and uhf has actually
> two different devices. So, it should be mapped as two separate devnodes.

I would say that it is still one device, one which happens to have two
outputs. Just because we want two dev nodes does not mean the dt has to
be structured as two devices.

> 
> > I'm not sure how generic these are, though we should certainly encourage
> > bindings that can be described this way to be described in the same way.
> > 
> > > +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> > > +	  the tx pins are wired up.
> > 
> > I have similar concerns here to those for the rx-mode property.
> > 
> > > +
> > > +Optional properties:
> > > +	- linux,rc-map-name: Linux specific remote control map name. Refer to
> > > +	  include/media/rc-map.h for full list of maps.
> > 
> > We shouldn't refer to Linux specifics (i.e. headers) in general in
> > bindings. While it's possible to relax that a bit for linux,*
> > properties, I'd prefer to explicitly list elements in the binding. That
> > prevents the ABI from changing under our feet by someone altering what
> > looks like a completely internal header file.
> 
> Well, the remote controller keymaps at include/media/rc-map.h is just a
> bunch of string names, defined as macro to avoid duplicating those names
> everywhere, to avoid typos and to help some userspace parsing logic to get
> all in just one single place. I don't see why the same names couldn't be 
> used on any other OS using DT.

To be used by another OS, they should be defined somewhere that's not
subject to arbitrary changes at any time at the whim of Linux
developers, without dt-related review.

That's not to say we couldn't use strings the kernel happened to use.
I'm saying that the names exposed by bindings should be well-defined,
and should not depend on the current state of a linux-internal header
file.

I think it would be nicer to have a way of defining the keymap in dt
anyway, so as to handle the general case and not get into the mess of
having an arbitrary set of strings we need to constantly update.

> 
> The logic behind include/media/rc-map.h, is that those names are used
> by:
> 
> 1) kernelspace: in order to locate a keytable with the same name, that
> would be loaded when the device is initialized;

The kernel uses the strings, so it has them defined in its include path
somewhere. If dt bindings wish to use the strings, they should be
defined somewhere. That somewhere should not be a Linux-internal header
file.

> 
> 2) userspace: to seek for a keytable with that name, allowing to
> dynamically load the keymap table on userspace, instead of hardwiring
> them on Kernelspace (or replacing the kernel's one by an user-customized
> one).

The name of each table is not exposed to userspace, they are not defined
under include/uapi. The fact that the names may be used to request other
data does not change the fact that the kernel has one copy, userspace
another. The set of strings the kernel understands *is* hard-wired.

> 
> So, I would simply call it as "keymap-name", keep pointing it to 
> include/media/rc-map.h.
> 
> That's said, this is actually a mandatory requirement, as without it,
> the RC core will not be able to load a keytable, and the userspace tool
> won't load the proper keymap, being confused on what to do.

It is possible to handle setting up the mapping within the kernel, or to
actually describe the general case, something like how gpio-keys works.
I think that would be preferable.

> 
> It should be noticed that, from time to time, manufacturers change the
> remote control unit, as those devices are generally manufactured by a
> third part. So, they change, for example, when they get a new BID to
> provide IRs for a cheaper cost, or when they need/want to provide a "deluxe"
> remote, a simplified "thin" one and/or when they need to provide customized
> remotes to some Cable company, for example.
> 
> So, it makes sense for it to be mandatory, as only with this information it
> is possible to load the keymap that matches that specific IR unit model.

While it's true that some description is necessary, I do nto believe
this is the right one.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[srinivas kandagatla]

>>>>  .../devicetree/bindings/media/remote-control.txt   |   31 ++++++++++++++++++++
>>>>  1 files changed, 31 insertions(+), 0 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/media/remote-control.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/remote-control.txt b/Documentation/devicetree/bindings/media/remote-control.txt
>>>> new file mode 100644
>>>> index 0000000..901ea56
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/remote-control.txt
>>>> @@ -0,0 +1,31 @@
>>>> +Generic device tree bindings for remote control.
>>>> +
>>>> +properties:
>>>> +	- compatible: Can contain any remote control driver compatible string.
>>>> +	  example: "st-comms-irb, "gpio-ir-receiver".
>>>
>>> This is more generic than remote control, could this not just be left
>>> for the specific binding to describe?
>>>
>>>> +	- reg: Base physical address of the controller and length of memory
>>>> +	  mapped region.
>>>
>>> What if it's on a bus that isn't memory mapped (e.g. i2c, SPI)?
>>>
>>>> +	- interrupts: Interrupt-specifier for the sole interrupt generated by
>>>> +	  the device. The interrupt specifier format depends on the
>>>> +	  interrupt controller parent. Iff the device supports interrupts.
>>>
>>> What if it has multiple interrupts, and has interrupts-names?

I think for properties like interrupts, reg can be left undocumented
here and let the actual device bindings document on this.

>>>
>>> It might be better to only describe the properties that relate
>>> specifically to remote controls, rather than listing all of the generic
>>> properties that device tree bidnings may have. That would match the
>>> style of the clock bindings.
>>>
>>>> +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
>>>> +	  the rx pins are wired up.
>>>
>>> I'm unsure on this. What if the device has multiple receivers that can
>>> be independently configured? 

Mauro C. had an option that this is not a real use-case and let's not
overdesign the API, thinking on a possible scenario that may never happen.

Do you still think that this use case should be considered in this
discussion?

>>
>> Well, if a given remote controller hardware has more than one independent
>> receiver (or transmitter), each one should have its own devnode.
>> Likely, two entries at DT.
> 
> Why? If an IP block happens to have support for N connections, that
> doesn't mean that each must be described individually. They likely share
> a bank of registers, and depending on the device they might not even be
> assigned consistently orgranised windows of that register bank.
> 
>>
>>> What if it supports something other than
>>> "infrared" or "uhf"? What if a device can only be wired up as
>>> "infrared"? 

I think "infrared" and "uhf" covers all the use cases for remote controls.

If the device only supports one mode. It can be documented in device
specific bindings. something like phy-mode of ethernet bindings.

The possibility of device supporting something other than
these("infrared" and "uhf")  L1 protocols is very little, unless a new
remote control PHY protocol comes up. I would like to know if there is a
new one you are already aware of.

>>
>> I would say that a hardware that has both infrared and uhf has actually
>> two different devices. So, it should be mapped as two separate devnodes.
> 
> I would say that it is still one device, one which happens to have two
> outputs. Just because we want two dev nodes does not mean the dt has to
> be structured as two devices.
> 
>>
>>> I'm not sure how generic these are, though we should certainly encourage
>>> bindings that can be described this way to be described in the same way.
>>>
>>>> +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
>>>> +	  the tx pins are wired up.
>>>
>>> I have similar concerns here to those for the rx-mode property.
>>>
>>>> +
>>>> +Optional properties:
>>>> +	- linux,rc-map-name: Linux specific remote control map name. Refer to
>>>> +	  include/media/rc-map.h for full list of maps.
>>>
>>> We shouldn't refer to Linux specifics (i.e. headers) in general in
>>> bindings. While it's possible to relax that a bit for linux,*
>>> properties, I'd prefer to explicitly list elements in the binding. That
>>> prevents the ABI from changing under our feet by someone altering what
>>> looks like a completely internal header file.
>>
>> Well, the remote controller keymaps at include/media/rc-map.h is just a
>> bunch of string names, defined as macro to avoid duplicating those names
>> everywhere, to avoid typos and to help some userspace parsing logic to get
>> all in just one single place. I don't see why the same names couldn't be 
>> used on any other OS using DT.
> 
> To be used by another OS, they should be defined somewhere that's not
> subject to arbitrary changes at any time at the whim of Linux
> developers, without dt-related review.
> 
> That's not to say we couldn't use strings the kernel happened to use.
> I'm saying that the names exposed by bindings should be well-defined,
> and should not depend on the current state of a linux-internal header
> file.
> 
> I think it would be nicer to have a way of defining the keymap in dt
> anyway, so as to handle the general case and not get into the mess of
> having an arbitrary set of strings we need to constantly update.
> 
>> The logic behind include/media/rc-map.h, is that those names are used
>> by:
>>
>> 1) kernelspace: in order to locate a keytable with the same name, that
>> would be loaded when the device is initialized;
> 
> The kernel uses the strings, so it has them defined in its include path
> somewhere. If dt bindings wish to use the strings, they should be
> defined somewhere. That somewhere should not be a Linux-internal header
> file.
> 
>>
>> 2) userspace: to seek for a keytable with that name, allowing to
>> dynamically load the keymap table on userspace, instead of hardwiring
>> them on Kernelspace (or replacing the kernel's one by an user-customized
>> one).
> 
> The name of each table is not exposed to userspace, they are not defined
> under include/uapi. The fact that the names may be used to request other
> data does not change the fact that the kernel has one copy, userspace
> another. The set of strings the kernel understands *is* hard-wired.
> 
>>
>> So, I would simply call it as "keymap-name", keep pointing it to 
>> include/media/rc-map.h.
>>
>> That's said, this is actually a mandatory requirement, as without it,
>> the RC core will not be able to load a keytable, and the userspace tool
>> won't load the proper keymap, being confused on what to do.
> 
> It is possible to handle setting up the mapping within the kernel, or to
> actually describe the general case, something like how gpio-keys works.
> I think that would be preferable.

Am sure we can do something on gpio-keys lines. This will be one more
method to load the keymaps to rc system.

Drivers can decide if they want a static keymaping which is already
compiled in kernel or a custom one which can be loaded via DT or user space.

my_keymap: keymap {
	rc-keymap-name = "my-keymap";
	rc-codes	= 	<0x12, KEY_POWER,
				0x01, KEY_TV,
				0x15, KEY_DVD>;
		...
 };

my-rc-device {
	compatible = "my,rc-device";
	rc-keymap = <&my_keymap>;
	rx-mode  = "infrared";
};

Mauro, Does this sound Ok to you, and correct me if I miss understood
some thing here.


So at the end the bindings doc will look something like:

== Generic device tree bindings for remote control ==

properties:
	- rx-mode: Can be "infrared" or "uhf". This property specifies the L1
protocol used for receiving the remote control signals. Some devices may
support only one of the modes, this restriction should be  documented in
there respective device binding document.

	- tx-mode: Can be "infrared" or "uhf". This property specifies the L1
protocol used for transmitting remote control signals. Some devices may
support only one of the modes, this restriction should be  documented in
there respective device binding document.

Optional properties:
	- rc-keymap: phandle to remote control keymap.

== Remote control Keymaps ==

properties:
	- rc-keymap-name: 	Should be the name of the keymap.
	- rc-keymaps:		Should be an array of pair of scan code and actual key
code with first cell representing rc scan code and second cell
representing actual keycode.

example:

my_keymap: keymap {
	rc-keymap-name = "my-keymap";
	rc-keymaps	= 	<0x12, KEY_POWER,
				0x01, KEY_TV,
				0x15, KEY_DVD>;
		...
 };

my-rc-device {
	compatible = "my,rc-device";
	rc-keymap = <&my_keymap>;
	rx-mode  = "infrared";
};




Thanks,
srini

> 
>>
>> It should be noticed that, from time to time, manufacturers change the
>> remote control unit, as those devices are generally manufactured by a
>> third part. So, they change, for example, when they get a new BID to
>> provide IRs for a cheaper cost, or when they need/want to provide a "deluxe"
>> remote, a simplified "thin" one and/or when they need to provide customized
>> remotes to some Cable company, for example.
>>
>> So, it makes sense for it to be mandatory, as only with this information it
>> is possible to load the keymap that matches that specific IR unit model.
> 
> While it's true that some description is necessary, I do nto believe
> this is the right one.
> 
> Thanks,
> Mark.
> 
> 

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[Mark Rutland]

On Wed, Oct 09, 2013 at 01:17:30PM +0100, srinivas kandagatla wrote:
> >>>>  .../devicetree/bindings/media/remote-control.txt   |   31 ++++++++++++++++++++
> >>>>  1 files changed, 31 insertions(+), 0 deletions(-)
> >>>>  create mode 100644 Documentation/devicetree/bindings/media/remote-control.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/remote-control.txt b/Documentation/devicetree/bindings/media/remote-control.txt
> >>>> new file mode 100644
> >>>> index 0000000..901ea56
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/media/remote-control.txt
> >>>> @@ -0,0 +1,31 @@
> >>>> +Generic device tree bindings for remote control.
> >>>> +
> >>>> +properties:
> >>>> +	- compatible: Can contain any remote control driver compatible string.
> >>>> +	  example: "st-comms-irb, "gpio-ir-receiver".
> >>>
> >>> This is more generic than remote control, could this not just be left
> >>> for the specific binding to describe?
> >>>
> >>>> +	- reg: Base physical address of the controller and length of memory
> >>>> +	  mapped region.
> >>>
> >>> What if it's on a bus that isn't memory mapped (e.g. i2c, SPI)?
> >>>
> >>>> +	- interrupts: Interrupt-specifier for the sole interrupt generated by
> >>>> +	  the device. The interrupt specifier format depends on the
> >>>> +	  interrupt controller parent. Iff the device supports interrupts.
> >>>
> >>> What if it has multiple interrupts, and has interrupts-names?
> 
> I think for properties like interrupts, reg can be left undocumented
> here and let the actual device bindings document on this.

If it has no particur meaning for the class of binding, leaving it to
the individual device bindings seems fine to me.

> 
> >>>
> >>> It might be better to only describe the properties that relate
> >>> specifically to remote controls, rather than listing all of the generic
> >>> properties that device tree bidnings may have. That would match the
> >>> style of the clock bindings.
> >>>
> >>>> +	- rx-mode: Can be "infrared" or "uhf". rx-mode should be present iff
> >>>> +	  the rx pins are wired up.
> >>>
> >>> I'm unsure on this. What if the device has multiple receivers that can
> >>> be independently configured? 
> 
> Mauro C. had an option that this is not a real use-case and let's not
> overdesign the API, thinking on a possible scenario that may never happen.
> 
> Do you still think that this use case should be considered in this
> discussion?

Given how simple a device we're attempting to describe, I'm not even
sure it makes sense to have a class of binding. We could leave this to
individual device bindings for the moment.

> 
> >>
> >> Well, if a given remote controller hardware has more than one independent
> >> receiver (or transmitter), each one should have its own devnode.
> >> Likely, two entries at DT.
> > 
> > Why? If an IP block happens to have support for N connections, that
> > doesn't mean that each must be described individually. They likely share
> > a bank of registers, and depending on the device they might not even be
> > assigned consistently orgranised windows of that register bank.
> > 
> >>
> >>> What if it supports something other than
> >>> "infrared" or "uhf"? What if a device can only be wired up as
> >>> "infrared"? 
> 
> I think "infrared" and "uhf" covers all the use cases for remote controls.
> 
> If the device only supports one mode. It can be documented in device
> specific bindings. something like phy-mode of ethernet bindings.

Given there's the possibility of a device supporting one mode, I think
it would make more sense to describe the *-mode properties in the
specific bindings which require them (though they may be identical).

> >>
> >>> I'm not sure how generic these are, though we should certainly encourage
> >>> bindings that can be described this way to be described in the same way.
> >>>
> >>>> +	- tx-mode: Can be "infrared" or "uhf". tx-mode should be present iff
> >>>> +	  the tx pins are wired up.
> >>>
> >>> I have similar concerns here to those for the rx-mode property.
> >>>
> >>>> +
> >>>> +Optional properties:
> >>>> +	- linux,rc-map-name: Linux specific remote control map name. Refer to
> >>>> +	  include/media/rc-map.h for full list of maps.
> >>>
> >>> We shouldn't refer to Linux specifics (i.e. headers) in general in
> >>> bindings. While it's possible to relax that a bit for linux,*
> >>> properties, I'd prefer to explicitly list elements in the binding. That
> >>> prevents the ABI from changing under our feet by someone altering what
> >>> looks like a completely internal header file.
> >>
> >> Well, the remote controller keymaps at include/media/rc-map.h is just a
> >> bunch of string names, defined as macro to avoid duplicating those names
> >> everywhere, to avoid typos and to help some userspace parsing logic to get
> >> all in just one single place. I don't see why the same names couldn't be 
> >> used on any other OS using DT.
> > 
> > To be used by another OS, they should be defined somewhere that's not
> > subject to arbitrary changes at any time at the whim of Linux
> > developers, without dt-related review.
> > 
> > That's not to say we couldn't use strings the kernel happened to use.
> > I'm saying that the names exposed by bindings should be well-defined,
> > and should not depend on the current state of a linux-internal header
> > file.
> > 
> > I think it would be nicer to have a way of defining the keymap in dt
> > anyway, so as to handle the general case and not get into the mess of
> > having an arbitrary set of strings we need to constantly update.
> > 
> >> The logic behind include/media/rc-map.h, is that those names are used
> >> by:
> >>
> >> 1) kernelspace: in order to locate a keytable with the same name, that
> >> would be loaded when the device is initialized;
> > 
> > The kernel uses the strings, so it has them defined in its include path
> > somewhere. If dt bindings wish to use the strings, they should be
> > defined somewhere. That somewhere should not be a Linux-internal header
> > file.
> > 
> >>
> >> 2) userspace: to seek for a keytable with that name, allowing to
> >> dynamically load the keymap table on userspace, instead of hardwiring
> >> them on Kernelspace (or replacing the kernel's one by an user-customized
> >> one).
> > 
> > The name of each table is not exposed to userspace, they are not defined
> > under include/uapi. The fact that the names may be used to request other
> > data does not change the fact that the kernel has one copy, userspace
> > another. The set of strings the kernel understands *is* hard-wired.
> > 
> >>
> >> So, I would simply call it as "keymap-name", keep pointing it to 
> >> include/media/rc-map.h.
> >>
> >> That's said, this is actually a mandatory requirement, as without it,
> >> the RC core will not be able to load a keytable, and the userspace tool
> >> won't load the proper keymap, being confused on what to do.
> > 
> > It is possible to handle setting up the mapping within the kernel, or to
> > actually describe the general case, something like how gpio-keys works.
> > I think that would be preferable.
> 
> Am sure we can do something on gpio-keys lines. This will be one more
> method to load the keymaps to rc system.
> 
> Drivers can decide if they want a static keymaping which is already
> compiled in kernel or a custom one which can be loaded via DT or user space.
> 
> my_keymap: keymap {
> 	rc-keymap-name = "my-keymap";
> 	rc-codes	= 	<0x12, KEY_POWER,
> 				0x01, KEY_TV,
> 				0x15, KEY_DVD>;
> 		...
>  };
> 
> my-rc-device {
> 	compatible = "my,rc-device";
> 	rc-keymap = <&my_keymap>;
> 	rx-mode  = "infrared";
> };

This may be ok, but we'll need to nail down the kaymap binding..

> 
> Mauro, Does this sound Ok to you, and correct me if I miss understood
> some thing here.
> 
> 
> So at the end the bindings doc will look something like:
> 
> == Generic device tree bindings for remote control ==
> 
> properties:
> 	- rx-mode: Can be "infrared" or "uhf". This property specifies the L1
> protocol used for receiving the remote control signals. Some devices may
> support only one of the modes, this restriction should be  documented in
> there respective device binding document.
> 
> 	- tx-mode: Can be "infrared" or "uhf". This property specifies the L1
> protocol used for transmitting remote control signals. Some devices may
> support only one of the modes, this restriction should be  documented in
> there respective device binding document.

I think it would be better to place these in the specific bindings for
the devices which need them.

> 
> Optional properties:
> 	- rc-keymap: phandle to remote control keymap.
> 
> == Remote control Keymaps ==
> 
> properties:
> 	- rc-keymap-name: 	Should be the name of the keymap.
> 	- rc-keymaps:		Should be an array of pair of scan code and actual key
> code with first cell representing rc scan code and second cell
> representing actual keycode.

Is one cell always enough for any scan code (or any actual keycode)?

As the format of the scan code will be device-specific, should this be
under the node for the device? Are we likely to have multiple rc devices
in a single system that can share a keymap?

What's the format of the actual keycode? What values are valid?

> 
> example:
> 
> my_keymap: keymap {
> 	rc-keymap-name = "my-keymap";
> 	rc-keymaps	= 	<0x12, KEY_POWER,
> 				0x01, KEY_TV,
> 				0x15, KEY_DVD>;
> 		...
>  };

Please bracket list entries individually (it makes it far easier for
humans to read arbitrary lists in dt, regardless of how consistent this
may be).

Also, commas shouldn't be between brackets, dtc will barf if they're
there.

So this should be something like:

	rc-keymaps = <0x12 KEY_POWER>,
	             <0x01 KEY_TV>,
	             <0x15 KEY_DVD>;

> 
> my-rc-device {
> 	compatible = "my,rc-device";
> 	rc-keymap = <&my_keymap>;
> 	rx-mode  = "infrared";
> };

Thanks,
Mark.

Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control

[srinivas kandagatla]

Thanks Mark,

The blocking issue for st-rc driver is now closed.

On 18/10/13 12:37, Mark Rutland wrote:
>>
>> Mauro C. had an option that this is not a real use-case and let's not
>> overdesign the API, thinking on a possible scenario that may never happen.
>>
>> Do you still think that this use case should be considered in this
>> discussion?
> 
> Given how simple a device we're attempting to describe, I'm not even
> sure it makes sense to have a class of binding. We could leave this to
> individual device bindings for the moment.

Its clear.
>>
>> my_keymap: keymap {
>> 	rc-keymap-name = "my-keymap";
>> 	rc-codes	= 	<0x12, KEY_POWER,
>> 				0x01, KEY_TV,
>> 				0x15, KEY_DVD>;
>> 		...
>>  };
>>
>> my-rc-device {
>> 	compatible = "my,rc-device";
>> 	rc-keymap = <&my_keymap>;
>> 	rx-mode  = "infrared";
>> };
> 
> This may be ok, but we'll need to nail down the kaymap binding..

Yes, If Mauro thinks that rc keymaps from device tree is good feature we
can start a new discussion on this.

>> == Remote control Keymaps ==
>>
>> properties:
>> 	- rc-keymap-name: 	Should be the name of the keymap.
>> 	- rc-keymaps:		Should be an array of pair of scan code and actual key
>> code with first cell representing rc scan code and second cell
>> representing actual keycode.
> 
> Is one cell always enough for any scan code (or any actual keycode)?
> 
> As the format of the scan code will be device-specific, should this be
> under the node for the device? Are we likely to have multiple rc devices
> in a single system that can share a keymap?

I will let Mauro answer this.
> 
> What's the format of the actual keycode? What values are valid?
> 
>>
>> example:
>>
>> my_keymap: keymap {
>> 	rc-keymap-name = "my-keymap";
>> 	rc-keymaps	= 	<0x12, KEY_POWER,
>> 				0x01, KEY_TV,
>> 				0x15, KEY_DVD>;
>> 		...
>>  };
> 
> Please bracket list entries individually (it makes it far easier for
> humans to read arbitrary lists in dt, regardless of how consistent this
> may be).
> 
> Also, commas shouldn't be between brackets, dtc will barf if they're
> there.
> 
> So this should be something like:
> 
> 	rc-keymaps = <0x12 KEY_POWER>,
> 	             <0x01 KEY_TV>,
> 	             <0x15 KEY_DVD>;
> 

I agree this is much readable.

>>
>> my-rc-device {
>> 	compatible = "my,rc-device";
>> 	rc-keymap = <&my_keymap>;
>> 	rx-mode  = "infrared";
>> };
> 
> Thanks,
> Mark.
> 
>