public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Antti Palosaari <crope@iki.fi>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH 2/3] dvb-usb: multi-frontend support (MFE)
Date: Sun, 31 Jul 2011 21:46:43 -0300	[thread overview]
Message-ID: <4E35F773.3060807@redhat.com> (raw)
In-Reply-To: <4E306BAE.1020302@iki.fi>

Em 27-07-2011 16:49, Antti Palosaari escreveu:
> On 07/27/2011 10:06 PM, Mauro Carvalho Chehab wrote:
> 
>>> +    for (i = 0; i<= x; i++) {
>>> +        ret = adap->props.frontend_attach(adap);
>>> +        if (ret || adap->fe[i] == NULL) {
>>> +            /* only print error when there is no FE at all */
>>> +            if (i == 0)
>>> +                err("no frontend was attached by '%s'",
>>> +                    adap->dev->desc->name);
>>
>> This doesn't seem right. One thing is to accept adap->fe[1] to be
>> NULL. Another thing is to accept an error at the attach. IMO, the
>> logic should be something like:
>>
>>     if (ret<  0)
>>         return ret;
>>
>>     if (!i&&  !adap->fe[0]) {
>>         err("no adapter!");
>>         return -ENODEV;
>>     }
> 
> Heh, I tried to keep it functioning as earlier not to break anything! Only thing it does now differently is that it keeps silent when 2nd FE attach fails since we don't know always before fe attach if there is fe or not.
> 
> So since it *does not change old behaviour* it must be OK. Let fix old problems later. There is millions of DVB USB callbacks failing silently - like tuner_attach etc.
> 
> Surely I want also fix many old issues but it is always too risky.

Added support for DRX-K way at dvb-usb:

http://git.linuxtv.org/mchehab/experimental.git/commitdiff/765b3db218f1e9af6432251c2ebe59bc9660cd42
http://git.linuxtv.org/mchehab/experimental.git/commitdiff/37fa5797c58068cc60cca6829bd662cd4f883cfa

One bad thing I noticed with the API is that it calls adap->props.frontend_attach(adap)
several times, instead of just one, without even passing an argument for the driver to
know that it was called twice.

IMO, there are two ways of doing the attach:

1) call it only once, and, inside the driver, it will loop to add the other FE's;
2) add a parameter, at the call, to say what FE needs to be initialized.

I think (1) is preferred, as it is more flexible, allowing the driver to test for
several types of frontends.

Regards,
Mauro

  parent reply	other threads:[~2011-08-01  0:46 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-26  0:17 [PATCH 2/3] dvb-usb: multi-frontend support (MFE) Antti Palosaari
2011-07-27 19:06 ` Mauro Carvalho Chehab
2011-07-27 19:49   ` Antti Palosaari
2011-07-27 20:06     ` Mauro Carvalho Chehab
2011-07-27 20:19       ` Antti Palosaari
2011-08-01  0:46     ` Mauro Carvalho Chehab [this message]
2011-08-01  1:22       ` Antti Palosaari
2011-08-01  2:24         ` Mauro Carvalho Chehab
2011-09-07 16:51           ` Antti Palosaari
2011-09-07 17:41             ` Michael Krufky
2011-09-07 17:45               ` Michael Krufky
2011-09-07 18:04                 ` Michael Krufky
2011-09-07 18:20                 ` Antti Palosaari
2011-09-07 18:36                   ` Michael Krufky
2011-09-07 21:10                     ` Antti Palosaari
2011-09-07 21:32                       ` Michael Krufky
2011-07-27 22:07   ` Malcolm Priestley
2011-07-27 22:23     ` Antti Palosaari
2011-07-31 18:28       ` Patrick Boettcher
2011-08-01  1:28         ` Antti Palosaari
  -- strict thread matches above, loose matches on Subject: below --
2011-07-28 21:35 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=4E35F773.3060807@redhat.com \
    --to=mchehab@redhat.com \
    --cc=crope@iki.fi \
    --cc=linux-media@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox