(first of all, what follows is just my opinion, I think it contains useful
advices and I hope it is heard in the general interest of the NuttX project, but
all decisions regarding the NuttX architecture belong to Greg).
I am not sure why you need a "i2cmux lower half". I would have expected this to
be integrated directly in the mux device driver. This looks like a very thin
layer added on top of the mux device.
Since this mux device will be instantiated in the board config, you can just add a :
int pca9540bdp_lower_half(FAR struct i2cmultiplexer_dev_s* i2cmux, FAR struct
i2c_master_s** vi2c, uint8_t port)
to your pca driver. I think that it's okay to do that, and that it saves you a
useless lower half driver.
Letting the user register his own /dev entries for the virtual bus is a good
thing, but I cannot understand why you need a dev entry for the mux itself...
this entry is far from necessary, I would have made it optional and I hope Greg
will require that.
Pursuing my MTD similarity: When you create MTD partitions, you need to invoke a
function from the "mtd partition driver" that creates a new MTD partition from a
parent MTD device, then you can register these partitions with the FTL layer or
something else. But the "MTD partition driver" that makes the partitions never
needs a /dev entry by itself.
Thats why the open/close/write routines of this character driver are empty. The
config can be made available to other parts of the system as an ioctl operation
of the i2c_master_s instances. I would prefer adding an optional ioctl interface
to i2c_master_s operations than use this design as-is. It is much more clean
imho. I can help with reviewing such patches, that would affect other i2c master
drivers (just the initialization of the ioctl entries to null).
Also, what is the goal ofÂ PCA9540BDPIOC_SEL_PORTin this driver ? I would have
thought that port switching was automatic? Is there a need to manually switch
the ports? This will just confuse the devices on the virtual ports.
But that's a very good start. Let's polish this.
Post by ***@robodev.eu [nuttx]
I have now implemented the suggested i2c multiplexer abstraction as described
files:Â Â https://bitbucket.org/ordsen/nuttx/src/77557554d22f6316f37c7b7a37c7e862c9b9cb2a/drivers/i2c/?at=feature-pca9540bdp-i2cmultiplexer
* I putÂ i2cmux_lower_half(..) ini2c_master.h (in accordance with
gpio.h/gpio_lower_half() ), if this is not okay please let me know; I can
also move it to a separate file.
* Virtual i2c busses are not automatically registered as /dev/i2cN. Instead,
an i2c_master_s pointer passed to i2cmux_lower_half(..)Â instance is set to
the virtual i2c_master_s instance; registering the virtual bus is left to
the initialization process
E.g. our initialization now looks something like
And this is the i2c sensor driver used with the
mux:Â https://bitbucket.org/ordsen/nuttx/src/8f2e6918cd55f47a804129794dea0eed4dfd2bc1/drivers/sensors/vcnl4040.c?at=feature-vcnl4040-driver&fileviewer=file-view-defaultÂ (usage
example in apps folder)
But you should be able to used it with any other existing i2c sensor driver
without changing the sensor driver.
I will create a PR for the mux interface, the pca9540bdp mux and the vcnl4040
sensor in the days to come.Â
Your thoughts on my code are appreciated!