linux-kernel - Re: [RFC PATCH] slimbus: Linux driver framework for SLIMbus.

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]

Message-ID: <20110814143401.GA25504@sirena.org.uk>
Date:	Sun, 14 Aug 2011 15:34:01 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Kenneth Heitke <kheitke@...eaurora.org>
Cc:	davidb@...eaurora.org, bryanh@...eaurora.org,
	linux-arm-msm@...r.kernel.org,
	Sagar Dharia <sdharia@...eaurora.org>, rdunlap@...otime.net,
	rmk+kernel@....linux.org.uk, john.stultz@...aro.org, arnd@...db.de,
	akpm@...ux-foundation.org, ohad@...ery.com, gregkh@...e.de,
	stefanr@...6.in-berlin.de, lethal@...ux-sh.org,
	linville@...driver.com, zajec5@...il.com,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] slimbus: Linux driver framework for SLIMbus.

On Wed, Aug 10, 2011 at 05:31:28PM -0600, Kenneth Heitke wrote:

> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.

Please CC me on any future spins of this patch.

> Signed-off-by: Sagar Dharia <sdharia@...eaurora.org>

You ought to sign off the patch as well as you're part of the chain for
it getting into the kernel.

> +#ifdef CONFIG_PM_SLEEP
> +static int slim_legacy_suspend(struct device *dev, pm_message_t mesg)
> +{
> +	struct slim_device *slim_dev = NULL;

Why have legacy stuff in a newly implemented subsystem?

> +static int slim_drv_probe(struct device *dev)
> +{
> +	const struct slim_driver *sdrv = to_slim_driver(dev->driver);
> +
> +	if (sdrv->probe)
> +		return sdrv->probe(to_slim_device(dev));
> +	return -ENODEV;
> +}

Why all the -ENODEVs if there's no function?  I'd expect that if there's
nothing to do it should be possible to omit functions from drivers.

> +int slim_driver_register(struct slim_driver *drv)
> +{
> +	drv->driver.bus = &slimbus_type;
> +	if (drv->probe)
> +		drv->driver.probe = slim_drv_probe;

Modifying the driver structure seems icky, why is this needed?  Other
bus types appear to manage without.

> +static u16 slim_slicecodefromsize(u32 req)
> +{
> +	u8 codetosize[8] = {1, 2, 3, 4, 6, 8, 12, 16};
> +	if (req >= 8)
> +		return 0;

ARRAY_SIZE()?  I guess the table should be static too.
--
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/