* [PATCH] iio: magnetometer: hmc5843: Clean up redundant code.
@ 2018-09-20 14:13 Song Qiang
2018-09-21 8:10 ` kbuild test robot
0 siblings, 1 reply; 5+ messages in thread
From: Song Qiang @ 2018-09-20 14:13 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, shubhrajyoti
Cc: linux-iio, linux-kernel, Song Qiang
Regmap tables are both defined in hmc5843_spi.c and hmc5843_i2c.c, while
the only difference between these two set of tables is SPI need a read
mask, which is set only in 'regmap_config' structrue. This patch moves
the other structrues into hmc5843_core.c to reduce redundance.
Signed-off-by: Song Qiang <songqiang1304521@gmail.com>
---
drivers/iio/magnetometer/hmc5843.h | 4 ++++
drivers/iio/magnetometer/hmc5843_core.c | 28 +++++++++++++++++++++++++
drivers/iio/magnetometer/hmc5843_i2c.c | 27 ------------------------
drivers/iio/magnetometer/hmc5843_spi.c | 27 ------------------------
4 files changed, 32 insertions(+), 54 deletions(-)
diff --git a/drivers/iio/magnetometer/hmc5843.h b/drivers/iio/magnetometer/hmc5843.h
index 76a5d7484d8d..0ab400f27c97 100644
--- a/drivers/iio/magnetometer/hmc5843.h
+++ b/drivers/iio/magnetometer/hmc5843.h
@@ -46,6 +46,10 @@ struct hmc5843_data {
__be16 buffer[8];
};
+extern const struct regmap_access_table hmc5843_readable_table;
+extern const struct regmap_access_table hmc5843_writable_table;
+extern const struct regmap_access_table hmc5843_volatile_table;
+
int hmc5843_common_probe(struct device *dev, struct regmap *regmap,
enum hmc5843_ids id, const char *name);
int hmc5843_common_remove(struct device *dev);
diff --git a/drivers/iio/magnetometer/hmc5843_core.c b/drivers/iio/magnetometer/hmc5843_core.c
index ada142fb7aa3..a18e6d89f42a 100644
--- a/drivers/iio/magnetometer/hmc5843_core.c
+++ b/drivers/iio/magnetometer/hmc5843_core.c
@@ -92,6 +92,34 @@ static const char *const hmc5843_meas_conf_modes[] = {"normal", "positivebias",
static const char *const hmc5983_meas_conf_modes[] = {"normal", "positivebias",
"negativebias",
"disabled"};
+
+static const struct regmap_range hmc5843_readable_ranges[] = {
+ regmap_reg_range(0, HMC5843_ID_END),
+};
+
+const 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),
+};
+
+const 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),
+};
+
+const struct regmap_access_table hmc5843_volatile_table = {
+ .yes_ranges = hmc5843_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(hmc5843_volatile_ranges),
+};
+
/* Scaling factors: 10000000/Gain */
static const int hmc5843_regval_to_nanoscale[] = {
6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
diff --git a/drivers/iio/magnetometer/hmc5843_i2c.c b/drivers/iio/magnetometer/hmc5843_i2c.c
index 3de7f4426ac4..a2c45a23e3c9 100644
--- a/drivers/iio/magnetometer/hmc5843_i2c.c
+++ b/drivers/iio/magnetometer/hmc5843_i2c.c
@@ -17,33 +17,6 @@
#include "hmc5843.h"
-static const struct regmap_range hmc5843_readable_ranges[] = {
- regmap_reg_range(0, HMC5843_ID_END),
-};
-
-static const 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 const 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 const struct regmap_access_table hmc5843_volatile_table = {
- .yes_ranges = hmc5843_volatile_ranges,
- .n_yes_ranges = ARRAY_SIZE(hmc5843_volatile_ranges),
-};
-
static const struct regmap_config hmc5843_i2c_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
diff --git a/drivers/iio/magnetometer/hmc5843_spi.c b/drivers/iio/magnetometer/hmc5843_spi.c
index 535f03a70d63..19541f5bb38f 100644
--- a/drivers/iio/magnetometer/hmc5843_spi.c
+++ b/drivers/iio/magnetometer/hmc5843_spi.c
@@ -14,33 +14,6 @@
#include "hmc5843.h"
-static const struct regmap_range hmc5843_readable_ranges[] = {
- regmap_reg_range(0, HMC5843_ID_END),
-};
-
-static const 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 const 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 const struct regmap_access_table hmc5843_volatile_table = {
- .yes_ranges = hmc5843_volatile_ranges,
- .n_yes_ranges = ARRAY_SIZE(hmc5843_volatile_ranges),
-};
-
static const struct regmap_config hmc5843_spi_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: magnetometer: hmc5843: Clean up redundant code.
2018-09-20 14:13 [PATCH] iio: magnetometer: hmc5843: Clean up redundant code Song Qiang
@ 2018-09-21 8:10 ` kbuild test robot
2018-09-21 18:26 ` Himanshu Jha
0 siblings, 1 reply; 5+ messages in thread
From: kbuild test robot @ 2018-09-21 8:10 UTC (permalink / raw)
To: Song Qiang
Cc: kbuild-all, jic23, knaack.h, lars, pmeerw, shubhrajyoti,
linux-iio, linux-kernel, Song Qiang
[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]
Hi Song,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on iio/togreg]
[also build test ERROR on v4.19-rc4 next-20180920]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Song-Qiang/iio-magnetometer-hmc5843-Clean-up-redundant-code/20180921-091239
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-randconfig-u0-09211331 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "hmc5843_volatile_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
>> ERROR: "hmc5843_readable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
>> ERROR: "hmc5843_writable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28234 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: magnetometer: hmc5843: Clean up redundant code.
2018-09-21 8:10 ` kbuild test robot
@ 2018-09-21 18:26 ` Himanshu Jha
2018-09-22 10:04 ` Song Qiang
0 siblings, 1 reply; 5+ messages in thread
From: Himanshu Jha @ 2018-09-21 18:26 UTC (permalink / raw)
To: songqiang1304521; +Cc: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
Hi Song,
On Fri, Sep 21, 2018 at 04:10:16PM +0800, kbuild test robot wrote:
> Hi Song,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on iio/togreg]
> [also build test ERROR on v4.19-rc4 next-20180920]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Song-Qiang/iio-magnetometer-hmc5843-Clean-up-redundant-code/20180921-091239
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: x86_64-randconfig-u0-09211331 (attached as .config)
> compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> >> ERROR: "hmc5843_volatile_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> >> ERROR: "hmc5843_readable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> >> ERROR: "hmc5843_writable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
You would need to export these above symbols using EXPORT_SYMBOL()
to be used by i2c/spi modules.
But on the other hand, exporting too many symbols is a bad idea since
it is only used for this driver and not at any other place in IIO.
So, in my opinion drop this patch and leave the code as-is.
https://lkml.org/lkml/2018/7/16/566 --> worth reading
Thanks
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: magnetometer: hmc5843: Clean up redundant code.
2018-09-21 18:26 ` Himanshu Jha
@ 2018-09-22 10:04 ` Song Qiang
2018-09-22 10:18 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Song Qiang @ 2018-09-22 10:04 UTC (permalink / raw)
To: Himanshu Jha; +Cc: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel
On Fri, Sep 21, 2018 at 11:56:16PM +0530, Himanshu Jha wrote:
> Hi Song,
>
> On Fri, Sep 21, 2018 at 04:10:16PM +0800, kbuild test robot wrote:
> > Hi Song,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on iio/togreg]
> > [also build test ERROR on v4.19-rc4 next-20180920]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url: https://github.com/0day-ci/linux/commits/Song-Qiang/iio-magnetometer-hmc5843-Clean-up-redundant-code/20180921-091239
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > config: x86_64-randconfig-u0-09211331 (attached as .config)
> > compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
> > reproduce:
> > # save the attached .config to linux build tree
> > make ARCH=x86_64
> >
> > All errors (new ones prefixed by >>):
> >
> > >> ERROR: "hmc5843_volatile_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> > >> ERROR: "hmc5843_readable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> > >> ERROR: "hmc5843_writable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
>
> You would need to export these above symbols using EXPORT_SYMBOL()
> to be used by i2c/spi modules.
>
> But on the other hand, exporting too many symbols is a bad idea since
> it is only used for this driver and not at any other place in IIO.
> So, in my opinion drop this patch and leave the code as-is.
>
> https://lkml.org/lkml/2018/7/16/566 --> worth reading
>
>
> Thanks
> --
> Himanshu Jha
> Undergraduate Student
> Department of Electronics & Communication
> Guru Tegh Bahadur Institute of Technology
Hi Himanshu,
You're right, that's exactly what I was missing!
I saw the link you mentioned above and I also think that's a very good
idea to limit the scope of symbols. But I don't know when this work can
be applied to the kernel, as it seems like a not little change for the
build infrastructure.
I think this maybe a common problem for some drivers.
Divers for bmc150 in drivers/iio/accel/bmc-150-accel-core.c did the same
exporting stuff as I was prefered. So I think even if either exporting or
duplicating is not good enough, one must be choosed for now.
I think this is a topic that I have some ideas but not experienced
enough to say what should we do is better. I would like to hear Jonathan's
ideas about this. If this patched shouldn't be applied, then maybe bmc150
should be patched.
yours,
Song Qiang
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: magnetometer: hmc5843: Clean up redundant code.
2018-09-22 10:04 ` Song Qiang
@ 2018-09-22 10:18 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2018-09-22 10:18 UTC (permalink / raw)
To: Song Qiang; +Cc: Himanshu Jha, knaack.h, lars, pmeerw, linux-iio, linux-kernel
On Sat, 22 Sep 2018 18:04:19 +0800
Song Qiang <songqiang1304521@gmail.com> wrote:
> On Fri, Sep 21, 2018 at 11:56:16PM +0530, Himanshu Jha wrote:
> > Hi Song,
> >
> > On Fri, Sep 21, 2018 at 04:10:16PM +0800, kbuild test robot wrote:
> > > Hi Song,
> > >
> > > Thank you for the patch! Yet something to improve:
> > >
> > > [auto build test ERROR on iio/togreg]
> > > [also build test ERROR on v4.19-rc4 next-20180920]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > >
> > > url: https://github.com/0day-ci/linux/commits/Song-Qiang/iio-magnetometer-hmc5843-Clean-up-redundant-code/20180921-091239
> > > base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > > config: x86_64-randconfig-u0-09211331 (attached as .config)
> > > compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
> > > reproduce:
> > > # save the attached .config to linux build tree
> > > make ARCH=x86_64
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> ERROR: "hmc5843_volatile_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> > > >> ERROR: "hmc5843_readable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> > > >> ERROR: "hmc5843_writable_table" [drivers/iio/magnetometer/hmc5843_i2c.ko] undefined!
> >
> > You would need to export these above symbols using EXPORT_SYMBOL()
> > to be used by i2c/spi modules.
> >
> > But on the other hand, exporting too many symbols is a bad idea since
> > it is only used for this driver and not at any other place in IIO.
> > So, in my opinion drop this patch and leave the code as-is.
> >
> > https://lkml.org/lkml/2018/7/16/566 --> worth reading
> >
> >
> > Thanks
> > --
> > Himanshu Jha
> > Undergraduate Student
> > Department of Electronics & Communication
> > Guru Tegh Bahadur Institute of Technology
>
> Hi Himanshu,
>
> You're right, that's exactly what I was missing!
> I saw the link you mentioned above and I also think that's a very good
> idea to limit the scope of symbols. But I don't know when this work can
> be applied to the kernel, as it seems like a not little change for the
> build infrastructure.
> I think this maybe a common problem for some drivers.
> Divers for bmc150 in drivers/iio/accel/bmc-150-accel-core.c did the same
> exporting stuff as I was prefered. So I think even if either exporting or
> duplicating is not good enough, one must be choosed for now.
>
> I think this is a topic that I have some ideas but not experienced
> enough to say what should we do is better. I would like to hear Jonathan's
> ideas about this. If this patched shouldn't be applied, then maybe bmc150
> should be patched.
As you say it's not a particularly clear advantage either way and will
depend on the size of the repetition.
So I'm afraid it's in the category of insisting on one option or the
other is just overly fussy. Al, for existing cases it's also not
worth the noise that changing to one or other option creates.
Whilst the noise effect on things like backporting fixes is minor for patches
like this, it isn't zero so there has to be a clear advantage in whatever
is changed. Hence I won't be taking this one, or any patches going the
other way for existing drivers.
Thanks,
Jonathan
>
> yours,
> Song Qiang
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-22 10:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-20 14:13 [PATCH] iio: magnetometer: hmc5843: Clean up redundant code Song Qiang
2018-09-21 8:10 ` kbuild test robot
2018-09-21 18:26 ` Himanshu Jha
2018-09-22 10:04 ` Song Qiang
2018-09-22 10:18 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).