-
Notifications
You must be signed in to change notification settings - Fork 911
Clean-up adsp-main-6.12 series #3010
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
base: adsp-6.12.0-y
Are you sure you want to change the base?
Conversation
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Utsav Agarwal <[email protected]>
Signed-off-by: Utsav Agarwal <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Only supports the ADZS-SC589-MINI Signed-off-by: Utsav Agarwal <[email protected]>
Signed-off-by: UtsavAgarwalADI <[email protected]>
Signed-off-by: Philip Molloy <[email protected]>
Signed-off-by: Utsav Agarwal <[email protected]> Signed-off-by: Arturs Artamonovs <[email protected]>
Co-developed-by: Nathan Barrett-Morrison <[email protected]> Signed-off-by: Nathan Barrett-Morrison <[email protected]> Co-developed-by: Greg Malysa <[email protected]> Signed-off-by: Greg Malysa <[email protected]> Signed-off-by: Utsav Agarwal <[email protected]> Signed-off-by: Arturs Artamonovs <[email protected]>
baabcea to
48807cd
Compare
|
There is still a lot to clean-up, but it isn't possible to fix everything in one go. I'll also fix some more when I rebase these on top of adsp-6.12.38-y that includes Artur's mainline series. @analogdevicesinc/linux-for-adsp please take a look through. There are a lot of fixup commits, but they should be very straight forward. |
|
The plan is to keep |
Yup, |
nunojsa
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.
Mostly looks good!
Regarding all the cast comments, I'm ok if you say you prefer to group all of those changes for a latter round
| struct ldr_hdr *block_hdr = NULL; | ||
| struct ldr_hdr *next_hdr = NULL; | ||
| uint8_t *virbuf = (uint8_t *) rproc_data->mem_virt; | ||
| u8 *virbuf = (u8 *) rproc_data->mem_virt; |
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.
I wonder if the cast is even needed. If mem_virt is void *, then it should be implicit already
| // Poll until Core is in reset | ||
| while (!(adi_rcu_readl(rcu, ADI_RCU_REG_CRSTAT) & (1 << coreid))); | ||
| while (!(adi_rcu_readl(rcu, ADI_RCU_REG_CRSTAT) & (1 << coreid))) | ||
| ; |
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.
Hmm? Is this just personal preference or did we get any tool complaining about it? If I'm not mistaken, I'm actually used to see the style we are changing.
|
|
||
| /** @brief Subdevice id to which the buffer should be attached */ | ||
| int32_t subdev_id; | ||
| s32 subdev_id; |
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.
For this, often one just uses plain int. I guess it will depend on the maintainer we're dealing with
| return ret; | ||
| } | ||
| sport->tx_dma_icap_buf_id = (uint32_t)ret; | ||
| sport->tx_dma_icap_buf_id = (u32)ret; |
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.
Same thing (since we're changing it already better to just drop it). Same for all other casts I'm seeing. I'm not 100% sure about all of them but gut feeling that we should be able to drop most (if not all) of them
| static u64 notrace read_sched_gptimer(void) | ||
| { | ||
| return (u64) get_gptimer_count(gptimer_controller.cs->timer); | ||
| return (u64)get_gptimer_count(gptimer_controller.cs->timer); |
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.
Likely the cast is not needed
Yah, I'd leave the cast clean-up to another round. The other comments were already fixed, but likely the result of rebasing or squashing fixups, which created weird diffs, but are correct in the end. |
nunojsa
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.
Given @pamolloy feedback, LGTM!
But we should have more eyes on this one!
This base series is a refactored version of the current
adsp-main-6.12branch. The minimal diff withadsp-main-6.12is below. Note this is getting merged into a new branch,adsp-6.12.0-y.I will then apply a set of fixup commits in an attempt to pass CI/CD. Only the fixup commits need to be reviewed.
Once merged
adsp-main-6.12can be replaced. And then this series can be manually rebased on top ofadsp-main-6.12.38-y, which contains Artur's series that is being mainlined.