* [PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type
@ 2018-11-20 17:00 Shreeya Patel
2018-11-21 8:21 ` Ardelean, Alexandru
0 siblings, 1 reply; 4+ messages in thread
From: Shreeya Patel @ 2018-11-20 17:00 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel
ADT7316 driver no more uses platform data and hence use device tree
data instead of platform data for assigning irq_type field.
Switch case figures out the type of irq and if it's the default case
then assign the default value to the irq_type i.e.
irq_type = IRQF_TRIGGER_LOW
Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
---
drivers/staging/iio/addac/adt7316.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
index 9c72538baf9e..c647875a64f5 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -2101,8 +2101,7 @@ int adt7316_probe(struct device *dev, struct adt7316_bus *bus,
{
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
- unsigned short *adt7316_platform_data = dev->platform_data;
- int irq_type = IRQF_TRIGGER_LOW;
+ int irq_type;
int ret = 0;
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2146,8 +2145,22 @@ int adt7316_probe(struct device *dev, struct adt7316_bus *bus,
indio_dev->modes = INDIO_DIRECT_MODE;
if (chip->bus.irq > 0) {
- if (adt7316_platform_data[0])
- irq_type = adt7316_platform_data[0];
+ irq_type =
+ irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
+
+ switch (irq_type) {
+ case IRQF_TRIGGER_HIGH:
+ case IRQF_TRIGGER_RISING:
+ break;
+ case IRQF_TRIGGER_LOW:
+ case IRQF_TRIGGER_FALLING:
+ break;
+ default:
+ dev_info(dev, "mode %d unsupported, using IRQF_TRIGGER_LOW\n",
+ irq_type);
+ irq_type = IRQF_TRIGGER_LOW;
+ break;
+ }
ret = devm_request_threaded_irq(dev, chip->bus.irq,
NULL,
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type
2018-11-20 17:00 [PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type Shreeya Patel
@ 2018-11-21 8:21 ` Ardelean, Alexandru
2018-11-21 9:32 ` Shreeya Patel
0 siblings, 1 reply; 4+ messages in thread
From: Ardelean, Alexandru @ 2018-11-21 8:21 UTC (permalink / raw)
To: lars, linux-kernel, knaack.h, jic23, Hennerich, Michael,
linux-iio, devel, pmeerw, gregkh, shreeya.patel23498
On Tue, 2018-11-20 at 22:30 +0530, Shreeya Patel wrote:
> ADT7316 driver no more uses platform data and hence use device tree
> data instead of platform data for assigning irq_type field.
> Switch case figures out the type of irq and if it's the default case
> then assign the default value to the irq_type i.e.
> irq_type = IRQF_TRIGGER_LOW
>
1 comment inline
> Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> ---
> drivers/staging/iio/addac/adt7316.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/addac/adt7316.c
> b/drivers/staging/iio/addac/adt7316.c
> index 9c72538baf9e..c647875a64f5 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -2101,8 +2101,7 @@ int adt7316_probe(struct device *dev, struct
> adt7316_bus *bus,
> {
> struct adt7316_chip_info *chip;
> struct iio_dev *indio_dev;
> - unsigned short *adt7316_platform_data = dev->platform_data;
> - int irq_type = IRQF_TRIGGER_LOW;
> + int irq_type;
> int ret = 0;
>
> indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
> @@ -2146,8 +2145,22 @@ int adt7316_probe(struct device *dev, struct
> adt7316_bus *bus,
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> if (chip->bus.irq > 0) {
> - if (adt7316_platform_data[0])
> - irq_type = adt7316_platform_data[0];
> + irq_type =
> + irqd_get_trigger_type(irq_get_irq_data(chip-
> >bus.irq));
> +
> + switch (irq_type) {
> + case IRQF_TRIGGER_HIGH:
> + case IRQF_TRIGGER_RISING:
> + break;
> + case IRQF_TRIGGER_LOW:
> + case IRQF_TRIGGER_FALLING:
> + break;
> + default:
> + dev_info(dev, "mode %d unsupported, using
> IRQF_TRIGGER_LOW\n",
> + irq_type);
> + irq_type = IRQF_TRIGGER_LOW;
> + break;
> + }
It would be an idea to move this part [together with
devm_request_threaded_irq()] into a "adt7316_setup_irq()" function. To un-
clutter the code in the adt7316_probe() function.
>
> ret = devm_request_threaded_irq(dev, chip->bus.irq,
> NULL,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type
2018-11-21 8:21 ` Ardelean, Alexandru
@ 2018-11-21 9:32 ` Shreeya Patel
2018-11-25 11:53 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Shreeya Patel @ 2018-11-21 9:32 UTC (permalink / raw)
To: Ardelean, Alexandru, lars, linux-kernel, knaack.h, jic23,
Hennerich, Michael, linux-iio, devel, pmeerw, gregkh
On Wed, 2018-11-21 at 08:21 +0000, Ardelean, Alexandru wrote:
> On Tue, 2018-11-20 at 22:30 +0530, Shreeya Patel wrote:
> > ADT7316 driver no more uses platform data and hence use device tree
> > data instead of platform data for assigning irq_type field.
> > Switch case figures out the type of irq and if it's the default
> > case
> > then assign the default value to the irq_type i.e.
> > irq_type = IRQF_TRIGGER_LOW
> >
>
> 1 comment inline
>
> > Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> > ---
> > drivers/staging/iio/addac/adt7316.c | 21 +++++++++++++++++----
> > 1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/iio/addac/adt7316.c
> > b/drivers/staging/iio/addac/adt7316.c
> > index 9c72538baf9e..c647875a64f5 100644
> > --- a/drivers/staging/iio/addac/adt7316.c
> > +++ b/drivers/staging/iio/addac/adt7316.c
> > @@ -2101,8 +2101,7 @@ int adt7316_probe(struct device *dev, struct
> > adt7316_bus *bus,
> > {
> > struct adt7316_chip_info *chip;
> > struct iio_dev *indio_dev;
> > - unsigned short *adt7316_platform_data = dev-
> > >platform_data;
> > - int irq_type = IRQF_TRIGGER_LOW;
> > + int irq_type;
> > int ret = 0;
> >
> > indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
> > @@ -2146,8 +2145,22 @@ int adt7316_probe(struct device *dev, struct
> > adt7316_bus *bus,
> > indio_dev->modes = INDIO_DIRECT_MODE;
> >
> > if (chip->bus.irq > 0) {
> > - if (adt7316_platform_data[0])
> > - irq_type = adt7316_platform_data[0];
> > + irq_type =
> > + irqd_get_trigger_type(irq_get_irq_data(chi
> > p-
> > > bus.irq));
> >
> > +
> > + switch (irq_type) {
> > + case IRQF_TRIGGER_HIGH:
> > + case IRQF_TRIGGER_RISING:
> > + break;
> > + case IRQF_TRIGGER_LOW:
> > + case IRQF_TRIGGER_FALLING:
> > + break;
> > + default:
> > + dev_info(dev, "mode %d unsupported, using
> > IRQF_TRIGGER_LOW\n",
> > + irq_type);
> > + irq_type = IRQF_TRIGGER_LOW;
> > + break;
> > + }
>
> It would be an idea to move this part [together with
> devm_request_threaded_irq()] into a "adt7316_setup_irq()" function.
> To un-
> clutter the code in the adt7316_probe() function.
>
Yes, seems like a good idea!
Even other drivers are doing the same as you told me to do...thanks :)
I'll do the change after Jonathan picks up the other patches and will
wait for some other reviews to come up if there are any.
Thanks
> >
> > ret = devm_request_threaded_irq(dev, chip-
> > >bus.irq,
> > NULL,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type
2018-11-21 9:32 ` Shreeya Patel
@ 2018-11-25 11:53 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2018-11-25 11:53 UTC (permalink / raw)
To: Shreeya Patel
Cc: Ardelean, Alexandru, lars, linux-kernel, knaack.h, Hennerich,
Michael, linux-iio, devel, pmeerw, gregkh
On Wed, 21 Nov 2018 15:02:52 +0530
Shreeya Patel <shreeya.patel23498@gmail.com> wrote:
> On Wed, 2018-11-21 at 08:21 +0000, Ardelean, Alexandru wrote:
> > On Tue, 2018-11-20 at 22:30 +0530, Shreeya Patel wrote:
> > > ADT7316 driver no more uses platform data and hence use device tree
> > > data instead of platform data for assigning irq_type field.
> > > Switch case figures out the type of irq and if it's the default
> > > case
> > > then assign the default value to the irq_type i.e.
> > > irq_type = IRQF_TRIGGER_LOW
> > >
> >
> > 1 comment inline
> >
> > > Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> > > ---
> > > drivers/staging/iio/addac/adt7316.c | 21 +++++++++++++++++----
> > > 1 file changed, 17 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/addac/adt7316.c
> > > b/drivers/staging/iio/addac/adt7316.c
> > > index 9c72538baf9e..c647875a64f5 100644
> > > --- a/drivers/staging/iio/addac/adt7316.c
> > > +++ b/drivers/staging/iio/addac/adt7316.c
> > > @@ -2101,8 +2101,7 @@ int adt7316_probe(struct device *dev, struct
> > > adt7316_bus *bus,
> > > {
> > > struct adt7316_chip_info *chip;
> > > struct iio_dev *indio_dev;
> > > - unsigned short *adt7316_platform_data = dev-
> > > >platform_data;
> > > - int irq_type = IRQF_TRIGGER_LOW;
> > > + int irq_type;
> > > int ret = 0;
> > >
> > > indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
> > > @@ -2146,8 +2145,22 @@ int adt7316_probe(struct device *dev, struct
> > > adt7316_bus *bus,
> > > indio_dev->modes = INDIO_DIRECT_MODE;
> > >
> > > if (chip->bus.irq > 0) {
> > > - if (adt7316_platform_data[0])
> > > - irq_type = adt7316_platform_data[0];
> > > + irq_type =
> > > + irqd_get_trigger_type(irq_get_irq_data(chi
> > > p-
> > > > bus.irq));
> > >
> > > +
> > > + switch (irq_type) {
> > > + case IRQF_TRIGGER_HIGH:
> > > + case IRQF_TRIGGER_RISING:
> > > + break;
> > > + case IRQF_TRIGGER_LOW:
> > > + case IRQF_TRIGGER_FALLING:
> > > + break;
> > > + default:
> > > + dev_info(dev, "mode %d unsupported, using
> > > IRQF_TRIGGER_LOW\n",
> > > + irq_type);
> > > + irq_type = IRQF_TRIGGER_LOW;
> > > + break;
> > > + }
> >
> > It would be an idea to move this part [together with
> > devm_request_threaded_irq()] into a "adt7316_setup_irq()" function.
> > To un-
> > clutter the code in the adt7316_probe() function.
> >
>
> Yes, seems like a good idea!
> Even other drivers are doing the same as you told me to do...thanks :)
>
> I'll do the change after Jonathan picks up the other patches and will
> wait for some other reviews to come up if there are any.
Agreed. This suggested change is good, so I'll leave this patch for now
on the basis it probably makes sense to do it as a short series focused
on that one element.
Thanks,
Jonathan
>
> Thanks
>
> > >
> > > ret = devm_request_threaded_irq(dev, chip-
> > > >bus.irq,
> > > NULL,
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-25 11:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-20 17:00 [PATCH v2 5/5] Staging: iio: adt7316: Use device tree data to assign irq_type Shreeya Patel
2018-11-21 8:21 ` Ardelean, Alexandru
2018-11-21 9:32 ` Shreeya Patel
2018-11-25 11:53 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).