drivers: dma: siwx917: Add DMA driver

[smalae]

Addition of DMA driver for Si91x

[jerome-pouiller]

Hello @smalae,

Could you configure git with your real name[1]:

git config --global user.name "Sai Santhosh Malae"
git config --global user.email "[email protected]"

(You can use git commit --amend --reset-author to change the author of existing commits).

In addition, could you follow the Zephyr Pull Request[2] guidelines[3].

[1] https://docs.zephyrproject.org/latest/contribute/guidelines.html#git-setup
[2] https://docs.zephyrproject.org/latest/contribute/guidelines.html#pull-request-guidelines
[3] https://docs.zephyrproject.org/latest/contribute/guidelines.html#contribution-workflow

[jerome-pouiller]

The introduction letter of your PR properly identify three steps to introduce this driver (driver by itself, DT configuration and tests). I suggest to split your commits to reflect these steps. Thus, the review you be more efficient.

Could you also enable your tests in CI (.github/workflows/build.yml and .github/workflows/upstream-build.yml)

[smalae]

Hello @smalae,

Could you configure git with your real name[1]:

git config --global user.name "Sai Santhosh Malae"
git config --global user.email "[email protected]"

(You can use git commit --amend --reset-author to change the author of existing commits).

In addition, could you follow the Zephyr Pull Request[2] guidelines[3].

[1] https://docs.zephyrproject.org/latest/contribute/guidelines.html#git-setup [2] https://docs.zephyrproject.org/latest/contribute/guidelines.html#pull-request-guidelines [3] https://docs.zephyrproject.org/latest/contribute/guidelines.html#contribution-workflow

Sure @jerome-pouiller

[smalae]

The introduction letter of your PR properly identify three steps to introduce this driver (driver by itself, DT configuration and tests). I suggest to split your commits to reflect these steps. Thus, the review you be more efficient.

Could you also enable your tests in CI (.github/workflows/build.yml and .github/workflows/upstream-build.yml)

Sure, I will split into multiple commits

[jhedberg]

@smalae could you explain a bit what you mean by this part related to the new test case you're adding:

This test case does not need to be pushed upstream. It can only reside in Downstream.

Even though it doesn't need to be upstream and can reside downstream, is there a good reason to do that? If it's generic, i.e. can be applied to any DMA driver, then I think upstream is the right place for it. Keeping code purely downstream always comes with a maintenance overhead, so it's good to avoid that whenever possible.

[smalae]

@smalae could you explain a bit what you mean by this part related to the new test case you're adding:

This test case does not need to be pushed upstream. It can only reside in Downstream.

Even though it doesn't need to be upstream and can reside downstream, is there a good reason to do that? If it's generic, i.e. can be applied to any DMA driver, then I think upstream is the right place for it. Keeping code purely downstream always comes with a maintenance overhead, so it's good to avoid that whenever possible.

@jhedberg agreed that it will cause the maintenance overhead. But this test case is almost similar to the existing test case with a few additional tests added (different data sizes and transfer lengths). I feel adding this test case upstream may not be necessary. Is it okay to remove this test case from the PR and retain it in my fork? Anyhow, the basic DMA test chan_blen_transfer can be run by CI. @jerome-pouiller, your thoughts?

[smalae]

But this test case is almost similar to the existing test case with a few additional tests added (different data sizes and transfer lengths). I feel adding this test case upstream may not be necessary.

I'm still a bit confused by what you mean with "not necessary". I don't think that's the metric we should use for making downstream/upstream decisions. "Is it useful/beneficial?" is a better one. If there's something that upstream tests don't cover and that "something" can be applied to any DMA controller (or most controllers), then proposing a change upstream would likely be well received as long as it's implemented in a reasonable way (e.g. does some refactoring if the alternative is to add lots of duplicated test code).

Thank you for your feedback. I see your point about considering whether something is 'useful/beneficial' rather than just 'necessary.' However, in this case, my concern was more about the immediate scope and focus of the current implementation. While I agree that upstream changes can be beneficial when they address broader use cases, I believe that the proposed change may not fit well into the current scope without introducing unnecessary complexity or duplication. I'll revisit the idea with this in mind and ensure we stay aligned with the overall goals for simplicity and maintainability. For now, I am only going with the existing UDMA test case in Zephyr.

[jerome-pouiller]

Sorry, I haven't been able spot all the issues in one go. I know it is a bit annoying. I believe this is the last round.

[jerome-pouiller]

Still this small issue with dma_rom_buff and we will be fine.

[jerome-pouiller]

Looks good. @smalae can you update the revision of hal_silabs in west.yml, so we can merge this PR.