Re: [RFC 5/7] [media] ir-lirc-codec: do not handle any buffer for raw transmitters

[Sean Young]

Hi Andi,

On Thu, Jul 21, 2016 at 09:48:12AM +0900, Andi Shyti wrote:
> > > Raw transmitters receive the data which need to be sent to
> > > receivers from userspace as stream of bits, they don't require
> > > any handling from the lirc framework.
> > 
> > No drivers of type RC_DRIVER_IR_RAW_TX should handle tx just like any
> > other device, so data should be provided as an array of u32 alternating
> > pulse-space. If your device does not handle input like that then convert
> > it into that format in the driver. Every other driver has to do some
> > sort of conversion of that kind.
> 
> I don't see anything wrong here, that's how it works for example
> in Tizen or in Android for the boards I'm on: userspace sends a
> stream of bits that are then submitted to the IR as they are.

This introduces a new, incompatible api with no way of detecting it.

It's not a good format. For example the leading pulse (9ms) for nec ir
with a carrier of 38000 will be 342 bits. With the pulse-space format
it will be 32 bits.

Doing the conversion in kernel space will be cheap.

> If I change it to only pulse-space domain, then I wouldn't
> provide support for those platforms. Eventually I can add a new
> protocol.

But this is forcing an new, incompatible api onto the rest of us. 

This is the code in tizen:

https://build.tizen.org/package/rdiff?linkrev=base=device-manager-plugin-exynos5433=Tizen%3AIVI=2

If this patch was merged as-is tizen would have to be changed anyway
to use different ioctls. If that is true, can it switch to use 
pulse-space format in the same change? If LIRC_GET_FREQUENCY fails then
it would be a main-line kernel, else the existent driver.

I could not find the code in android. It might be useful to see so we
can find a solution that works for everyone.


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: [RFC 5/7] [media] ir-lirc-codec: do not handle any buffer for raw transmitters

[Andi Shyti]

Hi Sean,

> > Raw transmitters receive the data which need to be sent to
> > receivers from userspace as stream of bits, they don't require
> > any handling from the lirc framework.
> 
> No drivers of type RC_DRIVER_IR_RAW_TX should handle tx just like any
> other device, so data should be provided as an array of u32 alternating
> pulse-space. If your device does not handle input like that then convert
> it into that format in the driver. Every other driver has to do some
> sort of conversion of that kind.

I don't see anything wrong here, that's how it works for example
in Tizen or in Android for the boards I'm on: userspace sends a
stream of bits that are then submitted to the IR as they are.

If I change it to only pulse-space domain, then I wouldn't
provide support for those platforms. Eventually I can add a new
protocol.

Andi
--
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: [RFC 5/7] [media] ir-lirc-codec: do not handle any buffer for raw transmitters

[Sean Young]

On Wed, Jul 20, 2016 at 12:56:56AM +0900, Andi Shyti wrote:
> Raw transmitters receive the data which need to be sent to
> receivers from userspace as stream of bits, they don't require
> any handling from the lirc framework.

No drivers of type RC_DRIVER_IR_RAW_TX should handle tx just like any
other device, so data should be provided as an array of u32 alternating
pulse-space. If your device does not handle input like that then convert
it into that format in the driver. Every other driver has to do some
sort of conversion of that kind.

Thanks

Sean


> 
> Signed-off-by: Andi Shyti 
> ---
>  drivers/media/rc/ir-lirc-codec.c | 30 +++---
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c 
> b/drivers/media/rc/ir-lirc-codec.c
> index 5effc65..80e94b6 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -121,17 +121,6 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, 
> const char __user *buf,
>   if (!lirc)
>   return -EFAULT;
>  
> - if (n < sizeof(unsigned) || n % sizeof(unsigned))
> - return -EINVAL;
> -
> - count = n / sizeof(unsigned);
> - if (count > LIRCBUF_SIZE || count % 2 == 0)
> - return -EINVAL;
> -
> - txbuf = memdup_user(buf, n);
> - if (IS_ERR(txbuf))
> - return PTR_ERR(txbuf);
> -
>   dev = lirc->dev;
>   if (!dev) {
>   ret = -EFAULT;
> @@ -143,6 +132,25 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, 
> const char __user *buf,
>   goto out;
>   }
>  
> + if (dev->driver_type == RC_DRIVER_IR_RAW_TX) {
> + txbuf = memdup_user(buf, n);
> + if (IS_ERR(txbuf))
> + return PTR_ERR(txbuf);
> +
> + return dev->tx_ir(dev, txbuf, n);
> + }
> +
> + if (n < sizeof(unsigned) || n % sizeof(unsigned))
> + return -EINVAL;
> +
> + count = n / sizeof(unsigned);
> + if (count > LIRCBUF_SIZE || count % 2 == 0)
> + return -EINVAL;
> +
> + txbuf = memdup_user(buf, n);
> + if (IS_ERR(txbuf))
> + return PTR_ERR(txbuf);
> +
>   for (i = 0; i < count; i++) {
>   if (txbuf[i] > IR_MAX_DURATION / 1000 - duration || !txbuf[i]) {
>   ret = -EINVAL;
> -- 
> 2.8.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
--
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

[RFC 5/7] [media] ir-lirc-codec: do not handle any buffer for raw transmitters

[Andi Shyti]

Raw transmitters receive the data which need to be sent to
receivers from userspace as stream of bits, they don't require
any handling from the lirc framework.

Signed-off-by: Andi Shyti 
---
 drivers/media/rc/ir-lirc-codec.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 5effc65..80e94b6 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -121,17 +121,6 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, 
const char __user *buf,
if (!lirc)
return -EFAULT;
 
-   if (n < sizeof(unsigned) || n % sizeof(unsigned))
-   return -EINVAL;
-
-   count = n / sizeof(unsigned);
-   if (count > LIRCBUF_SIZE || count % 2 == 0)
-   return -EINVAL;
-
-   txbuf = memdup_user(buf, n);
-   if (IS_ERR(txbuf))
-   return PTR_ERR(txbuf);
-
dev = lirc->dev;
if (!dev) {
ret = -EFAULT;
@@ -143,6 +132,25 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, 
const char __user *buf,
goto out;
}
 
+   if (dev->driver_type == RC_DRIVER_IR_RAW_TX) {
+   txbuf = memdup_user(buf, n);
+   if (IS_ERR(txbuf))
+   return PTR_ERR(txbuf);
+
+   return dev->tx_ir(dev, txbuf, n);
+   }
+
+   if (n < sizeof(unsigned) || n % sizeof(unsigned))
+   return -EINVAL;
+
+   count = n / sizeof(unsigned);
+   if (count > LIRCBUF_SIZE || count % 2 == 0)
+   return -EINVAL;
+
+   txbuf = memdup_user(buf, n);
+   if (IS_ERR(txbuf))
+   return PTR_ERR(txbuf);
+
for (i = 0; i < count; i++) {
if (txbuf[i] > IR_MAX_DURATION / 1000 - duration || !txbuf[i]) {
ret = -EINVAL;
-- 
2.8.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