linux-kernel - Re: [PATCH RFC 3/3] drm/exynos: use pending

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 17 Apr 2014 23:04:12 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Andrzej Hajda <a.hajda@...sung.com>
Cc:	dri-devel@...ts.freedesktop.org,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Inki Dae <inki.dae@...sung.com>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	"moderated list:ARM/S5P EXYNOS AR..." 
	<linux-samsung-soc@...r.kernel.org>,
	Tomasz Figa <t.figa@...sung.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	David Airlie <airlied@...ux.ie>,
	open list <linux-kernel@...r.kernel.org>,
	"moderated list:ARM/S5P EXYNOS AR..." 
	<linux-arm-kernel@...ts.infradead.org>,
	Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH RFC 3/3] drm/exynos: use pending_components for
	components tracking

On Thu, Apr 17, 2014 at 01:28:50PM +0200, Andrzej Hajda wrote:
> +static int exynos_drm_add_blocker(struct device *dev, void *data)
> +{
> +	struct device_driver *drv = data;
> +
> +	if (!platform_bus_type.match(dev, drv))
> +		return 0;
> +
> +	device_lock(dev);
> +	if (!dev->driver)
> +		exynos_drm_dev_busy(dev);
> +	device_unlock(dev);
> +
> +	return 0;
> +}
> +
> +static void exynos_drm_init_blockers(struct device_driver *drv)
> +{
> +	bus_for_each_dev(&platform_bus_type, NULL, drv, exynos_drm_add_blocker);
> +}

This feels very wrong to be dumping the above code into every driver which
wants to make use of some kind of componentised support.

You also appear to need to know the struct device_driver's for every
component.  While that may work for exynos, it doesn't work elsewhere
where the various components of the system are very real separate
kernel modules - for example, a separate I2C driver such as the TDA998x
case I mentioned in my first reply.

I can't see how your solution would be usable in that circumstance.

The third issue I have is that you're still needing to have internal
exynos sub-device management - you're having to add the individual
devices to some kind of list, array or static data, and during DRM
probe you're having to then walk these lists/arrays/static data to
locate these sub-devices and finish each of their individual
initialisations.  So you're ending up with a two-tier initialisation.

That's not particularly good because it means you're exposed to
problems where the state is different between two initialisations -
when the device is recreated, your component attempts to re-finalise
the initialisation a second time.  It wouldn't take much for a field
to be assumed to be zero at init time somewhere for a bug to creep
in.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
--
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/