[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <53CBEF61.40909@kernel.org>
Date: Sun, 20 Jul 2014 17:33:37 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Josef Gajdusek <atx@...lax.net>, linux-iio@...r.kernel.org
CC: devel@...verdev.osuosl.org, gregkh@...uxfoundation.org,
linux-kernel@...r.kernel.org, pmeerw@...erw.net,
dan.carpenter@...cle.com, lars@...afoo.de
Subject: Re: [PATCH v4 (staging-next) 2/5] staging:iio:hmc5843: Split hmc5843.c
to multiple files
On 20/07/14 17:32, Jonathan Cameron wrote:
> On 16/07/14 14:07, Josef Gajdusek wrote:
>> This patch splits hmc5843.c to multiple files - the interface-agnostic
>> hmc5843_core.c, i2c specific hmc5843_i2c.c and header file hmc5843.h. This is
>> another step to add support of SPI-enabled hmc5983.
>>
>> Signed-off-by: Josef Gajdusek <atx@....name>
> Unfortunately taking the tables non static just causes sparse to complain:
>
> drivers/staging/iio/magnetometer/hmc5843.h:62:28: warning: symbol 'hmc5843_readable_table' was not declared. Should it be static?
> drivers/staging/iio/magnetometer/hmc5843.h:71:28: warning: symbol 'hmc5843_writable_table' was not declared. Should it be static?
> drivers/staging/iio/magnetometer/hmc5843.h:80:28: warning: symbol 'hmc5843_volatile_table' was not declared. Should it be static?
> CC [M] drivers/staging/iio/magnetometer/hmc5843_core.o
> CHECK drivers/staging/iio/magnetometer/hmc5843_i2c.c
> drivers/staging/iio/magnetometer/hmc5843.h:62:28: warning: symbol 'hmc5843_readable_table' was not declared. Should it be static?
> drivers/staging/iio/magnetometer/hmc5843.h:71:28: warning: symbol 'hmc5843_writable_table' was not declared. Should it be static?
> drivers/staging/iio/magnetometer/hmc5843.h:80:28: warning: symbol 'hmc5843_volatile_table' was not declared. Should it be static?
> CC [M] drivers/staging/iio/magnetometer/hmc5843_i2c.o
>
> I think you'll probably just have to duplicate them (which is irritating, but there we are).
>
> Jonathan
p.s. For now I've backed out patch 1 as well so that we can keep this as a
clean series rather than splitting it through time!
>> ---
>> drivers/staging/iio/magnetometer/Kconfig | 14 +-
>> drivers/staging/iio/magnetometer/Makefile | 3 +-
>> drivers/staging/iio/magnetometer/hmc5843.h | 85 ++++++++++++
>> .../iio/magnetometer/{hmc5843.c => hmc5843_core.c} | 154 +++++----------------
>> drivers/staging/iio/magnetometer/hmc5843_i2c.c | 75 ++++++++++
>> 5 files changed, 203 insertions(+), 128 deletions(-)
>> create mode 100644 drivers/staging/iio/magnetometer/hmc5843.h
>> rename drivers/staging/iio/magnetometer/{hmc5843.c => hmc5843_core.c} (79%)
>> create mode 100644 drivers/staging/iio/magnetometer/hmc5843_i2c.c
>>
>> diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
>> index ad88d61..0a27f98 100644
>> --- a/drivers/staging/iio/magnetometer/Kconfig
>> +++ b/drivers/staging/iio/magnetometer/Kconfig
>> @@ -4,16 +4,22 @@
>> menu "Magnetometer sensors"
>>
>> config SENSORS_HMC5843
>> - tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer"
>> - depends on I2C
>> + tristate
>> select IIO_BUFFER
>> select IIO_TRIGGERED_BUFFER
>> +
>> +config SENSORS_HMC5843_I2C
>> + tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer (I2C)"
>> + depends on I2C
>> + select SENSORS_HMC5843
>> select REGMAP_I2C
>> help
>> Say Y here to add support for the Honeywell HMC5843, HMC5883 and
>> HMC5883L 3-Axis Magnetometer (digital compass).
>>
>> - To compile this driver as a module, choose M here: the module
>> - will be called hmc5843.
>> + This driver can also be compiled as a set of modules.
>> + If so, these modules will be created:
>> + - hmc5843_core (core functions)
>> + - hmc5843_i2c (support for HMC5843, HMC5883 and HMC5883L)
>>
>> endmenu
>> diff --git a/drivers/staging/iio/magnetometer/Makefile b/drivers/staging/iio/magnetometer/Makefile
>> index f9bfb2e..65baf1c 100644
>> --- a/drivers/staging/iio/magnetometer/Makefile
>> +++ b/drivers/staging/iio/magnetometer/Makefile
>> @@ -2,4 +2,5 @@
>> # Makefile for industrial I/O Magnetometer sensors
>> #
>>
>> -obj-$(CONFIG_SENSORS_HMC5843) += hmc5843.o
>> +obj-$(CONFIG_SENSORS_HMC5843) += hmc5843_core.o
>> +obj-$(CONFIG_SENSORS_HMC5843_I2C) += hmc5843_i2c.o
>> diff --git a/drivers/staging/iio/magnetometer/hmc5843.h b/drivers/staging/iio/magnetometer/hmc5843.h
>> new file mode 100644
>> index 0000000..0d9b02e
>> --- /dev/null
>> +++ b/drivers/staging/iio/magnetometer/hmc5843.h
>> @@ -0,0 +1,85 @@
>> +/*
>> + * Header file for hmc5843 driver
>> + *
>> + * Split from hmc5843.c
>> + * Copyright (C) Josef Gajdusek <atx@....name>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * */
>> +
>> +
>> +#ifndef HMC5843_CORE_H
>> +#define HMC5843_CORE_H
>> +
>> +#include <linux/regmap.h>
>> +#include <linux/iio/iio.h>
>> +
>> +#define HMC5843_CONFIG_REG_A 0x00
>> +#define HMC5843_CONFIG_REG_B 0x01
>> +#define HMC5843_MODE_REG 0x02
>> +#define HMC5843_DATA_OUT_MSB_REGS 0x03
>> +#define HMC5843_STATUS_REG 0x09
>> +#define HMC5843_ID_REG 0x0a
>> +#define HMC5843_ID_END 0x0c
>> +
>> +enum hmc5843_ids {
>> + HMC5843_ID,
>> + HMC5883_ID,
>> + HMC5883L_ID,
>> +};
>> +
>> +struct hmc5843_data {
>> + struct device *dev;
>> + struct mutex lock;
>> + struct regmap *regmap;
>> + const struct hmc5843_chip_info *variant;
>> + __be16 buffer[8]; /* 3x 16-bit channels + padding + 64-bit timestamp */
>> +};
>> +
>> +int hmc5843_common_probe(struct device *dev, struct regmap *regmap,
>> + enum hmc5843_ids id);
>> +int hmc5843_common_remove(struct device *dev);
>> +
>> +int hmc5843_common_suspend(struct device *dev);
>> +int hmc5843_common_resume(struct device *dev);
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static SIMPLE_DEV_PM_OPS(hmc5843_pm_ops,
>> + hmc5843_common_suspend,
>> + hmc5843_common_resume);
>> +#define HMC5843_PM_OPS (&hmc5843_pm_ops)
>> +#else
>> +#define HMC5843_PM_OPS NULL
>> +#endif
>> +
>> +static const struct regmap_range hmc5843_readable_ranges[] = {
>> + regmap_reg_range(0, HMC5843_ID_END),
>> +};
>> +
>> +struct regmap_access_table hmc5843_readable_table = {
>> + .yes_ranges = hmc5843_readable_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(hmc5843_readable_ranges),
>> +};
>> +
>> +static const struct regmap_range hmc5843_writable_ranges[] = {
>> + regmap_reg_range(0, HMC5843_MODE_REG),
>> +};
>> +
>> +struct regmap_access_table hmc5843_writable_table = {
>> + .yes_ranges = hmc5843_writable_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(hmc5843_writable_ranges),
>> +};
>> +
>> +static const struct regmap_range hmc5843_volatile_ranges[] = {
>> + regmap_reg_range(HMC5843_DATA_OUT_MSB_REGS, HMC5843_STATUS_REG),
>> +};
>> +
>> +struct regmap_access_table hmc5843_volatile_table = {
>> + .yes_ranges = hmc5843_volatile_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(hmc5843_volatile_ranges),
>> +};
>> +
>> +#endif /* HMC5843_CORE_H */
>> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843_core.c
>> similarity index 79%
>> rename from drivers/staging/iio/magnetometer/hmc5843.c
>> rename to drivers/staging/iio/magnetometer/hmc5843_core.c
>> index 6f06f98..bdeaf43 100644
>> --- a/drivers/staging/iio/magnetometer/hmc5843.c
>> +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c
>> @@ -4,6 +4,8 @@
>>
>> Support for HMC5883 and HMC5883L by Peter Meerwald <pmeerw@...erw.net>.
>>
>> + Split to multiple files by Josef Gajdusek <atx@....name> - 2014
>> +
>> This program is free software; you can redistribute it and/or modify
>> it under the terms of the GNU General Public License as published by
>> the Free Software Foundation; either version 2 of the License, or
>> @@ -20,7 +22,6 @@
>> */
>>
>> #include <linux/module.h>
>> -#include <linux/i2c.h>
>> #include <linux/regmap.h>
>> #include <linux/iio/iio.h>
>> #include <linux/iio/sysfs.h>
>> @@ -29,19 +30,7 @@
>> #include <linux/iio/triggered_buffer.h>
>> #include <linux/delay.h>
>>
>> -#define HMC5843_CONFIG_REG_A 0x00
>> -#define HMC5843_CONFIG_REG_B 0x01
>> -#define HMC5843_MODE_REG 0x02
>> -#define HMC5843_DATA_OUT_MSB_REGS 0x03
>> -#define HMC5843_STATUS_REG 0x09
>> -#define HMC5843_ID_REG 0x0a
>> -#define HMC5843_ID_END 0x0c
>> -
>> -enum hmc5843_ids {
>> - HMC5843_ID,
>> - HMC5883_ID,
>> - HMC5883L_ID,
>> -};
>> +#include "hmc5843.h"
>>
>> /*
>> * Range gain settings in (+-)Ga
>> @@ -121,15 +110,6 @@ struct hmc5843_chip_info {
>> const int *regval_to_nanoscale;
>> };
>>
>> -/* Each client has this additional data */
>> -struct hmc5843_data {
>> - struct i2c_client *client;
>> - struct mutex lock;
>> - struct regmap *regmap;
>> - const struct hmc5843_chip_info *variant;
>> - __be16 buffer[8]; /* 3x 16-bit channels + padding + 64-bit timestamp */
>> -};
>> -
>> /* The lower two bits contain the current conversion mode */
>> static s32 hmc5843_set_mode(struct hmc5843_data *data, u8 operating_mode)
>> {
>> @@ -159,7 +139,7 @@ static int hmc5843_wait_measurement(struct hmc5843_data *data)
>> }
>>
>> if (tries < 0) {
>> - dev_err(&data->client->dev, "data not ready\n");
>> + dev_err(data->dev, "data not ready\n");
>> return -EIO;
>> }
>>
>> @@ -524,7 +504,7 @@ static int hmc5843_init(struct hmc5843_data *data)
>> if (ret < 0)
>> return ret;
>> if (id[0] != 'H' || id[1] != '4' || id[2] != '3') {
>> - dev_err(&data->client->dev, "no HMC5843/5883/5883L sensor\n");
>> + dev_err(data->dev, "no HMC5843/5883/5883L sensor\n");
>> return -ENODEV;
>> }
>>
>> @@ -550,66 +530,43 @@ static const struct iio_info hmc5843_info = {
>>
>> static const unsigned long hmc5843_scan_masks[] = {0x7, 0};
>>
>> -static const struct regmap_range hmc5843_readable_ranges[] = {
>> - regmap_reg_range(0, HMC5843_ID_END),
>> -};
>> -
>> -static struct regmap_access_table hmc5843_readable_table = {
>> - .yes_ranges = hmc5843_readable_ranges,
>> - .n_yes_ranges = ARRAY_SIZE(hmc5843_readable_ranges),
>> -};
>>
>> -static const struct regmap_range hmc5843_writable_ranges[] = {
>> - regmap_reg_range(0, HMC5843_MODE_REG),
>> -};
>> -
>> -static struct regmap_access_table hmc5843_writable_table = {
>> - .yes_ranges = hmc5843_writable_ranges,
>> - .n_yes_ranges = ARRAY_SIZE(hmc5843_writable_ranges),
>> -};
>> -
>> -static const struct regmap_range hmc5843_volatile_ranges[] = {
>> - regmap_reg_range(HMC5843_DATA_OUT_MSB_REGS, HMC5843_STATUS_REG),
>> -};
>> -
>> -static struct regmap_access_table hmc5843_volatile_table = {
>> - .yes_ranges = hmc5843_volatile_ranges,
>> - .n_yes_ranges = ARRAY_SIZE(hmc5843_volatile_ranges),
>> -};
>> -
>> -static struct regmap_config hmc5843_regmap_config = {
>> - .reg_bits = 8,
>> - .val_bits = 8,
>> -
>> - .rd_table = &hmc5843_readable_table,
>> - .wr_table = &hmc5843_writable_table,
>> - .volatile_table = &hmc5843_volatile_table,
>> +int hmc5843_common_suspend(struct device *dev)
>> +{
>> + return hmc5843_set_mode(iio_priv(dev_get_drvdata(dev)),
>> + HMC5843_MODE_CONVERSION_CONTINUOUS);
>> +}
>> +EXPORT_SYMBOL(hmc5843_common_suspend);
>>
>> - .cache_type = REGCACHE_RBTREE,
>> -};
>> +int hmc5843_common_resume(struct device *dev)
>> +{
>> + return hmc5843_set_mode(iio_priv(dev_get_drvdata(dev)),
>> + HMC5843_MODE_SLEEP);
>> +}
>> +EXPORT_SYMBOL(hmc5843_common_resume);
>>
>> -static int hmc5843_probe(struct i2c_client *client,
>> - const struct i2c_device_id *id)
>> +int hmc5843_common_probe(struct device *dev, struct regmap *regmap,
>> + enum hmc5843_ids id)
>> {
>> struct hmc5843_data *data;
>> struct iio_dev *indio_dev;
>> int ret;
>>
>> - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>> if (indio_dev == NULL)
>> return -ENOMEM;
>>
>> + dev_set_drvdata(dev, indio_dev);
>> +
>> /* default settings at probe */
>> data = iio_priv(indio_dev);
>> - data->client = client;
>> - data->variant = &hmc5843_chip_info_tbl[id->driver_data];
>> - data->regmap = devm_regmap_init_i2c(client, &hmc5843_regmap_config);
>> + data->dev = dev;
>> + data->regmap = regmap;
>> + data->variant = &hmc5843_chip_info_tbl[id];
>> mutex_init(&data->lock);
>>
>> - i2c_set_clientdata(client, indio_dev);
>> + indio_dev->dev.parent = dev;
>> indio_dev->info = &hmc5843_info;
>> - indio_dev->name = id->name;
>> - indio_dev->dev.parent = &client->dev;
>> indio_dev->modes = INDIO_DIRECT_MODE;
>> indio_dev->channels = data->variant->channels;
>> indio_dev->num_channels = 4;
>> @@ -634,10 +591,11 @@ buffer_cleanup:
>> iio_triggered_buffer_cleanup(indio_dev);
>> return ret;
>> }
>> +EXPORT_SYMBOL(hmc5843_common_probe);
>>
>> -static int hmc5843_remove(struct i2c_client *client)
>> +int hmc5843_common_remove(struct device *dev)
>> {
>> - struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>
>> iio_device_unregister(indio_dev);
>> iio_triggered_buffer_cleanup(indio_dev);
>> @@ -647,58 +605,8 @@ static int hmc5843_remove(struct i2c_client *client)
>>
>> return 0;
>> }
>> -
>> -#ifdef CONFIG_PM_SLEEP
>> -static int hmc5843_suspend(struct device *dev)
>> -{
>> - struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
>> - to_i2c_client(dev)));
>> -
>> - return hmc5843_set_mode(data, HMC5843_MODE_SLEEP);
>> -}
>> -
>> -static int hmc5843_resume(struct device *dev)
>> -{
>> - struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
>> - to_i2c_client(dev)));
>> -
>> - return hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
>> -}
>> -
>> -static SIMPLE_DEV_PM_OPS(hmc5843_pm_ops, hmc5843_suspend, hmc5843_resume);
>> -#define HMC5843_PM_OPS (&hmc5843_pm_ops)
>> -#else
>> -#define HMC5843_PM_OPS NULL
>> -#endif
>> -
>> -static const struct i2c_device_id hmc5843_id[] = {
>> - { "hmc5843", HMC5843_ID },
>> - { "hmc5883", HMC5883_ID },
>> - { "hmc5883l", HMC5883L_ID },
>> - { }
>> -};
>> -MODULE_DEVICE_TABLE(i2c, hmc5843_id);
>> -
>> -static const struct of_device_id hmc5843_of_match[] = {
>> - { .compatible = "honeywell,hmc5843", .data = (void *)HMC5843_ID },
>> - { .compatible = "honeywell,hmc5883", .data = (void *)HMC5883_ID },
>> - { .compatible = "honeywell,hmc5883l", .data = (void *)HMC5883L_ID },
>> - {}
>> -};
>> -MODULE_DEVICE_TABLE(of, hmc5843_of_match);
>> -
>> -static struct i2c_driver hmc5843_driver = {
>> - .driver = {
>> - .name = "hmc5843",
>> - .pm = HMC5843_PM_OPS,
>> - .of_match_table = hmc5843_of_match,
>> - },
>> - .id_table = hmc5843_id,
>> - .probe = hmc5843_probe,
>> - .remove = hmc5843_remove,
>> -};
>> -module_i2c_driver(hmc5843_driver);
>> +EXPORT_SYMBOL(hmc5843_common_remove);
>>
>> MODULE_AUTHOR("Shubhrajyoti Datta <shubhrajyoti@...com>");
>> -MODULE_DESCRIPTION("HMC5843/5883/5883L driver");
>> +MODULE_DESCRIPTION("HMC5843/5883/5883L core driver");
>> MODULE_LICENSE("GPL");
>> diff --git a/drivers/staging/iio/magnetometer/hmc5843_i2c.c b/drivers/staging/iio/magnetometer/hmc5843_i2c.c
>> new file mode 100644
>> index 0000000..20b5c93
>> --- /dev/null
>> +++ b/drivers/staging/iio/magnetometer/hmc5843_i2c.c
>> @@ -0,0 +1,75 @@
>> +/*
>> + * i2c driver for hmc5843/5843/5883/5883l
>> + *
>> + * Split from hmc5843.c
>> + * Copyright (C) Josef Gajdusek <atx@....name>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * */
>> +
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +
>> +#include "hmc5843.h"
>> +
>> +static struct regmap_config hmc5843_i2c_regmap_config = {
>> + .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)
>> +{
>> + return hmc5843_common_probe(&client->dev,
>> + devm_regmap_init_i2c(client, &hmc5843_i2c_regmap_config),
>> + id->driver_data);
>> +}
>> +
>> +static int hmc5843_i2c_remove(struct i2c_client *client)
>> +{
>> + return hmc5843_common_remove(&client->dev);
>> +}
>> +
>> +static const struct i2c_device_id hmc5843_id[] = {
>> + { "hmc5843", HMC5843_ID },
>> + { "hmc5883", HMC5883_ID },
>> + { "hmc5883l", HMC5883L_ID },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, hmc5843_id);
>> +
>> +static const struct of_device_id hmc5843_of_match[] = {
>> + { .compatible = "honeywell,hmc5843", .data = (void *)HMC5843_ID },
>> + { .compatible = "honeywell,hmc5883", .data = (void *)HMC5883_ID },
>> + { .compatible = "honeywell,hmc5883l", .data = (void *)HMC5883L_ID },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, hmc5843_of_match);
>> +
>> +static struct i2c_driver hmc5843_driver = {
>> + .driver = {
>> + .name = "hmc5843",
>> + .pm = HMC5843_PM_OPS,
>> + .of_match_table = hmc5843_of_match,
>> + },
>> + .id_table = hmc5843_id,
>> + .probe = hmc5843_i2c_probe,
>> + .remove = hmc5843_i2c_remove,
>> +};
>> +module_i2c_driver(hmc5843_driver);
>> +
>> +MODULE_AUTHOR("Josef Gajdusek <atx@....name>");
>> +MODULE_DESCRIPTION("HMC5843/5883/5883L i2c driver");
>> +MODULE_LICENSE("GPL");
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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/
Powered by blists - more mailing lists