Re: [PATCH 00/13] Remaining coding style clean up of AS102 driver

2011-11-08 Thread Sylwester Nawrocki
On 11/08/2011 02:36 PM, Mauro Carvalho Chehab wrote:
> I got a few warnings here, after applying the patch series:
> 
> drivers/staging/media/as102/as102_drv.c: In function ‘as102_dvb_register’:
> drivers/staging/media/as102/as102_drv.c:223:3: warning: passing argument 1 of 
> ‘dev_err’ from incompatible pointer type [enabled by default]
> include/linux/device.h:812:12: note: expected ‘const struct device *’ but 
> argument is of type ‘char *’
> drivers/staging/media/as102/as102_drv.c:223:3: warning: too many arguments 
> for format [-Wformat-extra-args]
> 
> please check.

Yes, there was an error in patch 06/13, this has been pointed out yesterday.
I intended to resend the patch, but since you have already pulled the whole 
series.. :),
I'll post an additional patch to fix the problem.  

-- 
Cheers,
Sylwester
--
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 00/13] Remaining coding style clean up of AS102 driver

2011-11-08 Thread Mauro Carvalho Chehab
Em 06-11-2011 18:31, Sylwester Nawrocki escreveu:
> Hello,
> 
> the following patch set is a further cleanup of the AS102 DVB-T receiver 
> driver. I'm not sure if there are more issues to address before moving 
> the driver to drivers/media/dvb/, but checkpatch.pl seems now to be fairly
> happy about the driver's state:
> 
> 8<--
> $ scripts/checkpatch.pl -f drivers/staging/media/as102/*.[ch]
> WARNING: line over 80 characters
> #137: FILE: staging/media/as102/as102_drv.c:137:
> + dprintk(debug, "ADD_PID_FILTER([%02d -> %02d], 0x%04x) ret = 
> %d\n",
> 
> total: 0 errors, 1 warnings, 319 lines checked
> 
> drivers/staging/media/as102/as102_drv.c has style problems, please review.
> 
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> total: 0 errors, 0 warnings, 106 lines checked
> 
> drivers/staging/media/as102/as102_drv.h has no obvious style problems and is 
> ready for submission.
> total: 0 errors, 0 warnings, 599 lines checked
> 
> drivers/staging/media/as102/as102_fe.c has no obvious style problems and is 
> ready for submission.
> total: 0 errors, 0 warnings, 241 lines checked
> 
> drivers/staging/media/as102/as102_fw.c has no obvious style problems and is 
> ready for submission.
> total: 0 errors, 0 warnings, 38 lines checked
> 
> drivers/staging/media/as102/as102_fw.h has no obvious style problems and is 
> ready for submission.
> total: 0 errors, 0 warnings, 476 lines checked
> 
> drivers/staging/media/as102/as102_usb_drv.c has no obvious style problems and 
> is ready for submission.
> total: 0 errors, 0 warnings, 58 lines checked
> 
> drivers/staging/media/as102/as102_usb_drv.h has no obvious style problems and 
> is ready for submission.
> WARNING: line over 80 characters
> #349: FILE: staging/media/as102/as10x_cmd.c:349:
> + 
> le32_to_cpu(prsp->body.get_demod_stats.rsp.stats.bad_frame_count);
> 
> WARNING: line over 80 characters
> #351: FILE: staging/media/as102/as10x_cmd.c:351:
> + 
> le32_to_cpu(prsp->body.get_demod_stats.rsp.stats.bytes_fixed_by_rs);
> 
> total: 0 errors, 2 warnings, 453 lines checked
> 
> drivers/staging/media/as102/as10x_cmd.c has style problems, please review.
> 
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
> total: 0 errors, 0 warnings, 215 lines checked
> 
> drivers/staging/media/as102/as10x_cmd_cfg.c has no obvious style problems and 
> is ready for submission.
> total: 0 errors, 0 warnings, 529 lines checked
> 
> drivers/staging/media/as102/as10x_cmd.h has no obvious style problems and is 
> ready for submission.
> total: 0 errors, 0 warnings, 223 lines checked
> 
> drivers/staging/media/as102/as10x_cmd_stream.c has no obvious style problems 
> and is ready for submission.
> total: 0 errors, 0 warnings, 54 lines checked
> 
> drivers/staging/media/as102/as10x_handle.h has no obvious style problems and 
> is ready for submission.
> total: 0 errors, 0 warnings, 194 lines checked
> 
> drivers/staging/media/as102/as10x_types.h has no obvious style problems and 
> is ready for submission.
> >8
> 
> Thanks to Piotr Chmura for initially reviewing the series.
> 
> The patches can be pulled from:
> git://gitorious.org/linux-media/media_tree.git staging_media_as102_cleanup
> 
> The driver has been tested with PCTV picoStick (74e) DVB-T tuner. 

I got a few warnings here, after applying the patch series:

drivers/staging/media/as102/as102_drv.c: In function ‘as102_dvb_register’:
drivers/staging/media/as102/as102_drv.c:223:3: warning: passing argument 1 of 
‘dev_err’ from incompatible pointer type [enabled by default]
include/linux/device.h:812:12: note: expected ‘const struct device *’ but 
argument is of type ‘char *’
drivers/staging/media/as102/as102_drv.c:223:3: warning: too many arguments for 
format [-Wformat-extra-args]

please check.

> 
> The only issue I observed is at first run MeTV displays 
> "Failed to lock channel" error instead of playing the last selected 
> channel immediately.
> 
> I'm rather not planning to be doing much more work on this driver.
> 
> --
> Thanks,
> Sylwester
> 
> 
> Piotr Chmura (1):
>   staging: as102: Remove comment tags for editors configuration
> 
> Sylwester Nawrocki (12):
>   staging: as102: Remove unnecessary typedefs
>   staging: as102: Remove leftovers of the SPI bus driver
>   staging: as102: Make the driver select CONFIG_FW_LOADER
>   staging: as102: Replace pragma(pack) with attribute __packed
>   staging: as102: Fix the dvb device registration error path
>   staging: as102: Whitespace and indentation cleanup
>   staging: as102: Replace printk(KERN_ witk pr_
>   staging: as102: Remove linkage specifiers for C++
>   staging: as102: Use linux/uaccess.h instead of asm/uaccess.h
>   staging: as102: Move variable declarations to the header
>   staging: as102: Define device name string pointers constant
>   staging: as102: Eliminate as10x_handle_t ali

Re: [PATCH 00/13] Remaining coding style clean up of AS102 driver

2011-11-07 Thread Devin Heitmueller
On Mon, Nov 7, 2011 at 2:57 PM, Sylwester Nawrocki  wrote:
> My knowledge about this driver is rather limited, in case of any issues I
> guess it's best to ask Devin directly.

Part of the problem here is that the as102 chip is a fully
programmable part, and as a result the firmware may differ on a
product-by-product basis.  In fact, the engineer at Abilis told me
outright that the firmware provided for the PCTV device isn't
appropriate for the Elgato board, but it worked "good enough" for the
user.

In other words, users are free to try the firmware prepared for PCTV
against other devices, but be prepared for it to not be optimized
properly for that hardware design (meaning tuning quality issues or
perhaps not working at all), and if it destroys your board then don't
come crying to me.

The only way to know if the same firmware is being used would be to do
a USB capture, write a decoder which allows you to extract the
firmware being uploaded, and compare it against the PCTV blob.  And if
it doesn't match, there's no real way of knowing whether the delta is
significant.

Also worth noting that the only firmware we have the legal right to
freely redistribute is the one needed by the PCTV device.  Even if
somebody extracts another firmware blob from a USB trace, there are no
legal rights to redistribute it (meaning a firmware extract script
written to work against the Windows driver binary would be required).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
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 00/13] Remaining coding style clean up of AS102 driver

2011-11-07 Thread Sylwester Nawrocki
Hi Gianluca,

On 11/07/2011 05:09 PM, Gianluca Gennari wrote:
> Hi Sylwestwer,
> I was about to test the new driver when I discovered that my as102
> device is not included in the list of supported devices:
> 
> /* Super Digi KEY */
> #define AS102_SUPER_DIGI_NAME "Super Digi KEY"
> #define SUPER_DIGI_USB_VID0x2137
> #define SUPER_DIGI_USB_PID0x0001
> 
> It's a "Digital key" offered by Sky Italia to its customers to watch
> terrestrial programs with the Sky satellite decoders.

I'm not sure if your device needs some special handling, but it might work
if you repeat steps as in this patch: 
http://git.linuxtv.org/media_tree.git/commitdiff/5f9745b2c942b2ab220831b2c51a18c3f1374249

Have you tried it already ?

Possibly something like this is enough (only compile tested):

diff --git a/drivers/staging/media/as102/as102_usb_drv.c 
b/drivers/staging/media/as102/as102_usb_drv.c
index 9faab5b..9edb366 100644
--- a/drivers/staging/media/as102/as102_usb_drv.c
+++ b/drivers/staging/media/as102/as102_usb_drv.c
@@ -42,6 +42,7 @@ static struct usb_device_id as102_usb_id_table[] = {
{ USB_DEVICE(PCTV_74E_USB_VID, PCTV_74E_USB_PID) },
{ USB_DEVICE(ELGATO_EYETV_DTT_USB_VID, ELGATO_EYETV_DTT_USB_PID) },
{ USB_DEVICE(NBOX_DVBT_DONGLE_USB_VID, NBOX_DVBT_DONGLE_USB_PID) },
+   { USB_DEVICE(SUPER_DIGI_USB_VID, SUPER_DIGI_USB_PID) },
{ } /* Terminating entry */
 };
 
@@ -52,6 +53,7 @@ static const char * const as102_device_names[] = {
AS102_PCTV_74E,
AS102_ELGATO_EYETV_DTT_NAME,
AS102_NBOX_DVBT_DONGLE_NAME,
+   AS102_SUPER_DIGI_NAME,
NULL /* Terminating entry */
 };
 
diff --git a/drivers/staging/media/as102/as102_usb_drv.h 
b/drivers/staging/media/as102/as102_usb_drv.h
index 35925b7..6f95af2 100644
--- a/drivers/staging/media/as102/as102_usb_drv.h
+++ b/drivers/staging/media/as102/as102_usb_drv.h
@@ -47,6 +47,11 @@
 #define NBOX_DVBT_DONGLE_USB_VID   0x0b89
 #define NBOX_DVBT_DONGLE_USB_PID   0x0007
 
+/* Super Digi KEY */
+#define AS102_SUPER_DIGI_NAME  "Super Digi KEY"
+#define SUPER_DIGI_USB_VID 0x2137
+#define SUPER_DIGI_USB_PID 0x0001
+

My knowledge about this driver is rather limited, in case of any issues I
guess it's best to ask Devin directly.


-- 
Regards,
Sylwester
--
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 00/13] Remaining coding style clean up of AS102 driver

2011-11-07 Thread Sylwester Nawrocki
Hi Gianluca,

On 11/07/2011 04:19 PM, Gianluca Gennari wrote:
> Hi Sylwester,
> I spotted a minor error in file as102_drv.c line 233:
> in a call to function "dev_err", the first "dev" parameter is missing.
> This produces a compilation warning.

Thanks for pointing that out, not sure how I have managed to overlook this..
I'll wait for some time and I'll probably resend the whole series, including
correction of the above, at the end of this week.

-- 
Regards,
Sylwester
--
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 00/13] Remaining coding style clean up of AS102 driver

2011-11-06 Thread Sylwester Nawrocki
Hello,

the following patch set is a further cleanup of the AS102 DVB-T receiver 
driver. I'm not sure if there are more issues to address before moving 
the driver to drivers/media/dvb/, but checkpatch.pl seems now to be fairly
happy about the driver's state:

8<--
$ scripts/checkpatch.pl -f drivers/staging/media/as102/*.[ch]
WARNING: line over 80 characters
#137: FILE: staging/media/as102/as102_drv.c:137:
+   dprintk(debug, "ADD_PID_FILTER([%02d -> %02d], 0x%04x) ret = 
%d\n",

total: 0 errors, 1 warnings, 319 lines checked

drivers/staging/media/as102/as102_drv.c has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
total: 0 errors, 0 warnings, 106 lines checked

drivers/staging/media/as102/as102_drv.h has no obvious style problems and is 
ready for submission.
total: 0 errors, 0 warnings, 599 lines checked

drivers/staging/media/as102/as102_fe.c has no obvious style problems and is 
ready for submission.
total: 0 errors, 0 warnings, 241 lines checked

drivers/staging/media/as102/as102_fw.c has no obvious style problems and is 
ready for submission.
total: 0 errors, 0 warnings, 38 lines checked

drivers/staging/media/as102/as102_fw.h has no obvious style problems and is 
ready for submission.
total: 0 errors, 0 warnings, 476 lines checked

drivers/staging/media/as102/as102_usb_drv.c has no obvious style problems and 
is ready for submission.
total: 0 errors, 0 warnings, 58 lines checked

drivers/staging/media/as102/as102_usb_drv.h has no obvious style problems and 
is ready for submission.
WARNING: line over 80 characters
#349: FILE: staging/media/as102/as10x_cmd.c:349:
+   
le32_to_cpu(prsp->body.get_demod_stats.rsp.stats.bad_frame_count);

WARNING: line over 80 characters
#351: FILE: staging/media/as102/as10x_cmd.c:351:
+   
le32_to_cpu(prsp->body.get_demod_stats.rsp.stats.bytes_fixed_by_rs);

total: 0 errors, 2 warnings, 453 lines checked

drivers/staging/media/as102/as10x_cmd.c has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
total: 0 errors, 0 warnings, 215 lines checked

drivers/staging/media/as102/as10x_cmd_cfg.c has no obvious style problems and 
is ready for submission.
total: 0 errors, 0 warnings, 529 lines checked

drivers/staging/media/as102/as10x_cmd.h has no obvious style problems and is 
ready for submission.
total: 0 errors, 0 warnings, 223 lines checked

drivers/staging/media/as102/as10x_cmd_stream.c has no obvious style problems 
and is ready for submission.
total: 0 errors, 0 warnings, 54 lines checked

drivers/staging/media/as102/as10x_handle.h has no obvious style problems and is 
ready for submission.
total: 0 errors, 0 warnings, 194 lines checked

drivers/staging/media/as102/as10x_types.h has no obvious style problems and is 
ready for submission.
>8

Thanks to Piotr Chmura for initially reviewing the series.

The patches can be pulled from:
git://gitorious.org/linux-media/media_tree.git staging_media_as102_cleanup

The driver has been tested with PCTV picoStick (74e) DVB-T tuner. 

The only issue I observed is at first run MeTV displays 
"Failed to lock channel" error instead of playing the last selected 
channel immediately.

I'm rather not planning to be doing much more work on this driver.

--
Thanks,
Sylwester


Piotr Chmura (1):
  staging: as102: Remove comment tags for editors configuration

Sylwester Nawrocki (12):
  staging: as102: Remove unnecessary typedefs
  staging: as102: Remove leftovers of the SPI bus driver
  staging: as102: Make the driver select CONFIG_FW_LOADER
  staging: as102: Replace pragma(pack) with attribute __packed
  staging: as102: Fix the dvb device registration error path
  staging: as102: Whitespace and indentation cleanup
  staging: as102: Replace printk(KERN_ witk pr_
  staging: as102: Remove linkage specifiers for C++
  staging: as102: Use linux/uaccess.h instead of asm/uaccess.h
  staging: as102: Move variable declarations to the header
  staging: as102: Define device name string pointers constant
  staging: as102: Eliminate as10x_handle_t alias

 drivers/staging/media/as102/Kconfig|1 +
 drivers/staging/media/as102/Makefile   |2 +-
 drivers/staging/media/as102/as102_drv.c|  126 ++---
 drivers/staging/media/as102/as102_drv.h|   59 +--
 drivers/staging/media/as102/as102_fe.c |4 -
 drivers/staging/media/as102/as102_fw.c |   44 +-
 drivers/staging/media/as102/as102_fw.h |   10 +-
 drivers/staging/media/as102/as102_usb_drv.c|   34 +-
 drivers/staging/media/as102/as102_usb_drv.h|1 -
 drivers/staging/media/as102/as10x_cmd.c|  139 ++--
 drivers/staging/media/as102/as10x_cmd.h|  895 
 drivers/staging/media/as102/as10x_cmd_cfg.c|   66 +-
 drivers/staging/media/as102/as10x_cmd_stream.c |   56 +-