From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com ([62.209.51.94]:36827 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751766AbbHLEbD (ORCPT ); Wed, 12 Aug 2015 00:31:03 -0400 Message-ID: <55CACC13.7040907@st.com> Date: Wed, 12 Aug 2015 12:31:15 +0800 From: Denis Ciocca MIME-Version: 1.0 To: Linus Walleij , Jonathan Cameron , "linux-iio@vger.kernel.org" Subject: Re: [PATCH] iio: st_sensors: add debugfs register read hook References: <1439301665-4373-1-git-send-email-linus.walleij@linaro.org> In-Reply-To: <1439301665-4373-1-git-send-email-linus.walleij@linaro.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org Hi Linus, Thanks for your job! My comments inline. > This adds a debugfs hook to read/write registers in the ST > sensors using debugfs. Proved to be awesome help when trying > to debug why IRQs do not arrive. > > Cc: Denis Ciocca > Signed-off-by: Linus Walleij > --- > drivers/iio/accel/st_accel_core.c | 1 + > drivers/iio/common/st_sensors/st_sensors_core.c | 21 +++++++++++++++++++++ > drivers/iio/gyro/st_gyro_core.c | 1 + > drivers/iio/magnetometer/st_magn_core.c | 1 + > drivers/iio/pressure/st_pressure_core.c | 1 + > include/linux/iio/common/st_sensors.h | 4 ++++ > 6 files changed, 29 insertions(+) > > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c > index 6617e779aa87..f5f2684c0090 100644 > --- a/drivers/iio/accel/st_accel_core.c > +++ b/drivers/iio/accel/st_accel_core.c > @@ -612,6 +612,7 @@ static const struct iio_info accel_info = { > .attrs = &st_accel_attribute_group, > .read_raw = &st_accel_read_raw, > .write_raw = &st_accel_write_raw, > + .debugfs_reg_access = st_sensors_debugfs_reg_access, To keep the style, can u use & before the function name? > }; > > #ifdef CONFIG_IIO_TRIGGER > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c > index 8086cbcff87d..05b519ef111f 100644 > --- a/drivers/iio/common/st_sensors/st_sensors_core.c > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c > @@ -44,6 +44,27 @@ st_sensors_write_data_with_mask_error: > return err; > } > > +int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev, > + unsigned reg, unsigned writeval, > + unsigned *readval) > +{ > + struct st_sensor_data *sdata = iio_priv(indio_dev); > + u8 readdata; > + int err; > + > + if (!readval) > + return sdata->tf->write_byte(&sdata->tb, sdata->dev, > + (u8)reg, (u8)writeval); > + > + err = sdata->tf->read_byte(&sdata->tb, sdata->dev, (u8)reg, &readdata); > + if (err < 0) > + return err; > + > + *readval = (unsigned)readdata; > + > + return 0; > +} > + This function should export the symbol when modules are in use... > static int st_sensors_match_odr(struct st_sensor_settings *sensor_settings, > unsigned int odr, struct st_sensor_odr_avl *odr_out) > { > diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c > index ffe96642b6d0..39138e53ba24 100644 > --- a/drivers/iio/gyro/st_gyro_core.c > +++ b/drivers/iio/gyro/st_gyro_core.c > @@ -380,6 +380,7 @@ static const struct iio_info gyro_info = { > .attrs = &st_gyro_attribute_group, > .read_raw = &st_gyro_read_raw, > .write_raw = &st_gyro_write_raw, > + .debugfs_reg_access = st_sensors_debugfs_reg_access, > }; & same here > > #ifdef CONFIG_IIO_TRIGGER > diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c > index b4bcfb790f49..e4da514efa9f 100644 > --- a/drivers/iio/magnetometer/st_magn_core.c > +++ b/drivers/iio/magnetometer/st_magn_core.c > @@ -475,6 +475,7 @@ static const struct iio_info magn_info = { > .attrs = &st_magn_attribute_group, > .read_raw = &st_magn_read_raw, > .write_raw = &st_magn_write_raw, > + .debugfs_reg_access = st_sensors_debugfs_reg_access, > }; & same here > > int st_magn_common_probe(struct iio_dev *indio_dev) > diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c > index e881fa6291e9..1306d8284bdd 100644 > --- a/drivers/iio/pressure/st_pressure_core.c > +++ b/drivers/iio/pressure/st_pressure_core.c > @@ -397,6 +397,7 @@ static const struct iio_info press_info = { > .attrs = &st_press_attribute_group, > .read_raw = &st_press_read_raw, > .write_raw = &st_press_write_raw, > + .debugfs_reg_access = st_sensors_debugfs_reg_access, > }; & same here > > #ifdef CONFIG_IIO_TRIGGER > diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h > index 2c476acb87d9..d8b4a7bc4029 100644 > --- a/include/linux/iio/common/st_sensors.h > +++ b/include/linux/iio/common/st_sensors.h > @@ -269,6 +269,10 @@ void st_sensors_power_enable(struct iio_dev *indio_dev); > > void st_sensors_power_disable(struct iio_dev *indio_dev); > > +int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev, > + unsigned reg, unsigned writeval, > + unsigned *readval); > + > int st_sensors_set_odr(struct iio_dev *indio_dev, unsigned int odr); > > int st_sensors_set_dataready_irq(struct iio_dev *indio_dev, bool enable);