b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Russell King]

The above commit contains the following patch:

| --- a/drivers/serial/8250.c
| +++ b/drivers/serial/8250.c
| @@ -43,6 +43,7 @@
| 
|  #include <asm/io.h>
|  #include <asm/irq.h>
| +#include <asm/serial.h>
| 
|  #include "8250.h"
| 
| @@ -92,8 +93,6 @@ static unsigned int nr_uarts = CONFIG_SERIAL_8250_RUNTIME_UARTS;
|   */
|  #define CONFIG_HUB6 1
| 
| -#include <asm/serial.h>
| -
|  /*
|   * SERIAL_PORT_DFNS tells us about built-in ports that have no
|   * standard enumeration mechanism.   Platforms that can find all

The code between these two hunks contains the following:

| #ifdef CONFIG_SERIAL_8250_DETECT_IRQ
| #define CONFIG_SERIAL_DETECT_IRQ 1
| #endif
| #ifdef CONFIG_SERIAL_8250_MANY_PORTS
| #define CONFIG_SERIAL_MANY_PORTS 1
| #endif

and asm-*/serial.h contains:

| $ grep 'CONFIG_SERIAL_DETECT\|CONFIG_SERIAL_MANY' include/asm-*/serial.h
| include/asm-alpha/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
| include/asm-m68k/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
| include/asm-mn10300/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
| include/asm-mn10300/serial.h:#ifdef CONFIG_SERIAL_MANY_PORTS
| include/asm-x86/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ

So, all these ifdefs are now useless.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Javier Herrero]

Does the problem arise due to the change of inclusion order of 
asm/serial.h (from after 8250.h to before 8250.h) ?

Russell King escribió:
> The above commit contains the following patch:
>
> | --- a/drivers/serial/8250.c
> | +++ b/drivers/serial/8250.c
> | @@ -43,6 +43,7 @@
> | 
> |  #include <asm/io.h>
> |  #include <asm/irq.h>
> | +#include <asm/serial.h>
> | 
> |  #include "8250.h"
> | 
> | @@ -92,8 +93,6 @@ static unsigned int nr_uarts = CONFIG_SERIAL_8250_RUNTIME_UARTS;
> |   */
> |  #define CONFIG_HUB6 1
> | 
> | -#include <asm/serial.h>
> | -
> |  /*
> |   * SERIAL_PORT_DFNS tells us about built-in ports that have no
> |   * standard enumeration mechanism.   Platforms that can find all
>
> The code between these two hunks contains the following:
>
> | #ifdef CONFIG_SERIAL_8250_DETECT_IRQ
> | #define CONFIG_SERIAL_DETECT_IRQ 1
> | #endif
> | #ifdef CONFIG_SERIAL_8250_MANY_PORTS
> | #define CONFIG_SERIAL_MANY_PORTS 1
> | #endif
>
> and asm-*/serial.h contains:
>
> | $ grep 'CONFIG_SERIAL_DETECT\|CONFIG_SERIAL_MANY' include/asm-*/serial.h
> | include/asm-alpha/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
> | include/asm-m68k/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
> | include/asm-mn10300/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
> | include/asm-mn10300/serial.h:#ifdef CONFIG_SERIAL_MANY_PORTS
> | include/asm-x86/serial.h:#ifdef CONFIG_SERIAL_DETECT_IRQ
>
> So, all these ifdefs are now useless.
>

-- 
------------------------------------------------------------------------
Javier Herrero                            EMAIL: jherrero@hvsistemas.com
HV Sistemas S.L.                          PHONE:         +34 949 336 806
Los Charcones, 17A                        FAX:           +34 949 336 792
19170 El Casar - Guadalajara - Spain      WEB: http://www.hvsistemas.com 

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Russell King]

On Tue, May 20, 2008 at 10:07:11AM +0200, Javier Herrero wrote:
> Does the problem arise due to the change of inclusion order of 
> asm/serial.h (from after 8250.h to before 8250.h) ?

Yes.  It was placed where it was because of the dependencies of
asm/serial.h on the code between the two places.

The alternative solution is to get rid of the CONFIG_* compatibility
and update those asm/serial.h which reference the old symbols.
However, that's a larger patch.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Javier Herrero]

I see... would be OK to move the asm/serial.h include to its original 
position and to modify the asm-blackfin/serial.h in this way to avoid 
duplicate definition warnings, or would it be too ugly?:

/*
 * include/asm-blackfin/serial.h
 */

+#ifdef SERIAL_EXTRA_IRQ_FLAGS
+#undef SERIAL_EXTRA_IRQ_FLAGS
+#endif

#define SERIAL_EXTRA_IRQ_FLAGS IRQF_TRIGGER_HIGH

Regards,

Javier

Russell King escribió:
> On Tue, May 20, 2008 at 10:07:11AM +0200, Javier Herrero wrote:
>> Does the problem arise due to the change of inclusion order of 
>> asm/serial.h (from after 8250.h to before 8250.h) ?
>
> Yes.  It was placed where it was because of the dependencies of
> asm/serial.h on the code between the two places.
>
> The alternative solution is to get rid of the CONFIG_* compatibility
> and update those asm/serial.h which reference the old symbols.
> However, that's a larger patch.
>

-- 
------------------------------------------------------------------------
Javier Herrero                            EMAIL: jherrero@hvsistemas.com
HV Sistemas S.L.                          PHONE:         +34 949 336 806
Los Charcones, 17A                        FAX:           +34 949 336 792
19170 El Casar - Guadalajara - Spain      WEB: http://www.hvsistemas.com 

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Russell King]

On Tue, May 20, 2008 at 12:52:52PM +0200, Javier Herrero wrote:
> I see... would be OK to move the asm/serial.h include to its original 
> position and to modify the asm-blackfin/serial.h in this way to avoid 
> duplicate definition warnings, or would it be too ugly?:

Can blackfin systems accept PCMCIA cards?  Or PCI cards?  In which case
you probably don't want to implement this support like this.

A better solution may be to add some UPF_ flags to indicate the interrupt
polarity on a per-port basis.  Not sure I'm particularly thrilled by that
idea though, but other solutions I can think of inspire me even less.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Mike Frysinger]

On Tue, May 20, 2008 at 7:13 PM, Russell King wrote:
> On Tue, May 20, 2008 at 12:52:52PM +0200, Javier Herrero wrote:
>> I see... would be OK to move the asm/serial.h include to its original
>> position and to modify the asm-blackfin/serial.h in this way to avoid
>> duplicate definition warnings, or would it be too ugly?:
>
> Can blackfin systems accept PCMCIA cards?  Or PCI cards?  In which case
> you probably don't want to implement this support like this.

yes, you can hook PCMCIA cards up to a Blackfin proc (and we have).
we probably wont be supporting PCI since it tends to require MMU
support, and at least with all the Blackfins we support now, none have
PCI support on-chip.
-mike

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Javier Herrero]

Perhaps then, at least for now, in order to quickly restore the 
functionality of the driver with other platforms and to continue having 
the 8250 support in the blackfin, would be to apply this patch, that is 
a bit ugly but at least only affects to blackfin platforms using 
8250-class uarts :)

Regards,

Javier

Russell King escribió:
> On Tue, May 20, 2008 at 12:52:52PM +0200, Javier Herrero wrote:
>> I see... would be OK to move the asm/serial.h include to its original 
>> position and to modify the asm-blackfin/serial.h in this way to avoid 
>> duplicate definition warnings, or would it be too ugly?:
>
> Can blackfin systems accept PCMCIA cards?  Or PCI cards?  In which case
> you probably don't want to implement this support like this.
>
> A better solution may be to add some UPF_ flags to indicate the interrupt
> polarity on a per-port basis.  Not sure I'm particularly thrilled by that
> idea though, but other solutions I can think of inspire me even less.
>

-- 
------------------------------------------------------------------------
Javier Herrero                            EMAIL: jherrero@hvsistemas.com
HV Sistemas S.L.                          PHONE:         +34 949 336 806
Los Charcones, 17A                        FAX:           +34 949 336 792
19170 El Casar - Guadalajara - Spain      WEB: http://www.hvsistemas.com 

Re: b4aa54d951d38d7a989d6b6385494ef5ea7371d7 breaks some serial configurations

[Bryan Wu]

On Wed, May 21, 2008 at 1:45 PM, Javier Herrero <jherrero@hvsistemas.es> wrote:
> Perhaps then, at least for now, in order to quickly restore the
> functionality of the driver with other platforms and to continue having the
> 8250 support in the blackfin, would be to apply this patch, that is a bit
> ugly but at least only affects to blackfin platforms using 8250-class uarts
> :)
>

IMO, we need to revert this 8250 irq patch. Can we fix this just in
Blackfin code?
I found it was specific for our arch not for others.

-Bryan



> Regards,
>
> Javier
>
> Russell King escribió:
>>
>> On Tue, May 20, 2008 at 12:52:52PM +0200, Javier Herrero wrote:
>>>
>>> I see... would be OK to move the asm/serial.h include to its original
>>> position and to modify the asm-blackfin/serial.h in this way to avoid
>>> duplicate definition warnings, or would it be too ugly?:
>>
>> Can blackfin systems accept PCMCIA cards?  Or PCI cards?  In which case
>> you probably don't want to implement this support like this.
>>
>> A better solution may be to add some UPF_ flags to indicate the interrupt
>> polarity on a per-port basis.  Not sure I'm particularly thrilled by that
>> idea though, but other solutions I can think of inspire me even less.
>>
>
> --
> ------------------------------------------------------------------------
> Javier Herrero                            EMAIL: jherrero@hvsistemas.com
> HV Sistemas S.L.                          PHONE:         +34 949 336 806
> Los Charcones, 17A                        FAX:           +34 949 336 792
> 19170 El Casar - Guadalajara - Spain      WEB: http://www.hvsistemas.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>