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

pl35x-nand-controller: Enable dynamically setting clk rate in NAND. #210

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

DeborahOoi96
Copy link

@DeborahOoi96 DeborahOoi96 commented Mar 6, 2025

NI Zynq based devices require different nand memclk frequencies to be set at different timing modes, as seen in the bluefin branch of ni-zynq.dtsi.
This commit enables the memclk rate to be set according to the specifications in the device tree. If the device tree does not have this memclk-timing-frequency property specified, the driver proceeds as usual.

WI: AB#3024184

Testing:

Tested my changes on cDAQ9189, NAND timing mode was able to be set successfully and NAND device was detected.

image

Tested with memclk-timing-frequency property not set in device tree and we get the error as expected. This printk error message is not part of the commit, just there for my own checking. Purposely not including it in the commit as it is not actually an error if the device does not have that property.
image

Tested with passing a non-existent timing mode (99) into the timing mode parameter of of_get_memclk_freq. Got the expected error.
image

@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch 3 times, most recently from 03932ee to 893f109 Compare March 6, 2025 09:25
@gratian gratian requested a review from a team March 7, 2025 01:30
Copy link
Contributor

@chaitu236 chaitu236 left a comment

Choose a reason for hiding this comment

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

image

nit: I don't think tab/spacing in commit description is required.

@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from 893f109 to 2e83d53 Compare March 10, 2025 06:31
@DeborahOoi96 DeborahOoi96 requested a review from chaitu236 March 10, 2025 06:34
@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from 2e83d53 to 7e9b5f8 Compare March 11, 2025 10:21
@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from 7e9b5f8 to 2347fd1 Compare March 12, 2025 07:39
@DeborahOoi96 DeborahOoi96 requested a review from chaitu236 March 12, 2025 07:41
@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from 2347fd1 to 64c437d Compare March 12, 2025 08:11
Copy link
Contributor

@chaitu236 chaitu236 left a comment

Choose a reason for hiding this comment

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

Your PR description only mentions testing for "happy path".

Did you also test all the possible failure paths in this code?

@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from 64c437d to d52ed6e Compare March 13, 2025 06:34
@DeborahOoi96
Copy link
Author

Your PR description only mentions testing for "happy path".

Did you also test all the possible failure paths in this code?

Yes I did. Updated the PR description with all the test cases and their outcomes.

@DeborahOoi96 DeborahOoi96 requested a review from chaitu236 March 13, 2025 06:35
@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from d52ed6e to 4549e35 Compare March 14, 2025 06:47
NI Zynq based devices require different clk frequencies to be set at
different timing modes. Enable the memclk rate to be set according
to the specifications in the device tree. If the device tree does
not have this memclk-timing-frequency property specified, the driver
proceeds as usual.

Signed-off-by: deooi <[email protected]>
@DeborahOoi96 DeborahOoi96 force-pushed the deooi/NAND-timing-modes branch from 4549e35 to b90cd60 Compare March 14, 2025 06:51
@gratian gratian merged commit ea79ee4 into ni:nilrt/master/6.6 Mar 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants