core: interrupt: rename .add handler to .configure + minor inline comment changes #7267
Conversation
| @@ -132,8 +138,7 @@ static inline bool itr_chip_is_valid(struct itr_chip *chip) | |||
| is_unpaged((void *)chip->ops) && | |||
| chip->ops->mask && is_unpaged(chip->ops->mask) && | |||
| chip->ops->unmask && is_unpaged(chip->ops->unmask) && | |||
| chip->ops->enable && chip->ops->disable && | |||
| chip->ops->add; | |||
There was a problem hiding this comment.
I'd rather have a separate version of itr_chip_is_valid() for the DT use case, for clarity and consistent checking.
|
I've rebased the series to fix the merge conflict. |
| chip->ops->add(chip, itr_num, type, prio); | ||
| if (!chip->ops->configure) { | ||
| EMSG("No configure handler in itr_chip %s", chip->name); | ||
| return TEE_ERROR_NOT_IMPLEMENTED; |
There was a problem hiding this comment.
add_configure_handler() below should handle this error.
There was a problem hiding this comment.
Indeed! Thanks.
|
|
etienne-lms
force-pushed
the
itr-fix
branch
2 times, most recently
from
2e7403f to
91c272c
Compare
|
I've slightly modified the content of commits "core: interrupt: rename .add handler to .configure" and Aside * Handlers @enable, @disable, @mask and @unmask are mandated. Handlers
- * @mask and @unmask have unpaged memory contrainsts. See itr_chip_is_valid().
+ * @mask and @unmask have unpaged memory constraints. These requirements are
+ * verified by itr_chip_init() and itr_chip_dt_only_init().
*
- * Handler @configure is needed for interrupt provider which do not rely on
- * DT for consumer interrupt configuration, that is interrupt consumer using
+ * Handler @configure is needed for interrupt providers which do not rely on
+ * DT for consumers interrupt configuration, that is interrupt consumers using
* interrupt_configure() or friends (interrupt_add_handler(),
* interrupt_add_configure_handler(), interrupt_add_handler_with_chip(),
* etc...). |
|
Looks good if you remove from the commit message in "core: interrupt: itr_chip may not require configure handler" |
|
Right, sorry, i'll fix. |
Rename field add of struct itr_ops to configure for consistency since that handler is used the configure the interrupt. Update existing interrupt drivers accordingly. By the way fix inline comment spelling typo (s/contrainsts/constraints/). Signed-off-by: Etienne Carriere <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
The configure handler in struct itr_ops is not required for interrupt providers which consumers only use the DT to get and configure their interrupts (with interrupt_dt_get_by_*() and interrupt_create_handler()). Therefore change itr_chip_is_valid() to not enforce its support but add back that constraint for the interrupt main controller. Add an itr_chip_dt_only_init() helper function for interrupt controllers which consumers only use the DT to configure their interrupt, that is such controllers do not need a configure handler. itr_chip_is_valid() is not called outside interrupt.c where it is used in itr_chip_init() and itr_chip_dt_only_init() so make it a local function. Signed-off-by: Etienne Carriere <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
Add an inline comment telling struct itr_chip:dt_get_irq handler is needed only when interrupt consumer manually get configuration information from the DT to later configure the interrupt. The aim of this change is to clarify this handler is not needed for interrupt provider registered with interrupt_register_provider() and which consumer rely on interrupt_dt_get_by_*() to configure their interrupts. Signed-off-by: Etienne Carriere <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
Clarify inline comment in interrupt_create_handler() to explicit that this function request add_configure_handler() to not configure the interrupt (since it's already configured from interrupt_dt_get_by_*() API functions). Signed-off-by: Etienne Carriere <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
Fix inline description of itr_num argument for interrupt_set_affinity() and interrupt_set_wake(). Fixes: b2d6db2 ("core: interrupt: helper function for raise_pi, raise_sgi, set_affinity") Signed-off-by: Etienne Carriere <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
|
Commit log fixed and review tags applied. Failure of CI Qemu build/run on arm64 host seems unrelated to this change but it's strange it fails on this P-R (systematic over 3 recent series udpate) while I saw a recently updated P-R that successfully passed all CI tests. |
Rename field
addofstruct itr_opstoconfigurefor consistency since that handler is used the configure the interrupt. Update existing interrupt drivers accordingly.That
configurehandler is not required for interrupt providers which consumer only use the DT to get and configure their interrupts (withinterrupt_dt_get_by_*()andinterrupt_create_handler()). Therefore changeitr_chip_is_valid()to not enforce its support but add back that constraint for the interrupt main controller.Plus few minor changes in inline comments in interrupt.h and interrupt.c.