* David Herrmann <dh.herrmann@xxxxxxxxx> wrote:
> +/*
> + * Unregister the sysfb and prevent new sysfbs from getting registered. Can be
> + * called from any context except recursively or from sysfb_register().
> + * Used by remove_conflicting_framebuffers() and friends.
> + */
> +void sysfb_unregister(const struct apertures_struct *apert, bool primary)
> +{
> + mutex_lock(&sysfb_lock);
> + if (!IS_ERR(sysfb_dev) && sysfb_dev) {
> + if (sysfb_match(apert, primary)) {
> + platform_device_unregister(sysfb_dev);
> + sysfb_dev = ERR_PTR(-EALREADY);
> + }
> + } else {
> + /* if !sysfb_dev, set err so no new sysfb is probed later */
> + sysfb_dev = ERR_PTR(-EALREADY);
Just a small detail: we can get into this 'else' branch not just with
NULL, but also with IS_ERR(sysfb_dev). In that case we override
whatever error code is contained in sysfb_dev and overwrite it with
ERR_PTR(-EALREADY).
(Probably not a big deal, because we don't actually ever seem to
extract the error code from the pointer, but wanted to mention it.)
> +#ifdef CONFIG_X86_SYSFB
> +#include <asm/sysfb.h>
> +#endif
Pet peeve, this looks sexier:
#ifdef CONFIG_X86_SYSFB
# include <asm/sysfb.h>
#endif
> @@ -1604,6 +1607,17 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
> }
> }
>
> +static void remove_conflicting_sysfb(const struct apertures_struct *apert,
> + bool primary)
> +{
> + if (!apert)
> + return;
> +
> +#ifdef CONFIG_X86_SYSFB
> + sysfb_unregister(apert, primary);
> +#endif
> +}
So why not make sysfb_unregister() accept a !apert parameter (it would
simply return), at which point remove_conflicting_sysfb() could be
eliminated and just be replaced with a direct sysfb_unregister() call
- with no #ifdefs.
We only need #ifdefs for the sysfb_unregister() declaration in the .h
file.
At first sight this looks simpler and cleaner for the fix itself - no
need for extra cleanups for this detail.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html