| Message ID | Pine.LNX.4.58.0901082146470.1626@shell2.speakeasy.net (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Well that simplifies the ifdef and allows text editors to still see the braces as being balanced, at the expense of uglier code for all current kernel versions. On the other hand this clearly documents the backwards compatible change for all those kernels and won't affect anything going forward. I don't have a strong opinion about either. If I were to choose however, I'd probably pick Trent's version. -Mike On Thu, 8 Jan 2009, Trent Piepho wrote: > On Thu, 8 Jan 2009, Mike Isely wrote: > > > Yes... Anyway, this is the real patch. I've added a small comment about this > > > change... I'll commit this tomorrow, if you don't have a better suggestion. > > > > Looks good. > > Or maybe like this? > > diff -r f01b3897d141 linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c > --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 00:27:32 2009 -0200 > +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 02:45:48 2009 -0200 > @@ -3747,7 +3747,12 @@ > int ret; > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > - if (ret == 1) { > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 29) > + /* Due to the API changes, the ret value for success changed */ > + ret = ret != 1; > +#endif > + if (ret == 0) { > ret = usb_reset_device(hdw->usb_dev); > usb_unlock_device(hdw->usb_dev); > } else { > > > > diff -r f01b3897d141 linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c > > > --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 00:27:32 2009 -0200 > > > +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 02:45:48 2009 -0200 > > > @@ -3747,7 +3747,12 @@ > > > int ret; > > > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > > > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,29) > > > + if (ret == 0) { > > > +#else > > > + /* Due to the API changes, the ret value for success changed */ > > > if (ret == 1) { > > > +#endif > > > ret = usb_reset_device(hdw->usb_dev); > > > usb_unlock_device(hdw->usb_dev); > > > } else { >
On Thu, 8 Jan 2009 21:56:15 -0800 (PST) Trent Piepho <xyzzy@speakeasy.org> wrote: > On Thu, 8 Jan 2009, Mike Isely wrote: > > > Yes... Anyway, this is the real patch. I've added a small comment about this > > > change... I'll commit this tomorrow, if you don't have a better suggestion. > > > > Looks good. > > Or maybe like this? > > diff -r f01b3897d141 linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c > --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 00:27:32 2009 -0200 > +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 02:45:48 2009 -0200 > @@ -3747,7 +3747,12 @@ > int ret; > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > - if (ret == 1) { > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 29) > + /* Due to the API changes, the ret value for success changed */ > + ret = ret != 1; > +#endif > + if (ret == 0) { > ret = usb_reset_device(hdw->usb_dev); > usb_unlock_device(hdw->usb_dev); > } else { > Seems better! Could you please provide your SOB? I'll apply just the backport, then your patch. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 9 Jan 2009, Mauro Carvalho Chehab wrote: > On Thu, 8 Jan 2009 21:56:15 -0800 (PST) > Trent Piepho <xyzzy@speakeasy.org> wrote: > > > On Thu, 8 Jan 2009, Mike Isely wrote: > > > > Yes... Anyway, this is the real patch. I've added a small comment about this > > > > change... I'll commit this tomorrow, if you don't have a better suggestion. > > > > > > Looks good. > > > > Or maybe like this? > > > > diff -r f01b3897d141 linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c > > --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 00:27:32 2009 -0200 > > +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 02:45:48 2009 -0200 > > @@ -3747,7 +3747,12 @@ > > int ret; > > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > > - if (ret == 1) { > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 29) > > + /* Due to the API changes, the ret value for success changed */ > > + ret = ret != 1; > > +#endif > > + if (ret == 0) { > > ret = usb_reset_device(hdw->usb_dev); > > usb_unlock_device(hdw->usb_dev); > > } else { > > > > Seems better! Could you please provide your SOB? I'll apply just the backport, then your patch. Just for the record... Acked-By: Mike Isely <isely@pobox.com> -Mike
On Fri, 9 Jan 2009, Mauro Carvalho Chehab wrote: > On Thu, 8 Jan 2009 21:56:15 -0800 (PST) > Trent Piepho <xyzzy@speakeasy.org> wrote: > > On Thu, 8 Jan 2009, Mike Isely wrote: > > > > Yes... Anyway, this is the real patch. I've added a small comment about this > > > > change... I'll commit this tomorrow, if you don't have a better suggestion. > > > > > > Looks good. > > > > Or maybe like this? > > > > diff -r f01b3897d141 linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c > > --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 00:27:32 2009 -0200 > > +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 02:45:48 2009 -0200 > > @@ -3747,7 +3747,12 @@ > > int ret; > > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > > - if (ret == 1) { > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 29) > > + /* Due to the API changes, the ret value for success changed */ > > + ret = ret != 1; > > +#endif > > + if (ret == 0) { > > ret = usb_reset_device(hdw->usb_dev); > > usb_unlock_device(hdw->usb_dev); > > } else { > > > > Seems better! Could you please provide your SOB? I'll apply just the backport, then your patch. Signed-off-by: Trent Piepho <xyzzy@speakeasy.org> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -r f01b3897d141 linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c --- a/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 00:27:32 2009 -0200 +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c Fri Jan 09 02:45:48 2009 -0200 @@ -3747,7 +3747,12 @@ int ret; pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); - if (ret == 1) { +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 29) + /* Due to the API changes, the ret value for success changed */ + ret = ret != 1; +#endif + if (ret == 0) { ret = usb_reset_device(hdw->usb_dev); usb_unlock_device(hdw->usb_dev); } else {