Message ID | 20210511153108.14816-1-tangbin@cmss.chinamobile.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | iio: adc: ad7768-1: Fix the right interrupt interface calls | expand |
On 5/11/21 5:31 PM, Tang Bin wrote: > In the function ad7768_probe(), the devm_request_irq() should > call ad7768_interrupt, not &ad7768_interrupt, so fix this mistake. > > Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support") > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> > Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> Hi, Thanks for the patch. Aren't those two expressions equivalent? Are you seeing an issue with the current code? If so can you include that in the commit message? - Lars > --- > drivers/iio/adc/ad7768-1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > index 0e93b0766..9c9ab56d6 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -605,7 +605,7 @@ static int ad7768_probe(struct spi_device *spi) > init_completion(&st->completion); > > ret = devm_request_irq(&spi->dev, spi->irq, > - &ad7768_interrupt, > + ad7768_interrupt, > IRQF_TRIGGER_RISING | IRQF_ONESHOT, > indio_dev->name, indio_dev); > if (ret)
Hi Lars-Peter: Thanks for you reply! > Hi, > > Thanks for the patch. Aren't those two expressions equivalent? Are you > seeing an issue with the current code? If so can you include that in > the commit message? > > - Lars > > When submitting this patch, I actually thought about it for a while, but finally decided to submit it, my reason is as follows: In numerical data of address, &ad7768_interrupt is equal to ad7768_interrupt, and the compilation can pass. But I think they are not the same, ad7768_interrupt is the first address of the function, and its type is irqreturn_t, &ad7768_interrupt represents the address of an object that points to the function ad7768_interrupt(). So I think they are not the same, For previous experience with devm_request_irq(), I send this patch. If I'm wrong, I'm sorry to bother you. Thanks Tang Bin
On 5/12/21 10:39 AM, tangbin wrote: > Hi Lars-Peter: > > Thanks for you reply! > >> Hi, >> >> Thanks for the patch. Aren't those two expressions equivalent? Are >> you seeing an issue with the current code? If so can you include that >> in the commit message? >> >> - Lars >> >> > When submitting this patch, I actually thought about it for a > while, but finally decided to submit it, my reason is as follows: > > In numerical data of address, &ad7768_interrupt is equal to > ad7768_interrupt, and the compilation can pass. But I think they are > not the same, ad7768_interrupt is the first > > address of the function, and its type is irqreturn_t, > &ad7768_interrupt represents the address of an object that points to > the function ad7768_interrupt(). > > So I think they are not the same, For previous experience with > devm_request_irq(), I send this patch. If I'm wrong, I'm sorry to > bother you. > Have a look at https://stackoverflow.com/questions/6893285/why-do-function-pointer-definitions-work-with-any-number-of-ampersands-or-as for some background on this. You can also easily verify that they are the same with a simple test program static void foo(void) {} int main(void) { printf("%p %p %d\n", foo, &foo, foo == &foo); }
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index 0e93b0766..9c9ab56d6 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -605,7 +605,7 @@ static int ad7768_probe(struct spi_device *spi) init_completion(&st->completion); ret = devm_request_irq(&spi->dev, spi->irq, - &ad7768_interrupt, + ad7768_interrupt, IRQF_TRIGGER_RISING | IRQF_ONESHOT, indio_dev->name, indio_dev); if (ret)