Re: [PATCH] remove lots of IS_ERR_VALUE abuses
On 27/05/16 22:23, Arnd Bergmann wrote: drivers/acpi/acpi_dbg.c | 22 +++--- drivers/ata/sata_highbank.c | 2 +- drivers/clk/tegra/clk-tegra210.c | 2 +- drivers/cpufreq/omap-cpufreq.c | 2 +- drivers/crypto/caam/ctrl.c | 2 +- drivers/dma/sun4i-dma.c | 16 drivers/gpio/gpio-xlp.c | 2 +- drivers/gpu/drm/sti/sti_vtg.c| 4 ++-- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 2 +- drivers/gpu/host1x/hw/intr_hw.c | 2 +- drivers/iommu/arm-smmu-v3.c | 18 +- ... drivers/mmc/host/sdhci-of-at91.c | 2 +- drivers/mmc/host/sdhci.c | 4 ++-- drivers/net/ethernet/freescale/fman/fman.c | 2 +- drivers/net/ethernet/freescale/fman/fman_muram.c | 4 ++-- drivers/net/ethernet/freescale/fman/fman_muram.h | 4 ++-- drivers/net/wireless/ti/wlcore/spi.c | 4 ++-- drivers/nvmem/core.c | 22 +++--- For nvmem part, Acked-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> drivers/tty/serial/amba-pl011.c | 2 +- drivers/tty/serial/sprd_serial.c | 2 +- drivers/video/fbdev/da8xx-fb.c | 4 ++-- fs/afs/write.c | 4 fs/binfmt_flat.c | 6 +++--- fs/gfs2/dir.c| 15 +-- kernel/pid.c | 2 +- net/9p/client.c | 4 ++-- sound/soc/qcom/lpass-platform.c | 4 ++-- There is already a patch for this in Mark Brown's sound tree, https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/commit/?h=for-linus=cef794f76485f4a4e6affd851ae9f9d0eb4287a5 Thanks, srini 38 files changed, 100 insertions(+), 101 deletions(-) -- 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: [STLinux Kernel] [PATCH 1/3] media: st-rc: move to using reset_control_get_optional
Hi Pete, On 23/09/14 19:02, Peter Griffin wrote: Hi Srini, On Mon, 22 Sep 2014, Srinivas Kandagatla wrote: This patch fixes a compilation error while building with the random kernel configuration. drivers/media/rc/st_rc.c: In function 'st_rc_probe': drivers/media/rc/st_rc.c:281:2: error: implicit declaration of function 'reset_control_get' [-Werror=implicit-function-declaration] rc_dev-rstc = reset_control_get(dev, NULL); drivers/media/rc/st_rc.c:281:15: warning: assignment makes pointer from integer without a cast [enabled by default] rc_dev-rstc = reset_control_get(dev, NULL); Is managing the reset line actually optional though? I can't test atm as I don't have access to my board, but quite often if the IP's aren't taken out of reset reads / writes to the perhpiheral will hang the SoC. Yes and No. AFAIK reset line is optional on SOCs like 7108, 7141. I think having the driver function without reset might is a value add in case we plan to reuse the mainline driver for these SOCs. On latest ARM SOCs with SBC the IRB IP is moved to SBC and held in reset. Am not sure, if the reset line is optional in next generation SOCs? If managing the reset line isn't optional then I think the correct fix is to add depends on RESET_CONTROLLER in the kconfig. I agree. This would make the COMPILE_TEST less useful though. thanks, srini This will then do the right thing for randconfig builds as well. regards, Peter. -- 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
[PATCH 0/3] media:st-rc: Misc fixes.
Hi Mauro, Thankyou for the [media] enable COMPILE_TEST for media drivers patch which picked up few things in st-rc driver in linux-next testing. Here is a few minor fixes to the driver, could you consider them for the next merge window. Thanks, srini Srinivas Kandagatla (3): media: st-rc: move to using reset_control_get_optional media: st-rc: move pm ops setup out of conditional compilation. media: st-rc: Remove .owner field for driver drivers/media/rc/st_rc.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) -- 1.9.1 -- 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
[PATCH 1/3] media: st-rc: move to using reset_control_get_optional
This patch fixes a compilation error while building with the random kernel configuration. drivers/media/rc/st_rc.c: In function 'st_rc_probe': drivers/media/rc/st_rc.c:281:2: error: implicit declaration of function 'reset_control_get' [-Werror=implicit-function-declaration] rc_dev-rstc = reset_control_get(dev, NULL); drivers/media/rc/st_rc.c:281:15: warning: assignment makes pointer from integer without a cast [enabled by default] rc_dev-rstc = reset_control_get(dev, NULL); Reported-by: Jim Davis jim.ep...@gmail.com Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/media/rc/st_rc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c index 5c15135..e0f1312 100644 --- a/drivers/media/rc/st_rc.c +++ b/drivers/media/rc/st_rc.c @@ -278,7 +278,7 @@ static int st_rc_probe(struct platform_device *pdev) rc_dev-rx_base = rc_dev-base; - rc_dev-rstc = reset_control_get(dev, NULL); + rc_dev-rstc = reset_control_get_optional(dev, NULL); if (IS_ERR(rc_dev-rstc)) rc_dev-rstc = NULL; -- 1.9.1 -- 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
[PATCH 2/3] media: st-rc: move pm ops setup out of conditional compilation.
This patch moves setting of pm_ops out of the CONFIG_PM_SLEEP condition.
Setting pm ops under CONFIG_PM_SLEEP does not make any sense.
This patch also remove unnecessary also remove CONFIG_PM condition for pm
member in st_rc_driver structure.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
drivers/media/rc/st_rc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index e0f1312..03bbb09 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -376,9 +376,10 @@ static int st_rc_resume(struct device *dev)
return 0;
}
-static SIMPLE_DEV_PM_OPS(st_rc_pm_ops, st_rc_suspend, st_rc_resume);
#endif
+static SIMPLE_DEV_PM_OPS(st_rc_pm_ops, st_rc_suspend, st_rc_resume);
+
#ifdef CONFIG_OF
static struct of_device_id st_rc_match[] = {
{ .compatible = st,comms-irb, },
@@ -393,9 +394,7 @@ static struct platform_driver st_rc_driver = {
.name = IR_ST_NAME,
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(st_rc_match),
-#ifdef CONFIG_PM
.pm = st_rc_pm_ops,
-#endif
},
.probe = st_rc_probe,
.remove = st_rc_remove,
--
1.9.1
--
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
[PATCH 3/3] media: st-rc: Remove .owner field for driver
There is no need to init .owner field.
Based on the patch from Peter Griffin peter.grif...@linaro.org
mmc: remove .owner field for drivers using module_platform_driver
This patch removes the superflous .owner field for drivers which
use the module_platform_driver API, as this is overriden in
platform_driver_register anyway.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
drivers/media/rc/st_rc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index 03bbb09..e309441 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -392,7 +392,6 @@ MODULE_DEVICE_TABLE(of, st_rc_match);
static struct platform_driver st_rc_driver = {
.driver = {
.name = IR_ST_NAME,
- .owner = THIS_MODULE,
.of_match_table = of_match_ptr(st_rc_match),
.pm = st_rc_pm_ops,
},
--
1.9.1
--
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 RFC] media: rc: OF: Add Generic bindings for remote-control
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.
--
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 RFC] media: rc: OF: Add Generic bindings for remote-control
.../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 000..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
Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control
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 -- 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 RFC] media: rc: OF: Add Generic bindings for remote-control
On 27/09/13 14:57, Mauro Carvalho Chehab wrote: Em Fri, 27 Sep 2013 14:26:12 +0100 Srinivas KANDAGATLA srinivas.kandaga...@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 -- 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
[PATCH v5] media: st-rc: Add ST remote control driver
From: Srinivas Kandagatla srinivas.kandaga...@st.com
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.
This driver adds only Rx functionality via LIRC codec.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
Acked-by: Sean Young s...@mess.org
---
Hi Chehab,
This is a very simple rc driver for IRB controller found in STi SOCs.
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 v3.12-rc1.
Changes since v4:
- updated dt bindings doc with suggestions from Stephen W.
Changes since v3:
- updated dt bindings doc with suggestions from Mark R.
Changes since v2:
- Updated Kconfig dependencies as suggested by Sean and Chehab.
- Fixed a logical check spoted by Sean.
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.
Thankyou all for reviewing and commenting.
Thanks,
srini
Documentation/devicetree/bindings/media/st-rc.txt | 27 ++
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/st_rc.c | 396 +
4 files changed, 434 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..797ef39
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st-rc.txt
@@ -0,0 +1,27 @@
+Device-Tree bindings for ST IRB IP
+
+Required properties:
+ - compatible: Should contain st,comms-irb.
+ - 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.
+ - rx-mode: can be infrared or uhf. rx-mode should be present iff
+ the rx pins are wired up.
+ - tx-mode: should be infrared. tx-mode should be present iff the tx
+ pins are wired up.
+
+Optional properties:
+ - pinctrl-names, pinctrl-0: the pincontrol settings to configure muxing
+ properly for IRB pins.
+ - clocks : phandle with clock-specifier pair for IRB block.
+
+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..557afc5 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 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..e2dad9c
--- /dev/null
+++ b/drivers/media/rc/st_rc.c
@@ -0,0 +1,396 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited
+ * Author: Srinivas Kandagatla srinivas.kandaga...@st.com
+ *
+ * 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 linux/kernel.h
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include media/rc-core.h
+#include linux/pinctrl/consumer.h
[PATCH RFC] media: rc: OF: Add Generic bindings for remote-control
From: Srinivas Kandagatla srinivas.kandaga...@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.kandaga...@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 000..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
--
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 v5] media: st-rc: Add ST remote control driver
Thanks Prabhakar,
+config RC_ST
+ tristate ST remote control receiver
+ depends on ARCH_STI 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.
+
s/pluse/pulse
Yep.. Will fix it..
[...]
+
+/* Registers */
+#define IRB_SAMPLE_RATE_COMM 0x64/* sample freq divisor*/
+#define IRB_CLOCK_SEL 0x70/* clock select */
+#define IRB_CLOCK_SEL_STATUS 0x74/* clock status */
+/* IRB IR/UHF receiver registers */
+#define IRB_RX_ON 0x40 /* pulse time capture */
+#define IRB_RX_SYS 0X44 /* sym period capture */
+#define IRB_RX_INT_EN 0x48 /* IRQ enable (R/W) */
+#define IRB_RX_INT_STATUS 0x4C /* IRQ status (R/W) */
+#define IRB_RX_EN 0x50 /* Receive enablei*/
s/enablei/enable
+#define IRB_MAX_SYM_PERIOD 0x54 /* max sym value */
+#define IRB_RX_INT_CLEAR0x58 /* overrun status */
+#define IRB_RX_STATUS 0x6C /* receive status */
+#define IRB_RX_NOISE_SUPPR 0x5C /* noise suppression */
+#define IRB_RX_POLARITY_INV 0x68 /* polarity inverter */
+
generally smaller case hex letters are preferred
I will change this.
[...]
+
+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);
+
+ if (!rc_dev)
+ return -ENOMEM;
+
+ rdev = rc_allocate_device();
+
+ if (!rdev)
+ return -ENOMEM;
+
+ if (np !of_property_read_string(np, rx-mode, rx_mode)) {
+
+ if (!strcmp(rx_mode, uhf)) {
+ rc_dev-rxuhfmode = true;
+ } else if (!strcmp(rx_mode, infrared)) {
+ rc_dev-rxuhfmode = false;
+ } else {
+ dev_err(dev, Unsupported rx mode [%s]\n, rx_mode);
+ goto err;
+ }
+
+ } else {
+ goto err;
+ }
+
+ rc_dev-sys_clock = devm_clk_get(dev, NULL);
+ if (IS_ERR(rc_dev-sys_clock)) {
+ dev_err(dev, System clock not found\n);
+ ret = PTR_ERR(rc_dev-sys_clock);
+ goto err;
+ }
+
+ rc_dev-irq = platform_get_irq(pdev, 0);
+ if (rc_dev-irq 0) {
+ ret = rc_dev-irq;
+ goto clkerr;
+ }
+
+ ret = -ENODEV;
Not required
yes, its redundant here.
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ goto clkerr;
+
This check is not required.
Ok, I see your point.
+ rc_dev-base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(rc_dev-base))
+ goto clkerr;
+
In case of error assin ret to PTR_ERR(rc_dev-base)
Yes It makes sense .. I will assign it to ret.
Also found that there is a typo here.
It should be a goto err instead of goto clkerr.
Will fix that as well in next spin.
+ if (rc_dev-rxuhfmode)
+ rc_dev-rx_base = rc_dev-base + 0x40;
+ else
+ rc_dev-rx_base = rc_dev-base;
+
+ rc_dev-dev = dev;
+ platform_set_drvdata(pdev, rc_dev);
+ st_rc_hardware_init(rc_dev);
+
+ rdev-driver_type = RC_DRIVER_IR_RAW;
+ rdev-allowed_protos = RC_BIT_ALL;
+ /* rx sampling rate is 10Mhz */
+ rdev-rx_resolution = 100;
+ rdev-timeout = US_TO_NS(MAX_SYMB_TIME);
+ rdev-priv = rc_dev;
+ rdev-open = st_rc_open;
+ rdev-close = st_rc_close;
+ rdev-driver_name = IR_ST_NAME;
+ rdev-map_name = RC_MAP_LIRC;
+ rdev-input_name = ST Remote Control Receiver;
+
+ /* enable wake via this device */
+ device_set_wakeup_capable(dev, true);
+ device_set_wakeup_enable(dev, true);
+
+ ret = rc_register_device(rdev);
+ if (ret 0)
+ goto clkerr;
+
+ rc_dev-rdev = rdev;
+ if (devm_request_irq(dev, rc_dev-irq, st_rc_rx_interrupt,
+ IRQF_NO_SUSPEND, IR_ST_NAME, rc_dev) 0) {
+ dev_err(dev, IRQ %d register failed\n, rc_dev-irq);
+ ret = -EINVAL;
+ goto rcerr;
+ }
+
+ /**
+* for LIRC_MODE_MODE2 or LIRC_MODE_PULSE or LIRC_MODE_RAW
+* lircd expects a long space first before a signal train to sync.
+*/
+ st_rc_send_lirc_timeout(rdev);
+
+ dev_info(dev, setup in %s mode\n, rc_dev-rxuhfmode ? UHF :
IR);
+
+ return ret;
+rcerr:
+
Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control
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 -- 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 v4] media: st-rc: Add ST remote control driver
Hi Stephen, On 24/09/13 20:49, Stephen Warren wrote: Should those property names be prefixed with st,; I assume they're specific to this binding rather than something generic that applies to all IR controller bindings? If you expect them to be generic, it's fine. Officially these bindings are not specified in ePAPR specs Well, there are plenty of properties we now consider generic that aren't in ePAPR... but I see no reason for not having these properties as generic ones. Are you ok with that? I suppose that infrared-vs-uhf is a concept that's probably common enough across any similar HW device, so it may make sense for these properties to be generic. If we do intend them to be generic, I'd suggest they be defined in some generic binding document though; perhaps something like bindings/media/ir.txt or bindings/media/remote-control.txt? That way, a HW-specific binding isn't the only place where a supposedly generic property is defined. For now I will send a v5 for this driver with these generic properties. And, I will send an separate RFC for the generic binding document (bindings/media/remote-control.txt) so that we can get more inputs from others as well. Thanks, srini -- 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 v4] media: st-rc: Add ST remote control driver
Thanks Stephen, On 23/09/13 21:40, Stephen Warren wrote: On 09/19/2013 02:59 AM, Srinivas KANDAGATLA wrote: 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. diff --git a/Documentation/devicetree/bindings/media/st-rc.txt b/Documentation/devicetree/bindings/media/st-rc.txt +Required properties: +- compatible: should contain st,comms-irb. +- reg: base physical address of the controller and length of memory +mapped region. Nits: The indentation is a little odd here; I'd expect the - to be in column 1, at least that's how most other binding docs are written. Not a big deal though. It'd be nice to indent the continuation lines (e.g. mapped region) a bit more so it's easier to see where each new entry starts. There are two spaces in mapped region. I should have noticed it... I will fix it in next version. +- rx-mode: can be infrared or uhf. rx-mode should be present iff the + rx pins are wired up. +- tx-mode: should be infrared. tx-mode should be present iff the tx + pins are wired up. Should those property names be prefixed with st,; I assume they're specific to this binding rather than something generic that applies to all IR controller bindings? If you expect them to be generic, it's fine. Officially these bindings are not specified in ePAPR specs but I see no reason for not having these properties as generic ones. Are you ok with that? +- clocks : phandle with clock-specifier pair. For what clock? Its Clock for IRB block. I will add this in next version. Thanks, srini -- 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
[PATCH v4] media: st-rc: Add ST remote control driver
From: Srinivas Kandagatla srinivas.kandaga...@st.com
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 srinivas.kandaga...@st.com
Acked-by: Sean Young s...@mess.org
---
Hi Chehab,
This is a very simple rc driver for IRB controller found in STi ARM CA9 SOCs.
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 v3.12-rc1.
Changes since v3:
- updated dt bindings doc with suggestions from Mark R.
Changes since v2:
- Updated Kconfig dependencies as suggested by Sean and Chehab.
- Fixed a logical check spoted by Sean.
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.
Thankyou all for reviewing and commenting.
Thanks,
srini
Documentation/devicetree/bindings/media/st-rc.txt | 27 ++
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/st_rc.c | 396 +
4 files changed, 434 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..71de22b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st-rc.txt
@@ -0,0 +1,27 @@
+Device-Tree bindings for ST IRB IP
+
+Required properties:
+ - compatible: should contain st,comms-irb.
+ - 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.
+ - rx-mode: can be infrared or uhf. rx-mode should be present iff the
+ rx pins are wired up.
+ - tx-mode: should be infrared. tx-mode should be present iff the tx
+ pins are wired up.
+
+Optional properties:
+ - pinctrl-names, pinctrl-0: the pincontrol settings to configure
+ muxing properly for IRB pins.
+ - clocks : phandle with clock-specifier pair.
+
+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..557afc5 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 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..e2dad9c
--- /dev/null
+++ b/drivers/media/rc/st_rc.c
@@ -0,0 +1,396 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited
+ * Author: Srinivas Kandagatla srinivas.kandaga...@st.com
+ *
+ * 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 linux/kernel.h
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include media/rc-core.h
+#include linux/pinctrl/consumer.h
+
+struct st_rc_device {
+ struct device *dev;
+ int
Re: [PATCH v3] media: st-rc: Add ST remote control driver
Thanks Mark, On 16/09/13 15:10, Mark Rutland wrote: + +Required properties: + - compatible: should be st,comms-irb. This should probably say should contain rather than should be. There may be future vairants of this device, which will also have a more specific compatible string. Ok, will change it to the suggest. + - 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. I don't like the phrase interrupt number to the cpu. We already have the term interrupt-specifier to precisely define this. How about: - interrupts: interrupt-specifier for the sole interrupt generated by the device. TBH, I did copy them from one of the existing bindings docs. I will change it. + +Optional properties: + - rx-mode: can be infrared or uhf. + - tx-mode: should be infrared. Are these required to use rx/tx? Yes, these are required for driver to be in rx/tx mode. One of them can be optional depending on the board setup. So, Is it ok to move such properties to required properties section? If you don't have a tx-mode or rx-mode, I assume you can't do anything... Yes, driver errs out. + - pinctrl-names, pinctrl-0: the pincontrol settings to configure + muxing properly for IRB pins. If we're expecting names, the names we expect should be defined. + - clocks : phandle of clock. This is not just a phandle. This is a phandle + clock-specifier pair. Yep, will change it. Thanks, srini Cheers, -- 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 srinivas.kandaga...@st.com
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 srinivas.kandaga...@st.com
---
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 s...@mess.org
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 (RD) 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
On 29/08/13 11:49, Mauro Carvalho Chehab wrote: +MODULE_AUTHOR(STMicroelectronics (RD) 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 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 srinivas.kandaga...@st.com +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
[PATCH v3] media: st-rc: Add ST remote control driver
From: Srinivas Kandagatla srinivas.kandaga...@st.com
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 srinivas.kandaga...@st.com
Acked-by: Sean Young s...@mess.org
---
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.
If its not too late can you please consider this driver for 3.12.
Changes since v2:
- Updated Kconfig dependencies as suggested by Sean and Chehab.
- Fixed a logical check spoted by Sean.
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.
Thankyou all for reviewing and commenting.
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 | 396 +
4 files changed, 431 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..557afc5 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 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..e2dad9c
--- /dev/null
+++ b/drivers/media/rc/st_rc.c
@@ -0,0 +1,396 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited
+ * Author: Srinivas Kandagatla srinivas.kandaga...@st.com
+ *
+ * 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 linux/kernel.h
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include media/rc-core.h
+#include linux/pinctrl/consumer.h
+
+struct st_rc_device {
+ struct device *dev;
+ int irq;
+ int irq_wake;
+ struct clk *sys_clock;
+ void
[PATCH v2] media: st-rc: Add ST remote control driver
From: Srinivas Kandagatla srinivas.kandaga...@st.com
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 srinivas.kandaga...@st.com
---
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.
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
+ 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 srinivas.kandaga...@st.com
+ *
+ * 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 linux/kernel.h
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include media/rc-core.h
+#include linux/pinctrl/consumer.h
+
+struct st_rc_device {
+ struct device *dev;
+ int irq;
+ int irq_wake;
+ struct clk *sys_clock;
+ void*base; /* Register base address */
+ void*rx_base;/* RX Register base address */
+ struct rc_dev *rdev;
+ booloverclocking;
+ int
Re: [PATCH] media: st-rc: Add ST remote control driver
Thanks Sean for the comments.
On 16/08/13 09:38, Sean Young wrote:
On Wed, Aug 14, 2013 at 06:27:01PM +0100, Srinivas KANDAGATLA wrote:
[...]
Documentation/devicetree/bindings/media/st-rc.txt | 18 +
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/st_rc.c | 371
+
4 files changed, 400 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..57f9ee8
--- /dev/null
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
[...]
+static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
+{
+unsigned int symbol, mark = 0;
+struct st_rc_device *dev = data;
+int last_symbol = 0;
+u32 status;
+DEFINE_IR_RAW_EVENT(ev);
+
+if (dev-irq_wake)
+pm_wakeup_event(dev-dev, 0);
+
+status = readl(dev-rx_base + IRB_RX_STATUS);
+
+while (status (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW)) {
+u32 int_status = readl(dev-rx_base + IRB_RX_INT_STATUS);
+if (unlikely(int_status IRB_RX_OVERRUN_INT)) {
You should call ir_raw_event_reset() so that the in-kernel decoders
realise that the IR stream is not contiguous.
Yep... It makes sense.. I will add this.
+/* discard the entire collection in case of errors! */
+dev_info(dev-dev, IR RX overrun\n);
+writel(IRB_RX_OVERRUN_INT,
+dev-rx_base + IRB_RX_INT_CLEAR);
+continue;
+}
+
+symbol = readl(dev-rx_base + IRB_RX_SYS);
+mark = readl(dev-rx_base + IRB_RX_ON);
+
+if (symbol == IRB_TIMEOUT)
+last_symbol = 1;
+
+ /* Ignore any noise */
+if ((mark 2) (symbol 1)) {
+symbol -= mark;
+if (dev-overclocking) { /* adjustments to timings */
+symbol *= dev-sample_mult;
+symbol /= dev-sample_div;
+mark *= dev-sample_mult;
+mark /= dev-sample_div;
+}
+
+ev.duration = US_TO_NS(mark);
+ev.pulse = true;
+ir_raw_event_store(dev-rdev, ev);
+
+if (!last_symbol) {
+ev.duration = US_TO_NS(symbol);
+ev.pulse = false;
+ir_raw_event_store(dev-rdev, ev);
Make sure you call ir_raw_event_handle() once a while (maybe every time
the interrupt handler is called?) to prevent the ir kfifo from
overflowing in case of very long IR. ir_raw_event_store() just adds
new edges to the kfifo() but does not flush them to the decoders or
lirc.
I agree, but Am not sure it will really help in this case because, we
are going to stay in this interrupt handler till we get a
last_symbol(full key press/release event).. So calling
ir_raw_event_store mulitple times might not help because the
ir_raw_event kthread(which is clearing kfifo) which is only scheduled
after returning from this interrupt.
Correct me if Am wrong.
+} else {
+st_rc_send_lirc_timeout(dev-rdev);
+ir_raw_event_handle(dev-rdev);
+}
+}
+last_symbol = 0;
+status = readl(dev-rx_base + IRB_RX_STATUS);
+}
+
+writel(IRB_RX_INTS, dev-rx_base + IRB_RX_INT_CLEAR);
+
+return IRQ_HANDLED;
+}
+
[...]
+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;
+
+rc_dev = devm_kzalloc(dev, sizeof(struct st_rc_device), GFP_KERNEL);
+rdev = rc_allocate_device();
+
+if (!rc_dev || !rdev)
+return -ENOMEM;
+
+if (np)
+rc_dev-rxuhfmode = of_property_read_bool(np, st,uhfmode);
+
+rc_dev-sys_clock = devm_clk_get(dev, NULL);
+if (IS_ERR(rc_dev-sys_clock)) {
+dev_err(dev, System clock not found\n);
+ret = PTR_ERR(rc_dev-sys_clock);
+goto err;
+}
+
+rc_dev-irq = platform_get_irq(pdev, 0);
+if (rc_dev-irq 0) {
+ret = rc_dev-irq;
+goto clkerr;
+}
+
+ret = -ENODEV;
+res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+if (!res)
+goto clkerr;
+
+rc_dev-base = devm_ioremap_resource(dev, res
Re: [PATCH] media: st-rc: Add ST remote control driver
On 16/08/13 12:40, Sean Young wrote:
On Fri, Aug 16, 2013 at 11:53:48AM +0100, Srinivas KANDAGATLA wrote:
Thanks Sean for the comments.
On 16/08/13 09:38, Sean Young wrote:
On Wed, Aug 14, 2013 at 06:27:01PM +0100, Srinivas KANDAGATLA wrote:
[...]
+ /* discard the entire collection in case of errors! */
+ dev_info(dev-dev, IR RX overrun\n);
+ writel(IRB_RX_OVERRUN_INT,
+ dev-rx_base + IRB_RX_INT_CLEAR);
+ continue;
+ }
+
+ symbol = readl(dev-rx_base + IRB_RX_SYS);
+ mark = readl(dev-rx_base + IRB_RX_ON);
+
+ if (symbol == IRB_TIMEOUT)
+ last_symbol = 1;
+
+ /* Ignore any noise */
+ if ((mark 2) (symbol 1)) {
+ symbol -= mark;
+ if (dev-overclocking) { /* adjustments to timings */
+ symbol *= dev-sample_mult;
+ symbol /= dev-sample_div;
+ mark *= dev-sample_mult;
+ mark /= dev-sample_div;
+ }
+
+ ev.duration = US_TO_NS(mark);
+ ev.pulse = true;
+ ir_raw_event_store(dev-rdev, ev);
+
+ if (!last_symbol) {
+ ev.duration = US_TO_NS(symbol);
+ ev.pulse = false;
+ ir_raw_event_store(dev-rdev, ev);
Make sure you call ir_raw_event_handle() once a while (maybe every time
the interrupt handler is called?) to prevent the ir kfifo from
overflowing in case of very long IR. ir_raw_event_store() just adds
new edges to the kfifo() but does not flush them to the decoders or
lirc.
I agree, but Am not sure it will really help in this case because, we
are going to stay in this interrupt handler till we get a
last_symbol(full key press/release event).. So calling
ir_raw_event_store mulitple times might not help because the
ir_raw_event kthread(which is clearing kfifo) which is only scheduled
after returning from this interrupt.
If I read it correctly, then this is only true if the fifo contains an
IRB_TIMEOUT symbol. If not yet, then the interrupt handlers is not
waiting around for those symbols to arrive.
Yes, as we are clearing the fifo and fifo size is 64 words its always
highly possible that It will contain IRB_TIMEOUT for that interrupt.
Thanks,
srini
Thanks
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] media: st-rc: Add ST remote control driver
Thanks Mark for your comments.
On 15/08/13 09:49, Mark Rutland wrote:
On Wed, Aug 14, 2013 at 06:27:01PM +0100, Srinivas KANDAGATLA wrote:
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch adds support to ST RC driver, which is basically a IR/UHF
receiver and transmitter. This IP 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.
Is there anything that additionally needs to be in the dt to support Tx
functionality?
We need an additional boolean property for TX enable.
+
Two more configurable parameters for TX sub-carrier frequency and
duty-cycle. By default driver can set the sub carrier frequency to be
38Khz and duty cycle as 50%.
However I don't have strong use case to make these configurable.
So, I think just one boolean property to enable tx should be Ok.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
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.
Comments?
Thanks,
srini
Documentation/devicetree/bindings/media/st-rc.txt | 18 +
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/st_rc.c | 371
+
4 files changed, 400 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..57f9ee8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st-rc.txt
@@ -0,0 +1,18 @@
+Device-Tree bindings for ST IR and UHF receiver
+
+Required properties:
+ - compatible: should be st,rc.
That rc should be made more specific, and it seems like this is a
subset of a larger block of IP (the ST COMMS IP block). Is this really a
standalone piece of hardware, or is it always in the larger comms block?
COMMS block is a collection of communication peripherals, and IRB(Infra
red blaster) is always part of the COMMS block.
May renaming the compatible to st,comms-rc might be more clear.
What's the full name of this unit as it appears in documentation?
It is always refered as the Communication sub-system (COMMS) which is a
collection of communication peripherals like UART, I2C, SPI, IRB.
+ - st,uhfmode: boolean property to indicate if reception is in UHF.
That's not a very clear name. Is this a physical property of the device
(i.e. it's wired to either an IR receiver or a UHF receiver), or is this
a choice as to how it's used at runtime?
Its both, some boards have IR and others UHF receivers, So it becomes
board choice here.
And also the driver has different register set for UHF receiver and IR
receiver. This options selects which registers to use depending on mode
of reception.
If it's fixed property of how the device is wired, it might make more
sense to have a string mode property that's either uhf or infared.
Am ok with either approaches.
+ - 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.
+
+Example node:
+
+ rc: rc@fe518000 {
+ compatible = st,rc;
+ reg = 0xfe518000 0x234;
+ interrupts = 0 203 0;
+ };
+
[...]
+++ b/drivers/media/rc/st_rc.c
@@ -0,0 +1,371 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited
+ * Author: Srinivas Kandagatla srinivas.kandaga...@st.com
+ *
+ * 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 linux/kernel.h
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include media/rc-core.h
+#include linux/pinctrl/consumer.h
+
+struct st_rc_device {
+ struct device *dev;
+ int irq;
+ int irq_wake;
+ struct clk *sys_clock;
+ void*base; /* Register base address */
+ void*rx_base;/* RX Register base address
*/
+ struct rc_dev
Re: [PATCH] media: st-rc: Add ST remote control driver
On 15/08/13 14:30, Pawel Moll wrote:
On Wed, 2013-08-14 at 18:27 +0100, Srinivas KANDAGATLA wrote:
+Device-Tree bindings for ST IR and UHF receiver
+
+Required properties:
+ - compatible: should be st,rc.
+ - st,uhfmode: boolean property to indicate if reception is in UHF.
+ - 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.
+
+Example node:
+
+ rc: rc@fe518000 {
+ compatible = st,rc;
+ reg = 0xfe518000 0x234;
+ interrupts = 0 203 0;
+ };
So is st,uhfmode required or optional after all? If the former, the
example is wrong (doesn't specify required property). But as far as I
understand it's really optional...
You are right, I will move this to optional properties section.
Thanks,
srini
Paweł
--
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] media: st-rc: Add ST remote control driver
On 15/08/13 14:29, Mark Rutland wrote:
On Thu, Aug 15, 2013 at 01:57:13PM +0100, Srinivas KANDAGATLA wrote:
Thanks Mark for your comments.
On 15/08/13 09:49, Mark Rutland wrote:
On Wed, Aug 14, 2013 at 06:27:01PM +0100, Srinivas KANDAGATLA wrote:
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch adds support to ST RC driver, which is basically a IR/UHF
receiver and transmitter. This IP 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.
Is there anything that additionally needs to be in the dt to support Tx
functionality?
We need an additional boolean property for TX enable.
Why? Becuase you might not have something wired up for Tx?
Yes.
What needs to be present physically for Tx support?
An IR transmitter LEDs need to be physically connected.
Also driver need to know about this to setup the IP to do tx.
+
Two more configurable parameters for TX sub-carrier frequency and
duty-cycle. By default driver can set the sub carrier frequency to be
38Khz and duty cycle as 50%.
However I don't have strong use case to make these configurable.
So, I think just one boolean property to enable tx should be Ok.
+Device-Tree bindings for ST IR and UHF receiver
+
+Required properties:
+ - compatible: should be st,rc.
That rc should be made more specific, and it seems like this is a
subset of a larger block of IP (the ST COMMS IP block). Is this really a
standalone piece of hardware, or is it always in the larger comms block?
COMMS block is a collection of communication peripherals, and IRB(Infra
red blaster) is always part of the COMMS block.
May renaming the compatible to st,comms-rc might be more clear.
Given the actual name of the hardware seems to include irb, I think
irb makes more sense than rc in the compatible string. Is there no
more specific name than Infra Red Blaster?
There is'nt, its always referred as IRB.
I will rename the compatible to st,comms-irb does this sound Ok?
What's the full name of this unit as it appears in documentation?
It is always refered as the Communication sub-system (COMMS) which is a
collection of communication peripherals like UART, I2C, SPI, IRB.
Ok, are those separate IP blocks within a container, or is anything
shared? Does the COMMS block have any registers shared between those
units, for example?
No, registers are not shared across the IP blocks.
+ - st,uhfmode: boolean property to indicate if reception is in UHF.
That's not a very clear name. Is this a physical property of the device
(i.e. it's wired to either an IR receiver or a UHF receiver), or is this
a choice as to how it's used at runtime?
Its both, some boards have IR and others UHF receivers, So it becomes
board choice here.
When you say it's both, what do you mean? Is it possible for a unit to
be wired with both IR and UHF support simultaneously, even if the Linux
driver can't handle that?
I meant that this property is physical property of the device(board
wired up to either IR or UHF receiver) and also choice on how the IP is
configured.
The dt should encode information about the hardware, not the way you
intend to use the hardware at the present moment in time.
Ok.
And also the driver has different register set for UHF receiver and IR
receiver. This options selects which registers to use depending on mode
of reception.
Ok, but that's an implementation issue. If you described that IR *may*
be used, and UHF *may* be used, the driver can choose what to do based
on that.
If it's fixed property of how the device is wired, it might make more
sense to have a string mode property that's either uhf or infared.
Am ok with either approaches.
It sounds like we may need separate properties as mentioned above, or a
supported-modes list, perhaps.
I think having rx-mode and tx-mode properties makes more sense rather
than supported-modes.
like:
rx-mode = uhf;
or
rx-mode = infrared;
Similarly tx-mode.
+ - 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.
+
+Example node:
+
+ rc: rc@fe518000 {
+ compatible = st,rc;
+ reg = 0xfe518000 0x234;
+ interrupts = 0 203 0;
+ };
+
[...]
+++ b/drivers/media/rc/st_rc.c
@@ -0,0 +1,371 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited
+ * Author: Srinivas Kandagatla srinivas.kandaga...@st.com
+ *
+ * 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
[PATCH] media: st-rc: Add ST remote control driver
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch adds support to ST RC driver, which is basically a IR/UHF
receiver and transmitter. This IP 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 srinivas.kandaga...@st.com
---
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.
Comments?
Thanks,
srini
Documentation/devicetree/bindings/media/st-rc.txt | 18 +
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/st_rc.c | 371 +
4 files changed, 400 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..57f9ee8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st-rc.txt
@@ -0,0 +1,18 @@
+Device-Tree bindings for ST IR and UHF receiver
+
+Required properties:
+ - compatible: should be st,rc.
+ - st,uhfmode: boolean property to indicate if reception is in UHF.
+ - 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.
+
+Example node:
+
+ rc: rc@fe518000 {
+ compatible = st,rc;
+ reg = 0xfe518000 0x234;
+ interrupts = 0 203 0;
+ };
+
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 5a79c33..548a705 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -321,4 +321,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
+ help
+Say Y here if you want support for ST remote control driver
+which allows both IR and UHF RX. IR RX receiver is the default mode.
+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..712a2fb
--- /dev/null
+++ b/drivers/media/rc/st_rc.c
@@ -0,0 +1,371 @@
+/*
+ * Copyright (C) 2013 STMicroelectronics Limited
+ * Author: Srinivas Kandagatla srinivas.kandaga...@st.com
+ *
+ * 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 linux/kernel.h
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include media/rc-core.h
+#include linux/pinctrl/consumer.h
+
+struct st_rc_device {
+ struct device *dev;
+ int irq;
+ int irq_wake;
+ struct clk *sys_clock;
+ void*base; /* Register base address */
+ void*rx_base;/* RX Register base address */
+ struct rc_dev *rdev;
+ booloverclocking;
+ int sample_mult;
+ int sample_div;
+ boolrxuhfmode;
+};
+
+/* Registers */
+#define IRB_SAMPLE_RATE_COMM 0x64/* sample freq divisor*/
+#define IRB_CLOCK_SEL 0x70/* clock select */
+#define IRB_CLOCK_SEL_STATUS 0x74/* clock status */
+/* IRB IR/UHF receiver registers */
+#define IRB_RX_ON 0x40 /* pulse time capture */
+#define IRB_RX_SYS 0X44 /* sym period capture */
+#define IRB_RX_INT_EN 0x48 /* IRQ enable (R/W) */
+#define
[PATCH v2 2/2] media: lirc: Allow lirc dev to talk to rc device
From: Srinivas Kandagatla srinivas.kandaga...@st.com
The use case is simple, if any rc device has allowed protocols =
RC_TYPE_LIRC and map_name = RC_MAP_LIRC set, the driver open will be never
called. The reason for this is, all of the key maps except lirc have some
KEYS in there map, so during rc_register_device process these keys are
matched against the input drivers and open is performed, so for the case
of RC_MAP_EMPTY, a vt/keyboard is matched and the driver open is
performed.
In case of lirc, there is no match and result is that there is no open
performed, however the lirc-dev will go ahead and create a /dev/lirc0
node. Now when lircd/mode2 opens this device, no data is available
because the driver was never opened.
Other case pointed by Sean Young, As rc device gets opened via the
input interface. If the input device is never opened (e.g. embedded with
no console) then the rc open is never called and lirc will not work
either. So that's another case.
lirc_dev seems to have no link with actual rc device w.r.t open/close.
This patch adds rc_dev pointer to lirc_driver structure for cases like
this, so that it can do the open/close of the real driver in accordance
to lircd/mode2 open/close.
Without this patch its impossible to open a rc device which has
RC_TYPE_LIRC ad RC_MAP_LIRC set.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/ir-lirc-codec.c |1 +
drivers/media/rc/lirc_dev.c | 10 ++
include/media/lirc_dev.h |1 +
3 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index e456126..6858685 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -375,6 +375,7 @@ static int ir_lirc_register(struct rc_dev *dev)
drv-code_length = sizeof(struct ir_raw_event) * 8;
drv-fops = lirc_fops;
drv-dev = dev-dev;
+ drv-rdev = dev;
drv-owner = THIS_MODULE;
drv-minor = lirc_register_driver(drv);
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 8dc057b..dc5cbff 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -35,6 +35,7 @@
#include linux/device.h
#include linux/cdev.h
+#include media/rc-core.h
#include media/lirc.h
#include media/lirc_dev.h
@@ -467,6 +468,12 @@ int lirc_dev_fop_open(struct inode *inode, struct file
*file)
goto error;
}
+ if (ir-d.rdev) {
+ retval = rc_open(ir-d.rdev);
+ if (retval)
+ goto error;
+ }
+
cdev = ir-cdev;
if (try_module_get(cdev-owner)) {
ir-open++;
@@ -511,6 +518,9 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
WARN_ON(mutex_lock_killable(lirc_dev_lock));
+ if (ir-d.rdev)
+ rc_close(ir-d.rdev);
+
ir-open--;
if (ir-attached) {
ir-d.set_use_dec(ir-d.data);
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 168dd0b..78f0637 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -139,6 +139,7 @@ struct lirc_driver {
struct lirc_buffer *rbuf;
int (*set_use_inc) (void *data);
void (*set_use_dec) (void *data);
+ struct rc_dev *rdev;
const struct file_operations *fops;
struct device *dev;
struct module *owner;
--
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
[PATCH v2 1/2] media: rc: Add rc_open/close and use count to rc_dev.
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch adds user count to rc_dev structure, the reason to add this
new member is to allow other code like lirc to open rc device directly.
In the existing code, rc device is only opened by input subsystem which
works ok if we have any input drivers to match. But in case like lirc
where there will be no input driver, rc device will be never opened.
Having this user count variable will be usefull to allow rc device to be
opened from code other than rc-main.
This patch also adds rc_open and rc_close functions for other drivers
like lirc to open and close rc devices. This functions safely increment
and decrement the user count. Other driver wanting to open rc device
should call rc_open and rc_close, rather than directly modifying the
rc_dev structure.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/rc-main.c | 46 ---
include/media/rc-core.h|4 +++
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 1cf382a..1dedebd 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -699,19 +699,50 @@ void rc_keydown_notimeout(struct rc_dev *dev, int
scancode, u8 toggle)
}
EXPORT_SYMBOL_GPL(rc_keydown_notimeout);
+int rc_open(struct rc_dev *rdev)
+{
+ int rval = 0;
+
+ if (!rdev)
+ return -EINVAL;
+
+ mutex_lock(rdev-lock);
+ if (!rdev-users++)
+ rval = rdev-open(rdev);
+
+ if (rval)
+ rdev-users--;
+
+ mutex_unlock(rdev-lock);
+
+ return rval;
+}
+EXPORT_SYMBOL_GPL(rc_open);
+
static int ir_open(struct input_dev *idev)
{
struct rc_dev *rdev = input_get_drvdata(idev);
- return rdev-open(rdev);
+ return rc_open(rdev);
+}
+
+void rc_close(struct rc_dev *rdev)
+{
+ if (rdev) {
+ mutex_lock(rdev-lock);
+
+if (!--rdev-users)
+ rdev-close(rdev);
+
+ mutex_unlock(rdev-lock);
+ }
}
+EXPORT_SYMBOL_GPL(rc_close);
static void ir_close(struct input_dev *idev)
{
struct rc_dev *rdev = input_get_drvdata(idev);
-
-if (rdev)
- rdev-close(rdev);
+ rc_close(rdev);
}
/* class for /sys/class/rc */
@@ -1076,7 +1107,14 @@ int rc_register_device(struct rc_dev *dev)
memcpy(dev-input_dev-id, dev-input_id, sizeof(dev-input_id));
dev-input_dev-phys = dev-input_phys;
dev-input_dev-name = dev-input_name;
+
+ /* input_register_device can call ir_open, so unlock mutex here */
+ mutex_unlock(dev-lock);
+
rc = input_register_device(dev-input_dev);
+
+ mutex_lock(dev-lock);
+
if (rc)
goto out_table;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 06a75de..2f6f1f7 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -101,6 +101,7 @@ struct rc_dev {
boolidle;
u64 allowed_protos;
u64 enabled_protocols;
+ u32 users;
u32 scanmask;
void*priv;
spinlock_t keylock;
@@ -142,6 +143,9 @@ void rc_free_device(struct rc_dev *dev);
int rc_register_device(struct rc_dev *dev);
void rc_unregister_device(struct rc_dev *dev);
+int rc_open(struct rc_dev *rdev);
+void rc_close(struct rc_dev *rdev);
+
void rc_repeat(struct rc_dev *dev);
void rc_keydown(struct rc_dev *dev, int scancode, u8 toggle);
void rc_keydown_notimeout(struct rc_dev *dev, int scancode, u8 toggle);
--
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
[PATCH v2 0/2] Allow rc device to open from lirc
From: Srinivas Kandagatla srinivas.kandaga...@st.com Thankyou for providing comments on v1 patch. This patchset adds new members to rc_device structure to open rc device from code other than rc-main. In the current code rc-device is only opened via input driver. In cases where rc device is using lirc protocol, it will never be opened as there is no input driver associated with lirc. Thes patch set add fix to allow use cases like this to open rc device directly. Changes since v1: - used rcdev-lock around increments and decrements of users pointed by Sean Young. - Common up code in rc_open/rc_close. Thanks, srini Srinivas Kandagatla (2): media: rc: Add rc_open/close and use count to rc_dev. media: lirc: Allow lirc dev to talk to rc device drivers/media/rc/ir-lirc-codec.c |1 + drivers/media/rc/lirc_dev.c | 10 drivers/media/rc/rc-main.c | 46 ++--- include/media/lirc_dev.h |1 + include/media/rc-core.h |4 +++ 5 files changed, 58 insertions(+), 4 deletions(-) -- 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
[PATCH v1 1/2] media: rc: Add user count to rc dev.
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch adds user count to rc_dev structure, the reason to add this
new member is to allow other code like lirc to open rc device directly.
In the existing code, rc device is only opened by input subsystem which
works ok if we have any input drivers to match. But in case like lirc
where there will be no input driver, rc device will be never opened.
Having this user count variable will be useful to allow rc device to be
opened from code other than rc-main.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/rc-main.c | 11 +--
include/media/rc-core.h|1 +
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 1cf382a..e800b96 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -702,15 +702,22 @@ EXPORT_SYMBOL_GPL(rc_keydown_notimeout);
static int ir_open(struct input_dev *idev)
{
struct rc_dev *rdev = input_get_drvdata(idev);
+ int rval = 0;
- return rdev-open(rdev);
+ if (!rdev-users++)
+ rval = rdev-open(rdev);
+
+ if (rval)
+ rdev-users--;
+
+ return rval;
}
static void ir_close(struct input_dev *idev)
{
struct rc_dev *rdev = input_get_drvdata(idev);
-if (rdev)
+if (rdev !--rdev-users)
rdev-close(rdev);
}
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index 06a75de..b42016a 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -101,6 +101,7 @@ struct rc_dev {
boolidle;
u64 allowed_protos;
u64 enabled_protocols;
+ u32 users;
u32 scanmask;
void*priv;
spinlock_t keylock;
--
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
[PATCH v1 2/2] media: lirc: Allow lirc dev to talk to rc device
From: Srinivas Kandagatla srinivas.kandaga...@st.com
The use case is simple, if any rc device has allowed protocols =
RC_TYPE_LIRC and map_name = RC_MAP_LIRC set, the driver open will be never
called. The reason for this is, all of the key maps except lirc have some
KEYS in there map, so during rc_register_device process these keys are
matched against the input drivers and open is performed, so for the case
of RC_MAP_EMPTY, a vt/keyboard is matched and the driver open is
performed.
In case of lirc, there is no match and result is that there is no open
performed, however the lirc-dev will go ahead and create a /dev/lirc0
node. Now when lircd/mode2 opens this device, no data is available
because the driver was never opened.
Other case pointed by Sean Young, As rc device gets opened via the
input interface. If the input device is never opened (e.g. embedded with
no console) then the rc open is never called and lirc will not work
either. So that's another case.
lirc_dev seems to have no link with actual rc device w.r.t open/close.
This patch adds rc_dev pointer to lirc_driver structure for cases like
this, so that it can do the open/close of the real driver in accordance
to lircd/mode2 open/close.
Without this patch its impossible to open a rc device which has
RC_TYPE_LIRC ad RC_MAP_LIRC set.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/ir-lirc-codec.c |1 +
drivers/media/rc/lirc_dev.c | 16
include/media/lirc_dev.h |1 +
3 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index e456126..d5ad27f 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -375,6 +375,7 @@ static int ir_lirc_register(struct rc_dev *dev)
drv-code_length = sizeof(struct ir_raw_event) * 8;
drv-fops = lirc_fops;
drv-dev = dev-dev;
+ drv-rc_dev = dev;
drv-owner = THIS_MODULE;
drv-minor = lirc_register_driver(drv);
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 8dc057b..5f69fc0 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -35,6 +35,7 @@
#include linux/device.h
#include linux/cdev.h
+#include media/rc-core.h
#include media/lirc.h
#include media/lirc_dev.h
@@ -437,6 +438,7 @@ EXPORT_SYMBOL(lirc_unregister_driver);
int lirc_dev_fop_open(struct inode *inode, struct file *file)
{
struct irctl *ir;
+ struct rc_dev *rc_dev;
struct cdev *cdev;
int retval = 0;
@@ -467,6 +469,15 @@ int lirc_dev_fop_open(struct inode *inode, struct file
*file)
goto error;
}
+ rc_dev = ir-d.rc_dev;
+ if (rc_dev !rc_dev-users++ rc_dev-open) {
+ retval = rc_dev-open(rc_dev);
+ if (retval) {
+ rc_dev-users--;
+ goto error;
+ }
+ }
+
cdev = ir-cdev;
if (try_module_get(cdev-owner)) {
ir-open++;
@@ -499,6 +510,7 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
{
struct irctl *ir = irctls[iminor(inode)];
struct cdev *cdev;
+ struct rc_dev *rc_dev;
if (!ir) {
printk(KERN_ERR %s: called with invalid irctl\n, __func__);
@@ -511,6 +523,10 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
WARN_ON(mutex_lock_killable(lirc_dev_lock));
+ rc_dev = ir-d.rc_dev;
+ if (rc_dev !--rc_dev-users rc_dev-close)
+ rc_dev-close(rc_dev);
+
ir-open--;
if (ir-attached) {
ir-d.set_use_dec(ir-d.data);
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 168dd0b..96dccb6 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -139,6 +139,7 @@ struct lirc_driver {
struct lirc_buffer *rbuf;
int (*set_use_inc) (void *data);
void (*set_use_dec) (void *data);
+ struct rc_dev *rc_dev;
const struct file_operations *fops;
struct device *dev;
struct module *owner;
--
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
[PATCH v1 0/2] Allow rc device to open from lirc
From: Srinivas Kandagatla srinivas.kandaga...@st.com Thankyou for providing comments on RFC patch. This patchset adds new members to rc_device structure to open rc device from code other than rc-main. In the current code rc-device is only opened via input driver. In cases where rc device is using lirc protocol, it will never be opened as there is no input driver associated with lirc. Thes patch set add fix to allow use cases like this to open rc device directly. Thanks, srini Srinivas Kandagatla (2): media: rc: Add user count to rc dev. media: lirc: Allow lirc dev to talk to rc device drivers/media/rc/ir-lirc-codec.c |1 + drivers/media/rc/lirc_dev.c | 16 drivers/media/rc/rc-main.c | 11 +-- include/media/lirc_dev.h |1 + include/media/rc-core.h |1 + 5 files changed, 28 insertions(+), 2 deletions(-) -- 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 v1 1/2] media: rc: Add user count to rc dev.
On 19/07/13 12:01, Sean Young wrote: +int rval = 0; -return rdev-open(rdev); +if (!rdev-users++) +rval = rdev-open(rdev); + +if (rval) +rdev-users--; + +return rval; This looks racey. Some locking is needed, I think rc_dev-lock should work fine for this. Here and in the lirc code path too. thanks Sean, It makes sense, will fix this in v2. Srini 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 RFC] media: lirc: Allow lirc dev to talk to rc device
Thanks Sean for the comments,
On 12/07/13 13:46, Sean Young wrote:
On Fri, Jul 12, 2013 at 09:55:28AM +0100, Srinivas KANDAGATLA wrote:
From: Srinivas Kandagatla srinivas.kandaga...@st.com
The use case is simple, if any rc device has allowed protocols =
RC_TYPE_LIRC and map_name = RC_MAP_LIRC set, the driver open will be never
called. The reason for this is, all of the key maps except lirc have some
KEYS in there map, so during rc_register_device process these keys are
matched against the input drivers and open is performed, so for the case
of RC_MAP_EMPTY, a tty/vt/keyboard is matched and the driver open is
performed.
In case of lirc, there is no match and result is that there is no open
performed, however the lirc-dev will go ahead and create a /dev/lirc0
node. Now when lircd/mode2 opens this device, no data is available
because the driver was never opened.
The rc device gets opened via the input interface. If the input device is
never opened (e.g. embedded with no console) then the rc open is never
called and lirc will not work either. So that's another case.
Yes, this might be another case to add to the list.
diff --git a/drivers/media/rc/ir-lirc-codec.c
b/drivers/media/rc/ir-lirc-codec.c
index e456126..d5ad27f 100644
+rc_dev = ir-d.rc_dev;
+if (rc_dev rc_dev-open) {
+retval = rc_dev-open(rc_dev);
+if (retval)
+goto error;
+}
+
Now the rc device can have its open called twice; once via the input
system and then (while it is already opened) via lirc. The rc drivers do
not expect this.
I did think about this, and I thought managing this would be much easy
in the actual driver itself.
But given the generic nature, add this logic to rc infrastructure makes
much sense,
What I have done is added a users to rc_dev which will track howmany
users are actively using this device.
Here is the new patch, with the users count.
cdev = ir-cdev;
if (try_module_get(cdev-owner)) {
ir-open++;
@@ -499,6 +508,7 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
{
struct irctl *ir = irctls[iminor(inode)];
struct cdev *cdev;
+struct rc_dev *rc_dev;
if (!ir) {
printk(KERN_ERR %s: called with invalid irctl\n, __func__);
@@ -511,6 +521,10 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
WARN_ON(mutex_lock_killable(lirc_dev_lock));
+rc_dev = ir-d.rc_dev;
+if (rc_dev rc_dev-close)
+rc_dev-close(rc_dev);
+
iguanair, nuvoton and ene_ir will disable the ir receiver when their close
function is called. If the device was also opened via the input interface,
the input interface will receive no new ir activity.
I think there should be some sort of (atomic) use counter so that the
rc device open or close only gets called once, whether opened via the
input interface or via lirc.
new patch should address this using a users a use counter.
Thanks,
srini
From 06435a6ebd2374b500fbd0037e16a0451668f193 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla srinivas.kandaga...@st.com
Date: Fri, 12 Jul 2013 09:07:37 +0100
Subject: [PATCH RFC] media: lirc: Allow lirc dev to talk to rc device
The use case is simple, if any rc device has allowed protocols =
RC_TYPE_LIRC and map_name = RC_MAP_LIRC set, the driver open will be never
called. The reason for this is, all of the key maps except lirc have some
KEYS in there map, so during rc_register_device process these keys are
matched against the input drivers and open is performed, so for the case
of RC_MAP_EMPTY, a vt/keyboard is matched and the driver open is
performed.
In case of lirc, there is no match and result is that there is no open
performed, however the lirc-dev will go ahead and create a /dev/lirc0
node. Now when lircd/mode2 opens this device, no data is available
because the driver was never opened.
lirc_dev seems to have no link with actual rc device w.r.t open/close.
This patch adds rc_dev pointer to lirc_driver structure for cases like
this, so that it can do the open/close of the real driver in accordance
to lircd/mode2 open/close.
Without this patch its impossible to open a rc device which has
RC_TYPE_LIRC ad RC_MAP_LIRC set.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/ir-lirc-codec.c |1 +
drivers/media/rc/lirc_dev.c | 16
drivers/media/rc/rc-main.c | 11 +--
include/media/lirc_dev.h |1 +
include/media/rc-core.h |1 +
5 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index e456126..d5ad27f 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -375,6 +375,7 @@ static int ir_lirc_register(struct rc_dev *dev)
drv-code_length = sizeof(struct ir_raw_event) * 8;
drv-fops = lirc_fops;
drv-dev = dev-dev;
+ drv
[PATCH RFC] media: lirc: Allow lirc dev to talk to rc device
From: Srinivas Kandagatla srinivas.kandaga...@st.com
The use case is simple, if any rc device has allowed protocols =
RC_TYPE_LIRC and map_name = RC_MAP_LIRC set, the driver open will be never
called. The reason for this is, all of the key maps except lirc have some
KEYS in there map, so during rc_register_device process these keys are
matched against the input drivers and open is performed, so for the case
of RC_MAP_EMPTY, a tty/vt/keyboard is matched and the driver open is
performed.
In case of lirc, there is no match and result is that there is no open
performed, however the lirc-dev will go ahead and create a /dev/lirc0
node. Now when lircd/mode2 opens this device, no data is available
because the driver was never opened.
lirc_dev seems to have no link with actual rc device w.r.t open/close.
This patch adds the missing link rc_dev pointer to lirc_driver structure
for cases like this, so that it can do the open/close of the real driver
in accordance to lircd/mode2 open/close.
Without this patch its impossible to open a rc device which has
RC_TYPE_LIRC ad RC_MAP_LIRC set.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/ir-lirc-codec.c |1 +
drivers/media/rc/lirc_dev.c | 14 ++
include/media/lirc_dev.h |1 +
3 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index e456126..d5ad27f 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -375,6 +375,7 @@ static int ir_lirc_register(struct rc_dev *dev)
drv-code_length = sizeof(struct ir_raw_event) * 8;
drv-fops = lirc_fops;
drv-dev = dev-dev;
+ drv-rc_dev = dev;
drv-owner = THIS_MODULE;
drv-minor = lirc_register_driver(drv);
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 8dc057b..485bf7d 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -35,6 +35,7 @@
#include linux/device.h
#include linux/cdev.h
+#include media/rc-core.h
#include media/lirc.h
#include media/lirc_dev.h
@@ -437,6 +438,7 @@ EXPORT_SYMBOL(lirc_unregister_driver);
int lirc_dev_fop_open(struct inode *inode, struct file *file)
{
struct irctl *ir;
+ struct rc_dev *rc_dev;
struct cdev *cdev;
int retval = 0;
@@ -467,6 +469,13 @@ int lirc_dev_fop_open(struct inode *inode, struct file
*file)
goto error;
}
+ rc_dev = ir-d.rc_dev;
+ if (rc_dev rc_dev-open) {
+ retval = rc_dev-open(rc_dev);
+ if (retval)
+ goto error;
+ }
+
cdev = ir-cdev;
if (try_module_get(cdev-owner)) {
ir-open++;
@@ -499,6 +508,7 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
{
struct irctl *ir = irctls[iminor(inode)];
struct cdev *cdev;
+ struct rc_dev *rc_dev;
if (!ir) {
printk(KERN_ERR %s: called with invalid irctl\n, __func__);
@@ -511,6 +521,10 @@ int lirc_dev_fop_close(struct inode *inode, struct file
*file)
WARN_ON(mutex_lock_killable(lirc_dev_lock));
+ rc_dev = ir-d.rc_dev;
+ if (rc_dev rc_dev-close)
+ rc_dev-close(rc_dev);
+
ir-open--;
if (ir-attached) {
ir-d.set_use_dec(ir-d.data);
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 168dd0b..96dccb6 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -139,6 +139,7 @@ struct lirc_driver {
struct lirc_buffer *rbuf;
int (*set_use_inc) (void *data);
void (*set_use_dec) (void *data);
+ struct rc_dev *rc_dev;
const struct file_operations *fops;
struct device *dev;
struct module *owner;
--
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
[PATCH 3.6.0- 1/5] media/bfin: use module_platform_driver macro
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch removes some code duplication by using
module_platform_driver.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/platform/blackfin/bfin_capture.c | 14 +-
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/blackfin/bfin_capture.c
b/drivers/media/platform/blackfin/bfin_capture.c
index cb2eb26..ec476ef 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -1050,19 +1050,7 @@ static struct platform_driver bcap_driver = {
.probe = bcap_probe,
.remove = __devexit_p(bcap_remove),
};
-
-static __init int bcap_init(void)
-{
- return platform_driver_register(bcap_driver);
-}
-
-static __exit void bcap_exit(void)
-{
- platform_driver_unregister(bcap_driver);
-}
-
-module_init(bcap_init);
-module_exit(bcap_exit);
+module_platform_driver(bcap_driver);
MODULE_DESCRIPTION(Analog Devices blackfin video capture driver);
MODULE_AUTHOR(Scott Jiang scott.jiang.li...@gmail.com);
--
1.7.0.4
--
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
[PATCH 3.6.0- 2/5] media/m2m: use module_platform_driver macro
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch removes some code duplication by using
module_platform_driver.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/platform/m2m-deinterlace.c | 14 +-
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/m2m-deinterlace.c
b/drivers/media/platform/m2m-deinterlace.c
index 45164c4..fcdbb27 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -1108,17 +1108,5 @@ static struct platform_driver deinterlace_pdrv = {
.owner = THIS_MODULE,
},
};
-
-static void __exit deinterlace_exit(void)
-{
- platform_driver_unregister(deinterlace_pdrv);
-}
-
-static int __init deinterlace_init(void)
-{
- return platform_driver_register(deinterlace_pdrv);
-}
-
-module_init(deinterlace_init);
-module_exit(deinterlace_exit);
+module_platform_driver(deinterlace_pdrv);
--
1.7.0.4
--
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
[PATCH 3.6.0- 0/5] MEDIA: use module_platform_driver macro
From: Srinivas Kandagatla srinivas.kandaga...@st.com
Running below Coccinelle lookup pattern like below on the
latest kernel showed about 52 hits. This patch series is a subset
of those 52 patches, so that it will be easy for maintainers to review.
Hopefully these patches will get rid of some code duplication in kernel.
@ @
- initfunc(void)
- { return platform_driver_register(dr); }
...
- module_init(initfunc);
...
- exitfunc(void)
- { platform_driver_unregister(dr); }
...
- module_exit(exitfunc);
+ module_platform_driver(dr);
Srinivas Kandagatla (5):
media/bfin: use module_platform_driver macro
media/m2m: use module_platform_driver macro
media/mx2_emmaprp: use module_platform_driver macro
media/soc_camera: use module_platform_driver macro
media/ir_rx51: use module_platform_driver macro
drivers/media/platform/blackfin/bfin_capture.c | 14 +-
drivers/media/platform/m2m-deinterlace.c | 14 +-
drivers/media/platform/mx2_emmaprp.c | 14 +-
drivers/media/platform/soc_camera/soc_camera.c | 14 +-
drivers/media/rc/ir-rx51.c | 13 +
5 files changed, 5 insertions(+), 64 deletions(-)
--
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
[PATCH 3.6.0- 3/5] media/mx2_emmaprp: use module_platform_driver macro
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch removes some code duplication by using
module_platform_driver.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/platform/mx2_emmaprp.c | 14 +-
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/mx2_emmaprp.c
b/drivers/media/platform/mx2_emmaprp.c
index 8f22ce5..c45a9f5 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -1013,16 +1013,4 @@ static struct platform_driver emmaprp_pdrv = {
.owner = THIS_MODULE,
},
};
-
-static void __exit emmaprp_exit(void)
-{
- platform_driver_unregister(emmaprp_pdrv);
-}
-
-static int __init emmaprp_init(void)
-{
- return platform_driver_register(emmaprp_pdrv);
-}
-
-module_init(emmaprp_init);
-module_exit(emmaprp_exit);
+module_platform_driver(emmaprp_pdrv);
--
1.7.0.4
--
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
[PATCH 3.6.0- 4/5] media/soc_camera: use module_platform_driver macro
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch removes some code duplication by using
module_platform_driver.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/platform/soc_camera/soc_camera.c | 14 +-
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 3be9294..d4bfe29 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1585,19 +1585,7 @@ static struct platform_driver __refdata soc_camera_pdrv
= {
.owner = THIS_MODULE,
},
};
-
-static int __init soc_camera_init(void)
-{
- return platform_driver_register(soc_camera_pdrv);
-}
-
-static void __exit soc_camera_exit(void)
-{
- platform_driver_unregister(soc_camera_pdrv);
-}
-
-module_init(soc_camera_init);
-module_exit(soc_camera_exit);
+module_platform_driver(soc_camera_pdrv);
MODULE_DESCRIPTION(Image capture bus driver);
MODULE_AUTHOR(Guennadi Liakhovetski ker...@pengutronix.de);
--
1.7.0.4
--
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
[PATCH 3.6.0- 5/5] media/ir_rx51: use module_platform_driver macro
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch removes some code duplication by using
module_platform_driver.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
drivers/media/rc/ir-rx51.c | 13 +
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 546199e..8cfe316 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -480,18 +480,7 @@ struct platform_driver lirc_rx51_platform_driver = {
.owner = THIS_MODULE,
},
};
-
-static int __init lirc_rx51_init(void)
-{
- return platform_driver_register(lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
- platform_driver_unregister(lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
MODULE_DESCRIPTION(LIRC TX driver for Nokia RX51);
MODULE_AUTHOR(Nokia Corporation);
--
1.7.0.4
--
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
[PATCH 3.3.0] [media] ir-raw: Check available elements in kfifo before adding.
From: Srinivas Kandagatla srinivas.kandaga...@st.com This patch adds an additional availability check in ir_raw_event_store before adding an ir_raw_event into kfifo. The reason to do this is, Kfifo_alloc allocates fifo of size rounded down to 2. Which in this case makes sizeof ir_raw_event*MAX_IR_EVENT_SIZE = 6144 to 4096 bytes. Then again 4096 is not perfectly divisable by sizeof ir_raw_event(12). So before adding any element to kfifo checking howmany elements can be inserted into fifo is safe. This patch will make sure it inserts only sizeof(ev) into kfifo. Without this patch ir_raw_event_thread will trigger a bug. kernel BUG at drivers/media/rc/ir-raw.c:65! Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP Modules linked in: CPU: 0Not tainted (3.2.2_stm24_0208-b2000+ #31) PC is at ir_raw_event_thread+0xa4/0x10c LR is at ir_raw_event_thread+0xa4/0x10c pc : [c01e0ef4]lr : [c01e0ef4]psr: 6013 sp : df1d1f78 ip : df1d fp : 0004 r10: r9 : c041389c r8 : c0413848 r7 : df1d1f7c r6 : df1b6ecc r5 : df1b6ec0 r4 : df1d r3 : 000c r2 : df1d1f6c r1 : c0360798 r0 : 002f Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 10c53c7d Table: 5ece804a DAC: 0015 Process rc0 (pid: 577, stack limit = 0xdf1d02f0) This bug was identified as part of https://bugzilla.stlinux.com/show_bug.cgi?id=17387 triage. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com --- Hi All, Recently we have noticed a kernel BUG at drivers/media/rc/ir-raw.c:65! which shows up very frequently and sometimes even crashes our ARM based board. Digging a bit, found the reason for this is elements insertion into kfifo is not checking the available space in fifo before insertion. If ir_raw_event_thread is expecting kfifo out to give atleast sizeof(ev) bytes, This patch will make sure it inserts only sizeof(ev) into kfifo. This patch adds a check in ir_raw_event_store to fix this bug, because kfifo_alloc always rounds the size to power of 2. Thanks, srini drivers/media/rc/ir-raw.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c index 95e6309..863eee2 100644 --- a/drivers/media/rc/ir-raw.c +++ b/drivers/media/rc/ir-raw.c @@ -92,7 +92,9 @@ int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev) IR_dprintk(2, sample: (%05dus %s)\n, TO_US(ev-duration), TO_STR(ev-pulse)); - if (kfifo_in(dev-raw-kfifo, ev, sizeof(*ev)) != sizeof(*ev)) + if (kfifo_avail(dev-raw-kfifo) = sizeof(*ev)) + kfifo_in(dev-raw-kfifo, ev, sizeof(*ev)); + else return -ENOMEM; return 0; -- 1.6.3.3 -- 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
[PATCH 3.3.0] ir-raw: remove BUG_ON in ir_raw_event_thread.
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch removes BUG_ON in ir_raw_event_thread which IMO is a
over-kill, and this kills the ir_raw_event_thread too. With a bit of
additional logic in this patch, we nomore need to kill this thread.
Other disadvantage of having a BUG-ON is,
wake_up_process(dev-raw-thread) called on dead thread via
ir_raw_event_handle will result in total lockup in SMP system.
Advantage of this patch is ir-raw event thread is left in a usable state
even if the fifo does not have enough bytes.
This patch sets the thread into TASK_INTERRUPTIBLE if raw-fifo has less
then sizeof(struct ir_raw_event) bytes.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com
---
Hi All,
BUG-ON in ir_raw_event_thread on an SMP system will put the system in an
un-usable state, because..
BUG-ON actually kill the ir-raw-event kernel thread and my driver is
calling wake_up_process on a dead thread via ir_raw_event_handle from
interrupt context, which is why the system is left unusable.
However, my patch simplifies code in ir_raw_event_thread and remove BUG_ON
forever.
BUG_ON in this thread is a bit of over kill in my opinion, as the thread is
not is not in a very bad state, that it has to be killed.
Thanks,
srini
drivers/media/rc/ir-raw.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 95e6309..a820251 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -46,9 +46,9 @@ static int ir_raw_event_thread(void *data)
while (!kthread_should_stop()) {
spin_lock_irq(raw-lock);
- retval = kfifo_out(raw-kfifo, ev, sizeof(ev));
+ retval = kfifo_len(raw-kfifo);
- if (!retval) {
+ if (retval sizeof(ev)) {
set_current_state(TASK_INTERRUPTIBLE);
if (kthread_should_stop())
@@ -59,11 +59,9 @@ static int ir_raw_event_thread(void *data)
continue;
}
+ retval = kfifo_out(raw-kfifo, ev, sizeof(ev));
spin_unlock_irq(raw-lock);
-
- BUG_ON(retval != sizeof(ev));
-
mutex_lock(ir_raw_handler_lock);
list_for_each_entry(handler, ir_raw_handler_list, list)
handler-decode(raw-dev, ev);
--
1.6.3.3
--
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
