serial: sc16is7xx: Extend IRQ check for negative valus

[PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Andre Werner 2 weeks, 6 days ago

Fix the IRQ check to treat the negative values as No IRQ.

Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
---
 drivers/tty/serial/sc16is7xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 7b51cdc274fd..560f45ed19ae 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1561,7 +1561,7 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 	/* Always ask for fixed clock rate from a property. */
 	device_property_read_u32(dev, "clock-frequency", &uartclk);
 
-	s->polling = !!irq;
+	s->polling = (irq <= 0);
 	if (s->polling)
 		dev_dbg(dev,
 			"No interrupt pin definition, falling back to polling mode\n");
-- 
2.48.0

Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Jiri Slaby 2 weeks, 6 days ago

This is threaded weirdly.

On 16. 01. 25, 10:32, Andre Werner wrote:
> Fix the IRQ check to treat the negative values as No IRQ.

Care to describe on what HW that can happen?

> Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
> ---
>   drivers/tty/serial/sc16is7xx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 7b51cdc274fd..560f45ed19ae 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1561,7 +1561,7 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>   	/* Always ask for fixed clock rate from a property. */
>   	device_property_read_u32(dev, "clock-frequency", &uartclk);
>   
> -	s->polling = !!irq;
> +	s->polling = (irq <= 0);
>   	if (s->polling)
>   		dev_dbg(dev,
>   			"No interrupt pin definition, falling back to polling mode\n");


-- 
js
suse labs

Re: [External Email] Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Andre Werner 2 weeks, 6 days ago

On Thu, 16 Jan 2025, Jiri Slaby wrote:

> This is threaded weirdly.

Sorry for that.

>
> On 16. 01. 25, 10:32, Andre Werner wrote:
> > Fix the IRQ check to treat the negative values as No IRQ.
>
> Care to describe on what HW that can happen?

I have no example on that. In the previous thread it was mentioned that
it is not absolutely sure that the API is not called with negative values. Thus,
negative values shall also be treated as no IRQ.

>
> > Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
> > ---
> >   drivers/tty/serial/sc16is7xx.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> > index 7b51cdc274fd..560f45ed19ae 100644
> > --- a/drivers/tty/serial/sc16is7xx.c
> > +++ b/drivers/tty/serial/sc16is7xx.c
> > @@ -1561,7 +1561,7 @@ int sc16is7xx_probe(struct device *dev, const struct
> > sc16is7xx_devtype *devtype,
> >    /* Always ask for fixed clock rate from a property. */
> >    device_property_read_u32(dev, "clock-frequency", &uartclk);
> >   -	s->polling = !!irq;
> > +	s->polling = (irq <= 0);
> >    if (s->polling)
> >     dev_dbg(dev,
> >      "No interrupt pin definition, falling back to polling mode\n");
>
>
> --
> js
> suse labs
>
>

Regards,

André

Re: [External Email] Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Jiri Slaby 2 weeks, 6 days ago

On 16. 01. 25, 10:52, Andre Werner wrote:
> On Thu, 16 Jan 2025, Jiri Slaby wrote:
> 
>> This is threaded weirdly.
> 
> Sorry for that.
> 
>>
>> On 16. 01. 25, 10:32, Andre Werner wrote:
>>> Fix the IRQ check to treat the negative values as No IRQ.
>>
>> Care to describe on what HW that can happen?
> 
> I have no example on that. In the previous thread it was mentioned that
> it is not absolutely sure that the API is not called with negative values. Thus,
> negative values shall also be treated as no IRQ.

SPI:
         if (spi->irq < 0)
                 spi->irq = 0;

I2C:
                 if (irq < 0)
                         irq = 0;

So unlikely :).

>>> Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
>>> ---
>>>    drivers/tty/serial/sc16is7xx.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
>>> index 7b51cdc274fd..560f45ed19ae 100644
>>> --- a/drivers/tty/serial/sc16is7xx.c
>>> +++ b/drivers/tty/serial/sc16is7xx.c
>>> @@ -1561,7 +1561,7 @@ int sc16is7xx_probe(struct device *dev, const struct
>>> sc16is7xx_devtype *devtype,
>>>     /* Always ask for fixed clock rate from a property. */
>>>     device_property_read_u32(dev, "clock-frequency", &uartclk);
>>>    -	s->polling = !!irq;
>>> +	s->polling = (irq <= 0);
>>>     if (s->polling)
>>>      dev_dbg(dev,
>>>       "No interrupt pin definition, falling back to polling mode\n");
>>
>>
>> --
>> js
>> suse labs
>>
>>
> 
> Regards,
> 
> André


-- 
js
suse labs

Re: [External Email] Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Andy Shevchenko 2 weeks, 6 days ago

On Thu, Jan 16, 2025 at 11:01:30AM +0100, Jiri Slaby wrote:
> On 16. 01. 25, 10:52, Andre Werner wrote:
> > On Thu, 16 Jan 2025, Jiri Slaby wrote:
> > > On 16. 01. 25, 10:32, Andre Werner wrote:
> > > > Fix the IRQ check to treat the negative values as No IRQ.
> > > 
> > > Care to describe on what HW that can happen?
> > 
> > I have no example on that. In the previous thread it was mentioned that
> > it is not absolutely sure that the API is not called with negative values. Thus,
> > negative values shall also be treated as no IRQ.
> 
> SPI:
>         if (spi->irq < 0)
>                 spi->irq = 0;

In all cases of how SPI target device is instantiated? I haven't checked myself
all those (three or four? or even five?) ways.

> I2C:
>                 if (irq < 0)
>                         irq = 0;

> So unlikely :).

Yes, but none of them _guarantees_ that (looking at the documentation and
the field type in the respective data structures).

Until there no guarantees, I would consider this patch as robustness increase.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Andy Shevchenko 2 weeks, 6 days ago

On Thu, Jan 16, 2025 at 10:42:14AM +0100, Jiri Slaby wrote:
> This is threaded weirdly.

Yeah, new patch (version) — new email thread.

> On 16. 01. 25, 10:32, Andre Werner wrote:
> > Fix the IRQ check to treat the negative values as No IRQ.
> 
> Care to describe on what HW that can happen?

But how does this relevant? The whole idea is that neither I²C nor SPI
frameworks do not guarantee the IRQ field never be negative. This is
the fix to the previously submitted patch.

...

> > -	s->polling = !!irq;
> > +	s->polling = (irq <= 0);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Jiri Slaby 2 weeks, 6 days ago

On 16. 01. 25, 10:53, Andy Shevchenko wrote:
> On Thu, Jan 16, 2025 at 10:42:14AM +0100, Jiri Slaby wrote:
>> This is threaded weirdly.
> 
> Yeah, new patch (version) — new email thread.
> 
>> On 16. 01. 25, 10:32, Andre Werner wrote:
>>> Fix the IRQ check to treat the negative values as No IRQ.
>>
>> Care to describe on what HW that can happen?
> 
> But how does this relevant? The whole idea is that neither I²C nor SPI
> frameworks do not guarantee the IRQ field never be negative. This is
> the fix to the previously submitted patch.

They do AFAICS. Could you be more specific?

-- 
js
suse labs

Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Andy Shevchenko 2 weeks, 5 days ago

On Thu, Jan 16, 2025 at 12:02 PM Jiri Slaby <jirislaby@kernel.org> wrote:
> On 16. 01. 25, 10:53, Andy Shevchenko wrote:
> > On Thu, Jan 16, 2025 at 10:42:14AM +0100, Jiri Slaby wrote:
> >> This is threaded weirdly.
> >
> > Yeah, new patch (version) — new email thread.
> >
> >> On 16. 01. 25, 10:32, Andre Werner wrote:
> >>> Fix the IRQ check to treat the negative values as No IRQ.
> >>
> >> Care to describe on what HW that can happen?
> >
> > But how does this relevant? The whole idea is that neither I²C nor SPI
> > frameworks do not guarantee the IRQ field never be negative. This is
> > the fix to the previously submitted patch.
>
> They do AFAICS. Could you be more specific?

Hmm... I have looked into the SPI core and this behaviour seems quite
new. Documentation still says the same as before where it's not
guaranteed.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] serial: sc16is7xx: Extend IRQ check for negative valus

Posted by Andy Shevchenko 2 weeks, 6 days ago

On Thu, Jan 16, 2025 at 11:02:23AM +0100, Jiri Slaby wrote:
> On 16. 01. 25, 10:53, Andy Shevchenko wrote:
> > On Thu, Jan 16, 2025 at 10:42:14AM +0100, Jiri Slaby wrote:
> > > This is threaded weirdly.
> > 
> > Yeah, new patch (version) — new email thread.
> > 
> > > On 16. 01. 25, 10:32, Andre Werner wrote:
> > > > Fix the IRQ check to treat the negative values as No IRQ.
> > > 
> > > Care to describe on what HW that can happen?
> > 
> > But how does this relevant? The whole idea is that neither I²C nor SPI
> > frameworks do not guarantee the IRQ field never be negative. This is
> > the fix to the previously submitted patch.
> 
> They do AFAICS.

They don't AFAICT.

> Could you be more specific?

The documentation of the fields and their types suggests my way of perception.

-- 
With Best Regards,
Andy Shevchenko