Re: [PATCH 1/4] driver: add TXT driver in kernel

From: Matthew Garrett
Date: Tue May 07 2013 - 22:34:03 EST


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@xxxxxxxxxxxxx
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸ ¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðà nÆàþY&—