zephyr
zephyr copied to clipboard
zephyrproject-rtos
dts: Move pca95xx device from I2C0 to I2C1 on mec15xxevb
The on board device pca95xx is connected to I2C1, not I2C0
Fixes: #48071
Signed-off-by: Hu Zhenyu [email protected]
Do we have any datasheet to point for this, or was it discovered by trial and error?
Do we have any datasheet to point for this, or was it discovered by trial and error?
The datasheet is here: https://github.com/MicrochipTech/CPGZephyrDocs/tree/master/MEC152x I know your concern, the schematic shows the pca95xx is supposed to connect to I2C0. But I2C0 is used for UART logging. So we move pca95xx to I2C1. You can find the detailed description of this in tests/boards/mec15xxevb_assy6853/i2c_api/README.txt
Do we have any datasheet to point for this, or was it discovered by trial and error?
The datasheet is here: https://github.com/MicrochipTech/CPGZephyrDocs/tree/master/MEC152x I know your concern, the schematic shows the pca95xx is supposed to connect to I2C0. But I2C0 is used for UART logging. So we move pca95xx to I2C1. You can find the detailed description of this in tests/boards/mec15xxevb_assy6853/i2c_api/README.txt
Shouldn't we then use an overlay on the test only? As the test requires the extra effort?
Do we have any datasheet to point for this, or was it discovered by trial and error?
The datasheet is here: https://github.com/MicrochipTech/CPGZephyrDocs/tree/master/MEC152x I know your concern, the schematic shows the pca95xx is supposed to connect to I2C0. But I2C0 is used for UART logging. So we move pca95xx to I2C1. You can find the detailed description of this in tests/boards/mec15xxevb_assy6853/i2c_api/README.txt
Shouldn't we then use an overlay on the test only? As the test requires the extra effort?
I think it is OK to modify under board directory. As for MEC152x, pca95xx connected to I2C0 also needs a jumper, same as I2C1. To use the board, the UART is a must, it is highly probable that pca95xx is connecting to I2C1. Do you agree? If you insist to modify in overlay, I'll push a new patch.
Rebased to the latest. Tested OK on mec15. Got some CI failures which does not related to this change.
Do we have any datasheet to point for this, or was it discovered by trial and error?
The datasheet is here: https://github.com/MicrochipTech/CPGZephyrDocs/tree/master/MEC152x I know your concern, the schematic shows the pca95xx is supposed to connect to I2C0. But I2C0 is used for UART logging. So we move pca95xx to I2C1. You can find the detailed description of this in tests/boards/mec15xxevb_assy6853/i2c_api/README.txt
Shouldn't we then use an overlay on the test only? As the test requires the extra effort?
I think it is OK to modify under board directory. As for MEC152x, pca95xx connected to I2C0 also needs a jumper, same as I2C1. To use the board, the UART is a must, it is highly probable that pca95xx is connecting to I2C1. Do you agree? If you insist to modify in overlay, I'll push a new patch.
To make it board level, I think that the jumper configurations described at tests/boards/mec15xxevb_assy6853/i2c_api/README.txt should be moved to https://docs.zephyrproject.org/latest/boards/arm/mec15xxevb_assy6853/doc/index.html, don't you agree?
Do we have any datasheet to point for this, or was it discovered by trial and error?
The datasheet is here: https://github.com/MicrochipTech/CPGZephyrDocs/tree/master/MEC152x I know your concern, the schematic shows the pca95xx is supposed to connect to I2C0. But I2C0 is used for UART logging. So we move pca95xx to I2C1. You can find the detailed description of this in tests/boards/mec15xxevb_assy6853/i2c_api/README.txt
Shouldn't we then use an overlay on the test only? As the test requires the extra effort?
I think it is OK to modify under board directory. As for MEC152x, pca95xx connected to I2C0 also needs a jumper, same as I2C1. To use the board, the UART is a must, it is highly probable that pca95xx is connecting to I2C1. Do you agree? If you insist to modify in overlay, I'll push a new patch.
To make it board level, I think that the jumper configurations described at
tests/boards/mec15xxevb_assy6853/i2c_api/README.txtshould be moved to https://docs.zephyrproject.org/latest/boards/arm/mec15xxevb_assy6853/doc/index.html, don't you agree?
Agree. The documents will be updated later. For the PR, it does not need to be changed, right?
Do we have any datasheet to point for this, or was it discovered by trial and error?
The datasheet is here: https://github.com/MicrochipTech/CPGZephyrDocs/tree/master/MEC152x I know your concern, the schematic shows the pca95xx is supposed to connect to I2C0. But I2C0 is used for UART logging. So we move pca95xx to I2C1. You can find the detailed description of this in tests/boards/mec15xxevb_assy6853/i2c_api/README.txt
Shouldn't we then use an overlay on the test only? As the test requires the extra effort?
I think it is OK to modify under board directory. As for MEC152x, pca95xx connected to I2C0 also needs a jumper, same as I2C1. To use the board, the UART is a must, it is highly probable that pca95xx is connecting to I2C1. Do you agree? If you insist to modify in overlay, I'll push a new patch.
To make it board level, I think that the jumper configurations described at
tests/boards/mec15xxevb_assy6853/i2c_api/README.txtshould be moved to https://docs.zephyrproject.org/latest/boards/arm/mec15xxevb_assy6853/doc/index.html, don't you agree?Agree. The documents will be updated later. For the PR, it does not need to be changed, right?
If it's a simple enough update to the document, why not? Otherwise there will be a time that the documentation doesn't really match the code...
If it's a simple enough update to the document, why not? Otherwise there will be a time that the documentation doesn't really
match the code...
Well, it is not a simple work. In https://docs.zephyrproject.org/latest/boards/arm/mec15xxevb_assy6853/doc/index.htm, the board looks clean, but actually we have added dozens of wires on it. We have already made a plan to update it in Q3. To my understanding, this document is a minimal subset for making MEC15 to work, eg run a "hello world" demo. It does not contain the part to make every componment to work. In this PR, it looks like pca95xx works on I2C0 by default, as they are near. But actually it is not. To achieve this, you need to connect the JP44 pin1,3 to JP44 pin2,4 pin. But the JP44 is not show up in the document. So this document cannot cover the test case from the very beginning. To change the connection from I2C0 to I2C1 is simple, we just need to connect the JP44 pin 1,3 to I2C1, eg.JP25 pin 21,23.
If it's a simple enough update to the document, why not? Otherwise there will be a time that the documentation doesn't really match the code...Well, it is not a simple work. In https://docs.zephyrproject.org/latest/boards/arm/mec15xxevb_assy6853/doc/index.htm, the board looks clean, but actually we have added dozens of wires on it. We have already made a plan to update it in Q3. To my understanding, this document is a minimal subset for making MEC15 to work, eg run a "hello world" demo. It does not contain the part to make every componment to work. In this PR, it looks like pca95xx works on I2C0 by default, as they are near. But actually it is not. To achieve this, you need to connect the JP44 pin1,3 to JP44 pin2,4 pin. But the JP44 is not show up in the document. So this document cannot cover the test case from the very beginning. To change the connection from I2C0 to I2C1 is simple, we just need to connect the JP44 pin 1,3 to I2C1, eg.JP25 pin 21,23.
Ok, to fix the picture can be more work. But why not a simple note stating the need to set those jumpers and why (basically what you just mentioned)? After this patch, one would have to dig this information, so why not make it readily available?
@edersondisouza
https://github.com/zephyrproject-rtos/zephyr/pull/49097 is created for the document update. Thanks to review.
@edersondisouza
#49097 is created for the document update. Thanks to review.
It didn't need to be a new PR, just a new patch in this PR. But as long as both land together, it shall be fine.