linux-kernel - Re: [PATCH 1/3 v4] mfd: Add LPC driver for Intel ICH chipsets

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: <20120218204448.224903bf@endymion.delvare>
Date:	Sat, 18 Feb 2012 20:44:48 +0100
From:	Jean Delvare <khali@...ux-fr.org>
To:	Aaron Sierra <asierra@...-inc.com>
Cc:	Guenter Roeck <guenter@...ck-us.net>,
	Peter Tyser <ptyser@...-inc.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3 v4] mfd: Add LPC driver for Intel ICH chipsets

Oh, BTW...

On Fri, 17 Feb 2012 17:28:23 -0600 (CST), Aaron Sierra wrote:
> This driver currently creates resources for use by a forthcoming ICH
> chipset GPIO driver. It could be expanded to created the resources for
> converting the esb2rom (mtd) and iTCO_wdt (wdt), and potentially more,
> drivers to use the mfd model.
> 
> Signed-off-by: Aaron Sierra <asierra@...-inc.com>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
> (...)
> +static void lpc_ich_finalize_cell(struct mfd_cell *cell,
> +					const struct pci_device_id *id)
> +{
> +	cell->id = id->driver_data;

I don't think this makes any sense. By using global variables for
per-device resources and states, your driver pretty much assumes that
at most one supported device is present on every given system (which is
indeed always the case AFAIK.) So you should set id to 0 here...

> +	cell->platform_data = &lpc_chipset_info[id->driver_data];
> +	cell->pdata_size = sizeof(struct lpc_ich_info);
> +}
> (...)
> +	ret = mfd_add_devices(&dev->dev, 0, &lpc_ich_cells[LPC_GPIO],
> +				1, NULL, 0);

... and to -1 here, so that the platform devices created don't receive a
number at all. Many mfd drivers do exactly this.

-- 
Jean Delvare
--
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/