Re: [PATCH v3 2/5] staging:iio:hmc5843: Split hmc5843.c to multiple files

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

 



On 07/08/2014 03:39 PM, Josef Gajdusek wrote:
[...]
diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
index ad88d61..28c2612 100644
--- a/drivers/staging/iio/magnetometer/Kconfig
+++ b/drivers/staging/iio/magnetometer/Kconfig
@@ -5,15 +5,23 @@ menu "Magnetometer sensors"

  config SENSORS_HMC5843
  	tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer"
-	depends on I2C
+	depends on (I2C || SPI_MASTER)
  	select IIO_BUFFER
  	select IIO_TRIGGERED_BUFFER
-	select REGMAP_I2C
+	select SENSORS_HMC5843_I2C if (I2C)

This approach doesn't work to well and will cause randconfig build failures. If SPI_MASTER is 'y' and I2C is 'm' you'll be able to select this driver as built-in, which in turn will also select SENSORS_HMC5843_I2C as built-in, which means you'll get unresolved symbols during linking since core I2C support is built as a module. A better approach is to have a user-selectable symbol per bus and a non-user-selectable symbol for the core infrastructure of the driver. e.g.

config SENSORS_HMC5843
	tristate
	select IIO_BUFFER
	...

config SENSORS_HMC5843_I2C
	tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer (I2C)"
	select SENSORS_HMC5843
	select REGMAP_I2C

config SENSORS_HMC5843_SPI
	tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer (SPI)"
	select SENSORS_HMC5843
	select REGMAP_SPI



> +struct regmap_config hmc5843_i2c_regmap_config = {

static

> +		.reg_bits = 8,
> +		.val_bits = 8,
> +
> +		.rd_table = &hmc5843_readable_table,
> +		.wr_table = &hmc5843_writable_table,
> +		.volatile_table = &hmc5843_volatile_table,
> +
> +		.cache_type = REGCACHE_RBTREE,
> +};


> +static int hmc5843_i2c_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct hmc5843_data *data;
> +	struct iio_dev *indio_dev;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	data = iio_priv(indio_dev);
> +	data->dev = &client->dev;
> +	data->regmap = devm_regmap_init_i2c(client, &hmc5843_i2c_regmap_config);
> +
> +	indio_dev->name = id->name;
> +	indio_dev->dev.parent = &client->dev;
> +
> +	return hmc5843_common_probe(indio_dev, id->driver_data);
> +}

If you do the allocation of the IIO device in the common function this can be simplified a bit. E.g.

static int hmc5843_i2c_probe(struct i2c_client *client,
			 const struct i2c_device_id *id)
{
	return hmc5853_common_probe(&client->dev,
		devm_regmap_init_i2c(client, &mc5843_i2c_regmap_config),
		id->driver_data);
}


> +#ifdef CONFIG_PM_SLEEP
> +static int hmc5843_i2c_suspend(struct device *dev)
> +{
> +	return hmc5843_common_suspend(i2c_get_clientdata(to_i2c_client(dev)));
> +}
> +
> +static int hmc5843_i2c_resume(struct device *dev)
> +{
> +	return hmc5843_common_resume(i2c_get_clientdata(to_i2c_client(dev)));
> +}
> +
> +static SIMPLE_DEV_PM_OPS(hmc5843_pm_ops,
> +		hmc5843_i2c_suspend, hmc5843_i2c_resume);
> +#define HMC5843_PM_OPS (&hmc5843_pm_ops)
> +#else
> +#define HMC5843_PM_OPS NULL
> +#endif

Those ops will be the same for both SPI and I2C. i2c_get_clientdata(to_i2c_client(dev)) is the same as dev_get_drvdata(dev), so this can go into the core driver.


Also as a hint for future patches, if you rename a file use 'git-format-patch -M', this will make the patch a bit more legible.

- Lars
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-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