Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: i3c: npcx: introduce I3C driver #71366

Merged
merged 7 commits into from Apr 24, 2024

Conversation

alvsun
Copy link
Contributor

@alvsun alvsun commented Apr 11, 2024

This implements basic driver to utilize the I3C IP block on NPCX.

  1. I3C mode: Main controller mode only.
  2. Transfer: Support SDR only.
  3. IBI: Support Hot-Join, IBI(MDB). Controller request is not supported.
  4. Support 3 I3C modules: I3C1(3.3V), I3C2(1.8V, espi mode), (I3C3 1.8V or 3.3V)

Copy link

Hello @alvsun, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@alvsun
Copy link
Contributor Author

alvsun commented Apr 11, 2024

This PR depends on #70932.
After #70932 is merged into ToT, will begin the review.

@dcpleung
Copy link
Member

Could you split this into separate commits? I can see the I3C driver change, clock control change, and SoC/board change.

@alvsun
Copy link
Contributor Author

alvsun commented Apr 17, 2024

Could you split this into separate commits? I can see the I3C driver change, clock control change, and SoC/board change.

Done

dcpleung
dcpleung previously approved these changes Apr 17, 2024
drivers/i3c/i3c_common.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
@XenuIsWatching
Copy link
Member

XenuIsWatching commented Apr 21, 2024

With #70773 now merged... you'll need to add the compat macro to

DT_FOREACH_STATUS_OKAY(nxp_mcux_i3c, I3C_CTRL_FN)
and
DT_FOREACH_STATUS_OKAY(cdns_i3c, I3C_CTRL_LIST_ENTRY)
in a separate commit

@dcpleung dcpleung assigned MulinChao and ChiHuaL and unassigned dcpleung Apr 22, 2024
1. The only valid values of MCLKD clock frequency
   are between 40Mhz to 50Mhz.
2. If DMA is used, the APB4_CLK clock frequency must
   be equal to or higher than 20Mhz.

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
Add I3C device nodes.

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
Add I3C pinmux configuration.

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
During DAA, the responding device might not be in the device list.
This CL adds target device descriptor's pointer checking to prevent
getting unexpected results.

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
@alvsun alvsun force-pushed the npcx_i3c branch 2 times, most recently from f3fe722 to 6dd550f Compare April 23, 2024 09:12
@alvsun
Copy link
Contributor Author

alvsun commented Apr 23, 2024

With #70773 now merged... you'll need to add the compat macro to

DT_FOREACH_STATUS_OKAY(nxp_mcux_i3c, I3C_CTRL_FN)

and

DT_FOREACH_STATUS_OKAY(cdns_i3c, I3C_CTRL_LIST_ENTRY)

in a separate commit

Done (6dd550f)

dts/arm/nuvoton/npcx/npcx4.dtsi Outdated Show resolved Hide resolved
drivers/i3c/i3c_npcx.c Outdated Show resolved Hide resolved
This implements basic driver to utilize the I3C IP block
on NPCX.

1. I3C mode: Main controller mode only.
2. Transfer: Support SDR only.
3. IBI: Support Hot-Join, IBI(MDB).
   Controller request is not supported.
4. Support 3 I3C modules:
   I3C1(3.3V), I3C2(1.8V, espi mode), (I3C3 1.8V or 3.3V)

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
As titile.

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
Add /* zephyr-keep-sorted-start */ around DT_FOREACH_STATUS_OKAY.

Signed-off-by: Alvis Sun <yfsun@nuvoton.com>
Copy link
Collaborator

@MulinChao MulinChao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Comment on lines +1265 to +1282
/* Write data (defining byte or data bytes) for CCC if needed */
if (payload->ccc.data_len > 0) {
npcx_i3c_status_clear_all(inst);
npcx_i3c_errwarn_clear_all(inst);
xfered_len = npcx_i3c_xfer_write_fifo(inst, payload->ccc.data,
payload->ccc.data_len, false);
if (xfered_len < 0) {
LOG_ERR("CCC[0x%02x] %s command payload error (%d)", payload->ccc.id,
i3c_ccc_is_payload_broadcast(payload) ? "broadcast" : "direct",
ret);
ret = xfered_len;

goto out_do_ccc;
}

/* Write back the transferred bytes */
payload->ccc.num_xfer = xfered_len;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this distinguish the difference between the defining byte and payload for the direct CCC in I3C v1.1?

Copy link
Contributor Author

@alvsun alvsun Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the i3c_ccc APIs can tell the difference (e.g., i3c_ccc_do_getstatus()).
This part handles both broadcast and direct CCC write data operations in the earlier stage (marked with red color in the picture below).

image

@fabiobaltieri fabiobaltieri merged commit 629bfd9 into zephyrproject-rtos:main Apr 24, 2024
21 checks passed
Copy link

Hi @alvsun!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Clock Control area: Devicetree Binding PR modifies or adds a Device Tree binding area: I3C platform: Nuvoton NPCX Nuvoton NPCX platform: Nuvoton Numicro Numaker Nuvoton Technology Corporation, Numicro Numaker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants