Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Sean,

> >  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -	dev = rc_allocate_device();
> > +	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
> >  	if (!ir || !dev)
> >  		goto err_out_free;
> >  
> 
> If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE.
> 
> 
> > @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
> >  	dev->scancode_mask = hardware_mask;
> >  
> >  	if (ir->sampling) {
> > -		dev->driver_type = RC_DRIVER_IR_RAW;
> >  		dev->timeout = 10 * 1000 * 1000; /* 10 ms */
> >  	} else {
> >  		dev->driver_type = RC_DRIVER_SCANCODE;
> 
> That assignment shouldn't really be there any more.

I think this doesn't change the driver's behavior, because I
either do like:

  -   dev = rc_allocate_device();
  +   dev = rc_allocate_device(RC_DRIVER_SCANCODE);

  [ ... ]

      if (ir->sampling) {
              dev->driver_type = RC_DRIVER_IR_RAW;
              dev->timeout = 10 * 1000 * 1000; /* 10 ms */
      } else {
   -          dev->driver_type = RC_DRIVER_SCANCODE;

Or I would need to do aftr the long switch...case statement

    +  if (ir->sampling) {
    +          dev = rc_allocate_device(RC_DRIVER_IR_RAW);
    +          ...
    +  } else {
    +          dev = rc_allocate_device(RC_DRIVER_SCANCODE);
    +          ...

I prefered the first way because it doesn't alter much the
driver.

> >  	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -	rc = rc_allocate_device();
> > +	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> >  	if (!ir || !rc) {
> >  		err = -ENOMEM;
> >  		goto err_out_free;
> 
> This is not correct, I'm afraid. If you look at the code you can see that
> if raw_decode is true, then it should be RC_DRIVER_IR_RAW.

Same here, the driver doesn't change the behavior. raw_decode can
be both 'true' or 'false' it's set as default RC_DRIVER_SCANCODE
and depending on value of raw_decode it's chaged to
RC_DRIVER_IR_RAW.

also in this case I can do

    +  if (raw_decode) {
    +          rc = rc_allocate_device(RC_DRIVER_IR_RAW);
    +          ...
    +  } else {
    +          rc = rc_allocate_device(RC_DRIVER_SCANCODE);
    +          ...

but also in this case my original approach doesn't add much
changes.

Thanks,
Andi
--
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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux