2022-06-01 09:50:13

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

First of all, the additional conversion from vIRQ, and this is exactly
what is returned by platform_get_irq_byname(), to vIRQ is not needed.
Hence, drop no-op call to irq_of_parse_and_map().

Second, assign the firmware node instead of of_node.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/iio/adc/mxs-lradc-adc.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
index bca79a93cbe4..25292bb8a13f 100644
--- a/drivers/iio/adc/mxs-lradc-adc.c
+++ b/drivers/iio/adc/mxs-lradc-adc.c
@@ -17,7 +17,6 @@
#include <linux/mfd/core.h>
#include <linux/mfd/mxs-lradc.h>
#include <linux/module.h>
-#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/sysfs.h>

@@ -692,7 +691,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
struct mxs_lradc_adc *adc;
struct iio_dev *iio;
struct resource *iores;
- int ret, irq, virq, i, s, n;
+ int ret, irq, i, s, n;
u64 scale_uv;
const char **irq_name;

@@ -721,7 +720,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, iio);

iio->name = pdev->name;
- iio->dev.of_node = dev->parent->of_node;
+ device_set_node(&iio->dev, dev_fwnode(dev->parent));
iio->info = &mxs_lradc_adc_iio_info;
iio->modes = INDIO_DIRECT_MODE;
iio->masklength = LRADC_MAX_TOTAL_CHANS;
@@ -747,9 +746,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
if (irq < 0)
return irq;

- virq = irq_of_parse_and_map(dev->parent->of_node, irq);
-
- ret = devm_request_irq(dev, virq, mxs_lradc_adc_handle_irq,
+ ret = devm_request_irq(dev, irq, mxs_lradc_adc_handle_irq,
0, irq_name[i], iio);
if (ret)
return ret;
--
2.35.1



2022-06-06 05:15:20

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Mon, 30 May 2022 20:33:24 +0300
Andy Shevchenko <[email protected]> wrote:

> First of all, the additional conversion from vIRQ, and this is exactly
> what is returned by platform_get_irq_byname(), to vIRQ is not needed.
Confusing sentence form. Perhaps:

First, the additional conversion from vIRQ (returned by platform_get_irq_byname())
to vIRQ is not needed.

> Hence, drop no-op call to irq_of_parse_and_map().
>
> Second, assign the firmware node instead of of_node.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Hi,

Seems sensible to me, but I'd like a sanity check from someone more
familiar with this driver.

Thanks,

Jonathan

> ---
> drivers/iio/adc/mxs-lradc-adc.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
> index bca79a93cbe4..25292bb8a13f 100644
> --- a/drivers/iio/adc/mxs-lradc-adc.c
> +++ b/drivers/iio/adc/mxs-lradc-adc.c
> @@ -17,7 +17,6 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/mxs-lradc.h>
> #include <linux/module.h>
> -#include <linux/of_irq.h>
> #include <linux/platform_device.h>
> #include <linux/sysfs.h>
>
> @@ -692,7 +691,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
> struct mxs_lradc_adc *adc;
> struct iio_dev *iio;
> struct resource *iores;
> - int ret, irq, virq, i, s, n;
> + int ret, irq, i, s, n;
> u64 scale_uv;
> const char **irq_name;
>
> @@ -721,7 +720,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, iio);
>
> iio->name = pdev->name;
> - iio->dev.of_node = dev->parent->of_node;
> + device_set_node(&iio->dev, dev_fwnode(dev->parent));
> iio->info = &mxs_lradc_adc_iio_info;
> iio->modes = INDIO_DIRECT_MODE;
> iio->masklength = LRADC_MAX_TOTAL_CHANS;
> @@ -747,9 +746,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
> if (irq < 0)
> return irq;
>
> - virq = irq_of_parse_and_map(dev->parent->of_node, irq);
> -
> - ret = devm_request_irq(dev, virq, mxs_lradc_adc_handle_irq,
> + ret = devm_request_irq(dev, irq, mxs_lradc_adc_handle_irq,
> 0, irq_name[i], iio);
> if (ret)
> return ret;

2022-06-20 19:54:48

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Fri, 3 Jun 2022 18:10:06 +0100
Jonathan Cameron <[email protected]> wrote:

> On Mon, 30 May 2022 20:33:24 +0300
> Andy Shevchenko <[email protected]> wrote:
>
> > First of all, the additional conversion from vIRQ, and this is exactly
> > what is returned by platform_get_irq_byname(), to vIRQ is not needed.
> Confusing sentence form. Perhaps:
>
> First, the additional conversion from vIRQ (returned by platform_get_irq_byname())
> to vIRQ is not needed.
>
> > Hence, drop no-op call to irq_of_parse_and_map().
> >
> > Second, assign the firmware node instead of of_node.
> >
> > Signed-off-by: Andy Shevchenko <[email protected]>
> Hi,
>
> Seems sensible to me, but I'd like a sanity check from someone more
> familiar with this driver.

This one has been outstanding for a few weeks. I'd still like
an Ack or similar form someone who knows this device well.

If no one has looked at it in a week or so I'll just go with
my judgement and pick it up.

Thanks,

Jonathan

>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/adc/mxs-lradc-adc.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
> > index bca79a93cbe4..25292bb8a13f 100644
> > --- a/drivers/iio/adc/mxs-lradc-adc.c
> > +++ b/drivers/iio/adc/mxs-lradc-adc.c
> > @@ -17,7 +17,6 @@
> > #include <linux/mfd/core.h>
> > #include <linux/mfd/mxs-lradc.h>
> > #include <linux/module.h>
> > -#include <linux/of_irq.h>
> > #include <linux/platform_device.h>
> > #include <linux/sysfs.h>
> >
> > @@ -692,7 +691,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
> > struct mxs_lradc_adc *adc;
> > struct iio_dev *iio;
> > struct resource *iores;
> > - int ret, irq, virq, i, s, n;
> > + int ret, irq, i, s, n;
> > u64 scale_uv;
> > const char **irq_name;
> >
> > @@ -721,7 +720,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
> > platform_set_drvdata(pdev, iio);
> >
> > iio->name = pdev->name;
> > - iio->dev.of_node = dev->parent->of_node;
> > + device_set_node(&iio->dev, dev_fwnode(dev->parent));
> > iio->info = &mxs_lradc_adc_iio_info;
> > iio->modes = INDIO_DIRECT_MODE;
> > iio->masklength = LRADC_MAX_TOTAL_CHANS;
> > @@ -747,9 +746,7 @@ static int mxs_lradc_adc_probe(struct platform_device *pdev)
> > if (irq < 0)
> > return irq;
> >
> > - virq = irq_of_parse_and_map(dev->parent->of_node, irq);
> > -
> > - ret = devm_request_irq(dev, virq, mxs_lradc_adc_handle_irq,
> > + ret = devm_request_irq(dev, irq, mxs_lradc_adc_handle_irq,
> > 0, irq_name[i], iio);
> > if (ret)
> > return ret;
>

2022-06-20 21:33:03

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

Hi Andy,

On Mon, May 30, 2022 at 2:33 PM Andy Shevchenko
<[email protected]> wrote:
>
> First of all, the additional conversion from vIRQ, and this is exactly
> what is returned by platform_get_irq_byname(), to vIRQ is not needed.
> Hence, drop no-op call to irq_of_parse_and_map().
>
> Second, assign the firmware node instead of of_node.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/iio/adc/mxs-lradc-adc.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
> index bca79a93cbe4..25292bb8a13f 100644
> --- a/drivers/iio/adc/mxs-lradc-adc.c
> +++ b/drivers/iio/adc/mxs-lradc-adc.c
> @@ -17,7 +17,6 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/mxs-lradc.h>
> #include <linux/module.h>
> -#include <linux/of_irq.h>
> #include <linux/platform_device.h>
> #include <linux/sysfs.h>

#include <linux/property.h> needs to be included, otherwise the build fails.

> - virq = irq_of_parse_and_map(dev->parent->of_node, irq);
> -
> - ret = devm_request_irq(dev, virq, mxs_lradc_adc_handle_irq,
> + ret = devm_request_irq(dev, irq, mxs_lradc_adc_handle_irq,
> 0, irq_name[i], iio);

I tried to apply the same change inside
drivers/input/touchscreen/mxs-lradc-ts.c:

--- a/drivers/input/touchscreen/mxs-lradc-ts.c
+++ b/drivers/input/touchscreen/mxs-lradc-ts.c
@@ -675,11 +675,9 @@ static int mxs_lradc_ts_probe(struct platform_device *pdev)
if (irq < 0)
return irq;

- virq = irq_of_parse_and_map(node, irq);
-
mxs_lradc_ts_stop(ts);

- ret = devm_request_irq(dev, virq,
+ ret = devm_request_irq(dev, irq,
mxs_lradc_ts_handle_irq,
0, mxs_lradc_ts_irq_names[i], ts);
if (ret)

but I still get the following warning:

[ 6.135583] ------------[ cut here ]------------
[ 6.140366] WARNING: CPU: 0 PID: 1 at drivers/base/platform.c:449
__platform_get_irq_byname+0x74/0x90
[ 6.151053] 0 is an invalid IRQ number
[ 6.155201] Modules linked in:
[ 6.158444] CPU: 0 PID: 1 Comm: swapper Not tainted
5.18.5-00001-g3e38be7e4832 #108
[ 6.166537] Hardware name: Freescale MXS (Device Tree)
[ 6.172040] unwind_backtrace from show_stack+0x10/0x14
[ 6.177503] show_stack from __warn+0xc4/0x1cc
[ 6.182356] __warn from warn_slowpath_fmt+0x90/0xc8
[ 6.187549] warn_slowpath_fmt from __platform_get_irq_byname+0x74/0x90
[ 6.194698] __platform_get_irq_byname from platform_get_irq_byname+0x10/0x30
[ 6.202286] platform_get_irq_byname from mxs_lradc_ts_probe+0x19c/0x380
[ 6.209216] mxs_lradc_ts_probe from platform_probe+0x58/0xb8
[ 6.215383] platform_probe from really_probe+0xfc/0x288
[ 6.220907] really_probe from __driver_probe_device+0x80/0xe4
[ 6.227145] __driver_probe_device from driver_probe_device+0x30/0xd8
[ 6.234010] driver_probe_device from __driver_attach+0x70/0xf4
[ 6.240137] __driver_attach from bus_for_each_dev+0x74/0xc0
[ 6.246195] bus_for_each_dev from bus_add_driver+0x154/0x1e8
[ 6.252359] bus_add_driver from driver_register+0x74/0x108
[ 6.258131] driver_register from do_one_initcall+0x8c/0x2fc
[ 6.264198] do_one_initcall from kernel_init_freeable+0x184/0x210
[ 6.270588] kernel_init_freeable from kernel_init+0x10/0x108
[ 6.276756] kernel_init from ret_from_fork+0x14/0x3c
[ 6.282224] Exception stack(0xc8819fb0 to 0xc8819ff8)
[ 6.287444] 9fa0: 00000000
00000000 00000000 00000000
[ 6.295988] 9fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 6.304631] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 6.311392] irq event stamp: 128211
[ 6.315220] hardirqs last enabled at (128221): [<c0069c44>]
__up_console_sem+0x54/0x64
[ 6.323632] hardirqs last disabled at (128230): [<c0069c30>]
__up_console_sem+0x40/0x64
[ 6.331821] softirqs last enabled at (128200): [<c00098e4>]
__do_softirq+0x31c/0x4bc
[ 6.340041] softirqs last disabled at (128191): [<c0020f04>]
irq_exit+0x150/0x18c
[ 6.347913] ---[ end trace 0000000000000000 ]---
[ 6.364587] input: mxs-lradc-ts as
/devices/soc0/80000000.apb/80040000.apbx/80050000.lradc/mxs-lradc-ts/input/input0

Any suggestions?

Thanks

2022-06-20 22:19:56

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Mon, Jun 20, 2022 at 6:34 PM Andy Shevchenko
<[email protected]> wrote:

> Still? Does it mean you have it before my patch? If no, I will be very puzzled...

Yes, the warning is present before your patch.

> Otherwise does the touchscreen work?

I cannot test touchscreen at the moment.

>> Any suggestions?
>
> Perhaps, but we need to eliminate the proposed change from the equation

If you send a v2 with "#include <linux/property.h>" then you can add:

Tested-by: Fabio Estevam <[email protected]>

2022-06-21 10:29:30

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Mon, Jun 20, 2022 at 06:56:27PM -0300, Fabio Estevam wrote:
> On Mon, Jun 20, 2022 at 6:34 PM Andy Shevchenko
> <[email protected]> wrote:
>
> > Still? Does it mean you have it before my patch? If no, I will be very puzzled...
>
> Yes, the warning is present before your patch.
>
> > Otherwise does the touchscreen work?
>
> I cannot test touchscreen at the moment.
>
> >> Any suggestions?
> >
> > Perhaps, but we need to eliminate the proposed change from the equation
>
> If you send a v2 with "#include <linux/property.h>" then you can add:

Actually the entire thingy with setting node is not needed in this driver.

> Tested-by: Fabio Estevam <[email protected]>

Thanks!

--
With Best Regards,
Andy Shevchenko


2022-06-21 10:46:27

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Tue, Jun 21, 2022 at 01:26:29PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 20, 2022 at 06:56:27PM -0300, Fabio Estevam wrote:
> > On Mon, Jun 20, 2022 at 6:34 PM Andy Shevchenko
> > <[email protected]> wrote:

...

> > If you send a v2 with "#include <linux/property.h>" then you can add:
>
> Actually the entire thingy with setting node is not needed in this driver.

I stand corrected, the fwnode assignment is done fore the IIO parent's parent,
it's needed...


--
With Best Regards,
Andy Shevchenko


2022-06-21 11:19:59

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Mon, Jun 20, 2022 at 06:13:53PM -0300, Fabio Estevam wrote:
> On Mon, May 30, 2022 at 2:33 PM Andy Shevchenko
> <[email protected]> wrote:

...

> I tried to apply the same change inside
> drivers/input/touchscreen/mxs-lradc-ts.c:
>
> --- a/drivers/input/touchscreen/mxs-lradc-ts.c
> +++ b/drivers/input/touchscreen/mxs-lradc-ts.c
> @@ -675,11 +675,9 @@ static int mxs_lradc_ts_probe(struct platform_device *pdev)
> if (irq < 0)
> return irq;
>
> - virq = irq_of_parse_and_map(node, irq);
> -
> mxs_lradc_ts_stop(ts);
>
> - ret = devm_request_irq(dev, virq,
> + ret = devm_request_irq(dev, irq,
> mxs_lradc_ts_handle_irq,
> 0, mxs_lradc_ts_irq_names[i], ts);
> if (ret)
>
> but I still get the following warning:

So just to be sure. You got it before the above change applied, correct?

I'm wondering how this all LRADC was supposed to work. The IRQs are assigned
based on abstract numbering without any IRQ domain behind it. This is not how
it's designed in Linux. Adding Ksenija and Marek to shed a light.

> [ 6.135583] ------------[ cut here ]------------
> [ 6.140366] WARNING: CPU: 0 PID: 1 at drivers/base/platform.c:449
> __platform_get_irq_byname+0x74/0x90
> [ 6.151053] 0 is an invalid IRQ number
> [ 6.155201] Modules linked in:
> [ 6.158444] CPU: 0 PID: 1 Comm: swapper Not tainted
> 5.18.5-00001-g3e38be7e4832 #108
> [ 6.166537] Hardware name: Freescale MXS (Device Tree)
> [ 6.172040] unwind_backtrace from show_stack+0x10/0x14
> [ 6.177503] show_stack from __warn+0xc4/0x1cc
> [ 6.182356] __warn from warn_slowpath_fmt+0x90/0xc8
> [ 6.187549] warn_slowpath_fmt from __platform_get_irq_byname+0x74/0x90
> [ 6.194698] __platform_get_irq_byname from platform_get_irq_byname+0x10/0x30
> [ 6.202286] platform_get_irq_byname from mxs_lradc_ts_probe+0x19c/0x380
> [ 6.209216] mxs_lradc_ts_probe from platform_probe+0x58/0xb8
> [ 6.215383] platform_probe from really_probe+0xfc/0x288
> [ 6.220907] really_probe from __driver_probe_device+0x80/0xe4
> [ 6.227145] __driver_probe_device from driver_probe_device+0x30/0xd8
> [ 6.234010] driver_probe_device from __driver_attach+0x70/0xf4
> [ 6.240137] __driver_attach from bus_for_each_dev+0x74/0xc0
> [ 6.246195] bus_for_each_dev from bus_add_driver+0x154/0x1e8
> [ 6.252359] bus_add_driver from driver_register+0x74/0x108
> [ 6.258131] driver_register from do_one_initcall+0x8c/0x2fc
> [ 6.264198] do_one_initcall from kernel_init_freeable+0x184/0x210
> [ 6.270588] kernel_init_freeable from kernel_init+0x10/0x108
> [ 6.276756] kernel_init from ret_from_fork+0x14/0x3c
> [ 6.282224] Exception stack(0xc8819fb0 to 0xc8819ff8)
> [ 6.287444] 9fa0: 00000000
> 00000000 00000000 00000000
> [ 6.295988] 9fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 6.304631] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 6.311392] irq event stamp: 128211
> [ 6.315220] hardirqs last enabled at (128221): [<c0069c44>]
> __up_console_sem+0x54/0x64
> [ 6.323632] hardirqs last disabled at (128230): [<c0069c30>]
> __up_console_sem+0x40/0x64
> [ 6.331821] softirqs last enabled at (128200): [<c00098e4>]
> __do_softirq+0x31c/0x4bc
> [ 6.340041] softirqs last disabled at (128191): [<c0020f04>]
> irq_exit+0x150/0x18c
> [ 6.347913] ---[ end trace 0000000000000000 ]---
> [ 6.364587] input: mxs-lradc-ts as
> /devices/soc0/80000000.apb/80040000.apbx/80050000.lradc/mxs-lradc-ts/input/input0
>
> Any suggestions?

--
With Best Regards,
Andy Shevchenko


2022-06-21 11:36:00

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

Hi Andy,

On Tue, Jun 21, 2022 at 8:00 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Mon, Jun 20, 2022 at 06:13:53PM -0300, Fabio Estevam wrote:
> > On Mon, May 30, 2022 at 2:33 PM Andy Shevchenko
> > <[email protected]> wrote:
>
> ...
>
> > I tried to apply the same change inside
> > drivers/input/touchscreen/mxs-lradc-ts.c:
> >
> > --- a/drivers/input/touchscreen/mxs-lradc-ts.c
> > +++ b/drivers/input/touchscreen/mxs-lradc-ts.c
> > @@ -675,11 +675,9 @@ static int mxs_lradc_ts_probe(struct platform_device *pdev)
> > if (irq < 0)
> > return irq;
> >
> > - virq = irq_of_parse_and_map(node, irq);
> > -
> > mxs_lradc_ts_stop(ts);
> >
> > - ret = devm_request_irq(dev, virq,
> > + ret = devm_request_irq(dev, irq,
> > mxs_lradc_ts_handle_irq,
> > 0, mxs_lradc_ts_irq_names[i], ts);
> > if (ret)
> >
> > but I still get the following warning:
>
> So just to be sure. You got it before the above change applied, correct?

Correct. This warning is an old one. See:
https://lore.kernel.org/all/20200701224145.GA3616172@bjorn-Precision-5520/T/

> I'm wondering how this all LRADC was supposed to work. The IRQs are assigned
> based on abstract numbering without any IRQ domain behind it. This is not how
> it's designed in Linux. Adding Ksenija and Marek to shed a light.

2022-09-06 14:42:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Mon, Jun 20, 2022 at 08:42:25PM +0100, Jonathan Cameron wrote:
> On Fri, 3 Jun 2022 18:10:06 +0100
> Jonathan Cameron <[email protected]> wrote:
> > On Mon, 30 May 2022 20:33:24 +0300
> > Andy Shevchenko <[email protected]> wrote:
> >
> > > First of all, the additional conversion from vIRQ, and this is exactly
> > > what is returned by platform_get_irq_byname(), to vIRQ is not needed.
> > Confusing sentence form. Perhaps:
> >
> > First, the additional conversion from vIRQ (returned by platform_get_irq_byname())
> > to vIRQ is not needed.
> >
> > > Hence, drop no-op call to irq_of_parse_and_map().
> > >
> > > Second, assign the firmware node instead of of_node.

...

> > Seems sensible to me, but I'd like a sanity check from someone more
> > familiar with this driver.
>
> This one has been outstanding for a few weeks. I'd still like
> an Ack or similar form someone who knows this device well.
>
> If no one has looked at it in a week or so I'll just go with
> my judgement and pick it up.

Any news on this one? Maybe I need to resend with the better commit message?

--
With Best Regards,
Andy Shevchenko


2022-09-11 10:43:48

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Tue, 6 Sep 2022 16:36:20 +0300
Andy Shevchenko <[email protected]> wrote:

> On Mon, Jun 20, 2022 at 08:42:25PM +0100, Jonathan Cameron wrote:
> > On Fri, 3 Jun 2022 18:10:06 +0100
> > Jonathan Cameron <[email protected]> wrote:
> > > On Mon, 30 May 2022 20:33:24 +0300
> > > Andy Shevchenko <[email protected]> wrote:
> > >
> > > > First of all, the additional conversion from vIRQ, and this is exactly
> > > > what is returned by platform_get_irq_byname(), to vIRQ is not needed.
> > > Confusing sentence form. Perhaps:
> > >
> > > First, the additional conversion from vIRQ (returned by platform_get_irq_byname())
> > > to vIRQ is not needed.
> > >
> > > > Hence, drop no-op call to irq_of_parse_and_map().
> > > >
> > > > Second, assign the firmware node instead of of_node.
>
> ...
>
> > > Seems sensible to me, but I'd like a sanity check from someone more
> > > familiar with this driver.
> >
> > This one has been outstanding for a few weeks. I'd still like
> > an Ack or similar form someone who knows this device well.
> >
> > If no one has looked at it in a week or so I'll just go with
> > my judgement and pick it up.
>
> Any news on this one? Maybe I need to resend with the better commit message?
>

From glancing back at the thread, looks like you said you were going to send
a v2 with property.h include. I think the rest of the thread is concerned
with a different issue entirely.

Jonathan


2022-09-12 10:29:32

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Sun, Sep 11, 2022 at 10:52:15AM +0100, Jonathan Cameron wrote:
> On Tue, 6 Sep 2022 16:36:20 +0300
> Andy Shevchenko <[email protected]> wrote:
> > On Mon, Jun 20, 2022 at 08:42:25PM +0100, Jonathan Cameron wrote:

...

> > Any news on this one? Maybe I need to resend with the better commit message?
>
> From glancing back at the thread, looks like you said you were going to send
> a v2 with property.h include. I think the rest of the thread is concerned
> with a different issue entirely.

Ah, I only read last messages in the thread and didn't realized that I have
some ARs to accomplish. OK, v2 will come soon, thanks!

--
With Best Regards,
Andy Shevchenko


2022-09-12 10:29:54

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] iio: adc: mxs-lradc-adc: Get rid of OF specifics

On Mon, Sep 12, 2022 at 01:21:52PM +0300, Andy Shevchenko wrote:
> On Sun, Sep 11, 2022 at 10:52:15AM +0100, Jonathan Cameron wrote:
> > On Tue, 6 Sep 2022 16:36:20 +0300
> > Andy Shevchenko <[email protected]> wrote:
> > > On Mon, Jun 20, 2022 at 08:42:25PM +0100, Jonathan Cameron wrote:

...

> > > Any news on this one? Maybe I need to resend with the better commit message?
> >
> > From glancing back at the thread, looks like you said you were going to send
> > a v2 with property.h include. I think the rest of the thread is concerned
> > with a different issue entirely.
>
> Ah, I only read last messages in the thread and didn't realized that I have
> some ARs to accomplish. OK, v2 will come soon, thanks!

After reading more of the thread I think this patch should be in a different
form, so no v2 soon, needs more investigations.

--
With Best Regards,
Andy Shevchenko