On 15/03/2024 16:28, John Ogness wrote:
On 2024-03-15, Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:+static inline int drm_panic_trylock(struct drm_device *dev, unsigned long *flags) +{ + return raw_spin_trylock_irqsave(&dev->mode_config.panic_lock, *flags); +}[...]+static inline unsigned long drm_panic_lock(struct drm_device *dev) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&dev->mode_config.panic_lock, flags); + return flags; +}I find it a bit strange that @flags is passed as an argument in the trylock variant but returned in the lock variant. I would pass it as an argument in both variants. Just a suggestion.
Or maybe I can do it as a macro instead of a static inline function, to have the same semantic as raw_spin_lock_irqsave() which modify the flags, without passing it as pointer, because it's also a macro.
https://elixir.bootlin.com/linux/latest/source/include/linux/spinlock.h#L296 #define drm_panic_lock(dev, flags) \ raw_spin_lock_irqsave(&dev->mode_config.panic_lock, flags);
John Ogness
-- Jocelyn
