Skip to content

Conversation

@mniestroj
Copy link
Member

Up to now interrupts could be only configured once, with no way to disable them in runtime.

Allow interrupts to be disabled in runtime and then properly reenabled on user request. This allows to ignore interrupts when software is not expecting them.

The improvement over previously reverted patch [1] is that we disable interrupts only when we configure port for which interrupt line was previously selected. This for example prevents to disable interrupts line 2 in case PA2 was previously configured as interrupt source, but we are currently configuring PB2 as output.

Besides we make a small improvement by declaring gpio_stm32_enable_int as static and splitting into smaller functions in order to achieve better readability.

Those patches are a result of previously noticed bug #19177.

[1] 0951ce2 ("gpio: stm32: support disabling and reenabling interrupts on pin")

@erwango
Copy link
Member

erwango commented Sep 19, 2019

@mniestroj, btw I didn't mentioned it previously but since this change is going to have much more impact than initially, you should be aware of imminent driver rework due to new GPIO API introduction, merged yesterday actually (#16648).
So please have a look and check if it fits your need.

@mniestroj
Copy link
Member Author

So please have a look and check if it fits your need.

@erwango I don't see any problems, as those changes look unrelated. But thanks for pointing that, I'll try to follow it.

Copy link
Member

@mnkp mnkp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better if this PR targeted topic-gpio branch?

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mniestroj, this PR is of particular interest for driver adaptation for GPIO API.
I've reused it on #19607 (and hence validated it as part of my tetsing).
So to make things a little bit more simple, would be nice to have it on gpio-topic branch.
Also, as mentioned by @mnkp, some changes on gpio_stm32_config function would be needed, but are not particularly linked to the purpose of this PR.
So, I've pushed #19646 on topic-gpio branch. It would be required that you base your change on that as well.

To sum up, after updating this PR wrt requested changes, can you rebase on top of #19646 and then push it on topic-gpio branch?

Thanks for your understanding on this unusualy complex PR handling.

@mniestroj mniestroj changed the base branch from master to topic-gpio October 14, 2019 17:23
@mniestroj mniestroj force-pushed the gpio-stm32-int-disable branch from ef3fead to 466fabc Compare October 14, 2019 17:23
@zephyrbot
Copy link

zephyrbot commented Oct 14, 2019

All checks are passing now.

checkpatch (informational only, not a failure)

-:92: WARNING:PREFER_KERNEL_TYPES: Prefer kernel type 'u32_t' over 'uint32_t'
#92: FILE: drivers/gpio/gpio_stm32.c:244:
+	uint32_t line;

- total: 0 errors, 1 warnings, 174 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.

@mniestroj mniestroj force-pushed the gpio-stm32-int-disable branch from 466fabc to 0b25679 Compare October 14, 2019 17:26
@mniestroj
Copy link
Member Author

@erwango I've changed code according to review comments and rebased on top of topic-gpio branch.

@erwango
Copy link
Member

erwango commented Oct 15, 2019

@mniestroj, thanks!
I'm a little bit confused though, since I've finally done the changes myself directly in #19579 last week since we wanted to make it move fastly. I should have informed you to spare you the effort, sorry for this.
Anyway, your commits are present in #19579 and will be merged form this branch.
Thanks for this contribution that has been useful to initiate the driver re-spawn.

@mniestroj
Copy link
Member Author

@erwango Ahh, I see that you fixed those issues in separate commit :) Thanks for reviewing and pulling those changes!

erwango and others added 5 commits November 11, 2019 10:16
With dual core handling introduction, we now need to take care to
always release lock before exiting function.
Rework gpio_stm32_config to take this into account.
Additionally, since ENOSYS usage is resevred to system calls
handling, replace with EIO.


Signed-off-by: Erwan Gouriou <[email protected]>
This allows compiler to inline function body and reduce overall code
size.

Signed-off-by: Marcin Niestroj <[email protected]>
Check if CONFIG_SOC_SERIES_STM32MP1X is defined instead of its
value. While at it convert CONFIG_SOC_SERIES_STM32F1X check to be
consistent with others.

Signed-off-by: Marcin Niestroj <[email protected]>
This patch doesn't change functionality, but is only related to improved
readability and reusability.

Signed-off-by: Marcin Niestroj <[email protected]>
Up to now interrupts could be only configured once, with no way to
disable them in runtime.

Allow interrupts to be disabled in runtime and then properly reenabled
on user request. This allows to ignore interrupts when software is not
expecting them.

The improvement over previously reverted patch [1] is that we disable
interrupts only when we configure port for which interrupt line was
previously selected. This for example prevents to disable interrupts
line 2 in case PA2 was previously configured as interrupt source, but we
are currently configuring PB2 as output.

[1] 0951ce2 ("gpio: stm32: support disabling and reenabling
  interrupts on pin")

Signed-off-by: Marcin Niestroj <[email protected]>
@mniestroj
Copy link
Member Author

Main changes from this PR has been merged part of #20443 PR.

@mniestroj mniestroj closed this Nov 14, 2019
@mniestroj mniestroj deleted the gpio-stm32-int-disable branch August 12, 2020 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants