drivers: misc: adi-axi-tdd: Fix reg values in ms #2603
+18
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
podgori
requested review from
nunojsa,
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
nunojsa
reviewed
Oct 1, 2024
| #define ADI_REG_TDD_SYNC_COUNTER_LOW 0x0058 | ||
| #define ADI_REG_TDD_SYNC_COUNTER_HIGH 0x005c | ||
| #define ADI_REG_TDD_SYNC_PERIOD_LOW 0x0058 | ||
| #define ADI_REG_TDD_SYNC_PERIOD_HIGH 0x005c |
There was a problem hiding this comment.
Why changing the name? If it makes more sense to have the new name please mention it in the commit message to avoid comments like this. Typically commits are small and address only one issue. Hence, if you#re doing some additional thing (even if small like this) please mention it. A couple of issues with your commit:
- Run
git log --no-merges --oneline drivers/miscto see the used commit title in that dir. It seems to be a bit random but I would say the most used would be justmisc: adi-axi-tdd: .... That said, what you have seems to be acceptable so up to you; - You should describe and explain what you're fixing in the commit message. It helps in understanding what was the issue in the code (other than having to understand by looking at the code);
- You could also add a Fixes: tag. Not that important for ADI tree but it's a matter of keeping the same "routines" as upstream so it's easier then to send patches upstream.
drivers/misc/adi-axi-tdd.c
Outdated
| @@ -233,6 +233,7 @@ static ssize_t adi_axi_tdd_show(struct device *dev, | |||
| ret = regmap_read(st->regs, ADI_REG_TDD_STARTUP_DELAY, &data); | |||
| if (ret) | |||
| return ret; | |||
| if (data > 0) data = data + 1; | |||
There was a problem hiding this comment.
please do:
if (data)
data++;
Same for the other places
The raw register values of STARTUP_DELAY, FRAME_LENGTH and SYNC_PERIOD are expressed in clock cycles-1. In order to achieve a desired length, their actual value must be length-1. The 'SYNC_COUNTER' registers were renamed to 'SYNC_PERIOD' for a better understanding of the functionality. Fixes: 4519bcc ("drivers: misc: adi-axi-tdd: Add TDD engine") Signed-off-by: Ionut Podgoreanu <ionut.podgoreanu@analog.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
Fixed the computed values in [ms] for the following TDDN registers: STARTUP_DELAY, FRAME_LENGTH, SYNC_PERIOD (renamed from SYNC_COUNTER in the hdl documentation).
PR Type
PR Checklist