Pavel Pisa commented: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1218#note_149336


I agree with the formatting.

As for the **bsp/arm/tms570: unlock IOMM and disable interrupts before setting 
pins**, I am not fully sure if the unlock should be included in the individual 
pin mode manipulation functions.

I think that the unlock is safety related operation and if the low level pin 
manipulation function is called unintentionally by some bug it is better if it 
cannot change pin configuration. For some more dynamic system with lower 
security level it would be OK. But for some strict case it would be the best if 
the unlock setup of all pins and then lock is done during startup and there are 
no more places where unlocks is called which increase probability of misbehave 
in case of some software or even hardware error.

So I am not sure and it has been intentional that low level manipulation 
functions do not include unlock. There are `tms570_bsp_pin_config_one()` and 
`tms570_bsp_pinmmr_config` which take care about config unlock. The 
`tms570_bsp_pinmmr_config` is intended for startup where interrupts are not 
enabled yet. When the driver is dynamic, it can use `tms570_bsp_pin_config_one` 
which is safe to be called at runtime.

But I am for discussion about balance between API user friendliness and keeping 
as low as possible the probability that some random code execution can have 
some dangerous consequences.

-- 
View it on GitLab: 
https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1218#note_149336
You're receiving this email because of your account on gitlab.rtems.org.


_______________________________________________
bugs mailing list
[email protected]
http://lists.rtems.org/mailman/listinfo/bugs

Reply via email to