Re: lirc_dev: generic implementation of LIRC_{GET|SET}_SEND_MODE V2

[Maxim L. <max...@gm...>]

On Sun, 2010-05-02 at 09:30 +0200, Christoph Bartelmus wrote: 
> Hi!
> 
> Maxim Levitsky "max...@gm..." wrote:
> 
> > How about this:
> [...]
> 
> 
> I'd leave this in:
First I didn't understand you, but now I get it,
you want lirc to see if kernel driver is supported.
I agree with that.


> 
> > -static unsigned long supported_send_modes[]=
> > -{
> > -	/* LIRC_CAN_SEND_LIRCCODE, */
> > -	/* LIRC_CAN_SEND_MODE2, this one would be very easy */
> > -	LIRC_CAN_SEND_PULSE,
> > -	/* LIRC_CAN_SEND_RAW, */
> > -	0
> > -};
> > -static unsigned long supported_rec_modes[]=
> > -{
> > -	LIRC_CAN_REC_LIRCCODE,
> > -	LIRC_CAN_REC_MODE2,
> > -	/* LIRC_CAN_REC_PULSE, shouldn't be too hard */
> > -	/* LIRC_CAN_REC_RAW, */
> > -	0
> > -};
> > -
> >  struct hardware hw_default=
> >  {
> >  	LIRC_DRIVER_DEVICE, /* default device */
> > @@ -286,64 +269,9 @@ int default_init()
> >  	}
> >  #       endif
> >
> > -	/* set send/receive method */
> > -	hw.send_mode=0;
> > -	if(LIRC_CAN_SEND(hw.features))
> > -	{
> > -		for(i=0;supported_send_modes[i]!=0;i++)
> > -		{
> > -			if(hw.features&supported_send_modes[i])
> > -			{
> > -				unsigned long mode;
> > -
> > -				mode=LIRC_SEND2MODE(supported_send_modes[i]);
> 
> And just remove this part:
> 
> > -				if(default_ioctl(LIRC_SET_SEND_MODE, &mode)==-1)
> > -				{
> > -					logprintf(LOG_ERR,"could not set "
> > -						  "send mode");
> > -					logperror(LOG_ERR,"default_init()");
> > -					default_deinit();
> > -					return(0);
> > -				}
> 
> until here.
> 
> 
> > -				hw.send_mode=LIRC_SEND2MODE
> > -				(supported_send_modes[i]);
> > -				break;
> > -			}
> > -		}
> > -		if(supported_send_modes[i]==0)
> > -		{
> > -			logprintf(LOG_NOTICE,"the send method of the "
> > -				  "driver is not yet supported by lircd");
> > -		}
> > -	}
> > -	hw.rec_mode=0;
> > -	if(LIRC_CAN_REC(hw.features))
> > -	{
> > -		for(i=0;supported_rec_modes[i]!=0;i++)
> > -		{
> > -			if(hw.features&supported_rec_modes[i])
> > -			{
> > -				unsigned long mode;
> > +	hw.send_mode=LIRC_SEND2MODE(hw.features);
> > +	hw.rec_mode=LIRC_REC2MODE(hw.features);
> >
> > -				mode=LIRC_REC2MODE(supported_rec_modes[i]);
> 
> Remove:
> 
> > -				if(default_ioctl(LIRC_SET_REC_MODE, &mode)==-1)
> > -				{
> > -					logprintf(LOG_ERR,"could not set "
> > -						  "receive mode");
> > -					logperror(LOG_ERR,"default_init()");
> > -					return(0);
> > -				}
> 
> Keep:
> 
> 
> > -				hw.rec_mode=LIRC_REC2MODE
> > -				(supported_rec_modes[i]);
> > -				break;
> > -			}
> > -		}
> > -		if(supported_rec_modes[i]==0)
> > -		{
> > -			logprintf(LOG_NOTICE,"the receive method of the "
> > -				  "driver is not yet supported by lircd");
> > -		}
> > -	}
> >  	if(hw.rec_mode==LIRC_MODE_MODE2)
> >  	{
> >  		/* get resolution */
> > diff --git a/drivers/lirc_dev/lirc_dev.c b/drivers/lirc_dev/lirc_dev.c
> > index 2a54382..8c5a948 100644
> > -+- a/drivers/lirc_dev/lirc_dev.c
> > +++ b/drivers/lirc_dev/lirc_dev.c
> > @@ -695,6 +695,25 @@ static int irctl_ioctl(struct inode *inode, struct file
> > *file,  	case LIRC_GET_FEATURES:
> >  		result = put_user(ir->d.features, (unsigned long *)arg);
> >  		break;
> > +	case LIRC_GET_LENGTH:
> > +		result = put_user(ir->d.code_length, (unsigned long *) arg);
> > +		break;
> > +	case LIRC_GET_MIN_TIMEOUT:
> > +		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
> > +		    ir->d.min_timeout == 0)
> > +			return -ENOSYS;
> > +
> > +		result = put_user(ir->d.min_timeout, (lirc_t *) arg);
> > +		break;
> > +	case LIRC_GET_MAX_TIMEOUT:
> > +		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
> > +		    ir->d.max_timeout == 0)
> > +			return -ENOSYS;
> > +
> > +		result = put_user(ir->d.max_timeout, (lirc_t *) arg);
> > +		break;
> > +
> > +	/*obsolete */
> >  	case LIRC_GET_REC_MODE:
> >  		if (!(ir->d.features & LIRC_CAN_REC_MASK))
> >  			return -ENOSYS;
> > @@ -710,27 +729,23 @@ static int irctl_ioctl(struct inode *inode, struct
> > file *file,  		result = get_user(mode, (unsigned long *)arg);
> >  		if (!result && !(LIRC_MODE2REC(mode) & ir->d.features))
> >  			result = -EINVAL;
> 
> I'd add a printk here that this ioctl is obsolete and also mark it  
> obsolete in lirc.h.
> 
> > -		/*
> > -		 * FIXME: We should actually set the mode somehow but
> > -		 * for now, lirc_serial doesn't support mode changing either
> > -		 */
> >  		break;
> > -	case LIRC_GET_LENGTH:
> > -		result = put_user(ir->d.code_length, (unsigned long *) arg);
> > -		break;
> > -	case LIRC_GET_MIN_TIMEOUT:
> > -		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
> > -		    ir->d.min_timeout == 0)
> > +	case LIRC_GET_SEND_MODE:
> > +		if (!(ir->d.features & LIRC_CAN_SEND_MASK))
> >  			return -ENOSYS;
> > -
> > -		result = put_user(ir->d.min_timeout, (lirc_t *) arg);
> > +
> > +		result = put_user(LIRC_SEND2MODE
> > +				  (ir->d.features & LIRC_CAN_SEND_MASK),
> > +				  (unsigned long *)arg);
> >  		break;
> > -	case LIRC_GET_MAX_TIMEOUT:
> > -		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
> > -		    ir->d.max_timeout == 0)
> > +	case LIRC_SET_SEND_MODE:
> 
> Also mark this obsolete.
I think I put all obsolete ioctls to the end, after the line 'obsolete'
Which ones I missed?


Best regards,
Maxim Levitsky