All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antti Palosaari <crope@iki.fi>
To: Malcolm Priestley <tvboxspy@gmail.com>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@redhat.com>
Subject: Re: [PATCH] lmedm04 2.06 conversion to dvb-usb-v2 version 2
Date: Tue, 07 Aug 2012 19:28:31 +0300	[thread overview]
Message-ID: <5021422F.6080601@iki.fi> (raw)
In-Reply-To: <1344284500.12234.12.camel@router7789>

On 08/06/2012 11:21 PM, Malcolm Priestley wrote:
> Conversion of lmedm04 to dvb-usb-v2
>
> Functional changes m88rs2000 tuner now uses all callbacks.
> TODO migrate other tuners to the callbacks.
>
> This patch is applied on top of [BUG] Re: dvb_usb_lmedm04 crash Kernel (rs2000)
> http://patchwork.linuxtv.org/patch/13584/
>
>
> Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>

Could you try to make this driver more generic?

You use some internals of dvb-usb directly and most likely those are 
without a reason. For example data streaming, lme2510_kill_urb() kills 
directly urbs allocated and submitted by dvb-usb. Guess that driver is 
broken just after someone changes dvb-usb streaming code.

lme2510_usb_talk() could be replaced by generic dvb_usbv2_generic_rw().

What is function of lme2510_int_read() ? I see you use own low level URB 
routines for here too. It starts "polling", reads remote, tuner, demod, 
etc, and updates state. You would better to implement I2C-adapter 
correctly and then start Kernel work-queue, which reads same information 
using I2C-adapter. Or you could even abuse remote controller polling 
function provided by dvb-usb.

lme2510_get_stream_config() enables pid-filter again over the dvb-usb, 
but I can live with it because there is no dynamic configuration for 
that. Anyhow, is that really needed?

I can live with the pid-filter "abuse", but killing stream URBs on 
behalf of DVB-USB is something I don't like to see. If you have very 
good explanation and I cannot fix DVB USB to meet it I could consider 
that kind of hack. And it should be documented clearly adding necessary 
comments to code.

Re-implementing that driver to use 100% DVB-USB services will reduce 
around 50% of code or more.

regards
Antti


-- 
http://palosaari.fi/

  reply	other threads:[~2012-08-07 16:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-06 20:21 [PATCH] lmedm04 2.06 conversion to dvb-usb-v2 version 2 Malcolm Priestley
2012-08-07 16:28 ` Antti Palosaari [this message]
2012-08-07 19:12   ` Mauro Carvalho Chehab
2012-08-07 19:22     ` Antti Palosaari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5021422F.6080601@iki.fi \
    --to=crope@iki.fi \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@redhat.com \
    --cc=tvboxspy@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.