hi,

> > > - it looks weird a bit to define IPL_* macro as (PSL_S|PSL_IPL?)
> > >   because IPL_* macros are used to specify interrupt priority order
> > >   on some ports (hp300, some mips etc).
> > how are hp300 and mips related to this patch?
> 
> On hp300, IPL_* macros already have some meaning:
> >> /*
> >>  * Interrupt "levels".  These are a more abstract representation
> >>  * of interrupt levels, and do not have the same meaning as m68k
> >>  * CPU interrupt levels.  They serve two purposes:
> >>  *
> >>  *      - properly order ISRs in the list for that CPU ipl
> >>  *      - compute CPU PSL values for the spl*() calls.
> >>  */
> >>#define IPL_NONE        0       /* disable only this interrupt */
>  :
> 
> but in <sys/spl.h> IPL_* macros are used as opaque values passed
> to splraise(). Maybe we should rename either of them, but it
> might cause some confusion to use existing names for splraiseipl(),
> as you wrote in PR/33218.

i'm not sure if i understand your point.
the comment you cited ("compute CPU PSL values for the spl*() calls") is
what an argument of splraiseipl is for.
do you mean hp300 and luna68k should share IPL_xxx values?

> If we introduce new independent SPL_* macros, we could define them like:
> ---
> #define	IPL_NONE	0	/* disable only this interrupt */
> #define	IPL_BIO		1	/* disable block I/O interrupts */
> #define	IPL_NET		2	/* disable network interrupts */
> #define	IPL_TTY		3	/* disable terminal interrupts */
> #define	IPL_TTYNOBUF	4	/* IPL_TTY + higher ISR priority */
> #define	IPL_CLOCK	5	/* disable clock interrupts */
> #define	IPL_HIGH	6	/* disable all interrupts */
>  :
> #define	spllowersoftclock() spl1()
> 
> #define	SPL_SOFTCLOCK	(PSL_S|PSL_IPL1)
> #define	SPL_SOFTNET	(PSL_S|PSL_IPL1)
> #define	SPL_BIO		(hp300_ipls[HP300_IPL_BIO])
> #define	SPL_NET		(hp300_ipls[HP300_IPL_NET])
> #define	SPL_TTY		(hp300_ipls[HP300_IPL_TTY])
> #define	SPL_SERIAL	(hp300_ipls[HP300_IPL_TTY])
> #define	SPL_VM		(hp300_ipls[HP300_IPL_VM])
> #define	SPL_CLOCK	(PSL_S|PSL_IPL6)
> #define	SPL_STATCLOCK	SPL_CLOCK
> #define	SPL_SCHED	(PSL_S|PSL_IPL7)
> #define	SPL_LOCK	(PSL_S|PSL_IPL7)
> #define	SPL_HIGH	(PSL_S|PSL_IPL7)
>  :
> ---
> then all m68k ports could have the same meanings of SPL_*,
> definition of splraiseipl() could be defined as
> "#define splraiseipl(ipl) _splraise(ipl)" in common <m68k/psl.h>.

how about having another opaque type as the following?

- make sure that each IPL_xxx has different values, so that they can
  be converted to SPL_xxx when necessary.
- move a possibly slow part of splraiseipl (eg. IPL_xxx -> SPL_xxx conversion)
  into another function, makeiplcookie(), so that it isn't executed
  frequently.

	/*
	 * ipl_t: IPL_BIO, IPL_NET, ....
	 *
	 * MD type.  maybe int for all ports.
	 */
	typedef int ipl_t;

	/*
	 * ipl_cookie_t:
	 *
	 * MD type.
	 *
	 * for some ports, it can be the same as ipl_t.
	 * for other ports, it can be SPL_xxx, PSL_xxx, etc.
	 */
	typedef xxx ipl_cookie_t;

	int splraiseipl(ipl_cookie_t);

	/*
	 * makeiplcookie:
	 *
	 * calculate a cookie for splraiseipl.
	 *
	 * for some ports, this might do IPL_xxx -> SPL_xxx conversion.
	 */
	ipl_cookie_t makeiplcookie(ipl_t);


the following is an example of their usage.

	typedef struct {
		ipl_cookie_t iplcookie;
		int savedipl;
		struct simplelock lock;
	} lock_t; /* a simple ipl-protected spin lock */

	void
	lk_init(lock_t *lk, ipl_t ipl)
	{

		lk->iplcookie = makeiplcookie(ipl);
		simple_lock_init(&lk->lock);
	}

	void
	lk_acquire(lock_t *lk)
	{
		int s = splraiseipl(lk->iplcookie);
		simple_lock(&lk->lock);
		lk->savedipl = s;
	}

	void
	lk_release(lock_t *lk)
	{
		int s = lk->savedipl;
		simple_unlock(&lk->lock);
		splx(s);
	}

YAMAMOTO Takashi