linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Antti Palosaari <crope@iki.fi>
To: Michael Krufky <mkrufky@linuxtv.org>
Cc: Jean Delvare <khali@linux-fr.org>,
	Mauro Carvalho Chehab <m.chehab@samsung.com>,
	linux-media <linux-media@vger.kernel.org>
Subject: Re: [PATCH REVIEW] e4000: convert DVB tuner to I2C driver model
Date: Wed, 16 Oct 2013 20:45:39 +0300	[thread overview]
Message-ID: <525ED0C3.4010602@iki.fi> (raw)
In-Reply-To: <CAOcJUbxfaO9oc7QN-vHQwg461FnPdTPqMkoksGsdgMWedgoJmA@mail.gmail.com>

On 16.10.2013 20:33, Michael Krufky wrote:
> On Wed, Oct 16, 2013 at 1:22 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On 16.10.2013 20:19, Michael Krufky wrote:
>>>
>>> On Wed, Oct 16, 2013 at 1:09 PM, Jean Delvare <khali@linux-fr.org> wrote:
>>>>
>>>> Hi Michael,
>>>>
>>>> On Wed, 16 Oct 2013 13:04:42 -0400, Michael Krufky wrote:
>>>>>
>>>>> YIKES!!  i2c_new_probed_device() does indeed probe the hardware --
>>>>> this is unacceptable, as such an action can damage the ic.
>>>>>
>>>>> Is there some additional information that I'm missing that lets this
>>>>> perform an attach without probe?
>>>>
>>>>
>>>> Oh, i2c_new_probed_device() probes the device, what a surprise! :D
>>>>
>>>> Try, I don't know, i2c_new_device() maybe if you don't want the
>>>> probe? ;)
>>>>
>>>> --
>>>> Jean Delvare
>>>
>>>
>>> OK, so to confirm that I follow correctly, one can use
>>> i2c_new_device() to attach the sub-driver without probing, and the
>>> line that ensures that the correct sub-driver gets attached is
>>> "strlcpy(info.type, "e4000", I2C_NAME_SIZE);"  ??
>>>
>>> We're matching based on a string?  I think that's kinda yucky, but if
>>> that's what we're doing in i2c nowadays then I'm OK with it.
>>>
>>> If not, what prevents the wrong sub-driver from attaching to a device?
>>>    ...or conversely, how does the right sub-driver know which device to
>>> attach to?
>>
>>
>> Yes, it is that string. Driver has that string as a ID table entry. Then you
>> issue i2c_new_device() call with string and it attachs driver when strings
>> match.
>>
>>
>>> Again, if I'm asking "stupid questions" just point me to the
>>> documentation.
>>>
>>> -Mike
>>>
>>
>> regards
>> Antti
>
>
> OK, I get it and it does seem OK.  I'm just curious what kind of
> impact this refactoring would have over something like the
> b2c2-flexcop-fe driver, who does not know which ic's to attach based
> on device ids, but it does probe a few frontend combinations one after
> another, in an order that the driver authors knew was safe.  I'd
> imaging that we'd write some helper abstraction function to switch out
> the info.type string as each driver gets probed?  I think that can get
> quite ugly, but I know that the general population thinks dvb_attach()
> is even uglier, so maybe this could be the right path...
>
> Wanna take a crack at b2c2-flexcop-fe?
>
> -Mike
>

heh, look the rtl28xxu driver in question, it does same. It probes maybe 
total 10 tuners - checking their device ID too. Probing plain I2C device 
address and making devision from that is simply dead idea. "Standard" 
I2C address range for these silicon tuners are 0x60-0x64 => need to read 
chip ID in order to detect => i2c_new_probed_device() is quite useless.

regards
Antti

-- 
http://palosaari.fi/

  reply	other threads:[~2013-10-16 17:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-15 22:31 [PATCH REVIEW] e4000: convert DVB tuner to I2C driver model Antti Palosaari
2013-10-15 23:33 ` Mauro Carvalho Chehab
2013-10-16 15:54   ` Michael Krufky
2013-10-16 16:41     ` Mauro Carvalho Chehab
2013-10-16 16:43     ` Antti Palosaari
2013-10-16 17:01       ` Michael Krufky
2013-10-16 17:04         ` Michael Krufky
2013-10-16 17:09           ` Jean Delvare
2013-10-16 17:19             ` Michael Krufky
2013-10-16 17:22               ` Antti Palosaari
2013-10-16 17:33                 ` Michael Krufky
2013-10-16 17:45                   ` Antti Palosaari [this message]
2013-10-16 17:52                     ` Jean Delvare

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=525ED0C3.4010602@iki.fi \
    --to=crope@iki.fi \
    --cc=khali@linux-fr.org \
    --cc=linux-media@vger.kernel.org \
    --cc=m.chehab@samsung.com \
    --cc=mkrufky@linuxtv.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;
as well as URLs for NNTP newsgroup(s).