linux-kernel - Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <51D1A4B4.2040905@wwwdotorg.org>
Date:	Mon, 01 Jul 2013 09:48:04 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	David Herrmann <dh.herrmann@...il.com>
CC:	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Dave Airlie <airlied@...il.com>, linux-fbdev@...r.kernel.org,
	Stephen Warren <swarren@...dia.com>,
	Olof Johansson <olof@...om.net>
Subject: Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers

On 06/28/2013 04:11 AM, David Herrmann wrote:
> Hi
> 
> On Wed, Jun 26, 2013 at 10:49 PM, Stephen Warren <swarren@...dotorg.org> wrote:
>> On 06/24/2013 04:27 PM, David Herrmann wrote:
>>> The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
>>> x86 causes troubles when loading multiple fbdev drivers. The global
>>> "struct screen_info" does not provide any state-tracking about which
>>> drivers use the FBs. request_mem_region() theoretically works, but
>>> unfortunately vesafb/efifb ignore it due to quirks for broken boards.
>>>
>>> Avoid this by creating a "platform-framebuffer" device with a pointer
>>> to the "struct screen_info" as platform-data. Drivers can now create
>>> platform-drivers and the driver-core will refuse multiple drivers being
>>> active simultaneously.
>>>
>>> We keep the screen_info available for backwards-compatibility. Drivers
>>> can be converted in follow-up patches.
>>>
>>> Apart from "platform-framebuffer" devices, this also introduces a
>>> compatibility option for "simple-framebuffer" drivers which recently got
>>> introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
>>> try to match the screen_info against a simple-framebuffer supported
>>> format. If we succeed, we create a "simple-framebuffer" device instead
>>> of a platform-framebuffer.
...

>>> diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
...
>>> +#else /* CONFIG_X86_SYSFB */
>>> +
>>> +static bool parse_mode(const struct screen_info *si,
>>> +                    struct simplefb_platform_data *mode)
>>> +{
>>> +     return false;
>>> +}
>>> +
>>> +static int create_simplefb(const struct screen_info *si,
>>> +                        const struct simplefb_platform_data *mode)
>>> +{
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +#endif /* CONFIG_X86_SYSFB */
>>
>> Following on from my ifdef comment above, I believe those versions of
>> those functions will always cause add_sysfb() to return -ENODEV, so you
>> may as well provide a static inline for add_sysfb() instead.
> 
> No. add_sysfb() is supposed to always succeed. However, if
> parse_mode/create_simplefb fail, it creates a "platform-framebuffer"
> as fallback. I don't see any way to avoid these ifdefs. Considering
> the explanation above, could you elaborate how you think this should
> work?

Ah, I wasn't getting the fallback mechanism; that if creating a simplefb
wasn't possible or didn't succeed, a platformfb device would be created
instead.

Perhaps the following might be slightly clearer; there are certainly
fewer nesting levels:

static __init int add_sysfb(void)
{
	const struct screen_info *si = &screen_info;
	struct simplefb_platform_data mode;
	struct platform_device *pd;
	bool compatible = false;
	int ret;

	compatible = parse_mode(si, &mode);
	if (compatible) {
		ret = create_simplefb(si, &mode);
		if (!ret)
			return 0;
	}

	pd = platform_device_register_resndata(NULL,
					"platform-framebuffer", 0,
					NULL, 0, si, sizeof(*si));
	ret = IS_ERR(pd) ? PTR_ERR(pd) : 0;

	return ret;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/