Quoting Thomas Huth (2023-07-14 08:44:10) > On 13/07/2023 17.30, Nico Boehr wrote: > > Quoting Thomas Huth (2023-07-13 09:17:28) > >> On 12/07/2023 13.41, Nico Boehr wrote: > >>> When toggling DAT or switch address space modes, it is likely that > >>> interrupts should be handled in the same DAT or address space mode. > >>> > >>> Add a function which toggles DAT and address space mode for all > >>> interruptions, except restart interrupts. > >>> > >>> Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx> > >>> --- > >>> lib/s390x/asm/interrupt.h | 4 ++++ > >>> lib/s390x/interrupt.c | 36 ++++++++++++++++++++++++++++++++++++ > >>> lib/s390x/mmu.c | 5 +++-- > >>> 3 files changed, 43 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h > >>> index 35c1145f0349..55759002dce2 100644 > >>> --- a/lib/s390x/asm/interrupt.h > >>> +++ b/lib/s390x/asm/interrupt.h > >>> @@ -83,6 +83,10 @@ void expect_ext_int(void); > >>> uint16_t clear_pgm_int(void); > >>> void check_pgm_int_code(uint16_t code); > >>> > >>> +#define IRQ_DAT_ON true > >>> +#define IRQ_DAT_OFF false > >> > >> Just a matter of taste, but IMHO having defines like this for just using > >> them as boolean parameter to one function is a little bit overkill already. > >> I'd rather rename the "bool dat" below into "bool use_dat" and then use > >> "true" and "false" directly as a parameter for that function instead. > >> Anyway, just my 0.02 \u20ac. > > > > The point of having these defines was to convey the meaning of the parameter to my reader. > > > > When I read > > > > irq_set_dat_mode(true, AS_HOME); > > > > it's less clear to me that the first parameter toggles the DAT mode compared to this: > > > > irq_set_dat_mode(IRQ_DAT_ON, AS_HOME); > > > > That being said, here it's pretty clear from the function name what the first parameter is, so what's the preferred opinion? > > I see your point, but if it is clear from the function name like it is in > this case, I'd go with "true" and "false" directly, without the indirection > via #define. OK, will do. > ... > >>> + * Since enabling DAT needs initalized CRs and the restart new PSW is often used > >> > >> s/initalized/initialized/ > > > > Argh, thanks. > > > > *reprioritizes task to look for a spell checker* > > codespell (https://github.com/codespell-project/codespell) is your friend! Thanks!
