RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Hans Verkuil]

Hi all,

For video4linux we sometimes need to probe for a single i2c address. 
Normally you would do it like this:

static const unsigned short addrs[] = {
	addr, I2C_CLIENT_END
};

client = i2c_new_probed_device(adapter, &info, addrs);

This is a bit awkward and I came up with this macro:

#define V4L2_I2C_ADDRS(addr, addrs...) \
        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })

This can construct a list of one or more i2c addresses on the fly. But this 
is something that really belongs in i2c.h, renamed to I2C_ADDRS.

With this macro we can just do:

client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));

Comments?

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Trent Piepho]

On Sat, 6 Jun 2009, Hans Verkuil wrote:
> For video4linux we sometimes need to probe for a single i2c address.
> Normally you would do it like this:
>
> static const unsigned short addrs[] = {
> 	addr, I2C_CLIENT_END
> };
>
> client = i2c_new_probed_device(adapter, &info, addrs);
>
> This is a bit awkward and I came up with this macro:
>
> #define V4L2_I2C_ADDRS(addr, addrs...) \
>         ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
>
> This can construct a list of one or more i2c addresses on the fly. But this
> is something that really belongs in i2c.h, renamed to I2C_ADDRS.
>
> With this macro we can just do:
>
> client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
>
> Comments?

I agree it's better placed with i2c.h.  It's also possible to come up with
a more syntax more variadic function like, which can be called like this:

i2c_new_probed_device(adapter, &info, addr1);

i2c_new_probed_device(adapter, &info, addr1, addr2);


Using a macro like this...

#define i2c_new_probed_device(adap,info,addr,addrs...) ({ \
	const unsigned short _addrs[] = {addr, ## addrs, I2C_CLIENT_END }; \
	_i2c_new_probed_device(adap, info, _addrs); })

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Jon Smirl]

On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil@xs4all.nl> wrote:
> Hi all,
>
> For video4linux we sometimes need to probe for a single i2c address.
> Normally you would do it like this:

Why does video4linux need to probe to find i2c devices? Can't the
address be determined by knowing the PCI ID of the board?


>
> static const unsigned short addrs[] = {
>        addr, I2C_CLIENT_END
> };
>
> client = i2c_new_probed_device(adapter, &info, addrs);
>
> This is a bit awkward and I came up with this macro:
>
> #define V4L2_I2C_ADDRS(addr, addrs...) \
>        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
>
> This can construct a list of one or more i2c addresses on the fly. But this
> is something that really belongs in i2c.h, renamed to I2C_ADDRS.
>
> With this macro we can just do:
>
> client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
>
> Comments?
>
> Regards,
>
>        Hans
>
> --
> Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Jon Smirl
jonsmirl@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[hermann pitton]

Hi,

Am Samstag, den 06.06.2009, 18:20 -0400 schrieb Jon Smirl:
> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> > Hi all,
> >
> > For video4linux we sometimes need to probe for a single i2c address.
> > Normally you would do it like this:
> 
> Why does video4linux need to probe to find i2c devices? Can't the
> address be determined by knowing the PCI ID of the board?

NO, are you dreaming ?

Even the m$ attempts over additional stuff, that _eventually_ conforms
to something in the eeprom are doomed.

That is also not about video4linux anymore, since decades now soon, but
about v4l-dvb.

For real interesting stuff it does not work anymore and never did.

You should have a look at real hardware.

Cheers,
Hermann

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Hans Verkuil]

On Sunday 07 June 2009 00:20:26 Jon Smirl wrote:
> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil@xs4all.nl> wrote:
> > Hi all,
> >
> > For video4linux we sometimes need to probe for a single i2c address.
> > Normally you would do it like this:
>
> Why does video4linux need to probe to find i2c devices? Can't the
> address be determined by knowing the PCI ID of the board?

There are two reasons we need to probe: it is either because when the board 
was added no one bothered to record which chip was on what address (this 
happened in particular with old drivers like bttv) or because there is 
simply no other way to determine the presence or absence of an i2c device. 

E.g. there are three versions of one card: without upd64083 (Y/C separation 
device) and upd64031a (ghost reduction device), with only the upd64031a and 
one with both. Since they all have the same PCI ID the only way to 
determine the model is to probe.

Regards,

	Hans

>
> > static const unsigned short addrs[] = {
> >        addr, I2C_CLIENT_END
> > };
> >
> > client = i2c_new_probed_device(adapter, &info, addrs);
> >
> > This is a bit awkward and I came up with this macro:
> >
> > #define V4L2_I2C_ADDRS(addr, addrs...) \
> >        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> >
> > This can construct a list of one or more i2c addresses on the fly. But
> > this is something that really belongs in i2c.h, renamed to I2C_ADDRS.
> >
> > With this macro we can just do:
> >
> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> >
> > Comments?
> >
> > Regards,
> >
> >        Hans
> >
> > --
> > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Jon Smirl]

On Sun, Jun 7, 2009 at 2:35 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> On Sunday 07 June 2009 00:20:26 Jon Smirl wrote:
>> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>> > Hi all,
>> >
>> > For video4linux we sometimes need to probe for a single i2c address.
>> > Normally you would do it like this:
>>
>> Why does video4linux need to probe to find i2c devices? Can't the
>> address be determined by knowing the PCI ID of the board?
>
> There are two reasons we need to probe: it is either because when the board
> was added no one bothered to record which chip was on what address (this
> happened in particular with old drivers like bttv) or because there is
> simply no other way to determine the presence or absence of an i2c device.

Unrecorded boards could be handled by adding a printk at driver init
time asking people to email you the needed information. Then remove
the printks as soon as you get the answer.

>
> E.g. there are three versions of one card: without upd64083 (Y/C separation
> device) and upd64031a (ghost reduction device), with only the upd64031a and
> one with both. Since they all have the same PCI ID the only way to
> determine the model is to probe.

Did they happen to change the subsystem device_id? There are two pairs
of PCI IDs on each card. Most of the time the subsystem vendor/device
isn't set.

Getting rid of the probes altogether is the most reliable solution.
There is probably a way to identify these boards more specifically
that you haven't discovered yet.  PCI subsystem device ID is worth
checking.

>
> Regards,
>
>        Hans
>
>>
>> > static const unsigned short addrs[] = {
>> >        addr, I2C_CLIENT_END
>> > };
>> >
>> > client = i2c_new_probed_device(adapter, &info, addrs);
>> >
>> > This is a bit awkward and I came up with this macro:
>> >
>> > #define V4L2_I2C_ADDRS(addr, addrs...) \
>> >        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
>> >
>> > This can construct a list of one or more i2c addresses on the fly. But
>> > this is something that really belongs in i2c.h, renamed to I2C_ADDRS.
>> >
>> > With this macro we can just do:
>> >
>> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
>> >
>> > Comments?
>> >
>> > Regards,
>> >
>> >        Hans
>> >
>> > --
>> > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>



-- 
Jon Smirl
jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Jon Smirl]

On Sun, Jun 7, 2009 at 9:25 AM, Jon Smirl<jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sun, Jun 7, 2009 at 2:35 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>> On Sunday 07 June 2009 00:20:26 Jon Smirl wrote:
>>> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>>> > Hi all,
>>> >
>>> > For video4linux we sometimes need to probe for a single i2c address.
>>> > Normally you would do it like this:
>>>
>>> Why does video4linux need to probe to find i2c devices? Can't the
>>> address be determined by knowing the PCI ID of the board?
>>
>> There are two reasons we need to probe: it is either because when the board
>> was added no one bothered to record which chip was on what address (this
>> happened in particular with old drivers like bttv) or because there is
>> simply no other way to determine the presence or absence of an i2c device.
>
> Unrecorded boards could be handled by adding a printk at driver init
> time asking people to email you the needed information. Then remove
> the printks as soon as you get the answer.
>
>>
>> E.g. there are three versions of one card: without upd64083 (Y/C separation
>> device) and upd64031a (ghost reduction device), with only the upd64031a and
>> one with both. Since they all have the same PCI ID the only way to
>> determine the model is to probe.
>
> Did they happen to change the subsystem device_id? There are two pairs
> of PCI IDs on each card. Most of the time the subsystem vendor/device
> isn't set.

Example using lspci -vvv -nn

This is an Intel ICH8 but Dell as set in a subsystem id if 1028:01db
to indicate their custom use of the generic part.

00:1f.3 SMBus [0c05]: Intel Corporation 82801H (ICH8 Family) SMBus
Controller [8086:283e] (rev 02)
	Subsystem: Dell Device [1028:01db]
	Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Interrupt: pin C routed to IRQ 10
	Region 0: Memory at dffdab00 (32-bit, non-prefetchable) [size=256]
	Region 4: I/O ports at ece0 [size=32]
	Kernel modules: i2c-i801




>
> Getting rid of the probes altogether is the most reliable solution.
> There is probably a way to identify these boards more specifically
> that you haven't discovered yet.  PCI subsystem device ID is worth
> checking.
>
>>
>> Regards,
>>
>>        Hans
>>
>>>
>>> > static const unsigned short addrs[] = {
>>> >        addr, I2C_CLIENT_END
>>> > };
>>> >
>>> > client = i2c_new_probed_device(adapter, &info, addrs);
>>> >
>>> > This is a bit awkward and I came up with this macro:
>>> >
>>> > #define V4L2_I2C_ADDRS(addr, addrs...) \
>>> >        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
>>> >
>>> > This can construct a list of one or more i2c addresses on the fly. But
>>> > this is something that really belongs in i2c.h, renamed to I2C_ADDRS.
>>> >
>>> > With this macro we can just do:
>>> >
>>> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
>>> >
>>> > Comments?
>>> >
>>> > Regards,
>>> >
>>> >        Hans
>>> >
>>> > --
>>> > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>>> > --
>>> > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>>> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
>>
>
>
>
> --
> Jon Smirl
> jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>



-- 
Jon Smirl
jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Hans Verkuil]

On Sunday 07 June 2009 15:30:50 Jon Smirl wrote:
> On Sun, Jun 7, 2009 at 9:25 AM, Jon Smirl<jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Sun, Jun 7, 2009 at 2:35 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> >> On Sunday 07 June 2009 00:20:26 Jon Smirl wrote:
> >>> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> 
wrote:
> >>> > Hi all,
> >>> >
> >>> > For video4linux we sometimes need to probe for a single i2c
> >>> > address. Normally you would do it like this:
> >>>
> >>> Why does video4linux need to probe to find i2c devices? Can't the
> >>> address be determined by knowing the PCI ID of the board?
> >>
> >> There are two reasons we need to probe: it is either because when the
> >> board was added no one bothered to record which chip was on what
> >> address (this happened in particular with old drivers like bttv) or
> >> because there is simply no other way to determine the presence or
> >> absence of an i2c device.
> >
> > Unrecorded boards could be handled by adding a printk at driver init
> > time asking people to email you the needed information. Then remove
> > the printks as soon as you get the answer.

It's rather unfeasible when you are dealing with dozens and dozens of cards. 
We also know that there are variants of these cards out there where they 
changed audio chips or tuner chips without changing anything. It's not a 
big deal, that's why we have the i2c probing to solve these cases. Newer 
drivers try to avoid probing wherever possible, of course.

> >> E.g. there are three versions of one card: without upd64083 (Y/C
> >> separation device) and upd64031a (ghost reduction device), with only
> >> the upd64031a and one with both. Since they all have the same PCI ID
> >> the only way to determine the model is to probe.
> >
> > Did they happen to change the subsystem device_id? There are two pairs
> > of PCI IDs on each card. Most of the time the subsystem vendor/device
> > isn't set.

No, it's all the same. This is very common for the cheaper cards. They take 
a reference design, modify it and never bother to change the IDs.

Regards,

	Hans

>
> Example using lspci -vvv -nn
>
> This is an Intel ICH8 but Dell as set in a subsystem id if 1028:01db
> to indicate their custom use of the generic part.
>
> 00:1f.3 SMBus [0c05]: Intel Corporation 82801H (ICH8 Family) SMBus
> Controller [8086:283e] (rev 02)
> 	Subsystem: Dell Device [1028:01db]
> 	Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR+ FastB2B- DisINTx-
> 	Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Interrupt: pin C routed to IRQ 10
> 	Region 0: Memory at dffdab00 (32-bit, non-prefetchable) [size=256]
> 	Region 4: I/O ports at ece0 [size=32]
> 	Kernel modules: i2c-i801
>
> > Getting rid of the probes altogether is the most reliable solution.
> > There is probably a way to identify these boards more specifically
> > that you haven't discovered yet.  PCI subsystem device ID is worth
> > checking.
> >
> >> Regards,
> >>
> >>        Hans
> >>
> >>> > static const unsigned short addrs[] = {
> >>> >        addr, I2C_CLIENT_END
> >>> > };
> >>> >
> >>> > client = i2c_new_probed_device(adapter, &info, addrs);
> >>> >
> >>> > This is a bit awkward and I came up with this macro:
> >>> >
> >>> > #define V4L2_I2C_ADDRS(addr, addrs...) \
> >>> >        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END
> >>> > })
> >>> >
> >>> > This can construct a list of one or more i2c addresses on the fly.
> >>> > But this is something that really belongs in i2c.h, renamed to
> >>> > I2C_ADDRS.
> >>> >
> >>> > With this macro we can just do:
> >>> >
> >>> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> >>> >
> >>> > Comments?
> >>> >
> >>> > Regards,
> >>> >
> >>> >        Hans
> >>> >
> >>> > --
> >>> > Hans Verkuil - video4linux developer - sponsored by TANDBERG
> >>> > Telecom --
> >>> > To unsubscribe from this list: send the line "unsubscribe
> >>> > linux-i2c" in the body of a message to majordomo-u79uwXL29TaqPxH82wqD4g@public.gmane.orgg
> >>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >> --
> >> Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
> >
> > --
> > Jon Smirl
> > jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Andy Walls]

On Sun, 2009-06-07 at 09:25 -0400, Jon Smirl wrote:
> On Sun, Jun 7, 2009 at 2:35 AM, Hans Verkuil<hverkuil@xs4all.nl> wrote:
> > On Sunday 07 June 2009 00:20:26 Jon Smirl wrote:
> >> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil@xs4all.nl> wrote:
> >> > Hi all,
> >> >
> >> > For video4linux we sometimes need to probe for a single i2c address.
> >> > Normally you would do it like this:
> >>
> >> Why does video4linux need to probe to find i2c devices? Can't the
> >> address be determined by knowing the PCI ID of the board?
> >
> > There are two reasons we need to probe: it is either because when the board
> > was added no one bothered to record which chip was on what address (this
> > happened in particular with old drivers like bttv) or because there is
> > simply no other way to determine the presence or absence of an i2c device.
> 
> Unrecorded boards could be handled by adding a printk at driver init
> time asking people to email you the needed information. Then remove
> the printks as soon as you get the answer.
> 
> >
> > E.g. there are three versions of one card: without upd64083 (Y/C separation
> > device) and upd64031a (ghost reduction device), with only the upd64031a and
> > one with both. Since they all have the same PCI ID the only way to
> > determine the model is to probe.
> 
> Did they happen to change the subsystem device_id? There are two pairs
> of PCI IDs on each card. Most of the time the subsystem vendor/device
> isn't set.
> 
> Getting rid of the probes altogether is the most reliable solution.
> There is probably a way to identify these boards more specifically
> that you haven't discovered yet.  PCI subsystem device ID is worth
> checking.

Jon,

This really is a well beaten topic for v4l-dvb devices.

1. Device IDs are used to identify the bridge chip.
2. Subsystem IDs are used to identify the specfic card.
3. Vendor provided serial EEPROMs (sitting at 8-bit I2C address 0xA0)
can provide some amplifying information about the board layout.

There is variation in the I2C peripherals used on a video card PCB per
any Subsystem ID.  It's not a granular enough descriptor.

The data from serial EEPROMs, if the vendor was nice enough to even
include one, may not have a known decoding.  

I agree that I2C probing is bad/undesirable.  But for some video boards,
there is no other way than probing without each end user having expert
knowledge of the PCB layout.

Not probing, for cases where there is no other automated means to divine
the I2C devices used, would likely require an annoying or unsutainable
level of end user support be provided from a volunteer community.

Regards,
Andy


> > Regards,
> >
> >        Hans
> >
> >>
> >> > static const unsigned short addrs[] = {
> >> >        addr, I2C_CLIENT_END
> >> > };
> >> >
> >> > client = i2c_new_probed_device(adapter, &info, addrs);
> >> >
> >> > This is a bit awkward and I came up with this macro:
> >> >
> >> > #define V4L2_I2C_ADDRS(addr, addrs...) \
> >> >        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> >> >
> >> > This can construct a list of one or more i2c addresses on the fly. But
> >> > this is something that really belongs in i2c.h, renamed to I2C_ADDRS.
> >> >
> >> > With this macro we can just do:
> >> >
> >> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> >> >
> >> > Comments?
> >> >
> >> > Regards,
> >> >
> >> >        Hans
> >> >
> >> > --
> >> > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> >> > the body of a message to majordomo@vger.kernel.org
> >> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> >
> > --
> > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
> >
> 
> 
> 

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[hermann pitton]

Hi,

Am Sonntag, den 07.06.2009, 16:00 -0400 schrieb Andy Walls:
> On Sun, 2009-06-07 at 09:25 -0400, Jon Smirl wrote:
> > On Sun, Jun 7, 2009 at 2:35 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> > > On Sunday 07 June 2009 00:20:26 Jon Smirl wrote:
> > >> On Sat, Jun 6, 2009 at 9:00 AM, Hans Verkuil<hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> > >> > Hi all,
> > >> >
> > >> > For video4linux we sometimes need to probe for a single i2c address.
> > >> > Normally you would do it like this:
> > >>
> > >> Why does video4linux need to probe to find i2c devices? Can't the
> > >> address be determined by knowing the PCI ID of the board?
> > >
> > > There are two reasons we need to probe: it is either because when the board
> > > was added no one bothered to record which chip was on what address (this
> > > happened in particular with old drivers like bttv) or because there is
> > > simply no other way to determine the presence or absence of an i2c device.
> > 
> > Unrecorded boards could be handled by adding a printk at driver init
> > time asking people to email you the needed information. Then remove
> > the printks as soon as you get the answer.
> > 
> > >
> > > E.g. there are three versions of one card: without upd64083 (Y/C separation
> > > device) and upd64031a (ghost reduction device), with only the upd64031a and
> > > one with both. Since they all have the same PCI ID the only way to
> > > determine the model is to probe.
> > 
> > Did they happen to change the subsystem device_id? There are two pairs
> > of PCI IDs on each card. Most of the time the subsystem vendor/device
> > isn't set.
> > 
> > Getting rid of the probes altogether is the most reliable solution.
> > There is probably a way to identify these boards more specifically
> > that you haven't discovered yet.  PCI subsystem device ID is worth
> > checking.
> 
> Jon,
> 
> This really is a well beaten topic for v4l-dvb devices.
> 
> 1. Device IDs are used to identify the bridge chip.
> 2. Subsystem IDs are used to identify the specfic card.
> 3. Vendor provided serial EEPROMs (sitting at 8-bit I2C address 0xA0)
> can provide some amplifying information about the board layout.
> 
> There is variation in the I2C peripherals used on a video card PCB per
> any Subsystem ID.  It's not a granular enough descriptor.
> 
> The data from serial EEPROMs, if the vendor was nice enough to even
> include one, may not have a known decoding.  
> 
> I agree that I2C probing is bad/undesirable.  But for some video boards,
> there is no other way than probing without each end user having expert
> knowledge of the PCB layout.
> 
> Not probing, for cases where there is no other automated means to divine
> the I2C devices used, would likely require an annoying or unsutainable
> level of end user support be provided from a volunteer community.
> 
> Regards,
> Andy

about detecting and identifying i2c devices on a TV card it is really a
long story ...

Everybody talking about such should have at least a look at the bttv
driver before announcing RFC stuff.

It should make wonder, what all is used to come around.
(resistors on unused gpios for example)

On later drivers, like the saa7134, there were really high hopes to get
out of that previous total misery, highest respect to all sorting it
however possible and did it well then, by reliable PCI subsystem stuff.

But to give one more example, Asus, by far not the worst, did force the
users to uninstall all other TV card drivers on their m$ systems in
2005, if the tda8275a hybrid should be in use. (don't work anymore ...)

On that we have always been much better!

Jon's suggestions are on the level of 2002 and he is not in any details.

Cheers,
Hermann

 

> > > Regards,
> > >
> > >        Hans
> > >
> > >>
> > >> > static const unsigned short addrs[] = {
> > >> >        addr, I2C_CLIENT_END
> > >> > };
> > >> >
> > >> > client = i2c_new_probed_device(adapter, &info, addrs);
> > >> >
> > >> > This is a bit awkward and I came up with this macro:
> > >> >
> > >> > #define V4L2_I2C_ADDRS(addr, addrs...) \
> > >> >        ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> > >> >
> > >> > This can construct a list of one or more i2c addresses on the fly. But
> > >> > this is something that really belongs in i2c.h, renamed to I2C_ADDRS.
> > >> >
> > >> > With this macro we can just do:
> > >> >
> > >> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> > >> >
> > >> > Comments?
> > >> >
> > >> > Regards,
> > >> >
> > >> >        Hans
> > >> >

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Mauro Carvalho Chehab]

Em Sat, 6 Jun 2009 15:00:48 +0200
Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:

> Hi all,
> 
> For video4linux we sometimes need to probe for a single i2c address. 
> Normally you would do it like this:
> 
> static const unsigned short addrs[] = {
> 	addr, I2C_CLIENT_END
> };
> 
> client = i2c_new_probed_device(adapter, &info, addrs);
> 
> This is a bit awkward and I came up with this macro:
> 
> #define V4L2_I2C_ADDRS(addr, addrs...) \
>         ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> 
> This can construct a list of one or more i2c addresses on the fly. But this 
> is something that really belongs in i2c.h, renamed to I2C_ADDRS.
> 
> With this macro we can just do:
> 
> client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> 
> Comments?

Seems fine for me, but Your define has V4L2_foo.

Since this has nothing to do with V4L2, IMO, it is better to declare it as:

#define I2C_ADDRS(addr, addrs...) \
	((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })



Cheers,
Mauro

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Jean Delvare]

Hi Hans,

On Sat, 6 Jun 2009 15:00:48 +0200, Hans Verkuil wrote:
> For video4linux we sometimes need to probe for a single i2c address. 
> Normally you would do it like this:
> 
> static const unsigned short addrs[] = {
> 	addr, I2C_CLIENT_END
> };
> 
> client = i2c_new_probed_device(adapter, &info, addrs);
> 
> This is a bit awkward and I came up with this macro:
> 
> #define V4L2_I2C_ADDRS(addr, addrs...) \
>         ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> 
> This can construct a list of one or more i2c addresses on the fly. But this 
> is something that really belongs in i2c.h, renamed to I2C_ADDRS.
> 
> With this macro we can just do:
> 
> client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> 
> Comments?

I'm not a big fan of macros which hide how things work, but if this
makes your life easier, why not. Just send a patch and I'll queue it up
for 2.6.31.

-- 
Jean Delvare

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Hans Verkuil]

On Monday 08 June 2009 14:39:32 Jean Delvare wrote:
> Hi Hans,
>
> On Sat, 6 Jun 2009 15:00:48 +0200, Hans Verkuil wrote:
> > For video4linux we sometimes need to probe for a single i2c address.
> > Normally you would do it like this:
> >
> > static const unsigned short addrs[] = {
> > 	addr, I2C_CLIENT_END
> > };
> >
> > client = i2c_new_probed_device(adapter, &info, addrs);
> >
> > This is a bit awkward and I came up with this macro:
> >
> > #define V4L2_I2C_ADDRS(addr, addrs...) \
> >         ((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> >
> > This can construct a list of one or more i2c addresses on the fly. But
> > this is something that really belongs in i2c.h, renamed to I2C_ADDRS.
> >
> > With this macro we can just do:
> >
> > client = i2c_new_probed_device(adapter, &info, I2C_ADDRS(addr));
> >
> > Comments?
>
> I'm not a big fan of macros which hide how things work, but if this
> makes your life easier, why not. Just send a patch and I'll queue it up
> for 2.6.31.

Hi Jean,

Here it is:

--- linux-git/include/linux/i2c.h.orig	2009-06-09 14:53:32.000000000 +0200
+++ linux-git/include/linux/i2c.h	2009-06-09 15:03:24.000000000 +0200
@@ -412,6 +412,10 @@
 /* The numbers to use to set I2C bus address */
 #define ANY_I2C_BUS		0xffff
 
+/* Construct an I2C_CLIENT_END-terminated array of i2c addresses */
+#define I2C_ADDRS(addr, addrs...) \
+	((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
+
 
 /* ----- functions exported by i2c.o */
 

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>

Note that this can also be used to initialize an array:

static const unsigned short addrs[] = I2C_ADDRS(0x2a, 0x2c);

Whether you want to is another matter, but it works. This functionality is 
also available in the oldest supported gcc (3.2).

Thanks,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: RFC: proposal for new i2c.h macro to initialize i2c address lists on the fly

[Jean Delvare]

On Tue, 9 Jun 2009 15:04:36 +0200, Hans Verkuil wrote:
> Here it is:
> 
> --- linux-git/include/linux/i2c.h.orig	2009-06-09 14:53:32.000000000 +0200
> +++ linux-git/include/linux/i2c.h	2009-06-09 15:03:24.000000000 +0200
> @@ -412,6 +412,10 @@
>  /* The numbers to use to set I2C bus address */
>  #define ANY_I2C_BUS		0xffff
>  
> +/* Construct an I2C_CLIENT_END-terminated array of i2c addresses */
> +#define I2C_ADDRS(addr, addrs...) \
> +	((const unsigned short []){ addr, ## addrs, I2C_CLIENT_END })
> +
>  
>  /* ----- functions exported by i2c.o */
>  
> 
> Signed-off-by: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
> 
> Note that this can also be used to initialize an array:
> 
> static const unsigned short addrs[] = I2C_ADDRS(0x2a, 0x2c);
> 
> Whether you want to is another matter, but it works. This functionality is 
> also available in the oldest supported gcc (3.2).

Applied, thanks.

-- 
Jean Delvare