Re: [PATCH] driver for the "Fujitsu Tablet PC Buttons" device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 12, 2009 at 12:40:35PM +0200, Robert Gerlach wrote:
> +static struct {						/* fscbtns_t */
> +	struct platform_device *pdev;
> +	struct input_dev *idev_b;		/* tablet buttons */
> +	struct input_dev *idev_s;		/* orientation switch */
> +#if (defined(STICKY_TIMEOUT) && (STICKY_TIMEOUT > 0))
> +	struct timer_list timer;
> +#endif

Why would you not want this defined?

> +	unsigned int interrupt;
> +	unsigned int address;
> +	struct fscbtns_config config;
> +
> +	int orientation;
> +} fscbtns;
> +
> +static unsigned int user_model;
> +module_param_named(model, user_model, uint, 0);
> +MODULE_PARM_DESC(model, "model (1 = Stylistic, 2 = Lifebook T- and P-Series, 3 = Stylistic ST5xxx, 4 = Lifebook U800)");

I don't think you need this module parameter anymore, right?  Now that
you are matching on DMI strings, it can be detected automatically.

> +static struct dmi_system_id dmi_ids[] __initdata = {
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Fujitsu Siemens P/T Series",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK")
> +		},
> +		.driver_data = &config_Lifebook_Tseries
> +	},
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Fujitsu Lifebook T Series",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "LifeBook T")
> +		},
> +		.driver_data = &config_Lifebook_Tseries
> +	},
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Fujitsu Siemens Stylistic T Series",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Stylistic T")
> +		},
> +		.driver_data = &config_Stylistic_Tseries
> +	},
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Fujitsu LifeBook U810",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "LifeBook U810")
> +		},
> +		.driver_data = &config_Lifebook_U810
> +	},
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Fujitsu Siemens Stylistic ST5xxx Series",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "STYLISTIC ST5")
> +		},
> +		.driver_data = &config_Stylistic_ST5xxx
> +	},
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Fujitsu Siemens Stylistic ST5xxx Series",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Stylistic ST5")
> +		},
> +		.driver_data = &config_Stylistic_ST5xxx
> +	},
> +	{
> +		.callback = fscbtns_dmi_matched,
> +		.ident = "Unknown (using defaults)",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, ""),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "")
> +		},
> +		.driver_data = &config_Lifebook_Tseries
> +	},
> +	{ NULL }
> +};

Add a MODULE_DEVICE_TABLE for this structure, and then it will be loaded
automatically by the userspace module scripts if the device is present
on the machine.  It should be as simple as adding the line:
	MODULE_DEVICE_TABLE(dmi, dmi_ids);

other than that, looks good.  Take Jiri's advice and cc: the proper
people on the patch so it can be applied.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux