On Fri, Nov 10, 2023 at 6:12 PM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Thu, Nov 9, 2023 at 5:52 AM Rob Herring <[email protected]> wrote:
> >
> > > > End of background from Doug's cover letter.
> > >
> > > I think that using "status" is not a good idea, I find that confusing.
> >
> > "status" is what defines a device's state in terms of enabled,
> > present, available. That's exactly what we're expressing here.
> >
> > Now, I do not think we should be mixing the device class (e.g.
> > touchscreen) into status. I said this on v1, but apparently that was
> > not listened to.
>
> Interesting. I must have missed the "don't mix device class into
> status" part. Do you have a link to your post about that? Maybe
> there's other stuff I missed... Having the device class stuck at the
> end there was at least part of my last post [1] which gathered no
> response.
https://lore.kernel.org/all/CAL_JsqKK0tjeXNv=a8L3k0AjhCa15XOq1tPWqVod9mycsKXJHg@mail.gmail.com/
"I would not combine the 2 things. Knowing the class/type of the device
may be useful independent of your problem."
> I think one of the reasons that I felt we needed to mux the device
> class into status was that it was going to make the code a lot less
> fragile. Everything I've seen indicates that you don't want us to
> create a "HW prober" node that could be used to provide relevant
> phandles for different classes of devices, so the "HW prober" code
> needs to either search through the whole device tree for a status of
> "failed-needs-probe" or needs to contain per-board, hardcoded,
> fully-qualified paths.
>
> I don't think we want to include hardcoded, fully-qualified paths in
> the code. That would mean that if someone changed a node name
> somewhere in the path to one of the devices that we're dealing with
> then it would break.
Right, nothing should depend on the full path. That's not an ABI just
like the device path in sysfs is not (despite what Android HALs do).
> So if we're searching the whole device tree for "failed-needs-probe"
> then we need to figure out which devices are related to each other. If
> a given board has second sources for MIPI panels, touchscreens, and
> trackpads then we need to know which of the "failed-needs-probe"
> devices are trackpads, which are touchscreens, and which are MIPI
> panels. Do you have any suggestions for how we should do that? Maybe
> it was in some other thread that I missed? I guess we could have a
> board-specific table mapping (compatible + node name + reg) to a
> class, but that feels awkward.
Node name is supposed to correspond to device class, so why not use
that (no path or unit-address.) and nothing else (well, besides
"status")?
Rob
Hi,
On Wed, Nov 15, 2023 at 2:28 PM Rob Herring <[email protected]> wrote:
>
> > So if we're searching the whole device tree for "failed-needs-probe"
> > then we need to figure out which devices are related to each other. If
> > a given board has second sources for MIPI panels, touchscreens, and
> > trackpads then we need to know which of the "failed-needs-probe"
> > devices are trackpads, which are touchscreens, and which are MIPI
> > panels. Do you have any suggestions for how we should do that? Maybe
> > it was in some other thread that I missed? I guess we could have a
> > board-specific table mapping (compatible + node name + reg) to a
> > class, but that feels awkward.
>
> Node name is supposed to correspond to device class, so why not use
> that (no path or unit-address.) and nothing else (well, besides
> "status")?
One problem is that I could imagine having two second source trackpads
that both have the same i2c address. That would give them the same
name, right? I guess you could maybe come up with some sort of suffix
rule? Like
trackpad-1@10 {
compatible = "elan,blah";
ret = <0x10>;
status = "failed-needs-probe";
...
}
trackpad-2@10 {
compatible = "goodix,gt7375p";
ret = <0x10>;
status = "failed-needs-probe";
...
}
Then I guess the class would be "trackpad"?
-Doug
On Wed, Nov 15, 2023 at 2:45 PM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Wed, Nov 15, 2023 at 2:28 PM Rob Herring <[email protected]> wrote:
> >
> > > So if we're searching the whole device tree for "failed-needs-probe"
> > > then we need to figure out which devices are related to each other. If
> > > a given board has second sources for MIPI panels, touchscreens, and
> > > trackpads then we need to know which of the "failed-needs-probe"
> > > devices are trackpads, which are touchscreens, and which are MIPI
> > > panels. Do you have any suggestions for how we should do that? Maybe
> > > it was in some other thread that I missed? I guess we could have a
> > > board-specific table mapping (compatible + node name + reg) to a
> > > class, but that feels awkward.
> >
> > Node name is supposed to correspond to device class, so why not use
> > that (no path or unit-address.) and nothing else (well, besides
> > "status")?
>
> One problem is that I could imagine having two second source trackpads
> that both have the same i2c address. That would give them the same
> name, right? I guess you could maybe come up with some sort of suffix
> rule? Like
>
> trackpad-1@10 {
> compatible = "elan,blah";
> ret = <0x10>;
> status = "failed-needs-probe";
> ...
> }
> trackpad-2@10 {
> compatible = "goodix,gt7375p";
> ret = <0x10>;
> status = "failed-needs-probe";
> ...
> }
>
> Then I guess the class would be "trackpad"?
That issue is somewhat orthogonal because it is not following the spec.
I'm not sure mixing the 2 styles of node names is a good idea. While
not used too much, matching by node name does ignore the unit-address,
but I'm not sure we could ignore a '-N'.
I think our options are either add something to the unit-address or
use i2c-mux binding. Adding to the unit-address is not unprecedented.
I did that for some of the register bit level bindings where you have
a node for different bits at the same address. The downside is
unit-address is bus specific, so we'd have to add that for multiple
buses. For the i2c-mux, it's perhaps a bit complex and I'm not sure
what if anything you'd have to do to manage the mux that's not really
there.
Rob
Hi,
On Wed, Nov 15, 2023 at 4:35 PM Rob Herring <[email protected]> wrote:
>
> On Wed, Nov 15, 2023 at 2:45 PM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Wed, Nov 15, 2023 at 2:28 PM Rob Herring <[email protected]> wrote:
> > >
> > > > So if we're searching the whole device tree for "failed-needs-probe"
> > > > then we need to figure out which devices are related to each other. If
> > > > a given board has second sources for MIPI panels, touchscreens, and
> > > > trackpads then we need to know which of the "failed-needs-probe"
> > > > devices are trackpads, which are touchscreens, and which are MIPI
> > > > panels. Do you have any suggestions for how we should do that? Maybe
> > > > it was in some other thread that I missed? I guess we could have a
> > > > board-specific table mapping (compatible + node name + reg) to a
> > > > class, but that feels awkward.
> > >
> > > Node name is supposed to correspond to device class, so why not use
> > > that (no path or unit-address.) and nothing else (well, besides
> > > "status")?
> >
> > One problem is that I could imagine having two second source trackpads
> > that both have the same i2c address. That would give them the same
> > name, right? I guess you could maybe come up with some sort of suffix
> > rule? Like
> >
> > trackpad-1@10 {
> > compatible = "elan,blah";
> > ret = <0x10>;
> > status = "failed-needs-probe";
> > ...
> > }
> > trackpad-2@10 {
> > compatible = "goodix,gt7375p";
> > ret = <0x10>;
> > status = "failed-needs-probe";
> > ...
> > }
> >
> > Then I guess the class would be "trackpad"?
>
> That issue is somewhat orthogonal because it is not following the spec.
I'm not sure why you say it's orthogonal. The whole reason why we get
into the situation above is that we could have two devices, only one
of which is present (hence the status of "failed-needs-probe"), that
are the same type and have the same "reg" address.
Essentially the whole "failed-needs-probe" is extending the spec,
right? While extending the spec, we also need to talk about what to do
if some of the devices that we need to probe have the same class and
the same "reg".
> I'm not sure mixing the 2 styles of node names is a good idea. While
> not used too much, matching by node name does ignore the unit-address,
> but I'm not sure we could ignore a '-N'.
>
> I think our options are either add something to the unit-address or
> use i2c-mux binding. Adding to the unit-address is not unprecedented.
> I did that for some of the register bit level bindings where you have
> a node for different bits at the same address. The downside is
> unit-address is bus specific, so we'd have to add that for multiple
> buses. For the i2c-mux, it's perhaps a bit complex and I'm not sure
> what if anything you'd have to do to manage the mux that's not really
> there.
Somehow it feels weird to use an i2c-mux because there's no real mux
present, right? ...so this would be a virtual (bogus) device that
doesn't really exist in hardware.
...though I guess if the "mux" type binding is OK then maybe we just
use that as the HW prober, at least for i2c devices...
-Doug
On Thu, Nov 16, 2023 at 5:35 AM Rob Herring <[email protected]> wrote:
>
> On Wed, Nov 15, 2023 at 2:45 PM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Wed, Nov 15, 2023 at 2:28 PM Rob Herring <[email protected]> wrote:
> > >
> > > > So if we're searching the whole device tree for "failed-needs-probe"
> > > > then we need to figure out which devices are related to each other. If
> > > > a given board has second sources for MIPI panels, touchscreens, and
> > > > trackpads then we need to know which of the "failed-needs-probe"
> > > > devices are trackpads, which are touchscreens, and which are MIPI
> > > > panels. Do you have any suggestions for how we should do that? Maybe
> > > > it was in some other thread that I missed? I guess we could have a
> > > > board-specific table mapping (compatible + node name + reg) to a
> > > > class, but that feels awkward.
> > >
> > > Node name is supposed to correspond to device class, so why not use
> > > that (no path or unit-address.) and nothing else (well, besides
> > > "status")?
> >
> > One problem is that I could imagine having two second source trackpads
> > that both have the same i2c address. That would give them the same
> > name, right? I guess you could maybe come up with some sort of suffix
> > rule? Like
> >
> > trackpad-1@10 {
> > compatible = "elan,blah";
> > ret = <0x10>;
> > status = "failed-needs-probe";
> > ...
> > }
> > trackpad-2@10 {
> > compatible = "goodix,gt7375p";
> > ret = <0x10>;
> > status = "failed-needs-probe";
> > ...
> > }
> >
> > Then I guess the class would be "trackpad"?
>
> That issue is somewhat orthogonal because it is not following the spec.
>
> I'm not sure mixing the 2 styles of node names is a good idea. While
> not used too much, matching by node name does ignore the unit-address,
> but I'm not sure we could ignore a '-N'.
of_node_name_prefix() solves that. I assume that's the intended use case?
> I think our options are either add something to the unit-address or
> use i2c-mux binding. Adding to the unit-address is not unprecedented.
> I did that for some of the register bit level bindings where you have
> a node for different bits at the same address. The downside is
> unit-address is bus specific, so we'd have to add that for multiple
> buses. For the i2c-mux, it's perhaps a bit complex and I'm not sure
> what if anything you'd have to do to manage the mux that's not really
> there.
>
> Rob
On Thu, Nov 16, 2023 at 6:14 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Wed, Nov 15, 2023 at 4:35 PM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Nov 15, 2023 at 2:45 PM Doug Anderson <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Nov 15, 2023 at 2:28 PM Rob Herring <[email protected]> wrote:
> > > >
> > > > > So if we're searching the whole device tree for "failed-needs-probe"
> > > > > then we need to figure out which devices are related to each other. If
> > > > > a given board has second sources for MIPI panels, touchscreens, and
> > > > > trackpads then we need to know which of the "failed-needs-probe"
> > > > > devices are trackpads, which are touchscreens, and which are MIPI
> > > > > panels. Do you have any suggestions for how we should do that? Maybe
> > > > > it was in some other thread that I missed? I guess we could have a
> > > > > board-specific table mapping (compatible + node name + reg) to a
> > > > > class, but that feels awkward.
> > > >
> > > > Node name is supposed to correspond to device class, so why not use
> > > > that (no path or unit-address.) and nothing else (well, besides
> > > > "status")?
> > >
> > > One problem is that I could imagine having two second source trackpads
> > > that both have the same i2c address. That would give them the same
> > > name, right? I guess you could maybe come up with some sort of suffix
> > > rule? Like
> > >
> > > trackpad-1@10 {
> > > compatible = "elan,blah";
> > > ret = <0x10>;
> > > status = "failed-needs-probe";
> > > ...
> > > }
> > > trackpad-2@10 {
> > > compatible = "goodix,gt7375p";
> > > ret = <0x10>;
> > > status = "failed-needs-probe";
> > > ...
> > > }
> > >
> > > Then I guess the class would be "trackpad"?
> >
> > That issue is somewhat orthogonal because it is not following the spec.
>
> I'm not sure why you say it's orthogonal. The whole reason why we get
> into the situation above is that we could have two devices, only one
> of which is present (hence the status of "failed-needs-probe"), that
> are the same type and have the same "reg" address.
>
> Essentially the whole "failed-needs-probe" is extending the spec,
> right? While extending the spec, we also need to talk about what to do
> if some of the devices that we need to probe have the same class and
> the same "reg".
If it's at the same address, then you would need some device specific
probing code, like reading a register back and checking if matches
some known value. This could be done by passing in some kind of table,
but it is by no means as generic as checking if an address responds.
This is what we have on MT8186 Tentacruel. We do have the ability to
differentiate using the SKU ID for the same address case, but I'm happy
to try other methods if it helps everyone else.
ChenYu
> > I'm not sure mixing the 2 styles of node names is a good idea. While
> > not used too much, matching by node name does ignore the unit-address,
> > but I'm not sure we could ignore a '-N'.
> >
> > I think our options are either add something to the unit-address or
> > use i2c-mux binding. Adding to the unit-address is not unprecedented.
> > I did that for some of the register bit level bindings where you have
> > a node for different bits at the same address. The downside is
> > unit-address is bus specific, so we'd have to add that for multiple
> > buses. For the i2c-mux, it's perhaps a bit complex and I'm not sure
> > what if anything you'd have to do to manage the mux that's not really
> > there.
>
> Somehow it feels weird to use an i2c-mux because there's no real mux
> present, right? ...so this would be a virtual (bogus) device that
> doesn't really exist in hardware.
>
> ...though I guess if the "mux" type binding is OK then maybe we just
> use that as the HW prober, at least for i2c devices...
>
> -Doug
On Wed, Nov 15, 2023 at 05:13:50PM -0500, Doug Anderson wrote:
> Hi,
>
> On Wed, Nov 15, 2023 at 4:35 PM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Nov 15, 2023 at 2:45 PM Doug Anderson <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Nov 15, 2023 at 2:28 PM Rob Herring <[email protected]> wrote:
> > > >
> > > > > So if we're searching the whole device tree for "failed-needs-probe"
> > > > > then we need to figure out which devices are related to each other. If
> > > > > a given board has second sources for MIPI panels, touchscreens, and
> > > > > trackpads then we need to know which of the "failed-needs-probe"
> > > > > devices are trackpads, which are touchscreens, and which are MIPI
> > > > > panels. Do you have any suggestions for how we should do that? Maybe
> > > > > it was in some other thread that I missed? I guess we could have a
> > > > > board-specific table mapping (compatible + node name + reg) to a
> > > > > class, but that feels awkward.
> > > >
> > > > Node name is supposed to correspond to device class, so why not use
> > > > that (no path or unit-address.) and nothing else (well, besides
> > > > "status")?
> > >
> > > One problem is that I could imagine having two second source trackpads
> > > that both have the same i2c address. That would give them the same
> > > name, right? I guess you could maybe come up with some sort of suffix
> > > rule? Like
> > >
> > > trackpad-1@10 {
> > > compatible = "elan,blah";
> > > ret = <0x10>;
> > > status = "failed-needs-probe";
> > > ...
> > > }
> > > trackpad-2@10 {
> > > compatible = "goodix,gt7375p";
> > > ret = <0x10>;
> > > status = "failed-needs-probe";
> > > ...
> > > }
> > >
> > > Then I guess the class would be "trackpad"?
> >
> > That issue is somewhat orthogonal because it is not following the spec.
>
> I'm not sure why you say it's orthogonal. The whole reason why we get
> into the situation above is that we could have two devices, only one
> of which is present (hence the status of "failed-needs-probe"), that
> are the same type and have the same "reg" address.
I just mean that defining a node name for 2 devices of the same class
and at same address is separate problem from selecting which one to
enable.
>
> Essentially the whole "failed-needs-probe" is extending the spec,
> right? While extending the spec, we also need to talk about what to do
> if some of the devices that we need to probe have the same class and
> the same "reg".
>
>
> > I'm not sure mixing the 2 styles of node names is a good idea. While
> > not used too much, matching by node name does ignore the unit-address,
> > but I'm not sure we could ignore a '-N'.
> >
> > I think our options are either add something to the unit-address or
> > use i2c-mux binding. Adding to the unit-address is not unprecedented.
> > I did that for some of the register bit level bindings where you have
> > a node for different bits at the same address. The downside is
> > unit-address is bus specific, so we'd have to add that for multiple
> > buses. For the i2c-mux, it's perhaps a bit complex and I'm not sure
> > what if anything you'd have to do to manage the mux that's not really
> > there.
>
> Somehow it feels weird to use an i2c-mux because there's no real mux
> present, right? ...so this would be a virtual (bogus) device that
> doesn't really exist in hardware.
Yes, shrug...
> ...though I guess if the "mux" type binding is OK then maybe we just
> use that as the HW prober, at least for i2c devices...
Except for any other case where it's different addresses or different
buses. Maybe different addresses could still use the same thing. I
suppose the same class of device is unlikely to be on a different bus.
An extension to the unit-address would be the simpler solution, so we
should explore that first I think.
Rob