[PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Vasileios Amoiridis]

The iio_validate_own_trigger() function was added in this commit [1] but it is
the same with the below function called iio_trigger_validate_own_device(). The
bodies of the functions can be found in [2], [3].

[1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
[2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
[3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752

Vasileios Amoiridis (2):
  iio: Drop usage of iio_validate_own_trigger
  iio: remove iio_validate_own_trigger completely

 drivers/iio/accel/kionix-kx022a.c  |  2 +-
 drivers/iio/industrialio-trigger.c | 22 +---------------------
 drivers/iio/light/rohm-bu27008.c   |  2 +-
 include/linux/iio/trigger.h        |  1 -
 4 files changed, 3 insertions(+), 24 deletions(-)


base-commit: 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d
-- 
2.25.1

[PATCH v1 1/2] iio: Drop usage of iio_validate_own_trigger()

[Vasileios Amoiridis]

Change iio_validate_own_trigger() for iio_trigger_validate_own_device()
since they are the same function.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/accel/kionix-kx022a.c  | 2 +-
 drivers/iio/industrialio-trigger.c | 2 +-
 drivers/iio/light/rohm-bu27008.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 53d59a04ae15..0487610bab33 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -873,7 +873,7 @@ static const struct iio_info kx022a_info = {
 	.write_raw_get_fmt = &kx022a_write_raw_get_fmt,
 	.read_avail = &kx022a_read_avail,
 
-	.validate_trigger	= iio_validate_own_trigger,
+	.validate_trigger	= iio_trigger_validate_own_device,
 	.hwfifo_set_watermark	= kx022a_set_watermark,
 	.hwfifo_flush_to_buffer	= kx022a_fifo_flush,
 };
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..3da8b8c25185 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -315,7 +315,7 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
 	 * this is the case if the IIO device and the trigger device share the
 	 * same parent device.
 	 */
-	if (!iio_validate_own_trigger(pf->indio_dev, trig))
+	if (!iio_trigger_validate_own_device(trig, pf->indio_dev))
 		trig->attached_own_device = true;
 
 	return ret;
diff --git a/drivers/iio/light/rohm-bu27008.c b/drivers/iio/light/rohm-bu27008.c
index 0f010eff1981..12cfaa7614ce 100644
--- a/drivers/iio/light/rohm-bu27008.c
+++ b/drivers/iio/light/rohm-bu27008.c
@@ -1386,7 +1386,7 @@ static const struct iio_info bu27008_info = {
 	.write_raw_get_fmt = &bu27008_write_raw_get_fmt,
 	.read_avail = &bu27008_read_avail,
 	.update_scan_mode = bu27008_update_scan_mode,
-	.validate_trigger = iio_validate_own_trigger,
+	.validate_trigger = iio_trigger_validate_own_device,
 };
 
 static int bu27008_trigger_set_state(struct iio_trigger *trig, bool state)

base-commit: 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d
-- 
2.25.1

[PATCH v1 2/2] iio: remove iio_validate_own_trigger() completely

[Vasileios Amoiridis]

Remove completely the iio_validate_own_trigger() since it is not used
any more and since the iio_trigger_validate_own_device() can fully
replace the function.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/industrialio-trigger.c | 20 --------------------
 include/linux/iio/trigger.h        |  1 -
 2 files changed, 21 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 3da8b8c25185..b980843b5a3c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -718,26 +718,6 @@ bool iio_trigger_using_own(struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL(iio_trigger_using_own);
 
-/**
- * iio_validate_own_trigger - Check if a trigger and IIO device belong to
- *  the same device
- * @idev: the IIO device to check
- * @trig: the IIO trigger to check
- *
- * This function can be used as the validate_trigger callback for triggers that
- * can only be attached to their own device.
- *
- * Return: 0 if both the trigger and the IIO device belong to the same
- * device, -EINVAL otherwise.
- */
-int iio_validate_own_trigger(struct iio_dev *idev, struct iio_trigger *trig)
-{
-	if (idev->dev.parent != trig->dev.parent)
-		return -EINVAL;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(iio_validate_own_trigger);
-
 /**
  * iio_trigger_validate_own_device - Check if a trigger and IIO device belong to
  *  the same device
diff --git a/include/linux/iio/trigger.h b/include/linux/iio/trigger.h
index bce3b1788199..51f52c5c6092 100644
--- a/include/linux/iio/trigger.h
+++ b/include/linux/iio/trigger.h
@@ -171,7 +171,6 @@ void iio_trigger_free(struct iio_trigger *trig);
  */
 bool iio_trigger_using_own(struct iio_dev *indio_dev);
 
-int iio_validate_own_trigger(struct iio_dev *idev, struct iio_trigger *trig);
 int iio_trigger_validate_own_device(struct iio_trigger *trig,
 				     struct iio_dev *indio_dev);
 
-- 
2.25.1

Re: [PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Lars-Peter Clausen]

On 9/21/24 11:19, Vasileios Amoiridis wrote:
> The iio_validate_own_trigger() function was added in this commit [1] but it is
> the same with the below function called iio_trigger_validate_own_device(). The
> bodies of the functions can be found in [2], [3].
>
> [1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
> [2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
> [3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752

The signature of the two functions are different, the order of the 
parameters is switched. So you can't just swap them out for the 
`validate_trigger` callback since the signature is not compatible. But 
maybe you can update the implementation of one of the functions to 
calling the other function.

Re: [PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Vasileios Amoiridis]

On Sat, Sep 21, 2024 at 12:23:39PM -0700, Lars-Peter Clausen wrote:
> On 9/21/24 11:19, Vasileios Amoiridis wrote:
> > The iio_validate_own_trigger() function was added in this commit [1] but it is
> > the same with the below function called iio_trigger_validate_own_device(). The
> > bodies of the functions can be found in [2], [3].
> > 
> > [1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
> > [2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
> > [3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752
> 
> The signature of the two functions are different, the order of the
> parameters is switched. So you can't just swap them out for the
> `validate_trigger` callback since the signature is not compatible. But maybe
> you can update the implementation of one of the functions to calling the
> other function.
> 

Hi Lars,

Hmm, I see what you mean. Still though, do you think that we could do some
cleaning here? I can see 3 approaches:

1) One of the 2 functions calls the other internally and nothing else has
to change.

1) The default iio_validate_own_trigger() is used only by 2 out of many drivers
who use the .validate_trigger call. If this is deprecated, many function
signatures will need to change (swap the args) and then rename the rest.

The default iio_trigger_validate_own_device() is used in almost all of the
drivers apart from 3 
	* gyro/st_gyro_core.c
	* common/st_sensors/st_sensors_trigger.c
	* trigger/stm32-lptimer-trigger.c

So it will be less noise to change the iio_trigger_validate_own_device()
in the sense that the signature of 3 functions will need to change
(swap args) and then rename the rest.

1 is by far the less noisy as only a couple lines need to change but we
still endup with 2 functions doing the same thing. 2 and 3 require more
noise but we end up having 1 implementation which looks cleaner.

What would you say?

Cheers,
Vasilis

Re: [PATCH v1 1/2] iio: Drop usage of iio_validate_own_trigger()

[kernel test robot]

Hi Vasileios,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d]

url:    https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-Drop-usage-of-iio_validate_own_trigger/20240922-022124
base:   8bea3878a1511bceadc2fbf284b00bcc5a2ef28d
patch link:    https://lore.kernel.org/r/20240921181939.392517-2-vassilisamir%40gmail.com
patch subject: [PATCH v1 1/2] iio: Drop usage of iio_validate_own_trigger()
config: x86_64-buildonly-randconfig-002-20240922 (https://download.01.org/0day-ci/archive/20240922/202409221122.4BTAc5w1-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240922/202409221122.4BTAc5w1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409221122.4BTAc5w1-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/accel/kionix-kx022a.c:876:22: error: incompatible function pointer types initializing 'int (*)(struct iio_dev *, struct iio_trigger *)' with an expression of type 'int (struct iio_trigger *, struct iio_dev *)' [-Wincompatible-function-pointer-types]
     876 |         .validate_trigger       = iio_trigger_validate_own_device,
         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.
--
>> drivers/iio/light/rohm-bu27008.c:1389:22: error: incompatible function pointer types initializing 'int (*)(struct iio_dev *, struct iio_trigger *)' with an expression of type 'int (struct iio_trigger *, struct iio_dev *)' [-Wincompatible-function-pointer-types]
    1389 |         .validate_trigger = iio_trigger_validate_own_device,
         |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +876 drivers/iio/accel/kionix-kx022a.c

   869	
   870	static const struct iio_info kx022a_info = {
   871		.read_raw = &kx022a_read_raw,
   872		.write_raw = &kx022a_write_raw,
   873		.write_raw_get_fmt = &kx022a_write_raw_get_fmt,
   874		.read_avail = &kx022a_read_avail,
   875	
 > 876		.validate_trigger	= iio_trigger_validate_own_device,
   877		.hwfifo_set_watermark	= kx022a_set_watermark,
   878		.hwfifo_flush_to_buffer	= kx022a_fifo_flush,
   879	};
   880	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 1/2] iio: Drop usage of iio_validate_own_trigger()

[kernel test robot]

Hi Vasileios,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d]

url:    https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-Drop-usage-of-iio_validate_own_trigger/20240922-022124
base:   8bea3878a1511bceadc2fbf284b00bcc5a2ef28d
patch link:    https://lore.kernel.org/r/20240921181939.392517-2-vassilisamir%40gmail.com
patch subject: [PATCH v1 1/2] iio: Drop usage of iio_validate_own_trigger()
config: arc-randconfig-002-20240922 (https://download.01.org/0day-ci/archive/20240922/202409221140.AyQ2zQ81-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240922/202409221140.AyQ2zQ81-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409221140.AyQ2zQ81-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/light/rohm-bu27008.c:1389:29: error: initialization of 'int (*)(struct iio_dev *, struct iio_trigger *)' from incompatible pointer type 'int (*)(struct iio_trigger *, struct iio_dev *)' [-Werror=incompatible-pointer-types]
    1389 |         .validate_trigger = iio_trigger_validate_own_device,
         |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/light/rohm-bu27008.c:1389:29: note: (near initialization for 'bu27008_info.validate_trigger')
   cc1: some warnings being treated as errors
--
>> drivers/iio/accel/kionix-kx022a.c:876:35: error: initialization of 'int (*)(struct iio_dev *, struct iio_trigger *)' from incompatible pointer type 'int (*)(struct iio_trigger *, struct iio_dev *)' [-Werror=incompatible-pointer-types]
     876 |         .validate_trigger       = iio_trigger_validate_own_device,
         |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/kionix-kx022a.c:876:35: note: (near initialization for 'kx022a_info.validate_trigger')
   cc1: some warnings being treated as errors


vim +1389 drivers/iio/light/rohm-bu27008.c

  1382	
  1383	static const struct iio_info bu27008_info = {
  1384		.read_raw = &bu27008_read_raw,
  1385		.write_raw = &bu27008_write_raw,
  1386		.write_raw_get_fmt = &bu27008_write_raw_get_fmt,
  1387		.read_avail = &bu27008_read_avail,
  1388		.update_scan_mode = bu27008_update_scan_mode,
> 1389		.validate_trigger = iio_trigger_validate_own_device,
  1390	};
  1391	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Matti Vaittinen]

On 9/21/24 23:07, Vasileios Amoiridis wrote:
> On Sat, Sep 21, 2024 at 12:23:39PM -0700, Lars-Peter Clausen wrote:
>> On 9/21/24 11:19, Vasileios Amoiridis wrote:
>>> The iio_validate_own_trigger() function was added in this commit [1] but it is
>>> the same with the below function called iio_trigger_validate_own_device(). The
>>> bodies of the functions can be found in [2], [3].
>>>
>>> [1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
>>> [2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
>>> [3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752
>>
>> The signature of the two functions are different, the order of the
>> parameters is switched. So you can't just swap them out for the
>> `validate_trigger` callback since the signature is not compatible. But maybe
>> you can update the implementation of one of the functions to calling the
>> other function.
>>
> 
> Hi Lars,
> 
> Hmm, I see what you mean. Still though, do you think that we could do some
> cleaning here? I can see 3 approaches:
> 
> 1) One of the 2 functions calls the other internally and nothing else has
> to change.

I would go with this. Changing the signatures to be the same would be 
(in my, not always humble enough, opinion) wrong. The different order of 
parameters reflects the different idea. One checks if device for trigger 
is the right one, the other checks if the trigger for the device is the 
right one. Thus, the order of parameters should be different.

Calling the same implementation internally is fine with me. Maybe 
Jonathan will share his opinion when recovers from all the plumbing in 
Vienna ;)

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Vasileios Amoiridis]

On Sun, Sep 22, 2024 at 12:44:15PM +0300, Matti Vaittinen wrote:
> On 9/21/24 23:07, Vasileios Amoiridis wrote:
> > On Sat, Sep 21, 2024 at 12:23:39PM -0700, Lars-Peter Clausen wrote:
> > > On 9/21/24 11:19, Vasileios Amoiridis wrote:
> > > > The iio_validate_own_trigger() function was added in this commit [1] but it is
> > > > the same with the below function called iio_trigger_validate_own_device(). The
> > > > bodies of the functions can be found in [2], [3].
> > > > 
> > > > [1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
> > > > [2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
> > > > [3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752
> > > 
> > > The signature of the two functions are different, the order of the
> > > parameters is switched. So you can't just swap them out for the
> > > `validate_trigger` callback since the signature is not compatible. But maybe
> > > you can update the implementation of one of the functions to calling the
> > > other function.
> > > 
> > 
> > Hi Lars,
> > 
> > Hmm, I see what you mean. Still though, do you think that we could do some
> > cleaning here? I can see 3 approaches:
> > 
> > 1) One of the 2 functions calls the other internally and nothing else has
> > to change.
> 
> I would go with this. Changing the signatures to be the same would be (in
> my, not always humble enough, opinion) wrong. The different order of
> parameters reflects the different idea. One checks if device for trigger is
> the right one, the other checks if the trigger for the device is the right
> one. Thus, the order of parameters should be different.
> 
> Calling the same implementation internally is fine with me. Maybe Jonathan
> will share his opinion when recovers from all the plumbing in Vienna ;)
> 
> Yours,
> 	-- Matti
> 
> -- 
> Matti Vaittinen
> Linux kernel developer at ROHM Semiconductors
> Oulu Finland
> 

Hi Matti!

Thanks for your comment! Well, I still think in my eyes is better to
have one function do one thing instead of multiple. Also, I didn't
think of this argument with the order of arguments, it makes sense.
My experience is quite limited to how things should be in such a
large project so I trust your opinion. I would still like to see
what Jonathan has to say on this though, maybe he had some
reasoning behind!!!

Have a nice day!

Cheers,
Vasilis

Re: [PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Jonathan Cameron]

On Sun, 22 Sep 2024 13:07:21 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> On Sun, Sep 22, 2024 at 12:44:15PM +0300, Matti Vaittinen wrote:
> > On 9/21/24 23:07, Vasileios Amoiridis wrote:  
> > > On Sat, Sep 21, 2024 at 12:23:39PM -0700, Lars-Peter Clausen wrote:  
> > > > On 9/21/24 11:19, Vasileios Amoiridis wrote:  
> > > > > The iio_validate_own_trigger() function was added in this commit [1] but it is
> > > > > the same with the below function called iio_trigger_validate_own_device(). The
> > > > > bodies of the functions can be found in [2], [3].
> > > > > 
> > > > > [1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
> > > > > [2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
> > > > > [3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752  
> > > > 
> > > > The signature of the two functions are different, the order of the
> > > > parameters is switched. So you can't just swap them out for the
> > > > `validate_trigger` callback since the signature is not compatible. But maybe
> > > > you can update the implementation of one of the functions to calling the
> > > > other function.
> > > >   
> > > 
> > > Hi Lars,
> > > 
> > > Hmm, I see what you mean. Still though, do you think that we could do some
> > > cleaning here? I can see 3 approaches:
> > > 
> > > 1) One of the 2 functions calls the other internally and nothing else has
> > > to change.  
> > 
> > I would go with this. Changing the signatures to be the same would be (in
> > my, not always humble enough, opinion) wrong. The different order of
> > parameters reflects the different idea. One checks if device for trigger is
> > the right one, the other checks if the trigger for the device is the right
> > one. Thus, the order of parameters should be different.
> > 
> > Calling the same implementation internally is fine with me. Maybe Jonathan
> > will share his opinion when recovers from all the plumbing in Vienna ;)
> > 
> > Yours,
> > 	-- Matti
> > 
> > -- 
> > Matti Vaittinen
> > Linux kernel developer at ROHM Semiconductors
> > Oulu Finland
> >   
> 
> Hi Matti!
> 
> Thanks for your comment! Well, I still think in my eyes is better to
> have one function do one thing instead of multiple. Also, I didn't
> think of this argument with the order of arguments, it makes sense.
> My experience is quite limited to how things should be in such a
> large project so I trust your opinion. I would still like to see
> what Jonathan has to say on this though, maybe he had some
> reasoning behind!!!
> 
No to changing the signatures. It removes the difference
in meaning of the callbacks even though they happen to have
the same implementation in this very simple (and common case).

In the trigger first one, that is the subject.  We are asking the
question 'is this trigger ok being used for this device'.
In the other the device is the subject and we asking the
question 'is this device ok to use this trigger'

When we are checking the combination you have here, sure they
become the same thing but there are devices where it
matters that the trigger is not used to drive other devices
(typically because it's a hardware line that goes nowhere
else, so no interrupts etc) but other triggers can be used
to drive this device (often by software triggering the scan).
We have the opposite case as well but that's often
a shortcut when it just happens to be really complex to get
the trigger to reset (often requires reading all the data
or similar) - that condition can almost always be relaxed
but sometimes it's a lot of code for a niche case.

So fine to change the implementation of one of these
checks on tightly coupled device and trigger to call the other
but don't touch the callback signatures as to that breaks the
logical parameter ordering.

Jonathan

> Have a nice day!
> 
> Cheers,
> Vasilis

Re: [PATCH v1 0/2] iio: core: remove iio_validate_own_trigger() function

[Vasileios Amoiridis]

On Sat, Sep 28, 2024 at 03:55:19PM +0100, Jonathan Cameron wrote:
> On Sun, 22 Sep 2024 13:07:21 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > On Sun, Sep 22, 2024 at 12:44:15PM +0300, Matti Vaittinen wrote:
> > > On 9/21/24 23:07, Vasileios Amoiridis wrote:  
> > > > On Sat, Sep 21, 2024 at 12:23:39PM -0700, Lars-Peter Clausen wrote:  
> > > > > On 9/21/24 11:19, Vasileios Amoiridis wrote:  
> > > > > > The iio_validate_own_trigger() function was added in this commit [1] but it is
> > > > > > the same with the below function called iio_trigger_validate_own_device(). The
> > > > > > bodies of the functions can be found in [2], [3].
> > > > > > 
> > > > > > [1]: https://lore.kernel.org/all/51cd3e3e74a6addf8d333f4a109fb9c5a11086ee.1683541225.git.mazziesaccount@gmail.com/
> > > > > > [2]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L732
> > > > > > [3]: https://elixir.bootlin.com/linux/v6.11/source/drivers/iio/industrialio-trigger.c#L752  
> > > > > 
> > > > > The signature of the two functions are different, the order of the
> > > > > parameters is switched. So you can't just swap them out for the
> > > > > `validate_trigger` callback since the signature is not compatible. But maybe
> > > > > you can update the implementation of one of the functions to calling the
> > > > > other function.
> > > > >   
> > > > 
> > > > Hi Lars,
> > > > 
> > > > Hmm, I see what you mean. Still though, do you think that we could do some
> > > > cleaning here? I can see 3 approaches:
> > > > 
> > > > 1) One of the 2 functions calls the other internally and nothing else has
> > > > to change.  
> > > 
> > > I would go with this. Changing the signatures to be the same would be (in
> > > my, not always humble enough, opinion) wrong. The different order of
> > > parameters reflects the different idea. One checks if device for trigger is
> > > the right one, the other checks if the trigger for the device is the right
> > > one. Thus, the order of parameters should be different.
> > > 
> > > Calling the same implementation internally is fine with me. Maybe Jonathan
> > > will share his opinion when recovers from all the plumbing in Vienna ;)
> > > 
> > > Yours,
> > > 	-- Matti
> > > 
> > > -- 
> > > Matti Vaittinen
> > > Linux kernel developer at ROHM Semiconductors
> > > Oulu Finland
> > >   
> > 
> > Hi Matti!
> > 
> > Thanks for your comment! Well, I still think in my eyes is better to
> > have one function do one thing instead of multiple. Also, I didn't
> > think of this argument with the order of arguments, it makes sense.
> > My experience is quite limited to how things should be in such a
> > large project so I trust your opinion. I would still like to see
> > what Jonathan has to say on this though, maybe he had some
> > reasoning behind!!!
> > 
> No to changing the signatures. It removes the difference
> in meaning of the callbacks even though they happen to have
> the same implementation in this very simple (and common case).
> 
> In the trigger first one, that is the subject.  We are asking the
> question 'is this trigger ok being used for this device'.
> In the other the device is the subject and we asking the
> question 'is this device ok to use this trigger'
> 
> When we are checking the combination you have here, sure they
> become the same thing but there are devices where it
> matters that the trigger is not used to drive other devices
> (typically because it's a hardware line that goes nowhere
> else, so no interrupts etc) but other triggers can be used
> to drive this device (often by software triggering the scan).
> We have the opposite case as well but that's often
> a shortcut when it just happens to be really complex to get
> the trigger to reset (often requires reading all the data
> or similar) - that condition can almost always be relaxed
> but sometimes it's a lot of code for a niche case.
> 
> So fine to change the implementation of one of these
> checks on tightly coupled device and trigger to call the other
> but don't touch the callback signatures as to that breaks the
> logical parameter ordering.
> 
> Jonathan
> 

Hi Jonathan,

Thank you very much for the explanation, it makes total sense.
No need to change everything I think, it is a very small thing
and maybe even better like how it is now from what I understand.

Cheers,
Vasilis

> > Have a nice day!
> > 
> > Cheers,
> > Vasilis
>