[PATCH] iio: sx9360: enable push iio event

[PATCH] iio: sx9360: enable push iio event

[Gwendal Grignou]

From: Jongpil Jung <jongpil19.jung@samsung.com>

Fixes: f75095753 ("iio:proximity:sx9360: Add sx9360 support")

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.

Signed-off-by: Jongpil Jung <jongpil19.jung@samsung.com>
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 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
-- 
2.34.1.703.g22d0c6ccf7-goog

Re: [PATCH] iio: sx9360: enable push iio event

[Jonathan Cameron]

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

Re: [PATCH] iio: sx9360: enable push iio event

[Andy Shevchenko]

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).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: sx9360: enable push iio event

[Jonathan Cameron]

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

Re: [PATCH] iio: sx9360: enable push iio event

[Jonathan Cameron]

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

Re: [PATCH] iio: sx9360: enable push iio event

[Gwendal Grignou]

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