Skip to content
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

soc: arm: cypress: psoc6: Add Cortex-M0+ interrupt mux support #29459

Merged
merged 3 commits into from
Jan 20, 2021

Conversation

nandojve
Copy link
Member

@nandojve nandojve commented Oct 22, 2020

PSoC-6 SoC needs that user define the nvic interrupt number to bind with the peripheral interrupt line for the Cortex-M0+ CPU. It uses a multiplex before any NVIC interrupt line. The minimal support was added to allow interrupts be available for both CPUs.

Note: The PSoC-6 SoC allows that both CPUs receive the same interrupt. A tipical use is GPIO interrupt handle and user is responsable to define interrupt line, priority and take care of enable same peripheral instance on both CPUs only when appropriated.

This update <soc.h> include files removing the unnecessary <kernel_includes.h> file. In addition, add <sys/util.h> to expose macros and <devicetree.h> following general standards.

This solves #28442 and allows development of drivers that can be shared between SoC cores including, external interrupts by GPIO, serial, spi etc.

@nandojve nandojve added this to the v2.5.0 milestone Oct 22, 2020
@nandojve nandojve requested a review from galak as a code owner October 22, 2020 21:50
@github-actions github-actions bot added area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding labels Oct 22, 2020
@nandojve
Copy link
Member Author

Rebase and add missing map-nvic documentation.

@galak
Copy link
Collaborator

galak commented Oct 23, 2020

Need to look at this in more detail. Wondering how this compares to what we have for rv32m1_vega

@nandojve
Copy link
Member Author

nandojve commented Oct 23, 2020

Need to look at this in more detail. Wondering how this compares to what we have for rv32m1_vega

I know this is an intermediate solution. At drivers, the work was done on the macro that connects IRQ and it was defined externally for an rebase, if necessary. @mbolivar already told me about rv32m1_vega but I didn't see how user define the interrupt on DT, Cy m0 problem.

What I think it will be easy to user is described at #28442 but I face the constfy problem with DT_INST_FOREACH_CHILD.

@mbolivar-nordic
Copy link
Contributor

I didn't see how user define the interrupt on DT, Cy m0 problem.

I don't understand "Cy m0 problem", sorry.

Perhaps this is a chance to clean up the devicetree documentation (and maybe implementation) with regards to multi-level interrupts, but can you explain what you tried when you tried to make intermediate interrupt controller nodes for the interrupt mux? The idea is to set the interrupt-parent to point to those and to use the normal interrupts property to specify interrupts in DT. Can you explain what didn't work the way you expected when you tried that?

@nandojve
Copy link
Member Author

I didn't see how user define the interrupt on DT, Cy m0 problem.

I don't understand "Cy m0 problem", sorry.

Perhaps this is a chance to clean up the devicetree documentation (and maybe implementation) with regards to multi-level interrupts, but can you explain what you tried when you tried to make intermediate interrupt controller nodes for the interrupt mux? The idea is to set the interrupt-parent to point to those and to use the normal interrupts property to specify interrupts in DT. Can you explain what didn't work the way you expected when you tried that?

I think I understand my difficult, for some reason I didn't look at some file on rv32m1_vega dts directory. That create a gap that I wasn't understand how to fill. I hope this update is close to what I rely need to do.
Thanks!

@nandojve nandojve linked an issue Oct 28, 2020 that may be closed by this pull request
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

This looks nice to me! Just a couple of minor questions but I think this seems sane.

soc/arm/cypress/common/cypress_psoc6_dt.h Outdated Show resolved Hide resolved
dts/arm/cypress/psoc6_cm0.dtsi Outdated Show resolved Hide resolved
@nandojve
Copy link
Member Author

nandojve commented Nov 7, 2020

rebase

@nandojve nandojve force-pushed the atl/psoc6-zephyr-int-mux branch 2 times, most recently from c5d2b9c to 2b0c1b5 Compare November 22, 2020 19:17
@nandojve
Copy link
Member Author

Hello @galak !
I was wondering if there is anything missing here?

@ADEscobar
Copy link

Is this ready to be merged then?

@mbolivar-nordic
Copy link
Contributor

Is this ready to be merged then?

No, it needs another +1, ideally from @galak

Update <soc.h> include files.  This removes the unnecessary
<kernel_includes.h> file.  In addition, add <sys/util.h> to
expose macros and <devicetree.h> following general standards.

Signed-off-by: Gerson Fernando Budke <[email protected]>
The psoc6.dtsi file declare a reference to nvic.  Since it was proper
defined at psoc6_cm0/4.dtsi files this entry is redundant.  Drop the
useless entry.

Signed-off-by: Gerson Fernando Budke <[email protected]>
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

  • Change #size-cells to 1 for mux registers. Even if size is just 1, since these are still MMIO registers.
  • Add some comments on what / how the mux values are figured out. Seen inline.

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

Some minor tweaks, and on general question if we should just remove status="disabled" for the "cypress,psoc6-intmux-ch" nodes. Means one less thing the board dts file would have to modify.

@galak
Copy link
Collaborator

galak commented Jan 20, 2021

One other nit: can you see if we can remove label. Want to phase it out in the future and I don't think you need it at all.

PSoC-6 SoC needs that user define the nvic interrupt number to bind
with the peripheral interrupt line for the Cortex-M0+ CPU.  It uses
a multiplex before any NVIC interrupt line.  The interrupt vector is
selected using interrupt-parent property with the intmux_chN number
reference.

Note: The PSoC-6 SoC allows that both CPUs receive the same interrupt.
A tipical use is GPIO interrupt handle and user is responsable to
define interrupt line, priority and take care of enable same peripheral
instance on both CPUs only when appropriated.

Signed-off-by: Gerson Fernando Budke <[email protected]>
@nandojve
Copy link
Member Author

One other nit: can you see if we can remove label. Want to phase it out in the future and I don't think you need it at all.

Yes, I removed and tested with GPIO driver which depends on this.

@galak galak merged commit f93ee95 into zephyrproject-rtos:master Jan 20, 2021
@nandojve nandojve deleted the atl/psoc6-zephyr-int-mux branch January 20, 2021 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How handle IRQ_CONNECT const requirement?
5 participants