Re: [GIT PULL] SDR stuff
Em Fri, 18 Jul 2014 04:14:32 +0300 Antti Palosaari cr...@iki.fi escreveu: * AirSpy SDR driver * all SDR drivers moved out of staging * few new SDR stream formats regards Antti The following changes since commit 3445857b22eafb70a6ac258979e955b116bfd2c6: [media] hdpvr: fix two audio bugs (2014-07-04 15:13:02 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_pull for you to fetch changes up to 1c3378e1c17d6acd9b6d392ff75addad4c63cc6c: MAINTAINERS: add airspy driver (2014-07-18 04:12:27 +0300) Antti Palosaari (23): v4l: uapi: add SDR format RU12LE DocBook: V4L: add V4L2_SDR_FMT_RU12LE - 'RU12' airspy: AirSpy SDR driver v4l: uapi: add SDR format CS8 DocBook: V4L: add V4L2_SDR_FMT_CS8 - 'CS08' v4l: uapi: add SDR format CS14 DocBook: V4L: add V4L2_SDR_FMT_CS14LE - 'CS14' msi001: move out of staging MAINTAINERS: update MSI001 driver location Kconfig: add SDR support Kconfig: sub-driver auto-select SPI bus msi2500: move msi3101 out of staging and rename There are several issues pointed by checkpath on this driver: WARNING: line over 80 characters #55: FILE: drivers/media/usb/msi2500/msi2500.c:55: +#define V4L2_PIX_FMT_SDR_S8 v4l2_fourcc('D', 'S', '0', '8') /* signed 8-bit */ WARNING: line over 80 characters #56: FILE: drivers/media/usb/msi2500/msi2500.c:56: +#define V4L2_PIX_FMT_SDR_S12v4l2_fourcc('D', 'S', '1', '2') /* signed 12-bit */ WARNING: line over 80 characters #57: FILE: drivers/media/usb/msi2500/msi2500.c:57: +#define V4L2_PIX_FMT_SDR_S14v4l2_fourcc('D', 'S', '1', '4') /* signed 14-bit */ WARNING: line over 80 characters #58: FILE: drivers/media/usb/msi2500/msi2500.c:58: +#define V4L2_PIX_FMT_SDR_MSI2500_384 v4l2_fourcc('M', '3', '8', '4') /* Mirics MSi2500 format 384 */ The above are OK, however those formats should be moved to videodev2.h, where those API bits belong. There are several warnings, not all are mandatory for moving it out of staging. I'll point the critical ones below: WARNING: Missing a blank line after declarations #135: FILE: drivers/media/usb/msi2500/msi2500.c:135: + struct urb *urbs[MAX_ISO_BUFS]; + int (*convert_stream)(struct msi3101_state *s, u8 *dst, u8 *src, WARNING: line over 80 characters #188: FILE: drivers/media/usb/msi2500/msi2500.c:188: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #211: FILE: drivers/media/usb/msi2500/msi2500.c:211: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { This one is a real bug, as jiffies may reset to zero. you should, instead, use the time macros, like time_is_before_jiffies() and time_is_after_jiffies(). WARNING: line over 80 characters #213: FILE: drivers/media/usb/msi2500/msi2500.c:213: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); This also seems wrong for the same reasons. WARNING: Missing a blank line after declarations #215: FILE: drivers/media/usb/msi2500/msi2500.c:215: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies_now; WARNING: line over 80 characters #242: FILE: drivers/media/usb/msi2500/msi2500.c:242: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Missing a blank line after declarations #272: FILE: drivers/media/usb/msi2500/msi2500.c:272: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies + msecs_to_jiffies(MSECS); WARNING: line over 80 characters #339: FILE: drivers/media/usb/msi2500/msi2500.c:339: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #363: FILE: drivers/media/usb/msi2500/msi2500.c:363: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { Same here. WARNING: line over 80 characters #365: FILE: drivers/media/usb/msi2500/msi2500.c:365: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); Same here. WARNING: Missing a blank line after declarations #367: FILE: drivers/media/usb/msi2500/msi2500.c:367: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies_now; WARNING: line over 80 characters #405: FILE: drivers/media/usb/msi2500/msi2500.c:405: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #428: FILE: drivers/media/usb/msi2500/msi2500.c:428: + if ((s-jiffies_next +
Re: [GIT PULL] SDR stuff
Em Mon, 21 Jul 2014 20:50:05 -0300 Mauro Carvalho Chehab m.che...@samsung.com escreveu: Em Fri, 18 Jul 2014 04:14:32 +0300 Antti Palosaari cr...@iki.fi escreveu: * AirSpy SDR driver * all SDR drivers moved out of staging * few new SDR stream formats regards Antti The following changes since commit 3445857b22eafb70a6ac258979e955b116bfd2c6: [media] hdpvr: fix two audio bugs (2014-07-04 15:13:02 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_pull for you to fetch changes up to 1c3378e1c17d6acd9b6d392ff75addad4c63cc6c: MAINTAINERS: add airspy driver (2014-07-18 04:12:27 +0300) Antti Palosaari (23): v4l: uapi: add SDR format RU12LE DocBook: V4L: add V4L2_SDR_FMT_RU12LE - 'RU12' airspy: AirSpy SDR driver v4l: uapi: add SDR format CS8 DocBook: V4L: add V4L2_SDR_FMT_CS8 - 'CS08' v4l: uapi: add SDR format CS14 DocBook: V4L: add V4L2_SDR_FMT_CS14LE - 'CS14' msi001: move out of staging MAINTAINERS: update MSI001 driver location Kconfig: add SDR support Kconfig: sub-driver auto-select SPI bus msi2500: move msi3101 out of staging and rename There are several issues pointed by checkpath on this driver: WARNING: line over 80 characters #55: FILE: drivers/media/usb/msi2500/msi2500.c:55: +#define V4L2_PIX_FMT_SDR_S8 v4l2_fourcc('D', 'S', '0', '8') /* signed 8-bit */ WARNING: line over 80 characters #56: FILE: drivers/media/usb/msi2500/msi2500.c:56: +#define V4L2_PIX_FMT_SDR_S12v4l2_fourcc('D', 'S', '1', '2') /* signed 12-bit */ WARNING: line over 80 characters #57: FILE: drivers/media/usb/msi2500/msi2500.c:57: +#define V4L2_PIX_FMT_SDR_S14v4l2_fourcc('D', 'S', '1', '4') /* signed 14-bit */ WARNING: line over 80 characters #58: FILE: drivers/media/usb/msi2500/msi2500.c:58: +#define V4L2_PIX_FMT_SDR_MSI2500_384 v4l2_fourcc('M', '3', '8', '4') /* Mirics MSi2500 format 384 */ The above are OK, however those formats should be moved to videodev2.h, where those API bits belong. There are several warnings, not all are mandatory for moving it out of staging. I'll point the critical ones below: WARNING: Missing a blank line after declarations #135: FILE: drivers/media/usb/msi2500/msi2500.c:135: + struct urb *urbs[MAX_ISO_BUFS]; + int (*convert_stream)(struct msi3101_state *s, u8 *dst, u8 *src, WARNING: line over 80 characters #188: FILE: drivers/media/usb/msi2500/msi2500.c:188: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #211: FILE: drivers/media/usb/msi2500/msi2500.c:211: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { This one is a real bug, as jiffies may reset to zero. you should, instead, use the time macros, like time_is_before_jiffies() and time_is_after_jiffies(). WARNING: line over 80 characters #213: FILE: drivers/media/usb/msi2500/msi2500.c:213: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); This also seems wrong for the same reasons. WARNING: Missing a blank line after declarations #215: FILE: drivers/media/usb/msi2500/msi2500.c:215: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies_now; WARNING: line over 80 characters #242: FILE: drivers/media/usb/msi2500/msi2500.c:242: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Missing a blank line after declarations #272: FILE: drivers/media/usb/msi2500/msi2500.c:272: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies + msecs_to_jiffies(MSECS); WARNING: line over 80 characters #339: FILE: drivers/media/usb/msi2500/msi2500.c:339: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #363: FILE: drivers/media/usb/msi2500/msi2500.c:363: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { Same here. WARNING: line over 80 characters #365: FILE: drivers/media/usb/msi2500/msi2500.c:365: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); Same here. WARNING: Missing a blank line after declarations #367: FILE: drivers/media/usb/msi2500/msi2500.c:367: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies_now; WARNING: line over 80 characters #405: FILE: drivers/media/usb/msi2500/msi2500.c:405: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0;
Re: [GIT PULL] SDR stuff
So what. Those were mostly WARNING only and all but long lines were some new checks added to checkpatch recently. chekcpatch gets all the time new and new checks, these were added after I have made that driver. I will surely clean those later when I do some new changes to driver and my checkpatch updates. regards Antti On 07/22/2014 02:50 AM, Mauro Carvalho Chehab wrote: Em Fri, 18 Jul 2014 04:14:32 +0300 Antti Palosaari cr...@iki.fi escreveu: * AirSpy SDR driver * all SDR drivers moved out of staging * few new SDR stream formats regards Antti The following changes since commit 3445857b22eafb70a6ac258979e955b116bfd2c6: [media] hdpvr: fix two audio bugs (2014-07-04 15:13:02 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_pull for you to fetch changes up to 1c3378e1c17d6acd9b6d392ff75addad4c63cc6c: MAINTAINERS: add airspy driver (2014-07-18 04:12:27 +0300) Antti Palosaari (23): v4l: uapi: add SDR format RU12LE DocBook: V4L: add V4L2_SDR_FMT_RU12LE - 'RU12' airspy: AirSpy SDR driver v4l: uapi: add SDR format CS8 DocBook: V4L: add V4L2_SDR_FMT_CS8 - 'CS08' v4l: uapi: add SDR format CS14 DocBook: V4L: add V4L2_SDR_FMT_CS14LE - 'CS14' msi001: move out of staging MAINTAINERS: update MSI001 driver location Kconfig: add SDR support Kconfig: sub-driver auto-select SPI bus msi2500: move msi3101 out of staging and rename There are several issues pointed by checkpath on this driver: WARNING: line over 80 characters #55: FILE: drivers/media/usb/msi2500/msi2500.c:55: +#define V4L2_PIX_FMT_SDR_S8 v4l2_fourcc('D', 'S', '0', '8') /* signed 8-bit */ WARNING: line over 80 characters #56: FILE: drivers/media/usb/msi2500/msi2500.c:56: +#define V4L2_PIX_FMT_SDR_S12v4l2_fourcc('D', 'S', '1', '2') /* signed 12-bit */ WARNING: line over 80 characters #57: FILE: drivers/media/usb/msi2500/msi2500.c:57: +#define V4L2_PIX_FMT_SDR_S14v4l2_fourcc('D', 'S', '1', '4') /* signed 14-bit */ WARNING: line over 80 characters #58: FILE: drivers/media/usb/msi2500/msi2500.c:58: +#define V4L2_PIX_FMT_SDR_MSI2500_384 v4l2_fourcc('M', '3', '8', '4') /* Mirics MSi2500 format 384 */ The above are OK, however those formats should be moved to videodev2.h, where those API bits belong. There are several warnings, not all are mandatory for moving it out of staging. I'll point the critical ones below: WARNING: Missing a blank line after declarations #135: FILE: drivers/media/usb/msi2500/msi2500.c:135: + struct urb *urbs[MAX_ISO_BUFS]; + int (*convert_stream)(struct msi3101_state *s, u8 *dst, u8 *src, WARNING: line over 80 characters #188: FILE: drivers/media/usb/msi2500/msi2500.c:188: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #211: FILE: drivers/media/usb/msi2500/msi2500.c:211: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { This one is a real bug, as jiffies may reset to zero. you should, instead, use the time macros, like time_is_before_jiffies() and time_is_after_jiffies(). WARNING: line over 80 characters #213: FILE: drivers/media/usb/msi2500/msi2500.c:213: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); This also seems wrong for the same reasons. WARNING: Missing a blank line after declarations #215: FILE: drivers/media/usb/msi2500/msi2500.c:215: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies_now; WARNING: line over 80 characters #242: FILE: drivers/media/usb/msi2500/msi2500.c:242: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Missing a blank line after declarations #272: FILE: drivers/media/usb/msi2500/msi2500.c:272: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies + msecs_to_jiffies(MSECS); WARNING: line over 80 characters #339: FILE: drivers/media/usb/msi2500/msi2500.c:339: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #363: FILE: drivers/media/usb/msi2500/msi2500.c:363: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { Same here. WARNING: line over 80 characters #365: FILE: drivers/media/usb/msi2500/msi2500.c:365: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); Same here. WARNING: Missing a blank line after declarations #367: FILE: drivers/media/usb/msi2500/msi2500.c:367: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next =
Re: [GIT PULL] SDR stuff
On 07/22/2014 03:05 AM, Mauro Carvalho Chehab wrote: total: 1 errors, 45 warnings, 1517 lines checked drivers/media/usb/msi2500/msi2500.c has style problems, please review. FYI, I applied the rest of this patch series, except for those patches: msi2500: move msi3101 out of staging and rename MAINTAINERS: update MSI3101 / MSI2500 driver location msi2500: change supported formats msi2500: print notice to point SDR API is not 100% stable yet Because the latter ones depend on the first patch. Hey, just apply these too. As I explained, those are coming from new checkpatch checks added recently, after I have made that MSi3101/MSi2500 driver. We should not start begin run new checkpatch tests for old drivers. One reason I really want these out from staging is checkpatch terrorism newbies are doing in staging. There is all kind of people doing some eucalyptys challenge, running very latest checkpatch and sending useless patches for these driver, just wasting only my time. regards Antti -- http://palosaari.fi/ -- 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: [GIT PULL] SDR stuff
Em Tue, 22 Jul 2014 03:08:19 +0300 Antti Palosaari cr...@iki.fi escreveu: So what. Those were mostly WARNING only and all but long lines were some new checks added to checkpatch recently. chekcpatch gets all the time new and new checks, these were added after I have made that driver. I will surely clean those later when I do some new changes to driver and my checkpatch updates. Antti, I think you didn't read my comments in the middle of the checkpatch stuff. Please read my email again. I'm not requiring you to fix the newer checkpatch warning (Missing a blank line after declarations), and not even about the 80-cols warning. The thing is that there are two issues there: 1) you're adding API bits at msi2500 driver, instead of moving them to videodev2.h (or reusing the fourcc types you already added there); 2) you're handling jiffies wrong inside the driver. As you may know, adding a driver at staging is easier than at the main tree, as we don't care much about checkpatch issues (and not even about some more serious issues). However, when moving stuff out of staging, we review the entire driver again, to be sure that it is ok. Regards, Mauro regards Antti On 07/22/2014 02:50 AM, Mauro Carvalho Chehab wrote: Em Fri, 18 Jul 2014 04:14:32 +0300 Antti Palosaari cr...@iki.fi escreveu: * AirSpy SDR driver * all SDR drivers moved out of staging * few new SDR stream formats regards Antti The following changes since commit 3445857b22eafb70a6ac258979e955b116bfd2c6: [media] hdpvr: fix two audio bugs (2014-07-04 15:13:02 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_pull for you to fetch changes up to 1c3378e1c17d6acd9b6d392ff75addad4c63cc6c: MAINTAINERS: add airspy driver (2014-07-18 04:12:27 +0300) Antti Palosaari (23): v4l: uapi: add SDR format RU12LE DocBook: V4L: add V4L2_SDR_FMT_RU12LE - 'RU12' airspy: AirSpy SDR driver v4l: uapi: add SDR format CS8 DocBook: V4L: add V4L2_SDR_FMT_CS8 - 'CS08' v4l: uapi: add SDR format CS14 DocBook: V4L: add V4L2_SDR_FMT_CS14LE - 'CS14' msi001: move out of staging MAINTAINERS: update MSI001 driver location Kconfig: add SDR support Kconfig: sub-driver auto-select SPI bus msi2500: move msi3101 out of staging and rename There are several issues pointed by checkpath on this driver: WARNING: line over 80 characters #55: FILE: drivers/media/usb/msi2500/msi2500.c:55: +#define V4L2_PIX_FMT_SDR_S8 v4l2_fourcc('D', 'S', '0', '8') /* signed 8-bit */ WARNING: line over 80 characters #56: FILE: drivers/media/usb/msi2500/msi2500.c:56: +#define V4L2_PIX_FMT_SDR_S12v4l2_fourcc('D', 'S', '1', '2') /* signed 12-bit */ WARNING: line over 80 characters #57: FILE: drivers/media/usb/msi2500/msi2500.c:57: +#define V4L2_PIX_FMT_SDR_S14v4l2_fourcc('D', 'S', '1', '4') /* signed 14-bit */ WARNING: line over 80 characters #58: FILE: drivers/media/usb/msi2500/msi2500.c:58: +#define V4L2_PIX_FMT_SDR_MSI2500_384 v4l2_fourcc('M', '3', '8', '4') /* Mirics MSi2500 format 384 */ The above are OK, however those formats should be moved to videodev2.h, where those API bits belong. There are several warnings, not all are mandatory for moving it out of staging. I'll point the critical ones below: WARNING: Missing a blank line after declarations #135: FILE: drivers/media/usb/msi2500/msi2500.c:135: + struct urb *urbs[MAX_ISO_BUFS]; + int (*convert_stream)(struct msi3101_state *s, u8 *dst, u8 *src, WARNING: line over 80 characters #188: FILE: drivers/media/usb/msi2500/msi2500.c:188: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Comparing jiffies is almost always wrong; prefer time_after, time_before and friends #211: FILE: drivers/media/usb/msi2500/msi2500.c:211: + if ((s-jiffies_next + msecs_to_jiffies(1)) = jiffies) { This one is a real bug, as jiffies may reset to zero. you should, instead, use the time macros, like time_is_before_jiffies() and time_is_after_jiffies(). WARNING: line over 80 characters #213: FILE: drivers/media/usb/msi2500/msi2500.c:213: + unsigned long msecs = jiffies_to_msecs(jiffies_now) - jiffies_to_msecs(s-jiffies_next); This also seems wrong for the same reasons. WARNING: Missing a blank line after declarations #215: FILE: drivers/media/usb/msi2500/msi2500.c:215: + unsigned int samples = sample_num[i_max - 1] - s-sample; + s-jiffies_next = jiffies_now; WARNING: line over 80 characters #242: FILE: drivers/media/usb/msi2500/msi2500.c:242: + sample_num[i] = src[3] 24 | src[2] 16 | src[1] 8 | src[0] 0; WARNING: Missing a blank line after
Re: [GIT PULL] SDR stuff
On 07/22/2014 03:51 AM, Mauro Carvalho Chehab wrote: Em Tue, 22 Jul 2014 03:08:19 +0300 Antti Palosaari cr...@iki.fi escreveu: So what. Those were mostly WARNING only and all but long lines were some new checks added to checkpatch recently. chekcpatch gets all the time new and new checks, these were added after I have made that driver. I will surely clean those later when I do some new changes to driver and my checkpatch updates. Antti, I think you didn't read my comments in the middle of the checkpatch stuff. Please read my email again. I'm not requiring you to fix the newer checkpatch warning (Missing a blank line after declarations), and not even about the 80-cols warning. The thing is that there are two issues there: 1) you're adding API bits at msi2500 driver, instead of moving them to videodev2.h (or reusing the fourcc types you already added there); If you look inside driver code, you will see those defines are not used - but commented out. It is simply dead definition compiler optimizes away. It is code I used on my tests, but finally decided to comment out to leave some time add those later to API. I later moved 2 of those to API, that is done in same patch serie. No issue here. 2) you're handling jiffies wrong inside the driver. As you may know, adding a driver at staging is easier than at the main tree, as we don't care much about checkpatch issues (and not even about some more serious issues). However, when moving stuff out of staging, we review the entire driver again, to be sure that it is ok. That jiffie check is also rather new and didn't exists time drive was done. Jiffie is used to calculate debug sample rate. There is multiple times very similar code piece, which could be optimized to one. My plan merge all those ~5 functions to one and use jiffies using macros as checkpatch now likes. I don't see meaningful fix it now as you are going to rewrite that stuff in near future in any case. Silencing all those checkpatch things is not very hard job though. If you merge that stuff to media/master I can do it right away (I am running older kernel and older checkpatch currently). regards Antti -- http://palosaari.fi/ -- 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: [GIT PULL] SDR stuff
Em Tue, 22 Jul 2014 04:05:05 +0300 Antti Palosaari cr...@iki.fi escreveu: On 07/22/2014 03:51 AM, Mauro Carvalho Chehab wrote: Em Tue, 22 Jul 2014 03:08:19 +0300 Antti Palosaari cr...@iki.fi escreveu: So what. Those were mostly WARNING only and all but long lines were some new checks added to checkpatch recently. chekcpatch gets all the time new and new checks, these were added after I have made that driver. I will surely clean those later when I do some new changes to driver and my checkpatch updates. Antti, I think you didn't read my comments in the middle of the checkpatch stuff. Please read my email again. I'm not requiring you to fix the newer checkpatch warning (Missing a blank line after declarations), and not even about the 80-cols warning. The thing is that there are two issues there: 1) you're adding API bits at msi2500 driver, instead of moving them to videodev2.h (or reusing the fourcc types you already added there); If you look inside driver code, you will see those defines are not used - but commented out. It is simply dead definition compiler optimizes away. It is code I used on my tests, but finally decided to comment out to leave some time add those later to API. I later moved 2 of those to API, that is done in same patch serie. No issue here. 2) you're handling jiffies wrong inside the driver. As you may know, adding a driver at staging is easier than at the main tree, as we don't care much about checkpatch issues (and not even about some more serious issues). However, when moving stuff out of staging, we review the entire driver again, to be sure that it is ok. That jiffie check is also rather new and didn't exists time drive was done. Jiffie is used to calculate debug sample rate. There is multiple times very similar code piece, which could be optimized to one. My plan merge all those ~5 functions to one and use jiffies using macros as checkpatch now likes. I don't see meaningful fix it now as you are going to rewrite that stuff in near future in any case. Ok, I'll apply the remaining patches. Silencing all those checkpatch things is not very hard job though. If you merge that stuff to media/master I can do it right away (I am running older kernel and older checkpatch currently). FYI, I always use the checkpatch available on our tree, no matter what Kernel I'm running. My scripts just call ./scripts/checkpatch.pl. 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: [GIT PULL] SDR stuff
On 07/22/2014 05:09 AM, Mauro Carvalho Chehab wrote: Em Tue, 22 Jul 2014 04:05:05 +0300 Antti Palosaari cr...@iki.fi escreveu: On 07/22/2014 03:51 AM, Mauro Carvalho Chehab wrote: Em Tue, 22 Jul 2014 03:08:19 +0300 Antti Palosaari cr...@iki.fi escreveu: So what. Those were mostly WARNING only and all but long lines were some new checks added to checkpatch recently. chekcpatch gets all the time new and new checks, these were added after I have made that driver. I will surely clean those later when I do some new changes to driver and my checkpatch updates. Antti, I think you didn't read my comments in the middle of the checkpatch stuff. Please read my email again. I'm not requiring you to fix the newer checkpatch warning (Missing a blank line after declarations), and not even about the 80-cols warning. The thing is that there are two issues there: 1) you're adding API bits at msi2500 driver, instead of moving them to videodev2.h (or reusing the fourcc types you already added there); If you look inside driver code, you will see those defines are not used - but commented out. It is simply dead definition compiler optimizes away. It is code I used on my tests, but finally decided to comment out to leave some time add those later to API. I later moved 2 of those to API, that is done in same patch serie. No issue here. 2) you're handling jiffies wrong inside the driver. As you may know, adding a driver at staging is easier than at the main tree, as we don't care much about checkpatch issues (and not even about some more serious issues). However, when moving stuff out of staging, we review the entire driver again, to be sure that it is ok. That jiffie check is also rather new and didn't exists time drive was done. Jiffie is used to calculate debug sample rate. There is multiple times very similar code piece, which could be optimized to one. My plan merge all those ~5 functions to one and use jiffies using macros as checkpatch now likes. I don't see meaningful fix it now as you are going to rewrite that stuff in near future in any case. Ok, I'll apply the remaining patches. Silencing all those checkpatch things is not very hard job though. If you merge that stuff to media/master I can do it right away (I am running older kernel and older checkpatch currently). FYI, I always use the checkpatch available on our tree, no matter what Kernel I'm running. My scripts just call ./scripts/checkpatch.pl. I would like to also run/use *always* media/master, but it is not usually possible. Let me list here all the issues which makes it impossible in practice (issues which I see on current development process): 1) media/master is far behind Linus kernel master tree. It is usually updated only 2 times per RC cycle, RC1 and around RC5. Even I want test upstream RC, that makes it impossible. 2) RC1 is very often (more than 50%) unusable. It is simply too buggy. For example 3.16 there was GPU related problems at least (and again). 3) I send one fix (PULL requested) for brand new Silicon Labs Si2168 driver on 2014-06-15. That was just before RC1 was released. It was merged to media/fixes, but not media/master. Unfortunately that driver gets very much interest and multiple other developers started hacking with it, adding support for new hw and so. There was huge headache to lead that development because code base was divided to 2 upstream git trees; one in media/fixes and another in media/development. Critical and problematic patch was still only in media/fixes, leaving media/master driver broken. So that forces me to use media/fixes as a base and add new patches top of it. Media/fixes is 3.15 whilst media/master is 3.16. I mentioned that already you and I have got bug reports too, latest one 6 hours ago reporting his PCTV 292e stopped working... So could it be possible to address those issues? Like media/master contains all latest upstream patches and is updated/rebased weekly top of Linus main tree? regards Antti -- http://palosaari.fi/ -- 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: [GIT PULL] SDR stuff
Em Tue, 22 Jul 2014 05:48:26 +0300 Antti Palosaari cr...@iki.fi escreveu: On 07/22/2014 05:09 AM, Mauro Carvalho Chehab wrote: Em Tue, 22 Jul 2014 04:05:05 +0300 Antti Palosaari cr...@iki.fi escreveu: On 07/22/2014 03:51 AM, Mauro Carvalho Chehab wrote: Em Tue, 22 Jul 2014 03:08:19 +0300 Antti Palosaari cr...@iki.fi escreveu: So what. Those were mostly WARNING only and all but long lines were some new checks added to checkpatch recently. chekcpatch gets all the time new and new checks, these were added after I have made that driver. I will surely clean those later when I do some new changes to driver and my checkpatch updates. Antti, I think you didn't read my comments in the middle of the checkpatch stuff. Please read my email again. I'm not requiring you to fix the newer checkpatch warning (Missing a blank line after declarations), and not even about the 80-cols warning. The thing is that there are two issues there: 1) you're adding API bits at msi2500 driver, instead of moving them to videodev2.h (or reusing the fourcc types you already added there); If you look inside driver code, you will see those defines are not used - but commented out. It is simply dead definition compiler optimizes away. It is code I used on my tests, but finally decided to comment out to leave some time add those later to API. I later moved 2 of those to API, that is done in same patch serie. No issue here. 2) you're handling jiffies wrong inside the driver. As you may know, adding a driver at staging is easier than at the main tree, as we don't care much about checkpatch issues (and not even about some more serious issues). However, when moving stuff out of staging, we review the entire driver again, to be sure that it is ok. That jiffie check is also rather new and didn't exists time drive was done. Jiffie is used to calculate debug sample rate. There is multiple times very similar code piece, which could be optimized to one. My plan merge all those ~5 functions to one and use jiffies using macros as checkpatch now likes. I don't see meaningful fix it now as you are going to rewrite that stuff in near future in any case. Ok, I'll apply the remaining patches. Silencing all those checkpatch things is not very hard job though. If you merge that stuff to media/master I can do it right away (I am running older kernel and older checkpatch currently). FYI, I always use the checkpatch available on our tree, no matter what Kernel I'm running. My scripts just call ./scripts/checkpatch.pl. I would like to also run/use *always* media/master, but it is not usually possible. Let me list here all the issues which makes it impossible in practice (issues which I see on current development process): 1) media/master is far behind Linus kernel master tree. It is usually updated only 2 times per RC cycle, RC1 and around RC5. Even I want test upstream RC, that makes it impossible. Hmm... you complained before that I was using a too new version of checkpatch... Now you're complaining about just the opposite... Anyway, nothing prevents you to pull from both Linus and my tree at least for your testing purposes. 2) RC1 is very often (more than 50%) unusable. It is simply too buggy. For example 3.16 there was GPU related problems at least (and again). 3) I send one fix (PULL requested) for brand new Silicon Labs Si2168 driver on 2014-06-15. That was just before RC1 was released. It was merged to media/fixes, but not media/master. Unfortunately that driver gets very much interest and multiple other developers started hacking with it, adding support for new hw and so. There was huge headache to lead that development because code base was divided to 2 upstream git trees; one in media/fixes and another in media/development. Critical and problematic patch was still only in media/fixes, leaving media/master driver broken. So that forces me to use media/fixes as a base and add new patches top of it. Media/fixes is 3.15 whilst media/master is 3.16. I mentioned that already you and I have got bug reports too, latest one 6 hours ago reporting his PCTV 292e stopped working... So could it be possible to address those issues? Like media/master contains all latest upstream patches and is updated/rebased weekly top of Linus main tree? If you take a look at other big subsystems (x86 tip, arm, network, ...) most of them are even more fragmented: they use topic branches, e. g. each conceptual patchset goes to its own branch. One big advantage of topic branches is that maintainers can send pull requests from each topic directly to Linus, avoiding the risk that one broken topic would harm the entire patch merging. I use topic branch on some cases, but v4l2 core changes too often to allow doing it on a broader scale. My main concern, as maintainer, is to be sure that the patches will flow fine during
Re: [GIT PULL] SDR stuff
On 07/18/2014 04:14 AM, Antti Palosaari wrote: * AirSpy SDR driver * all SDR drivers moved out of staging * few new SDR stream formats Added few patches more. Antti The following changes since commit 3445857b22eafb70a6ac258979e955b116bfd2c6: [media] hdpvr: fix two audio bugs (2014-07-04 15:13:02 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_pull for you to fetch changes up to 57c6d1bcea459f50bfe1b8a47f575655deca888a: airspy: fill FMT buffer size (2014-07-20 19:37:34 +0300) Antti Palosaari (28): v4l: uapi: add SDR format RU12LE DocBook: V4L: add V4L2_SDR_FMT_RU12LE - 'RU12' airspy: AirSpy SDR driver v4l: uapi: add SDR format CS8 DocBook: V4L: add V4L2_SDR_FMT_CS8 - 'CS08' v4l: uapi: add SDR format CS14 DocBook: V4L: add V4L2_SDR_FMT_CS14LE - 'CS14' msi001: move out of staging MAINTAINERS: update MSI001 driver location Kconfig: add SDR support Kconfig: sub-driver auto-select SPI bus msi2500: move msi3101 out of staging and rename MAINTAINERS: update MSI3101 / MSI2500 driver location msi2500: change supported formats msi2500: print notice to point SDR API is not 100% stable yet rtl2832_sdr: move from staging to media rtl2832_sdr: put complex U16 format behind module parameter rtl2832_sdr: print notice to point SDR API is not 100% stable yet MAINTAINERS: update RTL2832_SDR location airspy: remove v4l2-compliance workaround airspy: move out of staging into drivers/media/usb airspy: print notice to point SDR API is not 100% stable yet MAINTAINERS: add airspy driver v4l: videodev2: add buffer size to SDR format rtl2832_sdr: fill FMT buffer size DocBook media: v4l2_sdr_format buffersize field msi2500: fill FMT buffer size airspy: fill FMT buffer size Documentation/DocBook/media/v4l/dev-sdr.xml | 18 +- Documentation/DocBook/media/v4l/pixfmt-sdr-cs08.xml | 44 Documentation/DocBook/media/v4l/pixfmt-sdr-cs14le.xml | 47 + Documentation/DocBook/media/v4l/pixfmt-sdr-ru12le.xml | 40 Documentation/DocBook/media/v4l/pixfmt.xml |3 + MAINTAINERS | 18 +- drivers/media/Kconfig | 12 +- drivers/media/dvb-frontends/Kconfig |9 + drivers/media/dvb-frontends/Makefile |6 + drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.c| 48 +++-- drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.h|0 drivers/media/tuners/Kconfig |6 + drivers/media/tuners/Makefile |1 + drivers/{staging/media/msi3101 = media/tuners}/msi001.c |0 drivers/media/usb/Kconfig |6 + drivers/media/usb/Makefile |2 + drivers/media/usb/airspy/Kconfig | 10 + drivers/media/usb/airspy/Makefile |1 + drivers/media/usb/airspy/airspy.c | 1134 +++ drivers/media/usb/dvb-usb-v2/Kconfig |1 + drivers/media/usb/msi2500/Kconfig |5 + drivers/media/usb/msi2500/Makefile |1 + drivers/{staging/media/msi3101/sdr-msi3101.c = media/usb/msi2500/msi2500.c} | 78 --- drivers/staging/media/Kconfig |4 - drivers/staging/media/Makefile |2 - drivers/staging/media/msi3101/Kconfig | 10 - drivers/staging/media/msi3101/Makefile |2 - drivers/staging/media/rtl2832u_sdr/Kconfig |7 - drivers/staging/media/rtl2832u_sdr/Makefile |6 - include/uapi/linux/videodev2.h |7 +- 30 files changed, 1444 insertions(+), 84 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-cs08.xml create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-cs14le.xml create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-ru12le.xml rename drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.c (96%) rename drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.h (100%) rename drivers/{staging/media/msi3101 = media/tuners}/msi001.c (100%) create mode 100644 drivers/media/usb/airspy/Kconfig create mode 100644 drivers/media/usb/airspy/Makefile create mode 100644 drivers/media/usb/airspy/airspy.c create mode 100644 drivers/media/usb/msi2500/Kconfig create mode 100644 drivers/media/usb/msi2500/Makefile rename drivers/{staging/media/msi3101/sdr-msi3101.c = media/usb/msi2500/msi2500.c} (96%) delete mode 100644 drivers/staging/media/msi3101/Kconfig delete mode 100644 drivers/staging/media/msi3101/Makefile delete mode 100644 drivers/staging/media/rtl2832u_sdr/Kconfig delete mode 100644 drivers/staging/media/rtl2832u_sdr/Makefile -- http://palosaari.fi/ -- To unsubscribe from
[GIT PULL] SDR stuff
* AirSpy SDR driver * all SDR drivers moved out of staging * few new SDR stream formats regards Antti The following changes since commit 3445857b22eafb70a6ac258979e955b116bfd2c6: [media] hdpvr: fix two audio bugs (2014-07-04 15:13:02 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git sdr_pull for you to fetch changes up to 1c3378e1c17d6acd9b6d392ff75addad4c63cc6c: MAINTAINERS: add airspy driver (2014-07-18 04:12:27 +0300) Antti Palosaari (23): v4l: uapi: add SDR format RU12LE DocBook: V4L: add V4L2_SDR_FMT_RU12LE - 'RU12' airspy: AirSpy SDR driver v4l: uapi: add SDR format CS8 DocBook: V4L: add V4L2_SDR_FMT_CS8 - 'CS08' v4l: uapi: add SDR format CS14 DocBook: V4L: add V4L2_SDR_FMT_CS14LE - 'CS14' msi001: move out of staging MAINTAINERS: update MSI001 driver location Kconfig: add SDR support Kconfig: sub-driver auto-select SPI bus msi2500: move msi3101 out of staging and rename MAINTAINERS: update MSI3101 / MSI2500 driver location msi2500: change supported formats msi2500: print notice to point SDR API is not 100% stable yet rtl2832_sdr: move from staging to media rtl2832_sdr: put complex U16 format behind module parameter rtl2832_sdr: print notice to point SDR API is not 100% stable yet MAINTAINERS: update RTL2832_SDR location airspy: remove v4l2-compliance workaround airspy: move out of staging into drivers/media/usb airspy: print notice to point SDR API is not 100% stable yet MAINTAINERS: add airspy driver Documentation/DocBook/media/v4l/pixfmt-sdr-cs08.xml | 44 Documentation/DocBook/media/v4l/pixfmt-sdr-cs14le.xml | 47 + Documentation/DocBook/media/v4l/pixfmt-sdr-ru12le.xml | 40 Documentation/DocBook/media/v4l/pixfmt.xml |3 + MAINTAINERS | 18 +- drivers/media/Kconfig | 12 +- drivers/media/dvb-frontends/Kconfig |9 + drivers/media/dvb-frontends/Makefile |6 + drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.c| 21 +- drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.h|0 drivers/media/tuners/Kconfig |6 + drivers/media/tuners/Makefile |1 + drivers/{staging/media/msi3101 = media/tuners}/msi001.c |0 drivers/media/usb/Kconfig |6 + drivers/media/usb/Makefile |2 + drivers/media/usb/airspy/Kconfig | 10 + drivers/media/usb/airspy/Makefile |1 + drivers/media/usb/airspy/airspy.c | 1122 +++ drivers/media/usb/dvb-usb-v2/Kconfig |1 + drivers/media/usb/msi2500/Kconfig |5 + drivers/media/usb/msi2500/Makefile |1 + drivers/{staging/media/msi3101/sdr-msi3101.c = media/usb/msi2500/msi2500.c} | 47 +++-- drivers/staging/media/Kconfig |4 - drivers/staging/media/Makefile |2 - drivers/staging/media/msi3101/Kconfig | 10 - drivers/staging/media/msi3101/Makefile |2 - drivers/staging/media/rtl2832u_sdr/Kconfig |7 - drivers/staging/media/rtl2832u_sdr/Makefile |6 - include/uapi/linux/videodev2.h |3 + 29 files changed, 1375 insertions(+), 61 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-cs08.xml create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-cs14le.xml create mode 100644 Documentation/DocBook/media/v4l/pixfmt-sdr-ru12le.xml rename drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.c (98%) rename drivers/{staging/media/rtl2832u_sdr = media/dvb-frontends}/rtl2832_sdr.h (100%) rename drivers/{staging/media/msi3101 = media/tuners}/msi001.c (100%) create mode 100644 drivers/media/usb/airspy/Kconfig create mode 100644 drivers/media/usb/airspy/Makefile create mode 100644 drivers/media/usb/airspy/airspy.c create mode 100644 drivers/media/usb/msi2500/Kconfig create mode 100644 drivers/media/usb/msi2500/Makefile rename drivers/{staging/media/msi3101/sdr-msi3101.c = media/usb/msi2500/msi2500.c} (98%) delete mode 100644 drivers/staging/media/msi3101/Kconfig delete mode 100644 drivers/staging/media/msi3101/Makefile delete mode 100644 drivers/staging/media/rtl2832u_sdr/Kconfig delete mode 100644 drivers/staging/media/rtl2832u_sdr/Makefile -- http://palosaari.fi/ -- 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