Skip to content

Conversation

@LaurentiuM1234
Copy link
Contributor

For now, this sums up to supporting some 64-bit TCD registers and the MUX register being in the MP range.

@zephyrbot
Copy link

zephyrbot commented Sep 12, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@7781570 zephyrproject-rtos/hal_nxp@17aac63 (master) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Sep 12, 2024
@LaurentiuM1234 LaurentiuM1234 marked this pull request as ready for review September 12, 2024 12:11
@zephyrbot zephyrbot added area: DMA Direct Memory Access platform: NXP Drivers NXP Semiconductors, drivers labels Sep 12, 2024
@teburd teburd assigned dbaluta and unassigned teburd Sep 12, 2024
@teburd
Copy link
Contributor

teburd commented Sep 12, 2024

Re-assigned to NXP SoF lead (@dbaluta) as I understand it, please re-assign as needed to @dleach02 otherwise.

@decsny decsny removed their request for review September 12, 2024 16:03
@dbaluta
Copy link
Contributor

dbaluta commented Sep 16, 2024

Change looks good to me.

Please resolve west.yml conflict and also these warnings:

image

Then is good to merge.

@LaurentiuM1234
Copy link
Contributor Author

@teburd what's the policy of the DMA subsystem w.r.t the warnings issued by clang-format? As far as I can tell there's no rule regarding the Zephyr code base having to be clang-format-compliant. Also, the header file for which the warnings were issued was never clang-format-compliant to begin with and the suggested changes don't improve the readability of the code.

@dbaluta dbaluta assigned dleach02 and unassigned dbaluta Sep 16, 2024
@dbaluta
Copy link
Contributor

dbaluta commented Sep 16, 2024

@dleach02 I will be assigning this to you as I will be out of office this week.

@LaurentiuM1234
Copy link
Contributor Author

Rebased and fixed clang-format warnings.

Update hal_nxp hash to point to the latest version.

Signed-off-by: Laurentiu Mihalcea <[email protected]>
Some EDMA versions may have the channel MUX register in the MP
region. To support this scenario, use the `EDMA_HAS_MP_MUX_FLAG`
flag to figure out which channel MUX register to use (TCD or MP).

Signed-off-by: Laurentiu Mihalcea <[email protected]>
On some EDMA versions, some TCD registers (e.g: SADDR, DADDR,
SLAST, DLAST, etc...) are extended to 64 bits via adding a new
HIGH register holding the value of bits [63:32]. Since, for now,
the driver doesn't support 64-bit addresses, this scenario is
supported by sign-extending the 32-bit value written to SLAST/DLAST.
SADDR and DADDR are taken care of on HAL side.

Signed-off-by: Laurentiu Mihalcea <[email protected]>
@LaurentiuM1234
Copy link
Contributor Author

Dependency merged. Rebased and updated hal_nxp hash.

@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Sep 18, 2024
@teburd
Copy link
Contributor

teburd commented Sep 19, 2024

@teburd what's the policy of the DMA subsystem w.r.t the warnings issued by clang-format? As far as I can tell there's no rule regarding the Zephyr code base having to be clang-format-compliant. Also, the header file for which the warnings were issued was never clang-format-compliant to begin with and the suggested changes don't improve the readability of the code.

These are warnings and should be looked at, but may be ignored if the suggestion results in less readable code today. Ideally we'd get to a place where clang-format is so good we just default to it, but there's some places where it can still do a worse job today.

@nashif nashif merged commit b26d21f into zephyrproject-rtos:main Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: DMA Direct Memory Access manifest manifest-hal_nxp platform: NXP Drivers NXP Semiconductors, drivers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants