Date: Sun, 8 Jul 2012 22:37:07 -0600 From: Warner Losh <imp@bsdimp.com> To: Arnaud Lacombe <lacombar@gmail.com> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org>, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: newbus' ivar's limitation.. Message-ID: <8048FFC5-6952-49FC-849D-EA1A5675ACBE@bsdimp.com> In-Reply-To: <CACqU3MUcbozpyqRLUS91p-%2BXANsisLoHzYpbQ8KjCr02=kMHYg@mail.gmail.com> References: <CACqU3MVh6shncm2Vtqj9oe_HxowWscCZ1eJf0q2F%2B=t_xKKBfQ@mail.gmail.com> <31A0DCE7-3B93-41BC-805A-E0B163892112@bsdimp.com> <CACqU3MVy65ck%2Bb8TKXwfXnBV9iuFzj%2ButRBH4Ecg6XDz3Vg5kQ@mail.gmail.com> <5C18109D-E7A8-4868-BEA9-26B63360BB24@bsdimp.com> <CACqU3MUcbozpyqRLUS91p-%2BXANsisLoHzYpbQ8KjCr02=kMHYg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Jul 8, 2012, at 9:46 PM, Arnaud Lacombe wrote:
> On Sun, Jul 8, 2012 at 11:31 PM, Warner Losh <imp@bsdimp.com> wrote:
>>=20
>> On Jul 8, 2012, at 9:26 PM, Arnaud Lacombe wrote:
>>=20
>>> Hi,
>>>=20
>>> On Sun, Jul 8, 2012 at 10:07 PM, Warner Losh <imp@bsdimp.com> wrote:
>>>>=20
>>>> On Jul 8, 2012, at 7:22 PM, Arnaud Lacombe wrote:
>>>>> Ok, yet another Newbus' limitation. Assuming a device exports more
>>>>> than one interface, and one of its child has need to use more than =
one
>>>>> interface, each interfaces cannot register, concurrently, its own
>>>>> ivar. While I try to always have a single child per
>>>>> interface/resource, I need to keep some compatibility with the old =
way
>>>>> of doing thing (POLA wrt. drivers I cannot/will not convert and
>>>>> userland). So, it would have been nice if ivar had been =
per-interface,
>>>>> not global and unique to one device.
>>>>=20
>>>> There's one pointer for the ivars. The bus code gets to determine =
what the ivar looks like, because the interface is totally private to =
the bus. So long as it returns the right thing for any key that's =
presented, it doesn't matter quite how things are done.
>>>>=20
>>>> So I'm not sure I understand what you are saying here.
>>>>=20
>>> dev0 implements two interfaces: A and B. dev1, child of dev0, needs =
to
>>> use both interfaces. There is no generic way for dev0 to export
>>> independent ivars for both interface. For now, I restricted the
>>> function of the second interface not to need ivar, but that's kind =
of
>>> hackish.
>>=20
>> Only if the IVARs for interface A and interface B have overlapping =
values. If the Ivar keys don't overlap, then there's no problems at =
all. Certainly less hackish than not using them at all. Since dev0 =
knows the layout of the ivar that it set on its child, this presents no =
problems at all. It would return the values from A from the right part =
of the ivar, and those from B in the right part. Apart from the =
coordination of Ivar numbers, as I outlined in my last post, there's no =
issue here.
>>=20
> I think we should not be talking about the same API here. I have no
> idea what you mean by "the key to value translation", nor "Ivar
> numbers". What I refer to is that device_set_ivars() /
> device_get_ivars() acts on a single instance variables from `struct
> device': `ivars'. In that case, I do not really see how to set that
> specific field to two distinct values for each interfaces.
We are talking about the ivar interface. You are just misunderstanding =
how it is used.
Let us say that dev0 wants to implement interface A and B. The =
interface it implements is the device_read _ivar method. That method =
looks like:
int pci_read_ivar(device_t dev, device_t child, int which, uintptr_t =
*result)
{
struct pci_ivar *iv =3D device_get_ivar(child);
switch (which) {
case PCI_IVAR_SUBVENDOR:
*result =3D iv->subvendor;
break;
...
default:
return EINVAL;
}
return 0;
}
So, if you wanted to implement interface A and interface B, you would =
have to support reading and writing A_IVAR_* and B_IVAR_*. The above =
routine would look like:
struct mydev_ivar {
int a_1;
int a_2;
int b_1;
int b_2;
};
int mydev_read_ivar(device_t dev, device child, int which, uintpr_t =
*result)
{
struct mydev_ivar *iv =3D device_get_ivar(child);
switch (which) {
case A_IVAR_1:
*result =3D iv->a_1;
break;
case A_IVAR_2:
*result =3D iv->a_2;
break;
case B_IVAR_1:
*result =3D iv->b_1;
break;
case B_IVAR_2:
*result =3D iv->b_2;
break;
default:
return EINVAL;
}
return 0;
}
and similar for the write routine. Now, there are some busses that =
provide convenience structures and functions for dealing with the ivars. =
In that case, you'd be able to use the wrapper structures and routines =
like so:
struct mdev_ivar {
struct a_ivar a;
struct b_ivar b;
};
int mydev_read_ivar(device_t dev, device child, int which, uintpr_t =
*result)
{
struct mydev_ivar *iv =3D device_get_ivar(child);
switch (which) {
case A_IVAR_1:
case A_IVAR_2:
return a_helper_read_ivar(&iv->a, which, result);
case B_IVAR_1:
case B_IVAR_2:
return b_helper_read_ivar(&iv->b, which, result);
default:
return EINVAL;
}
return 0;
}
Both of these work only if the A_IVAR values are disjoint from the =
B_IVAR values. Your mydev device is the one that sets the ivar on dev1, =
in your example, so it would know how to malloc and create the =
mydev_ivar structure, even if the interfaces it is implementing provide =
helper functions, like I've outlined above.
Warner
> - Arnaud
>=20
>> Warner
>>=20
>>> - Arnaud
>>>=20
>>>> The problem, more basically, is that the ivar keys are not unique. =
Currently, there's no bits used in the key to define the values to be =
non-overlapping. For example:
>>>> enum pci_device_ivars {
>>>> PCI_IVAR_SUBVENDOR,
>>>> PCI_IVAR_SUBDEVICE,
>>>> PCI_IVAR_VENDOR,
>>>> ....
>>>> };
>>>>=20
>>>> We could easily reserve the upper 16-bits of this field to be that =
key. This value could then be used to differentiate them. But this =
wouldn't scale too well. Given that there's only about a dozen or two =
in the tree, that's right at the moment, it wouldn't be hard to do =
something like:
>>>>=20
>>>> enum ivar_namespace {
>>>> IVAR_PCI =3D 1,
>>>> IVAR_PCCARD,
>>>> IVAR_USB,
>>>> etc
>>>> };
>>>> #define IVAR_SHIFT 16
>>>>=20
>>>> and the above could be changed to:
>>>>=20
>>>> enum pci_device_ivars {
>>>> PCI_IVAR_SUBVENDOR =3D IVAR_PCI << IVAR_SHIFT,
>>>> PCI_IVAR_SUBDEVICE,
>>>> PCI_IVAR_VENDOR,
>>>> ....
>>>> };
>>>>=20
>>>> and then we'd have an unambiguous key, and the bus could easily =
implement multiple interfaces.
>>>>=20
>>>> but then again, most of the existing interfaces in the kernel are =
mutually exclusive, so you could implement this just for your new =
interfaces.
>>>>=20
>>>>> Unless I am mistaken, ivar are the only way for a parent can =
transmit
>>>>> information to a child. I can not simply implement a new METHOD to =
get
>>>>> that ivar as the device implements multiple time the same function
>>>>> (actually, up to 4 time for one, 3 for the other, with possible
>>>>> crossovers...), each one physically distinct. Each child is being =
tied
>>>>> to a pair. Thus, I need to pass each child discriminator(s) for =
each
>>>>> interfaces right after having been *created*, which cannot be done
>>>>> later on. Of course, it is out-of-question to have crossover in =
the
>>>>> interfaces definitions.
>>>>=20
>>>> ivars are but one way to communicate this. However, they are the =
generic way to convert a key to a value and store a key on a value. I =
don't really understand what you are trying to say here, perhaps an =
example would help illustrate what you are trying to do, since I don't =
quite understand the problem here.
>>>>=20
>>>>> The best way I could achieve this currently is to pass the child's
>>>>> device to its parent, and do a lookup based on that pointer to get
>>>>> information I need, but erk....
>>>>=20
>>>> That doesn't make any sense. The child's parent already sets that =
child's ivar when the child is created. The child's parent already gets =
a pointer to the child when asked to do the key to value translation. =
Again, perhaps an example would help here.
>>>>=20
>>>> Warner
>>=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8048FFC5-6952-49FC-849D-EA1A5675ACBE>