From: Matthew Garrett <matthew.garrett@nebula.com>
To: Qiaowei Ren <qiaowei.ren@intel.com>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>,
        Xiaoyan Zhang <xiaoyan.zhang@intel.com>, Gang Wei <gang.wei@intel.com>
Subject: Re: [PATCH 1/4] driver: add TXT driver in kernel
Thread-Topic: [PATCH 1/4] driver: add TXT driver in kernel
Thread-Index: AQHOSvE9AcunBqr7IUa9oxQD3ESIK5j6kqOA
Date: Wed, 8 May 2013 02:32:19 +0000
Message-ID: <1367980339.2425.7.camel@x230>
References: <1367938519-840-1-git-send-email-qiaowei.ren@intel.com>
	 <1367938519-840-2-git-send-email-qiaowei.ren@intel.com>
In-Reply-To: <1367938519-840-2-git-send-email-qiaowei.ren@intel.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
Content-ID: <E76B48CC4F8F0F46BE86747DC3B95E5C@namprd05.prod.outlook.com>
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 1447
Lines: 38

On Tue, 2013-05-07 at 22:55 +0800, Qiaowei Ren wrote:
> TXT driver is expected to be a better tool to access below resources:
> TXT config space, TXT log and SMX parameter.

Please explain what these things are in the commit log, the config help
and the comment at the top of the driver - it'll make things easier for
people later.

> +         To compile this driver as a module, choose M here; the module will be
> +         called txt.

That's an awfully generic name. intel_txt would be better.

> + * This module is expected to be a better tool to access below resources

Expected to be or is? :)

> + * Data is currently found below

You're adding it to ABI, so it shouldn't just be current - are you
planning on moving it at any point? Is this functionality that should be
under securityfs instead?

> +	txt_pdev = platform_device_register_simple(DEV_NAME, -1, NULL, 0);

This will register even if there's no TXT support on the platform,
right? Is there no mechanism for determining whether a platform has TXT
support before registering a device?

> +module_init(txt_sysfs_init);
> +module_exit(txt_sysfs_exit);

And you've got no module aliases, so this will never be autoloaded. How
does the OS know that the platform implements TXT?

-- 
Matthew Garrett | mjg59@srcf.ucam.org
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?