Merge clean up / refactoring commits from topic-gpio to master

[mnkp]

This is a collection of mostly unrelated commits which were merged on topic-gpio branch as part of a clean up effort. They are compatible with the current GPIO API and can be merged with master before the work on topic-gpio branch is finished.

Affected platforms: SiLabs, NXP, STM32. Includes commits from #19256 by @mniestroj.

[andrewboie]

Hmm, did CI get stuck?

[mniestroj]

@mnkp Could you pull latest patches from #19256? There are now 4 instead of 3 patches. Most importantly things like#elif CONFIG_SOC_SERIES_STM32MP1X have been converted to #elif defined(CONFIG_SOC_SERIES_STM32MP1X).

[galak]

Hmm, did CI get stuck?

Weird, rebased to kick CI off again.

[mnkp]

@mnkp Could you pull latest patches from #19256? There are now 4 instead of 3 patches. Most importantly things like#elif CONFIG_SOC_SERIES_STM32MP1X have been converted to #elif defined(CONFIG_SOC_SERIES_STM32MP1X).

@mniestroj I've tried to cherry-pick the commit in question but unfortunately there were merge conflicts which I would rather not attempt to resolve. Your PR has undergone some modifications on the topic-gpio branch due to review process. Could you please verify that the version in this PR is fine with you. The commit modifying #elif CONFIG_SOC_SERIES_STM32MP1X needs to be recreated on top of this PR. Fortunately it's a minor change. I'm sorry about it.

[mniestroj]

@mniestroj I've tried to cherry-pick the commit in question but unfortunately there were merge conflicts which I would rather not attempt to resolve. Your PR has undergone some modifications on the topic-gpio branch due to review process. Could you please verify that the version in this PR is fine with you. The commit modifying #elif CONFIG_SOC_SERIES_STM32MP1X needs to be recreated on top of this PR. Fortunately it's a minor change. I'm sorry about it.

@mnkp I've successfully rebased those 4 commits that I mentioned on top of d4fba7eba7da79753dd38ce79ae730d645533dd6 from your branch. The result is here: https://github.com/mniestroj/zephyr/tree/mnkp-gpio-stm32-int-disable. Then I rebased your topic-gpio-fixes on top (with skipping my old 3 patches) and the result is here: https://github.com/mniestroj/zephyr/tree/mnkp-topic-gpio-fixes. All without any conflicts. Basically you cannot just apply 1 additional patch, because those old 3 patches are not subset of new 4 (things were modified during review process).

[zephyrbot]

All checks passed.

checkpatch (informational only, not a failure)

-:356: WARNING:PREFER_KERNEL_TYPES: Prefer kernel type 'u32_t' over 'uint32_t'
#356: FILE: drivers/gpio/gpio_stm32.c:204:
+	uint32_t line = gpio_stm32_pin_to_exti_line(pin);

-:382: WARNING:PREFER_KERNEL_TYPES: Prefer kernel type 'u32_t' over 'uint32_t'
#382: FILE: drivers/gpio/gpio_stm32.c:230:
+	uint32_t line = gpio_stm32_pin_to_exti_line(pin);

-:783: WARNING:LONG_LINE: line over 80 characters
#783: FILE: soc/arm/silabs_exx32/efr32mg12p/dts_fixup.h:29:
+#define DT_GPIO_GECKO_PORTI_NAME	DT_SILABS_EFR32MG_GPIO_PORT_4000A180_LABEL

-:784: WARNING:LONG_LINE: line over 80 characters
#784: FILE: soc/arm/silabs_exx32/efr32mg12p/dts_fixup.h:30:
+#define DT_GPIO_GECKO_PORTJ_NAME	DT_SILABS_EFR32MG_GPIO_PORT_4000A1B0_LABEL

-:785: WARNING:LONG_LINE: line over 80 characters
#785: FILE: soc/arm/silabs_exx32/efr32mg12p/dts_fixup.h:31:
+#define DT_GPIO_GECKO_PORTK_NAME	DT_SILABS_EFR32MG_GPIO_PORT_4000A1E0_LABEL

- total: 0 errors, 5 warnings, 692 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[mnkp]

Basically you cannot just apply 1 additional patch, because those old 3 patches are not subset of new 4 (things were modified during review process).

Sure, that's clear. I didn't want to replace the old 3 patches with the 4 new since the old 3 may have included changes not done in the new 4. The work was happening on the topic-gpio branch and the old 3 patches could have been modified via fixup.

I'll drop STM32 patches from this PR. As reported by checkpatch there is some more clean up which needs to be done and the required extra commit can't be merged as it includes unrelated changes.

@mniestroj @erwango If we want to get this into 2.1. it may be best if we merge the original #19256 PR. It would need to be reverted to target the master branch again. This would need to be coordinated between you too since you know what was modified the best. @mniestroj once again sorry for the trouble.

[mniestroj]

@mniestroj @erwango If we want to get this into 2.1. it may be best if we merge the original #19256 PR. It would need to be reverted to target the master branch again. This would need to be coordinated between you too since you know what was modified the best. @mniestroj once again sorry for the trouble.

Okay, then I can prepare a PR with only stm32 improvements. Looks like targetting those to topic-gpio branch was a bad idea in the first place, because they were never new GPIO API related. And because of this approach we are delaying fixes for dual core and disabling interrupts, almost by 2 months already.

@mnkp I'll prepare a PR with stm32 patches once you drop them here.

[erwango]

@mnkp, @mniestroj , #19256 is not in a state that could be merged right from what I can see.
If main difference is 95fdeb9, this is rather a cosmetic change, and I don't see why this should block merging of other commits.
Besides, aim of current PR is to ease merge of up coming gpio api changes. I don't guarantee these will fit nicely on top of #19256 (I might have tweaked minor stuff in the process). Though I know this works on top of the 3 commits present here.

[mnkp]

Making sure that rebasing of topic-gpio goes smoothly would indeed be an issue if the work on #19256 continues. We should have waited for #19256 to be merged or close it once the commits were pulled to other PR. That was communication failure on our part.

I let others decide how to handle this issue.

If this PR is to be merged fully we still need to fix checkpatch warnings. There is a commit on topic-gpio branch that does it but I would need to split it and apply only partially.

[mniestroj]

@mnkp, @mniestroj , #19256 is not in a state that could be merged right from what I can see.

@erwango What problem do you see? I've rebased my commits on top of master once again, with your commit fixing dual core systems.

If main difference is 95fdeb9, this is rather a cosmetic change, and I don't see why this should block merging of other commits.

This is not the only difference. I suggest to use git diff to see what are the other changes. In fact, those changes were requested by both @erwango and @mnkp during review process. So I do not understand your reasoning here, because you are trying to pull old version of patches, without changes you requested.

[erwango]

o I do not understand your reasoning here, because you are trying to pull old version of patches, without changes you requested.

@mniestroj, I've finally done the changes directly myself after pulling them (at that time I couldn't wait for your changes, sorry if this made additional work for you), so commits in topic-gpio branch are conform to what is expected. Besides there are commits on top of these ones in topic-gpio branch, that ensure the transition to new GPIO API. This set of changes have been reviewed tested and validated on all STM32 series as a whole.
Aim of current pull request is to prepare the landing of topic-gpio branch into main branch by merging the commits that do not touch directly the API. Taking commits that are not part of topic-gpio branch would imply the risks:

• of introducing new bugs, or requesting need to review or testing once again
• of merge issue when trying to merge remaining part of topic-gpio branch

So, there are 2 cases:

• Differences between Factorize out socket management from LwM2M engine #19526 and topic-gpio branch are minor/cosmetic: Then, I don't want to introduce chances to break topic-gpio branch merge-ability for this.
• Differences between Factorize out socket management from LwM2M engine #19526 and topic-gpio branch are significant: Then I prefer we introduce PR on topic-gpio and study change in this context (review and retesting).

[mniestroj]

@mniestroj, I've finally done the changes directly myself after pulling them (at that time I couldn't wait for your changes, sorry if this made additional work for you), so commits in topic-gpio branch are conform to what is expected.

@erwango You have done the changes part of another commit a070416 ("drivers/gpio: stm32: various clean up"), which is not part of this PR. My commits haven't been modified as far as I can see. Also #19256 contains improvements (made according to review process) which are not part of topic-gpio branch.

Besides there are commits on top of these ones in topic-gpio branch, that ensure the transition to new GPIO API. This set of changes have been reviewed tested and validated on all STM32 series as a whole.

I don't think this is a good approach to look at topic-gpio branch. This doesn't scale. If there would be 15 topic-something branches and you would want to make sure nothing gets broken there before merging other commits to master, then you would simply end up in a deadlock. It should be one way, so topic-gpio branch should be rebased on top of master whenever there are conflicts, but master should go with its own way applying fixes and minor improvements at a constant rate.

Taking commits that are not part of topic-gpio branch would imply the risks:

• of introducing new bugs, or requesting need to review or testing once again

I bet that commits on topic-gpio branches were tested with all other changes together. Are you 100% sure that everything works with the cherry-picked commits on this PR? :) Did someone tested them that way?

• of merge issue when trying to merge remaining part of topic-gpio branch

git rebase is very easy to use here. All conflicts will be automatically detected and you can easily drop patches with functionality that has been already introduced, even those done in slightly different way. And by saying that you want to achieve easier merge of topi-gpio I think that dropping patches with functionality already in master is what you really want to do.

So, there are 2 cases:

• Differences between Factorize out socket management from LwM2M engine #19526 and topic-gpio branch are minor/cosmetic: Then, I don't want to introduce chances to break topic-gpio branch merge-ability for this.
• Differences between Factorize out socket management from LwM2M engine #19526 and topic-gpio branch are significant: Then I prefer we introduce PR on topic-gpio and study change in this context (review and retesting).

As I said earlier, this should go the other way around. Fixes should have a priority in landing in master branch, so they should be based on master. I think that this is why @mnkp started this PR, so topic-gpio branch can be freed from random improvements and fixes and include only changes related to new GPIO API.

And last thing... checking what are the real differences in codebase is as simple and quick as executing git diff one-branch..another-branch. It takes 10 seconds and another 20 seconds to look at the changes. We just discuss too much about such simple thing.

[galak]

Approved by TSC for merge into 2.1