pca954x: harmonize pca9547 and pca954x and add pca9540 support

[bz]

Summary

The two implementations for the pca9548 switch and the pca9547 mux
seemed close enough so we can put them together and with a bit more
abstraction add pca9540 support.

While here apply a bit of consistency in variable and driver naming and
use device_has_property instead of the FDT-only OF_ variant.

This disconnects pca9547 from the build but does not yet delete it.

Test Plan

I cannot test either pca9547 or pca9548. And frankly I did this by
visual inspection and not much thinking in order to not add yet another
almost similar file for the pca9540.

Would be great if you could test things still work as expected in addition
to reviewing.

[mmel]

sys/dev/iicbus/mux/pca954x.c

158

I don't think it should be #ifdefed. The "okay" test is obviously missing in the original probe method.

[bz]

sys/dev/iicbus/mux/pca954x.c

158

Is that status inherited? I never saw a '''status = "okay"''' inside the '''i2c-switch''' blocks, only in the upper '''i2c'' node.

[imp]

sys/dev/iicbus/mux/pca954x.c

12

this drops a comma.

158

Generally only direct children of ofwbus/fdtbus need to check this.

Also, I have a huge patch set I've been rebasing forward for years that hoists this check into the bus code and now seems like a good time to solicit comments on that :)

[bz]

sys/dev/iicbus/mux/pca954x.c

12

Given this got merged from two files, ... also the comma is not in /usr/share/etc/bsd-style-copyright . I can add this back is needed but eventually we'll need to align this "templates" (also with the one for the SPDX tag)

158

Right but please on a separate review... I'll just remove the #if 0 block..., right?

[bz]

remove the #if 0 block for status valid check.

[avg]

I think that this is a very good change. Some minor code style / improvement suggestions from me.
Thank you.

sys/dev/iicbus/mux/pca954x.c

120

Style nit: I'd add an empty line before the multi-line comment.

133–141

Can this be "collapsed" into 'else if'?
Also, there is no default /catch-all else clause... maybe it's worthwhile adding it with some error and diagnostic? Or panic?

137

I think that what should be asserted here is that en is a power of 2, not just an even number.
Also, if en == 0 is allowed here, then this clause would cover the next clause as well.

145

What does the XXX comment suggest here?
It wasn't clear to me.
I do not see how anything from iic_reqbus_data::flags could affect iic_msg::flags.

[avg]

sys/dev/iicbus/mux/pca954x.c

158

I think that adding the okay status was a correct change.
A mux / switch is a device tree node, so it can be disabled via its status property.
This was an oversight in my code.

[bz]

sys/dev/iicbus/mux/pca954x.c

137

I cannot follow the en == 0 logic as

busidx != (1 << busdix);

I am probably misunderstanding here.

I'll do the power2 check though.

[bz]

Correct all but on coments from avg; KASSERT to be discussed still. I didn't understand.

[avg]

sys/dev/iicbus/mux/pca954x.c

137

Apologies, I misread the code.

[bz]

Does that mean we are good now (again)? Sorry the approved went away with the other corrections.

[avg]

One final super-small nit: I am not sure what style(9) says about it, but my personal preference is that if one block of an if-else chain needs curly braces then all blocks in the chain should have them.
To my eye, that looks prettier.
But feel free to ignore this if it looks uglier or just the same to you.

Thank you!

[bz]

sys/dev/iicbus/mux/pca954x.c

139

Added { } here and below for consistency.

159

and this has become a return (NULL); c&p error.