Re: [PATCH v2] media: st-rc: Add ST remote control driver
On 29/08/13 13:01, Sean Young wrote: > On Thu, Aug 29, 2013 at 12:29:00PM +0100, Srinivas KANDAGATLA wrote: >> On 29/08/13 10:11, Sean Young wrote: >>> On Wed, Aug 28, 2013 at 04:33:50PM +0100, Srinivas KANDAGATLA wrote: From: Srinivas Kandagatla +config RC_ST + tristate "ST remote control receiver" + depends on ARCH_STI && LIRC && OF >>> >>> Minor nitpick, this should not depend on LIRC, it depends on RC_CORE. >> Yes, I will make it depend on RC_CORE, remove OF as suggested by Mauro >> CC and select LIRC to something like. >> >> depends on ARCH_STI && RC_CORE >> select LIRC > > The driver is usable with the kernel ir decoders, there is no need to > select LIRC or use lirc at all. > > You can either define a remote in drivers/media/rc/keymaps and set > the rcdev->map_name, or it can be loaded at runtime using ir-keytable(1); > either way you don't need to use a lirc userspace daemon. Yep.. got it. I will remove LIRC selection. Thanks, srini > > > Sean > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: st-rc: Add ST remote control driver
On Thu, Aug 29, 2013 at 12:29:00PM +0100, Srinivas KANDAGATLA wrote: > On 29/08/13 10:11, Sean Young wrote: > > On Wed, Aug 28, 2013 at 04:33:50PM +0100, Srinivas KANDAGATLA wrote: > >> From: Srinivas Kandagatla > >> +config RC_ST > >> + tristate "ST remote control receiver" > >> + depends on ARCH_STI && LIRC && OF > > > > Minor nitpick, this should not depend on LIRC, it depends on RC_CORE. > Yes, I will make it depend on RC_CORE, remove OF as suggested by Mauro > CC and select LIRC to something like. > > depends on ARCH_STI && RC_CORE > select LIRC The driver is usable with the kernel ir decoders, there is no need to select LIRC or use lirc at all. You can either define a remote in drivers/media/rc/keymaps and set the rcdev->map_name, or it can be loaded at runtime using ir-keytable(1); either way you don't need to use a lirc userspace daemon. Sean -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: st-rc: Add ST remote control driver
On 29/08/13 11:49, Mauro Carvalho Chehab wrote:
>>> +MODULE_AUTHOR("STMicroelectronics (R&D) Ltd");
>>> > > +MODULE_LICENSE("GPL");
>>> > > --
>>> > > 1.7.6.5
> Except for the few points that Sean commented, the patch seems ok on my eyes.
Thankyou for reviewing. I will send v3 with the fixes.
thanks,
srini
>
> Regards,
> Mauro
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: st-rc: Add ST remote control driver
On 29/08/13 10:11, Sean Young wrote:
> On Wed, Aug 28, 2013 at 04:33:50PM +0100, Srinivas KANDAGATLA wrote:
>> From: Srinivas Kandagatla
>>
>> This patch adds support to ST RC driver, which is basically a IR/UHF
>> receiver and transmitter. This IP (IRB) is common across all the ST
>> parts for settop box platforms. IRB is embedded in ST COMMS IP block.
>> It supports both Rx & Tx functionality.
>>
>> In this driver adds only Rx functionality via LIRC codec.
>>
>> Signed-off-by: Srinivas Kandagatla
>> ---
>> Hi Chehab,
>>
>> This is a very simple rc driver for IRB controller found in STi ARM CA9 SOCs.
>> STi ARM SOC support went in 3.11 recently.
>> This driver is a raw driver which feeds data to lirc codec for the user lircd
>> to decode the keys.
>>
>> This patch is based on git://linuxtv.org/media_tree.git master branch.
>>
>> Changes since v1:
>> - Device tree bindings cleaned up as suggested by Mark and Pawel
>> - use ir_raw_event_reset under overflow conditions as suggested by Sean.
>> - call ir_raw_event_handle in interrupt handler as suggested by Sean.
>> - correct allowed_protos flag with RC_BIT_ types as suggested by Sean.
>> - timeout and rx resolution added as suggested by Sean.
>
> Acked-by: Sean Young
Thankyou Sean for the Ack.
>
> Note minor nitpicks below.
>>
>> Thanks,
>> srini
>>
>> Documentation/devicetree/bindings/media/st-rc.txt | 24 ++
>> drivers/media/rc/Kconfig | 10 +
>> drivers/media/rc/Makefile |1 +
>> drivers/media/rc/st_rc.c | 392
>> +
>> 4 files changed, 427 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/media/st-rc.txt
>> create mode 100644 drivers/media/rc/st_rc.c
>>
>> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
>> index 11e84bc..bf301ed 100644
>> --- a/drivers/media/rc/Kconfig
>> +++ b/drivers/media/rc/Kconfig
>> @@ -322,4 +322,14 @@ config IR_GPIO_CIR
>> To compile this driver as a module, choose M here: the module will
>> be called gpio-ir-recv.
>>
>> +config RC_ST
>> +tristate "ST remote control receiver"
>> +depends on ARCH_STI && LIRC && OF
>
> Minor nitpick, this should not depend on LIRC, it depends on RC_CORE.
Yes, I will make it depend on RC_CORE, remove OF as suggested by Mauro
CC and select LIRC to something like.
depends on ARCH_STI && RC_CORE
select LIRC
>> +static int st_rc_probe(struct platform_device *pdev)
>> +{
>> +int ret = -EINVAL;
>> +struct rc_dev *rdev;
>> +struct device *dev = &pdev->dev;
>> +struct resource *res;
>> +struct st_rc_device *rc_dev;
>> +struct device_node *np = pdev->dev.of_node;
>> +const char *rx_mode;
>> +
>> +rc_dev = devm_kzalloc(dev, sizeof(struct st_rc_device), GFP_KERNEL);
>> +rdev = rc_allocate_device();
>> +
>> +if (!rc_dev || !rdev)
>> +return -ENOMEM;
>
> If one fails and the other succeeds you have a leak.
Yes... I will fix it in v3.
>
>> +
>> +if (np && !of_property_read_string(np, "rx-mode", &rx_mode)) {
>> +
[...]
>> +static SIMPLE_DEV_PM_OPS(st_rc_pm_ops, st_rc_suspend, st_rc_resume);
>> +#endif
>> +
>> +#ifdef CONFIG_OF
>
> Since this depends on OF it will always be defined.
Removed OF dependency in Kconfig.
>> +module_platform_driver(st_rc_driver);
>> +
>> +MODULE_DESCRIPTION("RC Transceiver driver for STMicroelectronics
>> platforms");
>> +MODULE_AUTHOR("STMicroelectronics (R&D) Ltd");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.7.6.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: st-rc: Add ST remote control driver
Em Thu, 29 Aug 2013 10:11:56 +0100
Sean Young escreveu:
> On Wed, Aug 28, 2013 at 04:33:50PM +0100, Srinivas KANDAGATLA wrote:
> > From: Srinivas Kandagatla
> >
> > This patch adds support to ST RC driver, which is basically a IR/UHF
> > receiver and transmitter. This IP (IRB) is common across all the ST
> > parts for settop box platforms. IRB is embedded in ST COMMS IP block.
> > It supports both Rx & Tx functionality.
> >
> > In this driver adds only Rx functionality via LIRC codec.
> >
> > Signed-off-by: Srinivas Kandagatla
> > ---
> > Hi Chehab,
> >
> > This is a very simple rc driver for IRB controller found in STi ARM CA9
> > SOCs.
> > STi ARM SOC support went in 3.11 recently.
> > This driver is a raw driver which feeds data to lirc codec for the user
> > lircd
> > to decode the keys.
> >
> > This patch is based on git://linuxtv.org/media_tree.git master branch.
> >
> > Changes since v1:
> > - Device tree bindings cleaned up as suggested by Mark and Pawel
> > - use ir_raw_event_reset under overflow conditions as suggested by Sean.
> > - call ir_raw_event_handle in interrupt handler as suggested by Sean.
> > - correct allowed_protos flag with RC_BIT_ types as suggested by Sean.
> > - timeout and rx resolution added as suggested by Sean.
>
> Acked-by: Sean Young
>
> Note minor nitpicks below.
> >
> > Thanks,
> > srini
> >
> > Documentation/devicetree/bindings/media/st-rc.txt | 24 ++
> > drivers/media/rc/Kconfig | 10 +
> > drivers/media/rc/Makefile |1 +
> > drivers/media/rc/st_rc.c | 392
> > +
> > 4 files changed, 427 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/st-rc.txt
> > create mode 100644 drivers/media/rc/st_rc.c
> >
> > diff --git a/Documentation/devicetree/bindings/media/st-rc.txt
> > b/Documentation/devicetree/bindings/media/st-rc.txt
> > new file mode 100644
> > index 000..20fe264
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/st-rc.txt
> > @@ -0,0 +1,24 @@
> > +Device-Tree bindings for ST IRB IP
> > +
> > +Required properties:
> > + - compatible: should be "st,comms-irb".
> > + - reg: base physical address of the controller and length of memory
> > + mapped region.
> > + - interrupts: interrupt number to the cpu. The interrupt specifier
> > + format depends on the interrupt controller parent.
> > +
> > +Optional properties:
> > + - rx-mode: can be "infrared" or "uhf".
> > + - tx-mode: should be "infrared".
> > + - pinctrl-names, pinctrl-0: the pincontrol settings to configure
> > + muxing properly for IRB pins.
> > + - clocks : phandle of clock.
> > +
> > +Example node:
> > +
> > + rc: rc@fe518000 {
> > + compatible = "st,comms-irb";
> > + reg = <0xfe518000 0x234>;
> > + interrupts = <0 203 0>;
> > + rx-mode = "infrared";
> > + };
> > diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> > index 11e84bc..bf301ed 100644
> > --- a/drivers/media/rc/Kconfig
> > +++ b/drivers/media/rc/Kconfig
> > @@ -322,4 +322,14 @@ config IR_GPIO_CIR
> >To compile this driver as a module, choose M here: the module will
> >be called gpio-ir-recv.
> >
> > +config RC_ST
> > + tristate "ST remote control receiver"
> > + depends on ARCH_STI && LIRC && OF
>
> Minor nitpick, this should not depend on LIRC, it depends on RC_CORE.
>
> > + help
> > +Say Y here if you want support for ST remote control driver
> > +which allows both IR and UHF RX.
> > +The driver passes raw pluse and space information to the LIRC decoder.
> > +
> > +If you're not sure, select N here.
> > +
> > endif #RC_DEVICES
> > diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> > index 56bacf0..f4eb32c 100644
> > --- a/drivers/media/rc/Makefile
> > +++ b/drivers/media/rc/Makefile
> > @@ -30,3 +30,4 @@ obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
> > obj-$(CONFIG_IR_GPIO_CIR) += gpio-ir-recv.o
> > obj-$(CONFIG_IR_IGUANA) += iguanair.o
> > obj-$(CONFIG_IR_TTUSBIR) += ttusbir.o
> > +obj-$(CONFIG_RC_ST) += st_rc.o
> > diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
> > new file mode 100644
> > index 000..5caa6c5
> > --- /dev/null
> > +++ b/drivers/media/rc/st_rc.c
> > @@ -0,0 +1,392 @@
> > +/*
> > + * Copyright (C) 2013 STMicroelectronics Limited
> > + * Author: Srinivas Kandagatla
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +struct st_rc_device {
> > + struct device
Re: [PATCH v2] media: st-rc: Add ST remote control driver
On Wed, Aug 28, 2013 at 04:33:50PM +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla
>
> This patch adds support to ST RC driver, which is basically a IR/UHF
> receiver and transmitter. This IP (IRB) is common across all the ST
> parts for settop box platforms. IRB is embedded in ST COMMS IP block.
> It supports both Rx & Tx functionality.
>
> In this driver adds only Rx functionality via LIRC codec.
>
> Signed-off-by: Srinivas Kandagatla
> ---
> Hi Chehab,
>
> This is a very simple rc driver for IRB controller found in STi ARM CA9 SOCs.
> STi ARM SOC support went in 3.11 recently.
> This driver is a raw driver which feeds data to lirc codec for the user lircd
> to decode the keys.
>
> This patch is based on git://linuxtv.org/media_tree.git master branch.
>
> Changes since v1:
> - Device tree bindings cleaned up as suggested by Mark and Pawel
> - use ir_raw_event_reset under overflow conditions as suggested by Sean.
> - call ir_raw_event_handle in interrupt handler as suggested by Sean.
> - correct allowed_protos flag with RC_BIT_ types as suggested by Sean.
> - timeout and rx resolution added as suggested by Sean.
Acked-by: Sean Young
Note minor nitpicks below.
>
> Thanks,
> srini
>
> Documentation/devicetree/bindings/media/st-rc.txt | 24 ++
> drivers/media/rc/Kconfig | 10 +
> drivers/media/rc/Makefile |1 +
> drivers/media/rc/st_rc.c | 392
> +
> 4 files changed, 427 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/st-rc.txt
> create mode 100644 drivers/media/rc/st_rc.c
>
> diff --git a/Documentation/devicetree/bindings/media/st-rc.txt
> b/Documentation/devicetree/bindings/media/st-rc.txt
> new file mode 100644
> index 000..20fe264
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/st-rc.txt
> @@ -0,0 +1,24 @@
> +Device-Tree bindings for ST IRB IP
> +
> +Required properties:
> + - compatible: should be "st,comms-irb".
> + - reg: base physical address of the controller and length of memory
> + mapped region.
> + - interrupts: interrupt number to the cpu. The interrupt specifier
> + format depends on the interrupt controller parent.
> +
> +Optional properties:
> + - rx-mode: can be "infrared" or "uhf".
> + - tx-mode: should be "infrared".
> + - pinctrl-names, pinctrl-0: the pincontrol settings to configure
> + muxing properly for IRB pins.
> + - clocks : phandle of clock.
> +
> +Example node:
> +
> + rc: rc@fe518000 {
> + compatible = "st,comms-irb";
> + reg = <0xfe518000 0x234>;
> + interrupts = <0 203 0>;
> + rx-mode = "infrared";
> + };
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 11e84bc..bf301ed 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -322,4 +322,14 @@ config IR_GPIO_CIR
> To compile this driver as a module, choose M here: the module will
> be called gpio-ir-recv.
>
> +config RC_ST
> + tristate "ST remote control receiver"
> + depends on ARCH_STI && LIRC && OF
Minor nitpick, this should not depend on LIRC, it depends on RC_CORE.
> + help
> + Say Y here if you want support for ST remote control driver
> + which allows both IR and UHF RX.
> + The driver passes raw pluse and space information to the LIRC decoder.
> +
> + If you're not sure, select N here.
> +
> endif #RC_DEVICES
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 56bacf0..f4eb32c 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
> obj-$(CONFIG_IR_GPIO_CIR) += gpio-ir-recv.o
> obj-$(CONFIG_IR_IGUANA) += iguanair.o
> obj-$(CONFIG_IR_TTUSBIR) += ttusbir.o
> +obj-$(CONFIG_RC_ST) += st_rc.o
> diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
> new file mode 100644
> index 000..5caa6c5
> --- /dev/null
> +++ b/drivers/media/rc/st_rc.c
> @@ -0,0 +1,392 @@
> +/*
> + * Copyright (C) 2013 STMicroelectronics Limited
> + * Author: Srinivas Kandagatla
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct st_rc_device {
> + struct device *dev;
> + int irq;
> + int irq_wake;
> + struct clk *sys_clock;
> + void*base; /* Register base address */
> + void
