From: Jonathan Cameron <jic23@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>, linux-iio@vger.kernel.org
Cc: Giuseppe Barba <giuseppe.barba@st.com>,
Denis Ciocca <denis.ciocca@st.com>,
Gregor Boirie <gregor.boirie@parrot.com>,
Crestez Dan Leonard <leonard.crestez@intel.com>
Subject: Re: [PATCH 1/2] iio: st_sensors: centralize I2C probe() code
Date: Sat, 12 Nov 2016 16:11:14 +0000 [thread overview]
Message-ID: <3761c454-10e5-ba26-bee3-2cc4df5025e0@kernel.org> (raw)
In-Reply-To: <1478773133-23034-1-git-send-email-linus.walleij@linaro.org>
On 10/11/16 10:18, Linus Walleij wrote:
> The probe() calls for the ST Sensors I2C modules were all doing
> pretty much the same things for the different IIO subsubsystems,
> they were all allocating the same state container struct just
> giving different names to the same variable. Move it all to the
> ommon code in st_sensors_i2c.c to avoid code duplication.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Looks fine to me other the missing c in the description above ;)
Would like some input from Denis and / or Giuseppe on this
though (partly as I'm falling asleep so might well miss something!)
Also wondering why we ended up with the divide between generic
and specific where it was rather than as you have it here.
oh well. Made sense at the time I guess ;)
Jonathan
> ---
> drivers/iio/accel/st_accel_i2c.c | 16 +++-----
> drivers/iio/common/st_sensors/st_sensors_i2c.c | 53 +++++++++++++++++---------
> drivers/iio/gyro/st_gyro_i2c.c | 16 +++-----
> drivers/iio/magnetometer/st_magn_i2c.c | 16 +++-----
> drivers/iio/pressure/st_pressure_i2c.c | 16 +++-----
> include/linux/iio/common/st_sensors_i2c.h | 15 ++------
> 6 files changed, 59 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> index e9d427a5df7c..1256bb5a59ac 100644
> --- a/drivers/iio/accel/st_accel_i2c.c
> +++ b/drivers/iio/accel/st_accel_i2c.c
> @@ -92,23 +92,17 @@ MODULE_DEVICE_TABLE(of, st_accel_of_match);
> #endif
>
> static int st_accel_i2c_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> + const struct i2c_device_id *id)
> {
> struct iio_dev *indio_dev;
> - struct st_sensor_data *adata;
> int err;
>
> - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*adata));
> - if (!indio_dev)
> - return -ENOMEM;
> -
> - adata = iio_priv(indio_dev);
> - st_sensors_of_i2c_probe(client, st_accel_of_match);
> -
> - st_sensors_i2c_configure(indio_dev, client, adata);
> + err = st_sensors_i2c_probe(client, st_accel_of_match, &indio_dev);
> + if (err)
> + return err;
>
> err = st_accel_common_probe(indio_dev);
> - if (err < 0)
> + if (err)
> return err;
>
> return 0;
> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> index b43aa36031f8..ed6bd865cc0d 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> @@ -64,20 +64,6 @@ static const struct st_sensor_transfer_function st_sensors_tf_i2c = {
> .read_multiple_byte = st_sensors_i2c_read_multiple_byte,
> };
>
> -void st_sensors_i2c_configure(struct iio_dev *indio_dev,
> - struct i2c_client *client, struct st_sensor_data *sdata)
> -{
> - i2c_set_clientdata(client, indio_dev);
> -
> - indio_dev->dev.parent = &client->dev;
> - indio_dev->name = client->name;
> -
> - sdata->dev = &client->dev;
> - sdata->tf = &st_sensors_tf_i2c;
> - sdata->get_irq_data_ready = st_sensors_i2c_get_irq;
> -}
> -EXPORT_SYMBOL(st_sensors_i2c_configure);
> -
> #ifdef CONFIG_OF
> /**
> * st_sensors_of_i2c_probe() - device tree probe for ST I2C sensors
> @@ -91,8 +77,8 @@ EXPORT_SYMBOL(st_sensors_i2c_configure);
> * rely on that name from this point on. I2C client devices will be renamed
> * to match the internal kernel convention.
> */
> -void st_sensors_of_i2c_probe(struct i2c_client *client,
> - const struct of_device_id *match)
> +static void st_sensors_of_i2c_probe(struct i2c_client *client,
> + const struct of_device_id *match)
> {
> const struct of_device_id *of_id;
>
> @@ -104,9 +90,42 @@ void st_sensors_of_i2c_probe(struct i2c_client *client,
> strncpy(client->name, of_id->data, sizeof(client->name));
> client->name[sizeof(client->name) - 1] = '\0';
> }
> -EXPORT_SYMBOL(st_sensors_of_i2c_probe);
> +#else
> +static void st_sensors_of_i2c_probe(struct i2c_client *client,
> + const struct of_device_id *match)
> +{
> +}
> #endif
>
> +int st_sensors_i2c_probe(struct i2c_client *client,
> + const struct of_device_id *match,
> + struct iio_dev **ret_indio_dev)
> +{
> + struct iio_dev *indio_dev;
> + struct st_sensor_data *sdata;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*sdata));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + sdata = iio_priv(indio_dev);
> + st_sensors_of_i2c_probe(client, match);
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->name = client->name;
> +
> + sdata->dev = &client->dev;
> + sdata->tf = &st_sensors_tf_i2c;
> + sdata->get_irq_data_ready = st_sensors_i2c_get_irq;
> +
> + i2c_set_clientdata(client, indio_dev);
> +
> + *ret_indio_dev = indio_dev;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(st_sensors_i2c_probe);
> +
> MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
> MODULE_DESCRIPTION("STMicroelectronics ST-sensors i2c driver");
> MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
> index 40056b821036..2fac5c63c8b3 100644
> --- a/drivers/iio/gyro/st_gyro_i2c.c
> +++ b/drivers/iio/gyro/st_gyro_i2c.c
> @@ -60,23 +60,17 @@ MODULE_DEVICE_TABLE(of, st_gyro_of_match);
> #endif
>
> static int st_gyro_i2c_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> + const struct i2c_device_id *id)
> {
> struct iio_dev *indio_dev;
> - struct st_sensor_data *gdata;
> int err;
>
> - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*gdata));
> - if (!indio_dev)
> - return -ENOMEM;
> -
> - gdata = iio_priv(indio_dev);
> - st_sensors_of_i2c_probe(client, st_gyro_of_match);
> -
> - st_sensors_i2c_configure(indio_dev, client, gdata);
> + err = st_sensors_i2c_probe(client, st_gyro_of_match, &indio_dev);
> + if (err)
> + return err;
>
> err = st_gyro_common_probe(indio_dev);
> - if (err < 0)
> + if (err)
> return err;
>
> return 0;
> diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> index 8aa37af306ed..7f519d3fa973 100644
> --- a/drivers/iio/magnetometer/st_magn_i2c.c
> +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> @@ -48,23 +48,17 @@ MODULE_DEVICE_TABLE(of, st_magn_of_match);
> #endif
>
> static int st_magn_i2c_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> + const struct i2c_device_id *id)
> {
> struct iio_dev *indio_dev;
> - struct st_sensor_data *mdata;
> int err;
>
> - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*mdata));
> - if (!indio_dev)
> - return -ENOMEM;
> -
> - mdata = iio_priv(indio_dev);
> - st_sensors_of_i2c_probe(client, st_magn_of_match);
> -
> - st_sensors_i2c_configure(indio_dev, client, mdata);
> + err = st_sensors_i2c_probe(client, st_magn_of_match, &indio_dev);
> + if (err)
> + return err;
>
> err = st_magn_common_probe(indio_dev);
> - if (err < 0)
> + if (err)
> return err;
>
> return 0;
> diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> index ed18701c68c9..4d78e39c8a2f 100644
> --- a/drivers/iio/pressure/st_pressure_i2c.c
> +++ b/drivers/iio/pressure/st_pressure_i2c.c
> @@ -44,23 +44,17 @@ MODULE_DEVICE_TABLE(of, st_press_of_match);
> #endif
>
> static int st_press_i2c_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> + const struct i2c_device_id *id)
> {
> struct iio_dev *indio_dev;
> - struct st_sensor_data *press_data;
> int err;
>
> - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*press_data));
> - if (!indio_dev)
> - return -ENOMEM;
> -
> - press_data = iio_priv(indio_dev);
> - st_sensors_of_i2c_probe(client, st_press_of_match);
> -
> - st_sensors_i2c_configure(indio_dev, client, press_data);
> + err = st_sensors_i2c_probe(client, st_press_of_match, &indio_dev);
> + if (err)
> + return err;
>
> err = st_press_common_probe(indio_dev);
> - if (err < 0)
> + if (err)
> return err;
>
> return 0;
> diff --git a/include/linux/iio/common/st_sensors_i2c.h b/include/linux/iio/common/st_sensors_i2c.h
> index 1796af093368..b0c5342942da 100644
> --- a/include/linux/iio/common/st_sensors_i2c.h
> +++ b/include/linux/iio/common/st_sensors_i2c.h
> @@ -15,17 +15,8 @@
> #include <linux/iio/common/st_sensors.h>
> #include <linux/of.h>
>
> -void st_sensors_i2c_configure(struct iio_dev *indio_dev,
> - struct i2c_client *client, struct st_sensor_data *sdata);
> -
> -#ifdef CONFIG_OF
> -void st_sensors_of_i2c_probe(struct i2c_client *client,
> - const struct of_device_id *match);
> -#else
> -static inline void st_sensors_of_i2c_probe(struct i2c_client *client,
> - const struct of_device_id *match)
> -{
> -}
> -#endif
> +int st_sensors_i2c_probe(struct i2c_client *client,
> + const struct of_device_id *match,
> + struct iio_dev **ret_indio_dev);
>
> #endif /* ST_SENSORS_I2C_H */
>
next prev parent reply other threads:[~2016-11-12 16:11 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-10 10:18 [PATCH 1/2] iio: st_sensors: centralize I2C probe() code Linus Walleij
2016-11-12 16:11 ` Jonathan Cameron [this message]
2016-11-14 12:25 ` Linus Walleij
2016-11-19 12:35 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3761c454-10e5-ba26-bee3-2cc4df5025e0@kernel.org \
--to=jic23@kernel.org \
--cc=denis.ciocca@st.com \
--cc=giuseppe.barba@st.com \
--cc=gregor.boirie@parrot.com \
--cc=leonard.crestez@intel.com \
--cc=linus.walleij@linaro.org \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.