Re: [PATCH 03/20] [media] lirc.h: remove several unused ioctls

2016-07-12 Thread Mauro Carvalho Chehab 
Em Tue, 12 Jul 2016 14:54:38 +0100
Sean Young  escreveu:

> On Tue, Jul 12, 2016 at 02:35:56PM +0100, Sean Young wrote:
> > On Tue, Jul 12, 2016 at 10:23:00AM -0300, Mauro Carvalho Chehab wrote:  
> > > Em Tue, 12 Jul 2016 14:14:06 +0100
> > > Sean Young  escreveu:  
> > > > On Tue, Jul 12, 2016 at 09:41:57AM -0300, Mauro Carvalho Chehab wrote:  
> > -snip-  
> > > > > -#define LIRC_SET_REC_DUTY_CYCLE_IOW('i', 0x0016, __u32)  
> > > > >   
> > > > 
> > > > Also remove LIRC_CAN_SET_REC_DUTY_CYCLE and 
> > > > LIRC_CAN_SET_REC_DUTY_CYCLE_RANGE.  
> > > 
> > > Removing the "LIRC_CAN" macros can break userspace, as some app could
> > > be using it to print the LIRC features. That's why I opted to keep
> > > them, but to document that those features are unused - this is at
> > > the next patch (04/20).  
> > 
> > How is that different from removing the ioctls? Might as well go the whole
> > hog.  

If someone implemented LIRC_GET_FEATURES and handled all flags, such
program would break, as the API change.

> 
> Ah you meant that if someone later adds a new feature then we might reuse
> an existing bit. Oops, sorry.

Yes, this is another possibility. In such case, the ABI will also 
break, with is more severe than a pure API change.

Removing the ioctl declarations will still work for compiled programs,
as they'll still receive an error code when the ioctl is issued.

> 
> > Also note that LIRC_CAN_SET_REC_DUTY_CYCLE has the same value as
> > LIRC_CAN_MEASURE_CARRIER, so if some userspace program uses this it might
> > end up in the mistaken belief its supports LIRC_CAN_SET_REC_DUTY_CYCLE.  
> 
> So there is an argument for removing LIRC_CAN_SET_REC_DUTY_CYCLE, but
> that should be a separate patch.

Yes. Yet, IMHO, the best would be to put those unused LIRC_CAN into a:

#ifndef __KERNEL

macro block, to:

1) avoid the risk of breaking userspace;
2) be clear that those are deprecated stuff and should not be used on
   newer programs;
3) Reserve the bits for not be used, to avoid possible conflicts.

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 03/20] [media] lirc.h: remove several unused ioctls

2016-07-12 Thread Sean Young 
On Tue, Jul 12, 2016 at 02:35:56PM +0100, Sean Young wrote:
> On Tue, Jul 12, 2016 at 10:23:00AM -0300, Mauro Carvalho Chehab wrote:
> > Em Tue, 12 Jul 2016 14:14:06 +0100
> > Sean Young  escreveu:
> > > On Tue, Jul 12, 2016 at 09:41:57AM -0300, Mauro Carvalho Chehab wrote:
> -snip-
> > > > -#define LIRC_SET_REC_DUTY_CYCLE_IOW('i', 0x0016, __u32)  
> > > 
> > > Also remove LIRC_CAN_SET_REC_DUTY_CYCLE and 
> > > LIRC_CAN_SET_REC_DUTY_CYCLE_RANGE.
> > 
> > Removing the "LIRC_CAN" macros can break userspace, as some app could
> > be using it to print the LIRC features. That's why I opted to keep
> > them, but to document that those features are unused - this is at
> > the next patch (04/20).
> 
> How is that different from removing the ioctls? Might as well go the whole
> hog.

Ah you meant that if someone later adds a new feature then we might reuse
an existing bit. Oops, sorry.

> Also note that LIRC_CAN_SET_REC_DUTY_CYCLE has the same value as
> LIRC_CAN_MEASURE_CARRIER, so if some userspace program uses this it might
> end up in the mistaken belief its supports LIRC_CAN_SET_REC_DUTY_CYCLE.

So there is an argument for removing LIRC_CAN_SET_REC_DUTY_CYCLE, but
that should be a separate patch.


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 03/20] [media] lirc.h: remove several unused ioctls

2016-07-12 Thread Sean Young 
On Tue, Jul 12, 2016 at 10:23:00AM -0300, Mauro Carvalho Chehab wrote:
> Em Tue, 12 Jul 2016 14:14:06 +0100
> Sean Young  escreveu:
> > On Tue, Jul 12, 2016 at 09:41:57AM -0300, Mauro Carvalho Chehab wrote:
-snip-
> > > -#define LIRC_SET_REC_DUTY_CYCLE_IOW('i', 0x0016, __u32)  
> > 
> > Also remove LIRC_CAN_SET_REC_DUTY_CYCLE and 
> > LIRC_CAN_SET_REC_DUTY_CYCLE_RANGE.
> 
> Removing the "LIRC_CAN" macros can break userspace, as some app could
> be using it to print the LIRC features. That's why I opted to keep
> them, but to document that those features are unused - this is at
> the next patch (04/20).

How is that different from removing the ioctls? Might as well go the whole
hog.

Also note that LIRC_CAN_SET_REC_DUTY_CYCLE has the same value as
LIRC_CAN_MEASURE_CARRIER, so if some userspace program uses this it might
end up in the mistaken belief its supports LIRC_CAN_SET_REC_DUTY_CYCLE.


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 03/20] [media] lirc.h: remove several unused ioctls

2016-07-12 Thread Mauro Carvalho Chehab 
Em Tue, 12 Jul 2016 14:14:06 +0100
Sean Young  escreveu:

> On Tue, Jul 12, 2016 at 09:41:57AM -0300, Mauro Carvalho Chehab wrote:
> > While reviewing the documentation gaps on LIRC, it was
> > noticed that several ioctls aren't used by any LIRC drivers
> > (nor at staging or mainstream).
> > 
> > It doesn't make sense to document them, as they're not used
> > anywhere. So, let's remove those from the lirc header.  
> 
> Good to see these go.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  include/uapi/linux/lirc.h | 39 ++-
> >  1 file changed, 2 insertions(+), 37 deletions(-)
> > 
> > diff --git a/include/uapi/linux/lirc.h b/include/uapi/linux/lirc.h
> > index 4b3ab2966b5a..991ab4570b8e 100644
> > --- a/include/uapi/linux/lirc.h
> > +++ b/include/uapi/linux/lirc.h
> > @@ -90,20 +90,11 @@
> >  
> >  #define LIRC_GET_SEND_MODE _IOR('i', 0x0001, __u32)
> >  #define LIRC_GET_REC_MODE  _IOR('i', 0x0002, __u32)
> > -#define LIRC_GET_SEND_CARRIER  _IOR('i', 0x0003, __u32)
> > -#define LIRC_GET_REC_CARRIER   _IOR('i', 0x0004, __u32)
> > -#define LIRC_GET_SEND_DUTY_CYCLE   _IOR('i', 0x0005, __u32)
> > -#define LIRC_GET_REC_DUTY_CYCLE_IOR('i', 0x0006, __u32)
> >  #define LIRC_GET_REC_RESOLUTION_IOR('i', 0x0007, __u32)
> >  
> >  #define LIRC_GET_MIN_TIMEOUT   _IOR('i', 0x0008, __u32)
> >  #define LIRC_GET_MAX_TIMEOUT   _IOR('i', 0x0009, __u32)
> >  
> > -#define LIRC_GET_MIN_FILTER_PULSE  _IOR('i', 0x000a, __u32)
> > -#define LIRC_GET_MAX_FILTER_PULSE  _IOR('i', 0x000b, __u32)
> > -#define LIRC_GET_MIN_FILTER_SPACE  _IOR('i', 0x000c, __u32)
> > -#define LIRC_GET_MAX_FILTER_SPACE  _IOR('i', 0x000d, __u32)
> > -
> >  /* code length in bits, currently only for LIRC_MODE_LIRCCODE */
> >  #define LIRC_GET_LENGTH_IOR('i', 0x000f, __u32)
> >  
> > @@ -113,7 +104,6 @@
> >  #define LIRC_SET_SEND_CARRIER  _IOW('i', 0x0013, __u32)
> >  #define LIRC_SET_REC_CARRIER   _IOW('i', 0x0014, __u32)
> >  #define LIRC_SET_SEND_DUTY_CYCLE   _IOW('i', 0x0015, __u32)
> > -#define LIRC_SET_REC_DUTY_CYCLE_IOW('i', 0x0016, __u32)  
> 
> Also remove LIRC_CAN_SET_REC_DUTY_CYCLE and 
> LIRC_CAN_SET_REC_DUTY_CYCLE_RANGE.

Removing the "LIRC_CAN" macros can break userspace, as some app could
be using it to print the LIRC features. That's why I opted to keep
them, but to document that those features are unused - this is at
the next patch (04/20).

Thanks,
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 03/20] [media] lirc.h: remove several unused ioctls

2016-07-12 Thread Sean Young 
On Tue, Jul 12, 2016 at 09:41:57AM -0300, Mauro Carvalho Chehab wrote:
> While reviewing the documentation gaps on LIRC, it was
> noticed that several ioctls aren't used by any LIRC drivers
> (nor at staging or mainstream).
> 
> It doesn't make sense to document them, as they're not used
> anywhere. So, let's remove those from the lirc header.

Good to see these go.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  include/uapi/linux/lirc.h | 39 ++-
>  1 file changed, 2 insertions(+), 37 deletions(-)
> 
> diff --git a/include/uapi/linux/lirc.h b/include/uapi/linux/lirc.h
> index 4b3ab2966b5a..991ab4570b8e 100644
> --- a/include/uapi/linux/lirc.h
> +++ b/include/uapi/linux/lirc.h
> @@ -90,20 +90,11 @@
>  
>  #define LIRC_GET_SEND_MODE _IOR('i', 0x0001, __u32)
>  #define LIRC_GET_REC_MODE  _IOR('i', 0x0002, __u32)
> -#define LIRC_GET_SEND_CARRIER  _IOR('i', 0x0003, __u32)
> -#define LIRC_GET_REC_CARRIER   _IOR('i', 0x0004, __u32)
> -#define LIRC_GET_SEND_DUTY_CYCLE   _IOR('i', 0x0005, __u32)
> -#define LIRC_GET_REC_DUTY_CYCLE_IOR('i', 0x0006, __u32)
>  #define LIRC_GET_REC_RESOLUTION_IOR('i', 0x0007, __u32)
>  
>  #define LIRC_GET_MIN_TIMEOUT   _IOR('i', 0x0008, __u32)
>  #define LIRC_GET_MAX_TIMEOUT   _IOR('i', 0x0009, __u32)
>  
> -#define LIRC_GET_MIN_FILTER_PULSE  _IOR('i', 0x000a, __u32)
> -#define LIRC_GET_MAX_FILTER_PULSE  _IOR('i', 0x000b, __u32)
> -#define LIRC_GET_MIN_FILTER_SPACE  _IOR('i', 0x000c, __u32)
> -#define LIRC_GET_MAX_FILTER_SPACE  _IOR('i', 0x000d, __u32)
> -
>  /* code length in bits, currently only for LIRC_MODE_LIRCCODE */
>  #define LIRC_GET_LENGTH_IOR('i', 0x000f, __u32)
>  
> @@ -113,7 +104,6 @@
>  #define LIRC_SET_SEND_CARRIER  _IOW('i', 0x0013, __u32)
>  #define LIRC_SET_REC_CARRIER   _IOW('i', 0x0014, __u32)
>  #define LIRC_SET_SEND_DUTY_CYCLE   _IOW('i', 0x0015, __u32)
> -#define LIRC_SET_REC_DUTY_CYCLE_IOW('i', 0x0016, __u32)

Also remove LIRC_CAN_SET_REC_DUTY_CYCLE and 
LIRC_CAN_SET_REC_DUTY_CYCLE_RANGE.

>  #define LIRC_SET_TRANSMITTER_MASK  _IOW('i', 0x0017, __u32)
>  
>  /*
> @@ -127,42 +117,17 @@
>  #define LIRC_SET_REC_TIMEOUT_REPORTS   _IOW('i', 0x0019, __u32)
>  
>  /*
> - * pulses shorter than this are filtered out by hardware (software
> - * emulation in lirc_dev?)
> - */
> -#define LIRC_SET_REC_FILTER_PULSE  _IOW('i', 0x001a, __u32)
> -/*
> - * spaces shorter than this are filtered out by hardware (software
> - * emulation in lirc_dev?)
> - */
> -#define LIRC_SET_REC_FILTER_SPACE  _IOW('i', 0x001b, __u32)
> -/*
> - * if filter cannot be set independently for pulse/space, this should
> - * be used
> - */
> -#define LIRC_SET_REC_FILTER_IOW('i', 0x001c, __u32)

Also remove LIRC_CAN_SET_REC_FILTER.
> -
> -/*
>   * if enabled from the next key press on the driver will send
>   * LIRC_MODE2_FREQUENCY packets
>   */
>  #define LIRC_SET_MEASURE_CARRIER_MODE_IOW('i', 0x001d, __u32)
>  
>  /*
> - * to set a range use
> - * LIRC_SET_REC_DUTY_CYCLE_RANGE/LIRC_SET_REC_CARRIER_RANGE with the
> - * lower bound first and later
> - * LIRC_SET_REC_DUTY_CYCLE/LIRC_SET_REC_CARRIER with the upper bound
> + * to set a range use LIRC_SET_REC_CARRIER_RANGE with the
> + * lower bound first and later LIRC_SET_REC_CARRIER with the upper bound
>   */
> -
> -#define LIRC_SET_REC_DUTY_CYCLE_RANGE  _IOW('i', 0x001e, __u32)
>  #define LIRC_SET_REC_CARRIER_RANGE _IOW('i', 0x001f, __u32)
>  
> -#define LIRC_NOTIFY_DECODE _IO('i', 0x0020)

Also remove LIRC_CAN_NOTIFY_DECODE.

> -
> -#define LIRC_SETUP_START   _IO('i', 0x0021)
> -#define LIRC_SETUP_END _IO('i', 0x0022)
> -
>  #define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x0023, __u32)
>  
>  #endif

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

[PATCH 03/20] [media] lirc.h: remove several unused ioctls

2016-07-12 Thread Mauro Carvalho Chehab 
While reviewing the documentation gaps on LIRC, it was
noticed that several ioctls aren't used by any LIRC drivers
(nor at staging or mainstream).

It doesn't make sense to document them, as they're not used
anywhere. So, let's remove those from the lirc header.

Signed-off-by: Mauro Carvalho Chehab 
---
 include/uapi/linux/lirc.h | 39 ++-
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/include/uapi/linux/lirc.h b/include/uapi/linux/lirc.h
index 4b3ab2966b5a..991ab4570b8e 100644
--- a/include/uapi/linux/lirc.h
+++ b/include/uapi/linux/lirc.h
@@ -90,20 +90,11 @@
 
 #define LIRC_GET_SEND_MODE _IOR('i', 0x0001, __u32)
 #define LIRC_GET_REC_MODE  _IOR('i', 0x0002, __u32)
-#define LIRC_GET_SEND_CARRIER  _IOR('i', 0x0003, __u32)
-#define LIRC_GET_REC_CARRIER   _IOR('i', 0x0004, __u32)
-#define LIRC_GET_SEND_DUTY_CYCLE   _IOR('i', 0x0005, __u32)
-#define LIRC_GET_REC_DUTY_CYCLE_IOR('i', 0x0006, __u32)
 #define LIRC_GET_REC_RESOLUTION_IOR('i', 0x0007, __u32)
 
 #define LIRC_GET_MIN_TIMEOUT   _IOR('i', 0x0008, __u32)
 #define LIRC_GET_MAX_TIMEOUT   _IOR('i', 0x0009, __u32)
 
-#define LIRC_GET_MIN_FILTER_PULSE  _IOR('i', 0x000a, __u32)
-#define LIRC_GET_MAX_FILTER_PULSE  _IOR('i', 0x000b, __u32)
-#define LIRC_GET_MIN_FILTER_SPACE  _IOR('i', 0x000c, __u32)
-#define LIRC_GET_MAX_FILTER_SPACE  _IOR('i', 0x000d, __u32)
-
 /* code length in bits, currently only for LIRC_MODE_LIRCCODE */
 #define LIRC_GET_LENGTH_IOR('i', 0x000f, __u32)
 
@@ -113,7 +104,6 @@
 #define LIRC_SET_SEND_CARRIER  _IOW('i', 0x0013, __u32)
 #define LIRC_SET_REC_CARRIER   _IOW('i', 0x0014, __u32)
 #define LIRC_SET_SEND_DUTY_CYCLE   _IOW('i', 0x0015, __u32)
-#define LIRC_SET_REC_DUTY_CYCLE_IOW('i', 0x0016, __u32)
 #define LIRC_SET_TRANSMITTER_MASK  _IOW('i', 0x0017, __u32)
 
 /*
@@ -127,42 +117,17 @@
 #define LIRC_SET_REC_TIMEOUT_REPORTS   _IOW('i', 0x0019, __u32)
 
 /*
- * pulses shorter than this are filtered out by hardware (software
- * emulation in lirc_dev?)
- */
-#define LIRC_SET_REC_FILTER_PULSE  _IOW('i', 0x001a, __u32)
-/*
- * spaces shorter than this are filtered out by hardware (software
- * emulation in lirc_dev?)
- */
-#define LIRC_SET_REC_FILTER_SPACE  _IOW('i', 0x001b, __u32)
-/*
- * if filter cannot be set independently for pulse/space, this should
- * be used
- */
-#define LIRC_SET_REC_FILTER_IOW('i', 0x001c, __u32)
-
-/*
  * if enabled from the next key press on the driver will send
  * LIRC_MODE2_FREQUENCY packets
  */
 #define LIRC_SET_MEASURE_CARRIER_MODE  _IOW('i', 0x001d, __u32)
 
 /*
- * to set a range use
- * LIRC_SET_REC_DUTY_CYCLE_RANGE/LIRC_SET_REC_CARRIER_RANGE with the
- * lower bound first and later
- * LIRC_SET_REC_DUTY_CYCLE/LIRC_SET_REC_CARRIER with the upper bound
+ * to set a range use LIRC_SET_REC_CARRIER_RANGE with the
+ * lower bound first and later LIRC_SET_REC_CARRIER with the upper bound
  */
-
-#define LIRC_SET_REC_DUTY_CYCLE_RANGE  _IOW('i', 0x001e, __u32)
 #define LIRC_SET_REC_CARRIER_RANGE _IOW('i', 0x001f, __u32)
 
-#define LIRC_NOTIFY_DECODE _IO('i', 0x0020)
-
-#define LIRC_SETUP_START   _IO('i', 0x0021)
-#define LIRC_SETUP_END _IO('i', 0x0022)
-
 #define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x0023, __u32)
 
 #endif
-- 
2.7.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