| Message ID | 20220118212504.832429-1-gwendal@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: sx9360: enable push iio event | expand |
On Tue, 18 Jan 2022 13:25:04 -0800 Gwendal Grignou <gwendal@chromium.org> wrote: > From: Jongpil Jung <jongpil19.jung@samsung.com> Hi, Patch title is not really clear. It suggests this is enabling a new feature rather than fixing anything. Please rephrase. > > Fixes: f75095753 ("iio:proximity:sx9360: Add sx9360 support") This is part of the tag block so should appear. > > To convert SX9360 status register ["REG_STAT"], into a channel > index, we need to right shift by |stat_offset|, not left shift. > Also the PROXSTAT bit (3) is for channel 1 (PHM, PHase Measured), not (PHR, > PHase Reference, channel 0), so the offset is 2 instead of 3. > Phase fixes tag should be here. > Signed-off-by: Jongpil Jung <jongpil19.jung@samsung.com> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Thanks, Jonathan > --- > drivers/iio/proximity/sx9360.c | 2 +- > drivers/iio/proximity/sx_common.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c > index 6fd6561bb6f5b8..3ebb30c8a4f61d 100644 > --- a/drivers/iio/proximity/sx9360.c > +++ b/drivers/iio/proximity/sx9360.c > @@ -775,7 +775,7 @@ static const struct sx_common_chip_info sx9360_chip_info = { > .reg_reset = SX9360_REG_RESET, > > .mask_enable_chan = SX9360_REG_GNRL_CTRL0_PHEN_MASK, > - .stat_offset = 3, > + .stat_offset = 2, > .num_channels = SX9360_NUM_CHANNELS, > .num_default_regs = ARRAY_SIZE(sx9360_default_regs), > > diff --git a/drivers/iio/proximity/sx_common.c b/drivers/iio/proximity/sx_common.c > index ac8fd5920481cb..a7c07316a0a91e 100644 > --- a/drivers/iio/proximity/sx_common.c > +++ b/drivers/iio/proximity/sx_common.c > @@ -87,7 +87,7 @@ static void sx_common_push_events(struct iio_dev *indio_dev) > return; > } > > - val <<= data->chip_info->stat_offset; > + val >>= data->chip_info->stat_offset; > > /* > * Only iterate over channels with changes on proximity status that have
On Sat, Jan 22, 2022 at 7:23 PM Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 18 Jan 2022 13:25:04 -0800 > Gwendal Grignou <gwendal@chromium.org> wrote: ... > > Fixes: f75095753 ("iio:proximity:sx9360: Add sx9360 support") > This is part of the tag block so should appear. > fixes tag should be here. > > Signed-off-by: Jongpil Jung <jongpil19.jung@samsung.com> > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> ...and the submitter's SoB must be last (according to the documentation).
On Sat, 22 Jan 2022 20:15:57 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Sat, Jan 22, 2022 at 7:23 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Tue, 18 Jan 2022 13:25:04 -0800 > > Gwendal Grignou <gwendal@chromium.org> wrote: > > ... > > > > Fixes: f75095753 ("iio:proximity:sx9360: Add sx9360 support") > > This is part of the tag block so should appear. > > > > fixes tag should be here. > > > Signed-off-by: Jongpil Jung <jongpil19.jung@samsung.com> > > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > ...and the submitter's SoB must be last (according to the documentation). > Hi Andy, If it's a handling chain, rather that about co development etc then I'd expect it to be in this order to indicate that Gwendal was on the route to upstream. "Any further SoBs (Signed-off-by:'s) following the author's SoB are from people handling and transporting the patch, but were not involved in its development. SoB chains should reflect the **real** route a patch took as it was propagated to the maintainers and ultimately to Linus, with the first SoB entry signalling primary authorship of a single author." Jonathan
On Sat, Jan 22, 2022 at 11:08 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 22 Jan 2022 20:15:57 +0200 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > On Sat, Jan 22, 2022 at 7:23 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > On Tue, 18 Jan 2022 13:25:04 -0800 > > > Gwendal Grignou <gwendal@chromium.org> wrote: > > > > ... > > > > > > Fixes: f75095753 ("iio:proximity:sx9360: Add sx9360 support") > > > This is part of the tag block so should appear. Done: The patch is at https://lore.kernel.org/all/20220122213444.745152-1-gwendal@chromium.org/. Given the title change, it is not automatically connected to this patch. I should have added a --in-reply-to argument to my git send-mail command, will do better next time. Gwendal. > > > > > > > fixes tag should be here. > > > > Signed-off-by: Jongpil Jung <jongpil19.jung@samsung.com> > > > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > > > ...and the submitter's SoB must be last (according to the documentation). > > > Hi Andy, > > If it's a handling chain, rather that about co development etc then > I'd expect it to be in this order to indicate that Gwendal was on the > route to upstream. > > "Any further SoBs (Signed-off-by:'s) following the author's SoB are from > people handling and transporting the patch, but were not involved in its > development. SoB chains should reflect the **real** route a patch took > as it was propagated to the maintainers and ultimately to Linus, with > the first SoB entry signalling primary authorship of a single author." > > Jonathan
diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c index 6fd6561bb6f5b8..3ebb30c8a4f61d 100644 --- a/drivers/iio/proximity/sx9360.c +++ b/drivers/iio/proximity/sx9360.c @@ -775,7 +775,7 @@ static const struct sx_common_chip_info sx9360_chip_info = { .reg_reset = SX9360_REG_RESET, .mask_enable_chan = SX9360_REG_GNRL_CTRL0_PHEN_MASK, - .stat_offset = 3, + .stat_offset = 2, .num_channels = SX9360_NUM_CHANNELS, .num_default_regs = ARRAY_SIZE(sx9360_default_regs), diff --git a/drivers/iio/proximity/sx_common.c b/drivers/iio/proximity/sx_common.c index ac8fd5920481cb..a7c07316a0a91e 100644 --- a/drivers/iio/proximity/sx_common.c +++ b/drivers/iio/proximity/sx_common.c @@ -87,7 +87,7 @@ static void sx_common_push_events(struct iio_dev *indio_dev) return; } - val <<= data->chip_info->stat_offset; + val >>= data->chip_info->stat_offset; /* * Only iterate over channels with changes on proximity status that have