-
Notifications
You must be signed in to change notification settings - Fork 0
Virgo soc driver atcdmac100 #7
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
Conversation
rahul-r-shah-pub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes and some things to discuss
| examples for andes_v5_ae350 DMA instance | ||
| dma0: dma0@f0c00000 { | ||
| compatible = "andestech,atcdmac300"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| compatible = "andestech,atcdmac300"; | |
| compatible = "andestech,atcdmac100"; |
| }; | ||
|
|
||
| spi0: spi@A0800000 { | ||
| compatible = "andestech,atcspi200"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file name in the code hierarchy is andestech.atcspi200.yaml, it should be andestech,atcspi200.yaml. Notice the full-stop should be replaced by comma
rahul-r-shah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last of reviews done, couple of changes
samples/hello_world/src/main.c
Outdated
| printf("%s flash has status disabled...%s\n", ATTR_ERR,ATTR_RST); | ||
| } else { | ||
| printf("%s flash Object is Created. Initialize to Test Via DMA%s\n", ATTR_INF,ATTR_RST); | ||
| if(flash != NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this extra check? Youve already check this above flash == NULL, or do you mean to be checking for DMA here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually now we dont need it now. Earlier i was doing deferred initialization of flash. There it was needed. that extra initialization line is removed. I will remove this.
drivers/dma/dma_andes_atcdmac100.c
Outdated
|
|
||
| struct __attribute__((packed, aligned(4))) chain_block { | ||
| uint32_t ctrl; | ||
| uint32_t srcaddrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, whats the 'l' at the end of srcaddr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in ATCDMAC300 we have low and high registers. I removed all the srcaddrh type variables and left srcaddrl type. May be i can rename it to remove this l part as well.
rahul-r-shah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Uh oh!
There was an error while loading. Please reload this page.