[PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Anson Huang]

All i.MX SoCs except i.MX1 have ONLY 1 IRQ, so it is better to check
the IRQ count before getting second/third IRQ to avoid below error
message during probe:

[    0.726219] imx-uart 30860000.serial: IRQ index 1 not found
[    0.731329] imx-uart 30860000.serial: IRQ index 2 not found

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/tty/serial/imx.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 504d81c..081fa82 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2198,6 +2198,7 @@ static int imx_uart_probe(struct platform_device *pdev)
 	u32 ucr1;
 	struct resource *res;
 	int txirq, rxirq, rtsirq;
+	int irq_count;
 
 	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
 	if (!sport)
@@ -2220,9 +2221,17 @@ static int imx_uart_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	irq_count = platform_irq_count(pdev);
+	if (irq_count < 0)
+		return irq_count;
+
 	rxirq = platform_get_irq(pdev, 0);
-	txirq = platform_get_irq(pdev, 1);
-	rtsirq = platform_get_irq(pdev, 2);
+	if (irq_count > 1) {
+		txirq = platform_get_irq(pdev, 1);
+		rtsirq = platform_get_irq(pdev, 2);
+	} else {
+		txirq = rtsirq = -ENXIO;
+	}
 
 	sport->port.dev = &pdev->dev;
 	sport->port.mapbase = res->start;
-- 
2.7.4

Re: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Uwe Kleine-König]

Hello,

On Wed, Oct 09, 2019 at 02:18:31PM +0800, Anson Huang wrote:
> All i.MX SoCs except i.MX1 have ONLY 1 IRQ, so it is better to check
> the IRQ count before getting second/third IRQ to avoid below error
> message during probe:
> 
> [    0.726219] imx-uart 30860000.serial: IRQ index 1 not found
> [    0.731329] imx-uart 30860000.serial: IRQ index 2 not found

This message was introduced in commit
7723f4c5ecdb8d832f049f8483beb0d1081cedf6 for 5.4-rc1. I added the
involved people to the recipents of this mail.

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/tty/serial/imx.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 504d81c..081fa82 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -2198,6 +2198,7 @@ static int imx_uart_probe(struct platform_device *pdev)
>  	u32 ucr1;
>  	struct resource *res;
>  	int txirq, rxirq, rtsirq;
> +	int irq_count;
>  
>  	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
>  	if (!sport)
> @@ -2220,9 +2221,17 @@ static int imx_uart_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	irq_count = platform_irq_count(pdev);
> +	if (irq_count < 0)
> +		return irq_count;
> +
>  	rxirq = platform_get_irq(pdev, 0);
> -	txirq = platform_get_irq(pdev, 1);
> -	rtsirq = platform_get_irq(pdev, 2);
> +	if (irq_count > 1) {
> +		txirq = platform_get_irq(pdev, 1);
> +		rtsirq = platform_get_irq(pdev, 2);
> +	} else {
> +		txirq = rtsirq = -ENXIO;
> +	}

The patch is fine given the changed behaviour of platform_get_irq. I
wonder if it is sensible to introduce a variant of platform_get_irq (say
platform_get_irq_nowarn) that behaves like __platform_get_irq does
today. Then the imx driver would just call platform_get_irq_nowarn
without having to check the number of available irqs first.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

RE: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Anson Huang]

Hi, Uwe

> On Wed, Oct 09, 2019 at 02:18:31PM +0800, Anson Huang wrote:
> > All i.MX SoCs except i.MX1 have ONLY 1 IRQ, so it is better to check
> > the IRQ count before getting second/third IRQ to avoid below error
> > message during probe:
> >
> > [    0.726219] imx-uart 30860000.serial: IRQ index 1 not found
> > [    0.731329] imx-uart 30860000.serial: IRQ index 2 not found
> 
> This message was introduced in commit
> 7723f4c5ecdb8d832f049f8483beb0d1081cedf6 for 5.4-rc1. I added the
> involved people to the recipents of this mail.

Yes, I noticed this, thanks.

> 
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/tty/serial/imx.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> > 504d81c..081fa82 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -2198,6 +2198,7 @@ static int imx_uart_probe(struct platform_device
> *pdev)
> >  	u32 ucr1;
> >  	struct resource *res;
> >  	int txirq, rxirq, rtsirq;
> > +	int irq_count;
> >
> >  	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
> >  	if (!sport)
> > @@ -2220,9 +2221,17 @@ static int imx_uart_probe(struct
> platform_device *pdev)
> >  	if (IS_ERR(base))
> >  		return PTR_ERR(base);
> >
> > +	irq_count = platform_irq_count(pdev);
> > +	if (irq_count < 0)
> > +		return irq_count;
> > +
> >  	rxirq = platform_get_irq(pdev, 0);
> > -	txirq = platform_get_irq(pdev, 1);
> > -	rtsirq = platform_get_irq(pdev, 2);
> > +	if (irq_count > 1) {
> > +		txirq = platform_get_irq(pdev, 1);
> > +		rtsirq = platform_get_irq(pdev, 2);
> > +	} else {
> > +		txirq = rtsirq = -ENXIO;
> > +	}
> 
> The patch is fine given the changed behaviour of platform_get_irq. I wonder
> if it is sensible to introduce a variant of platform_get_irq (say
> platform_get_irq_nowarn) that behaves like __platform_get_irq does t
> Then the imx driver would just call platform_get_irq_nowarn without having
> to check the number of available irqs first.

Agreed, it would be nice if we can fix this from the API level, this is to save many patches
from various drivers side, let me know if agreement is reached and I will do the patch.

Thanks,
Anson.

Re: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Uwe Kleine-König]

Hello,

On Wed, Oct 09, 2019 at 06:58:24AM +0000, Anson Huang wrote:
> > On Wed, Oct 09, 2019 at 02:18:31PM +0800, Anson Huang wrote:
> > > All i.MX SoCs except i.MX1 have ONLY 1 IRQ, so it is better to check
> > > the IRQ count before getting second/third IRQ to avoid below error
> > > message during probe:
> > >
> > > [    0.726219] imx-uart 30860000.serial: IRQ index 1 not found
> > > [    0.731329] imx-uart 30860000.serial: IRQ index 2 not found
> > 
> > This message was introduced in commit
> > 7723f4c5ecdb8d832f049f8483beb0d1081cedf6 for 5.4-rc1. I added the
> > involved people to the recipents of this mail.
> 
> Yes, I noticed this, thanks.
> 
> > 
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > >  drivers/tty/serial/imx.c | 13 +++++++++++--
> > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index
> > > 504d81c..081fa82 100644
> > > --- a/drivers/tty/serial/imx.c
> > > +++ b/drivers/tty/serial/imx.c
> > > @@ -2198,6 +2198,7 @@ static int imx_uart_probe(struct platform_device
> > *pdev)
> > >  	u32 ucr1;
> > >  	struct resource *res;
> > >  	int txirq, rxirq, rtsirq;
> > > +	int irq_count;
> > >
> > >  	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
> > >  	if (!sport)
> > > @@ -2220,9 +2221,17 @@ static int imx_uart_probe(struct
> > platform_device *pdev)
> > >  	if (IS_ERR(base))
> > >  		return PTR_ERR(base);
> > >
> > > +	irq_count = platform_irq_count(pdev);
> > > +	if (irq_count < 0)
> > > +		return irq_count;
> > > +
> > >  	rxirq = platform_get_irq(pdev, 0);
> > > -	txirq = platform_get_irq(pdev, 1);
> > > -	rtsirq = platform_get_irq(pdev, 2);
> > > +	if (irq_count > 1) {
> > > +		txirq = platform_get_irq(pdev, 1);
> > > +		rtsirq = platform_get_irq(pdev, 2);
> > > +	} else {
> > > +		txirq = rtsirq = -ENXIO;
> > > +	}
> > 
> > The patch is fine given the changed behaviour of platform_get_irq. I wonder
> > if it is sensible to introduce a variant of platform_get_irq (say
> > platform_get_irq_nowarn) that behaves like __platform_get_irq does t
> > Then the imx driver would just call platform_get_irq_nowarn without having
> > to check the number of available irqs first.
> 
> Agreed, it would be nice if we can fix this from the API level, this
> is to save many patches from various drivers side, let me know if
> agreement is reached and I will do the patch.

I wouldn't expect that most callers actually want an error message and
so these need a different patch (i.e. dropping the error message by the
caller). This type of patch is fine and the normal load when something
is consolidated.

Which other drivers do you have on your radar that don't want an error
message if platform_get_irq() fails?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

RE: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Anson Huang]

Hi, Uwe

> On Wed, Oct 09, 2019 at 06:58:24AM +0000, Anson Huang wrote:
> > > On Wed, Oct 09, 2019 at 02:18:31PM +0800, Anson Huang wrote:
> > > > All i.MX SoCs except i.MX1 have ONLY 1 IRQ, so it is better to
> > > > check the IRQ count before getting second/third IRQ to avoid below
> > > > error message during probe:
> > > >
> > > > [    0.726219] imx-uart 30860000.serial: IRQ index 1 not found
> > > > [    0.731329] imx-uart 30860000.serial: IRQ index 2 not found
> > >
> > > This message was introduced in commit
> > > 7723f4c5ecdb8d832f049f8483beb0d1081cedf6 for 5.4-rc1. I added the
> > > involved people to the recipents of this mail.
> >
> > Yes, I noticed this, thanks.
> >
> > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > >  drivers/tty/serial/imx.c | 13 +++++++++++--
> > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > > index
> > > > 504d81c..081fa82 100644
> > > > --- a/drivers/tty/serial/imx.c
> > > > +++ b/drivers/tty/serial/imx.c
> > > > @@ -2198,6 +2198,7 @@ static int imx_uart_probe(struct
> > > > platform_device
> > > *pdev)
> > > >  	u32 ucr1;
> > > >  	struct resource *res;
> > > >  	int txirq, rxirq, rtsirq;
> > > > +	int irq_count;
> > > >
> > > >  	sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL);
> > > >  	if (!sport)
> > > > @@ -2220,9 +2221,17 @@ static int imx_uart_probe(struct
> > > platform_device *pdev)
> > > >  	if (IS_ERR(base))
> > > >  		return PTR_ERR(base);
> > > >
> > > > +	irq_count = platform_irq_count(pdev);
> > > > +	if (irq_count < 0)
> > > > +		return irq_count;
> > > > +
> > > >  	rxirq = platform_get_irq(pdev, 0);
> > > > -	txirq = platform_get_irq(pdev, 1);
> > > > -	rtsirq = platform_get_irq(pdev, 2);
> > > > +	if (irq_count > 1) {
> > > > +		txirq = platform_get_irq(pdev, 1);
> > > > +		rtsirq = platform_get_irq(pdev, 2);
> > > > +	} else {
> > > > +		txirq = rtsirq = -ENXIO;
> > > > +	}
> > >
> > > The patch is fine given the changed behaviour of platform_get_irq. I
> > > wonder if it is sensible to introduce a variant of platform_get_irq
> > > (say
> > > platform_get_irq_nowarn) that behaves like __platform_get_irq does t
> > > Then the imx driver would just call platform_get_irq_nowarn without
> > > having to check the number of available irqs first.
> >
> > Agreed, it would be nice if we can fix this from the API level, this
> > is to save many patches from various drivers side, let me know if
> > agreement is reached and I will do the patch.
> 
> I wouldn't expect that most callers actually want an error message and so
> these need a different patch (i.e. dropping the error message by the caller).
> This type of patch is fine and the normal load when something is
> consolidated.
> 
> Which other drivers do you have on your radar that don't want an error
> message if platform_get_irq() fails?

I did NOT mean drivers don't want an error when getting irq failed, but I just
agree that introducing another API with nowarn as you mentioned upper, then
i.MX driver can call it. For now, the FEC driver also have many such error message,
we will fix later.

So if the API with nowarn is added, then I can change the API call in some i.MX driver
instead of getting irq_count first. Do you think I should add the nowarn API and redo
this patch to call it? 

Anson

Re: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Uwe Kleine-König]

On Wed, Oct 09, 2019 at 07:24:57AM +0000, Anson Huang wrote:
> > On Wed, Oct 09, 2019 at 06:58:24AM +0000, Anson Huang wrote:
> > > > The patch is fine given the changed behaviour of platform_get_irq. I
> > > > wonder if it is sensible to introduce a variant of platform_get_irq (say
> > > > platform_get_irq_nowarn) that behaves like __platform_get_irq does t
> > > > Then the imx driver would just call platform_get_irq_nowarn without
> > > > having to check the number of available irqs first.
> > >
> > > Agreed, it would be nice if we can fix this from the API level, this
> > > is to save many patches from various drivers side, let me know if
> > > agreement is reached and I will do the patch.
> > 
> > I wouldn't expect that most callers actually want an error message and so
> > these need a different patch (i.e. dropping the error message by the caller).
> > This type of patch is fine and the normal load when something is
> > consolidated.
> > 
> > Which other drivers do you have on your radar that don't want an error
> > message if platform_get_irq() fails?
> 
> I did NOT mean drivers don't want an error when getting irq failed, but I just
> agree that introducing another API with nowarn as you mentioned upper, then
> i.MX driver can call it. For now, the FEC driver also have many such error message,
> we will fix later.
> 
> So if the API with nowarn is added, then I can change the API call in some i.MX driver
> instead of getting irq_count first. Do you think I should add the nowarn API and redo
> this patch to call it? 

Having a patch (or a set of patches) is probably helpful to get forward
here, yes. You have my blessing to create a suggestion. (Not that you
actually need that :-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

RE: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Anson Huang]

Hi, Uwe

> On Wed, Oct 09, 2019 at 07:24:57AM +0000, Anson Huang wrote:
> > > On Wed, Oct 09, 2019 at 06:58:24AM +0000, Anson Huang wrote:
> > > > > The patch is fine given the changed behaviour of
> > > > > platform_get_irq. I wonder if it is sensible to introduce a
> > > > > variant of platform_get_irq (say
> > > > > platform_get_irq_nowarn) that behaves like __platform_get_irq
> > > > > does t Then the imx driver would just call
> > > > > platform_get_irq_nowarn without having to check the number of
> available irqs first.
> > > >
> > > > Agreed, it would be nice if we can fix this from the API level,
> > > > this is to save many patches from various drivers side, let me
> > > > know if agreement is reached and I will do the patch.
> > >
> > > I wouldn't expect that most callers actually want an error message
> > > and so these need a different patch (i.e. dropping the error message by
> the caller).
> > > This type of patch is fine and the normal load when something is
> > > consolidated.
> > >
> > > Which other drivers do you have on your radar that don't want an
> > > error message if platform_get_irq() fails?
> >
> > I did NOT mean drivers don't want an error when getting irq failed,
> > but I just agree that introducing another API with nowarn as you
> > mentioned upper, then i.MX driver can call it. For now, the FEC driver
> > also have many such error message, we will fix later.
> >
> > So if the API with nowarn is added, then I can change the API call in
> > some i.MX driver instead of getting irq_count first. Do you think I
> > should add the nowarn API and redo this patch to call it?
> 
> Having a patch (or a set of patches) is probably helpful to get forward here,
> yes. You have my blessing to create a suggestion. (Not that you actually need
> that :-)

Thanks, OK, then I will leave this patch as it is for now, and leave others to decide whether
to add a patch of adding new API of nowarn. Some drivers need to be changed anyway to avoid
this error message, either getting irq count first or calling new API once it is added.

Thanks,
Anson

Re: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Andy Shevchenko]

On Wed, Oct 9, 2019 at 9:53 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> The patch is fine given the changed behaviour of platform_get_irq. I
> wonder if it is sensible to introduce a variant of platform_get_irq (say
> platform_get_irq_nowarn) that behaves like __platform_get_irq does
> today. Then the imx driver would just call platform_get_irq_nowarn
> without having to check the number of available irqs first.

It's being discussed in parallel thread about
platform_get_irq_optional() which won't issue a warning.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Uwe Kleine-König]

On Wed, Oct 09, 2019 at 11:12:20AM +0300, Andy Shevchenko wrote:
> On Wed, Oct 9, 2019 at 9:53 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > The patch is fine given the changed behaviour of platform_get_irq. I
> > wonder if it is sensible to introduce a variant of platform_get_irq (say
> > platform_get_irq_nowarn) that behaves like __platform_get_irq does
> > today. Then the imx driver would just call platform_get_irq_nowarn
> > without having to check the number of available irqs first.
> 
> It's being discussed in parallel thread about
> platform_get_irq_optional() which won't issue a warning.

This is even already in 5.4-rc1 as
8973ea47901c81a1912bd05f1577bed9b5b52506.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

RE: [PATCH] tty: serial: imx: Only get second/third IRQ when there is more than one IRQ

[Anson Huang]

> On Wed, Oct 09, 2019 at 11:12:20AM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 9, 2019 at 9:53 AM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> >
> > > The patch is fine given the changed behaviour of platform_get_irq. I
> > > wonder if it is sensible to introduce a variant of platform_get_irq
> > > (say
> > > platform_get_irq_nowarn) that behaves like __platform_get_irq does
> > > today. Then the imx driver would just call platform_get_irq_nowarn
> > > without having to check the number of available irqs first.
> >
> > It's being discussed in parallel thread about
> > platform_get_irq_optional() which won't issue a warning.
> 
> This is even already in 5.4-rc1 as
> 8973ea47901c81a1912bd05f1577bed9b5b52506.

Great, I will send out a V2 using platform_get_irq_optional() for second/third IRQ.

Thanks,
Anson