diff for duplicates of <20181021151427.5b3dbb9b@archlinux> diff --git a/a/1.txt b/N1/1.txt index 4f4d232..54761ab 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,10 +1,10 @@ On Thu, 18 Oct 2018 16:24:15 +0800 Song Qiang <songqiang1304521@gmail.com> wrote: -> On 2018/10/13 =E4=B8=8B=E5=8D=886:19, Jonathan Cameron wrote: +> On 2018/10/13 下午6:19, Jonathan Cameron wrote: > > On Fri, 12 Oct 2018 15:35:36 +0800 > > Song Qiang<songqiang1304521@gmail.com> wrote: -> > =20 +> > > >> PNI RM3100 is a high resolution, large signal immunity magnetometer, > >> composed of 3 single sensors and a processing chip with a MagI2C > >> interface. @@ -18,7 +18,7 @@ Song Qiang <songqiang1304521@gmail.com> wrote: > >> - Both interrupt and polling measurement is supported, depends on if > >> the 'interrupts' in DT is declared. > >> -> >> Signed-off-by: Song Qiang<songqiang1304521@gmail.com> =20 +> >> Signed-off-by: Song Qiang<songqiang1304521@gmail.com> > > A few questions for you (getting very close to being good to go btw!) > > > > Why do we have the 3second additional wait for conversions? I know we @@ -28,32 +28,28 @@ Song Qiang <songqiang1304521@gmail.com> wrote: > > > > Thanks, > > -> > Jonathan =20 ->=20 +> > Jonathan +> > Hi Jonathan, ->=20 ->=20 -> The measurement time of this device varies from 1.7ms to 13 seconds, 3 se= -conds=20 -> is just a number in the middle between them. This may be worth discussing= -,=20 +> +> +> The measurement time of this device varies from 1.7ms to 13 seconds, 3 seconds +> is just a number in the middle between them. This may be worth discussing, > hoping to get a better solution from the community. -We should 'know' which of those it will be though as I assume it is depende= -nt +We should 'know' which of those it will be though as I assume it is dependent on the device configuration which we control. So waiting for say, double, the expected time should be sufficient to detect that things have gone horribly wrong. ->=20 ->=20 +> +> > >> --- > >> MAINTAINERS | 7 + > >> drivers/iio/magnetometer/Kconfig | 29 ++ > >> drivers/iio/magnetometer/Makefile | 4 + -> >> drivers/iio/magnetometer/rm3100-core.c | 627 +++++++++++++++++++++++= -++ +> >> drivers/iio/magnetometer/rm3100-core.c | 627 +++++++++++++++++++++++++ > >> drivers/iio/magnetometer/rm3100-i2c.c | 58 +++ > >> drivers/iio/magnetometer/rm3100-spi.c | 64 +++ > >> drivers/iio/magnetometer/rm3100.h | 17 + @@ -62,70 +58,60 @@ that things have gone horribly wrong. > >> create mode 100644 drivers/iio/magnetometer/rm3100-i2c.c > >> create mode 100644 drivers/iio/magnetometer/rm3100-spi.c > >> create mode 100644 drivers/iio/magnetometer/rm3100.h -> >> =20 ->=20 +> >> +> > ... ->=20 -> > =20 +> +> > > >> +static irqreturn_t rm3100_trigger_handler(int irq, void *p) > >> +{ -> >> + struct iio_poll_func *pf =3D p; -> >> + struct iio_dev *indio_dev =3D pf->indio_dev; -> >> + unsigned long scan_mask =3D *indio_dev->active_scan_mask; -> >> + unsigned int mask_len =3D indio_dev->masklength; -> >> + struct rm3100_data *data =3D iio_priv(indio_dev); -> >> + struct regmap *regmap =3D data->regmap; +> >> + struct iio_poll_func *pf = p; +> >> + struct iio_dev *indio_dev = pf->indio_dev; +> >> + unsigned long scan_mask = *indio_dev->active_scan_mask; +> >> + unsigned int mask_len = indio_dev->masklength; +> >> + struct rm3100_data *data = iio_priv(indio_dev); +> >> + struct regmap *regmap = data->regmap; > >> + int ret, i, bit; > >> + > >> + mutex_lock(&data->lock); > >> + switch (scan_mask) { > >> + case BIT(0) | BIT(1) | BIT(2): -> >> + ret =3D regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9); +> >> + ret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9); > >> + mutex_unlock(&data->lock); > >> + if (ret < 0) > >> + goto done; > >> + break; > >> + case BIT(0) | BIT(1): -> >> + ret =3D regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 6); +> >> + ret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 6); > >> + mutex_unlock(&data->lock); > >> + if (ret < 0) > >> + goto done; > >> + break; > >> + case BIT(1) | BIT(2): -> >> + ret =3D regmap_bulk_read(regmap, RM3100_REG_MY2, data->buffer, 6); +> >> + ret = regmap_bulk_read(regmap, RM3100_REG_MY2, data->buffer, 6); > >> + mutex_unlock(&data->lock); > >> + if (ret < 0) > >> + goto done; -> >> + break; =20 +> >> + break; > > What about BIT(0) | BIT(2)? > > -> > Now you can do it like you have here and on that one corner case let th= -e iio core -> > demux code take care of it, but then you will need to provide available= -_scan_masks +> > Now you can do it like you have here and on that one corner case let the iio core +> > demux code take care of it, but then you will need to provide available_scan_masks > > so the core knows it needs to handle this case. -> > =20 ->=20 -> This confused me a little. The available_scan_masks I was using is {BIT(0= -) |=20 -> BIT(1) | BIT(2), 0x0}. Apparently in this version of patch I would like i= -t to=20 -> handle every circumstances like BIT(0), BIT(0) | BIT(2), BIT(1) | BIT(2),= - etc.=20 -> Since Phil mentioned he would like this to reduce bus usage as much as we= - can=20 -> and I want it, too, I think these three circumstances can be read consecu= -tively=20 -> while others can be read one axis at a time. So I plan to let=C2=A0 BIT(0= -) | BIT(2)=20 +> > +> +> This confused me a little. The available_scan_masks I was using is {BIT(0) | +> BIT(1) | BIT(2), 0x0}. Apparently in this version of patch I would like it to +> handle every circumstances like BIT(0), BIT(0) | BIT(2), BIT(1) | BIT(2), etc. +> Since Phil mentioned he would like this to reduce bus usage as much as we can +> and I want it, too, I think these three circumstances can be read consecutively +> while others can be read one axis at a time. So I plan to let BIT(0) | BIT(2) > fall into the 'default' section, which reads axis one by one. ->=20 -> My question is, since this handles every possible combination, do I still= - need=20 +> +> My question is, since this handles every possible combination, do I still need > to list every available scan masks in available_scan_masks? -Ah. I see, I'd missed that the default was picking up that case as well as = -the +Ah. I see, I'd missed that the default was picking up that case as well as the single axes. It would be interesting to sanity check if it is quicker on a 'typical' platform to do the all axis read for the BIT(0) | BIT(2) case and drop the middle value (which would be done using available scan_masks) @@ -133,15 +119,15 @@ or to just do two independent reads. (I would guess it is worth reading the 'dead' axis). ->=20 ->=20 +> +> > All other problems will be fixed in the next patch. ->=20 +> > yours, ->=20 +> > Song Qiang ->=20 ->=20 +> +> > ... Thanks, diff --git a/a/content_digest b/N1/content_digest index 064ee59..9870c94 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -22,10 +22,10 @@ "On Thu, 18 Oct 2018 16:24:15 +0800\n" "Song Qiang <songqiang1304521@gmail.com> wrote:\n" "\n" - "> On 2018/10/13 =E4=B8=8B=E5=8D=886:19, Jonathan Cameron wrote:\n" + "> On 2018/10/13 \344\270\213\345\215\2106:19, Jonathan Cameron wrote:\n" "> > On Fri, 12 Oct 2018 15:35:36 +0800\n" "> > Song Qiang<songqiang1304521@gmail.com> wrote:\n" - "> > =20\n" + "> > \n" "> >> PNI RM3100 is a high resolution, large signal immunity magnetometer,\n" "> >> composed of 3 single sensors and a processing chip with a MagI2C\n" "> >> interface.\n" @@ -39,7 +39,7 @@ "> >> - Both interrupt and polling measurement is supported, depends on if\n" "> >> the 'interrupts' in DT is declared.\n" "> >>\n" - "> >> Signed-off-by: Song Qiang<songqiang1304521@gmail.com> =20\n" + "> >> Signed-off-by: Song Qiang<songqiang1304521@gmail.com> \n" "> > A few questions for you (getting very close to being good to go btw!)\n" "> >\n" "> > Why do we have the 3second additional wait for conversions? I know we\n" @@ -49,32 +49,28 @@ "> >\n" "> > Thanks,\n" "> >\n" - "> > Jonathan =20\n" - ">=20\n" + "> > Jonathan \n" + "> \n" "> Hi Jonathan,\n" - ">=20\n" - ">=20\n" - "> The measurement time of this device varies from 1.7ms to 13 seconds, 3 se=\n" - "conds=20\n" - "> is just a number in the middle between them. This may be worth discussing=\n" - ",=20\n" + "> \n" + "> \n" + "> The measurement time of this device varies from 1.7ms to 13 seconds, 3 seconds \n" + "> is just a number in the middle between them. This may be worth discussing, \n" "> hoping to get a better solution from the community.\n" "\n" - "We should 'know' which of those it will be though as I assume it is depende=\n" - "nt\n" + "We should 'know' which of those it will be though as I assume it is dependent\n" "on the device configuration which we control.\n" "\n" "So waiting for say, double, the expected time should be sufficient to detect\n" "that things have gone horribly wrong.\n" "\n" - ">=20\n" - ">=20\n" + "> \n" + "> \n" "> >> ---\n" "> >> MAINTAINERS | 7 +\n" "> >> drivers/iio/magnetometer/Kconfig | 29 ++\n" "> >> drivers/iio/magnetometer/Makefile | 4 +\n" - "> >> drivers/iio/magnetometer/rm3100-core.c | 627 +++++++++++++++++++++++=\n" - "++\n" + "> >> drivers/iio/magnetometer/rm3100-core.c | 627 +++++++++++++++++++++++++\n" "> >> drivers/iio/magnetometer/rm3100-i2c.c | 58 +++\n" "> >> drivers/iio/magnetometer/rm3100-spi.c | 64 +++\n" "> >> drivers/iio/magnetometer/rm3100.h | 17 +\n" @@ -83,70 +79,60 @@ "> >> create mode 100644 drivers/iio/magnetometer/rm3100-i2c.c\n" "> >> create mode 100644 drivers/iio/magnetometer/rm3100-spi.c\n" "> >> create mode 100644 drivers/iio/magnetometer/rm3100.h\n" - "> >> =20\n" - ">=20\n" + "> >> \n" + "> \n" "> ...\n" - ">=20\n" - "> > =20\n" + "> \n" + "> > \n" "> >> +static irqreturn_t rm3100_trigger_handler(int irq, void *p)\n" "> >> +{\n" - "> >> +\tstruct iio_poll_func *pf =3D p;\n" - "> >> +\tstruct iio_dev *indio_dev =3D pf->indio_dev;\n" - "> >> +\tunsigned long scan_mask =3D *indio_dev->active_scan_mask;\n" - "> >> +\tunsigned int mask_len =3D indio_dev->masklength;\n" - "> >> +\tstruct rm3100_data *data =3D iio_priv(indio_dev);\n" - "> >> +\tstruct regmap *regmap =3D data->regmap;\n" + "> >> +\tstruct iio_poll_func *pf = p;\n" + "> >> +\tstruct iio_dev *indio_dev = pf->indio_dev;\n" + "> >> +\tunsigned long scan_mask = *indio_dev->active_scan_mask;\n" + "> >> +\tunsigned int mask_len = indio_dev->masklength;\n" + "> >> +\tstruct rm3100_data *data = iio_priv(indio_dev);\n" + "> >> +\tstruct regmap *regmap = data->regmap;\n" "> >> +\tint ret, i, bit;\n" "> >> +\n" "> >> +\tmutex_lock(&data->lock);\n" "> >> +\tswitch (scan_mask) {\n" "> >> +\tcase BIT(0) | BIT(1) | BIT(2):\n" - "> >> +\t\tret =3D regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9);\n" + "> >> +\t\tret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9);\n" "> >> +\t\tmutex_unlock(&data->lock);\n" "> >> +\t\tif (ret < 0)\n" "> >> +\t\t\tgoto done;\n" "> >> +\tbreak;\n" "> >> +\tcase BIT(0) | BIT(1):\n" - "> >> +\t\tret =3D regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 6);\n" + "> >> +\t\tret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 6);\n" "> >> +\t\tmutex_unlock(&data->lock);\n" "> >> +\t\tif (ret < 0)\n" "> >> +\t\t\tgoto done;\n" "> >> +\tbreak;\n" "> >> +\tcase BIT(1) | BIT(2):\n" - "> >> +\t\tret =3D regmap_bulk_read(regmap, RM3100_REG_MY2, data->buffer, 6);\n" + "> >> +\t\tret = regmap_bulk_read(regmap, RM3100_REG_MY2, data->buffer, 6);\n" "> >> +\t\tmutex_unlock(&data->lock);\n" "> >> +\t\tif (ret < 0)\n" "> >> +\t\t\tgoto done;\n" - "> >> +\tbreak; =20\n" + "> >> +\tbreak; \n" "> > What about BIT(0) | BIT(2)?\n" "> >\n" - "> > Now you can do it like you have here and on that one corner case let th=\n" - "e iio core\n" - "> > demux code take care of it, but then you will need to provide available=\n" - "_scan_masks\n" + "> > Now you can do it like you have here and on that one corner case let the iio core\n" + "> > demux code take care of it, but then you will need to provide available_scan_masks\n" "> > so the core knows it needs to handle this case.\n" - "> > =20\n" - ">=20\n" - "> This confused me a little. The available_scan_masks I was using is {BIT(0=\n" - ") |=20\n" - "> BIT(1) | BIT(2), 0x0}. Apparently in this version of patch I would like i=\n" - "t to=20\n" - "> handle every circumstances like BIT(0), BIT(0) | BIT(2), BIT(1) | BIT(2),=\n" - " etc.=20\n" - "> Since Phil mentioned he would like this to reduce bus usage as much as we=\n" - " can=20\n" - "> and I want it, too, I think these three circumstances can be read consecu=\n" - "tively=20\n" - "> while others can be read one axis at a time. So I plan to let=C2=A0 BIT(0=\n" - ") | BIT(2)=20\n" + "> > \n" + "> \n" + "> This confused me a little. The available_scan_masks I was using is {BIT(0) | \n" + "> BIT(1) | BIT(2), 0x0}. Apparently in this version of patch I would like it to \n" + "> handle every circumstances like BIT(0), BIT(0) | BIT(2), BIT(1) | BIT(2), etc. \n" + "> Since Phil mentioned he would like this to reduce bus usage as much as we can \n" + "> and I want it, too, I think these three circumstances can be read consecutively \n" + "> while others can be read one axis at a time. So I plan to let\302\240 BIT(0) | BIT(2) \n" "> fall into the 'default' section, which reads axis one by one.\n" - ">=20\n" - "> My question is, since this handles every possible combination, do I still=\n" - " need=20\n" + "> \n" + "> My question is, since this handles every possible combination, do I still need \n" "> to list every available scan masks in available_scan_masks?\n" "\n" - "Ah. I see, I'd missed that the default was picking up that case as well as =\n" - "the\n" + "Ah. I see, I'd missed that the default was picking up that case as well as the\n" "single axes. It would be interesting to sanity check if it is quicker on\n" "a 'typical' platform to do the all axis read for the BIT(0) | BIT(2) case\n" "and drop the middle value (which would be done using available scan_masks)\n" @@ -154,18 +140,18 @@ "\n" "(I would guess it is worth reading the 'dead' axis).\n" "\n" - ">=20\n" - ">=20\n" + "> \n" + "> \n" "> All other problems will be fixed in the next patch.\n" - ">=20\n" + "> \n" "> yours,\n" - ">=20\n" + "> \n" "> Song Qiang\n" - ">=20\n" - ">=20\n" + "> \n" + "> \n" "> ...\n" "Thanks,\n" "\n" Jonathan -0ac0356fc29fe281b56737af18fe6a9e73b6e758a29ae792797459e656d8df56 +aac5cb45c1fba80e6b110eaee22d0c206c44ec0f59d3d11b10014270b5d1560b
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.