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 0000000..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);
>> + if (IS_ERR(rc_dev->base))
>> + goto clkerr;
>> +
>> + 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_TYPE_LIRC;
>
> allowed_protos is a bit field, so you need one of the RC_BIT_ types;
> you should use RC_BIT_ALL so that in-kernel IR decoders can be used
> for decoding if desired.
Ok, I miss interpreted the TYPE .. I will fix this in v2.
>
>> + 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";
>
> Please also set the timeout and rx_resolution according to the hardware.
Ok, I will add this info as well.
Thanks,
srini
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
