Linux PARISC architecture development
 help / color / mirror / Atom feed
* [parisc-linux] Patch for dino serial port on B-class workstations
@ 2001-05-31 15:53 Clement MOYROUD
  2001-06-03  7:21 ` Grant Grundler
  0 siblings, 1 reply; 4+ messages in thread
From: Clement MOYROUD @ 2001-05-31 15:53 UTC (permalink / raw)
  To: parisc-linux

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

Hi all !

I have made a patch for dino. It's a bit ugly, but before going on with 
a rewrite of the dino driver, I would like to have some feedback. So 
feel free to apply it on your kernel tree and give me some remarks about it.

Thanks,

Clement
ESIEE Team
mkhppa1.esiee.fr

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2902 bytes --]

diff -Nru linux.old/drivers/gsc/dino.c linux/drivers/gsc/dino.c
--- linux.old/drivers/gsc/dino.c	Thu May 31 16:56:20 2001
+++ linux/drivers/gsc/dino.c	Thu May 31 16:55:07 2001
@@ -66,6 +66,7 @@
 #include <asm/irq.h>		/* for "gsc" irq functions */
 #include <asm/gsc.h>
 
+#include "busdevice.h"
 
 #undef DINO_DEBUG
 
@@ -542,6 +543,14 @@
 	}
 }
 
+/* Here is where the dino's serial port gets its irq on B-class workstations */
+
+static int
+dino_find_irq(struct busdevice *dino_dev, struct hp_device *dev)
+{
+	return 10;
+}
+
 static void __init
 dino_bios_init(void)
 {
@@ -804,18 +813,33 @@
 }
 
 static int __init
-dino_common_init(struct dino_device *dino_dev)
+dino_common_init(struct hp_device *d, struct dino_device *dino_dev)
 {
 	int status;
 	u32 eim;
 	struct gsc_irq gsc_irq;
 	struct resource *res;
 
+	struct busdevice *dino;
+	int ret;
+
 	pcibios_register_hba((struct pci_hba_data *) dino_dev);
 
 	pci_bios = &dino_bios_ops;   /* used by pci_scan_bus() */
 	pci_port = &dino_port_ops;
 
+
+        /* Needed for the serial port to work. Quite ugly for now */
+
+	dino = kmalloc(sizeof(struct busdevice), GFP_KERNEL);
+	if(!dino)
+		return -ENOMEM;
+	
+	dino->name = "Dino";
+	dino->hpa = d->hpa;
+	dino->find_irq = dino_find_irq;
+	
+
 	/*
 	** Note: SMP systems can make use of IRR1/IAR1 registers
 	**   But it won't buy much performance except in very
@@ -859,6 +883,20 @@
 		return(1);
 	}
 
+	/* Register busdevice for the serial port */
+
+	dino->parent_irq = gsc_irq.irq;
+	dino->eim = ((u32) gsc_irq.txn_addr) | gsc_irq.txn_data;
+
+	ret = register_busdevice(d,dino);
+	if (ret) {
+	    kfree(dino);
+	    return ret;
+	}
+
+	dino->busdev_region = dino_dev->dino_region;
+	
+
 	/*
 	** This enables DINO to generate interrupts when it sees
 	** any of it's inputs *change*. Just asserting an IRQ
@@ -945,7 +983,7 @@
 		dino_bridge_init(dino_dev);
 	}
 
-	if (dino_common_init(dino_dev))
+	if (dino_common_init(d,dino_dev))
 		return(1);
 
 	/*
diff -Nru linux.old/drivers/gsc/serial.c linux/drivers/gsc/serial.c
--- linux.old/drivers/gsc/serial.c	Thu May 31 16:56:14 2001
+++ linux/drivers/gsc/serial.c	Thu May 31 16:54:57 2001
@@ -35,6 +35,7 @@
 
 #include "busdevice.h"
 
+
 static int serial_line_nr;
 
 static int __init 
@@ -87,9 +89,12 @@
 	return 0;
 }
 
-
-static struct pa_iodc_driver serial_drivers_for[] = {
-  {HPHW_FIO, 0x05F, 0x0, 0x00081, 0x0, 0,		/* A-class 180 */
+static struct pa_iodc_driver serial_drivers_for[] = {  
+   {HPHW_FIO, 0x022, 0x0, 0x0008C, 0x0, 0,		/* B-Class 132 & 180 */
+	DRIVER_CHECK_HVERSION + DRIVER_CHECK_HVERSION_REV + 
+	DRIVER_CHECK_SVERSION + DRIVER_CHECK_HWTYPE,
+	"serial device", "B-132 L+", serial_init_chip},
+   {HPHW_FIO, 0x05F, 0x0, 0x00081, 0x0, 0,		/* A-class 180 */
 	DRIVER_CHECK_HVERSION + DRIVER_CHECK_HVERSION_REV + 
 	DRIVER_CHECK_SVERSION + DRIVER_CHECK_HWTYPE,
 	"serial device", "unknown", serial_init_chip},

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [parisc-linux] Patch for dino serial port on B-class workstations
  2001-05-31 15:53 [parisc-linux] Patch for dino serial port on B-class workstations Clement MOYROUD
@ 2001-06-03  7:21 ` Grant Grundler
  2001-06-03 19:20   ` Clément MOYROUD
  0 siblings, 1 reply; 4+ messages in thread
From: Grant Grundler @ 2001-06-03  7:21 UTC (permalink / raw)
  To: Clement MOYROUD; +Cc: parisc-linux

Clement MOYROUD wrote:
> Hi all !
> 
> I have made a patch for dino. It's a bit ugly, but before going on with 
> a rewrite of the dino driver, I would like to have some feedback.

Clement! This is great!

Could you explain why you think yuo need to rewrite the dino driver?
This is basically how I expected it to work.

>  So 
> feel free to apply it on your kernel tree and give me some remarks about it.

I've neither applied nor tested it.

> diff -Nru linux.old/drivers/gsc/dino.c linux/drivers/gsc/dino.c
> --- linux.old/drivers/gsc/dino.c	Thu May 31 16:56:20 2001
> +++ linux/drivers/gsc/dino.c	Thu May 31 16:55:07 2001
> @@ -66,6 +66,7 @@
>  #include <asm/irq.h>		/* for "gsc" irq functions */
>  #include <asm/gsc.h>
>  
> +#include "busdevice.h"
>  
>  #undef DINO_DEBUG
>  
> @@ -542,6 +543,14 @@
>  	}
>  }
>  
> +/* Here is where the dino's serial port gets its irq on B-class workstations
>    */
> +
> +static int
> +dino_find_irq(struct busdevice *dino_dev, struct hp_device *dev)
> +{

Some upper portion of the address bits are already validated
but the code in bus_device.c.  Need to valid some of the lower
address bits here.

Want to make sure it's really the serial device that we want
to talk to and not the PS/2 port or "fire extinguisher" (only
used on 743 or Hitachi box, I forgot).


> +	return 10;
> +}
> +
>  static void __init
>  dino_bios_init(void)
>  {
> @@ -804,18 +813,33 @@
>  }
>  
>  static int __init
> -dino_common_init(struct dino_device *dino_dev)
> +dino_common_init(struct hp_device *d, struct dino_device *dino_dev)
>  {
>  	int status;
>  	u32 eim;
>  	struct gsc_irq gsc_irq;
>  	struct resource *res;
>  
> +	struct busdevice *dino;

Please call this dino_busdev or something like that to differentiate
it from the other "dino" data structures.

> +	int ret;
> +
>  	pcibios_register_hba((struct pci_hba_data *) dino_dev);
>  
>  	pci_bios = &dino_bios_ops;   /* used by pci_scan_bus() */
>  	pci_port = &dino_port_ops;
>  
> +
> +        /* Needed for the serial port to work. Quite ugly for now */
> +
> +	dino = kmalloc(sizeof(struct busdevice), GFP_KERNEL);
> +	if(!dino)
> +		return -ENOMEM;
> +	
> +	dino->name = "Dino";
> +	dino->hpa = d->hpa;
> +	dino->find_irq = dino_find_irq;

I don't think this in an ugly hack.
Overall, this is how I expected it to work.
Might not even need to fill in the rest of the functions
in the bus_device jump table.

> +	
> +
>  	/*
>  	** Note: SMP systems can make use of IRR1/IAR1 registers
>  	**   But it won't buy much performance except in very

offhand, the rest looks ok.

grant

Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [parisc-linux] Patch for dino serial port on B-class workstations
  2001-06-03  7:21 ` Grant Grundler
@ 2001-06-03 19:20   ` Clément MOYROUD
  2001-06-03 21:19     ` Grant Grundler
  0 siblings, 1 reply; 4+ messages in thread
From: Clément MOYROUD @ 2001-06-03 19:20 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux

> Clement MOYROUD wrote:
> > Hi all !
> >
> > I have made a patch for dino. It's a bit ugly, but before going on with
> > a rewrite of the dino driver, I would like to have some feedback.
>
> Clement! This is great!
>
> Could you explain why you think yuo need to rewrite the dino driver?
> This is basically how I expected it to work.
>

I think (as well as the other members of the ESIEE team) that the dino
driver has not been written in order to "comply" with some sort of a GSC
driver standard - i.e. it could make use of the register_busdevice instead
of requiring it's own irq region (e.g. lasi). I have started to do this, but
it would require some modifications of the busdevice structure to add some
support for the pci-specific part of dino. With this modification of the
busdevice struct (and some other functions), I would be much more clean.....
But if you think it's clear enough, I've got other things to do :)

> >  So
> > feel free to apply it on your kernel tree and give me some remarks about
it.
>
> I've neither applied nor tested it.
>
> > diff -Nru linux.old/drivers/gsc/dino.c linux/drivers/gsc/dino.c
> > --- linux.old/drivers/gsc/dino.c Thu May 31 16:56:20 2001
> > +++ linux/drivers/gsc/dino.c Thu May 31 16:55:07 2001
> > @@ -66,6 +66,7 @@
> >  #include <asm/irq.h> /* for "gsc" irq functions */
> >  #include <asm/gsc.h>
> >
> > +#include "busdevice.h"
> >
> >  #undef DINO_DEBUG
> >
> > @@ -542,6 +543,14 @@
> >  }
> >  }
> >
> > +/* Here is where the dino's serial port gets its irq on B-class
workstations
> >    */
> > +
> > +static int
> > +dino_find_irq(struct busdevice *dino_dev, struct hp_device *dev)
> > +{
>
> Some upper portion of the address bits are already validated
> but the code in bus_device.c.  Need to valid some of the lower
> address bits here.
>
> Want to make sure it's really the serial device that we want
> to talk to and not the PS/2 port or "fire extinguisher" (only
> used on 743 or Hitachi box, I forgot).
>
>

As I wrote, it's something that works on a b132/180, and I didn't check in
the hardware database to see if it is in use in other workstations. But I
know I'll have to make some tests to find which irq return....

> > + return 10;
> > +}
> > +
> >  static void __init
> >  dino_bios_init(void)
> >  {
> > @@ -804,18 +813,33 @@
> >  }
> >
> >  static int __init
> > -dino_common_init(struct dino_device *dino_dev)
> > +dino_common_init(struct hp_device *d, struct dino_device *dino_dev)
> >  {
> >  int status;
> >  u32 eim;
> >  struct gsc_irq gsc_irq;
> >  struct resource *res;
> >
> > + struct busdevice *dino;
>
> Please call this dino_busdev or something like that to differentiate
> it from the other "dino" data structures.
>

I agree it might not be that clear. In fact, my future plans are to use only
one struct (bye-bye  struct dino_device !), and I didn't take care of the
clarity of this....

> > + int ret;
> > +
> >  pcibios_register_hba((struct pci_hba_data *) dino_dev);
> >
> >  pci_bios = &dino_bios_ops;   /* used by pci_scan_bus() */
> >  pci_port = &dino_port_ops;
> >
> > +
> > +        /* Needed for the serial port to work. Quite ugly for now */
> > +
> > + dino = kmalloc(sizeof(struct busdevice), GFP_KERNEL);
> > + if(!dino)
> > + return -ENOMEM;
> > +
> > + dino->name = "Dino";
> > + dino->hpa = d->hpa;
> > + dino->find_irq = dino_find_irq;
>
> I don't think this in an ugly hack.
> Overall, this is how I expected it to work.
> Might not even need to fill in the rest of the functions
> in the bus_device jump table.
>

As I wrote before, it looks ugly to me because I think the driver would look
much better if dino.c was rewritten. But again, if you don't feel like it
is, I won't complain !

> > +
> > +
> >  /*
> >  ** Note: SMP systems can make use of IRR1/IAR1 registers
> >  **   But it won't buy much performance except in very
>
> offhand, the rest looks ok.
>

ok

> grant
>

So, are you happy with the actual look of the driver or do you think it is
worth rewriting it ?

Clement MOYROUD
Puffin ESIEE Team

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [parisc-linux] Patch for dino serial port on B-class workstations
  2001-06-03 19:20   ` Clément MOYROUD
@ 2001-06-03 21:19     ` Grant Grundler
  0 siblings, 0 replies; 4+ messages in thread
From: Grant Grundler @ 2001-06-03 21:19 UTC (permalink / raw)
  To: Clément MOYROUD; +Cc: parisc-linux

Clement MOYROUD wrote:
...
> I think (as well as the other members of the ESIEE team) that the dino
> driver has not been written in order to "comply" with some sort of a GSC
> driver standard - i.e. it could make use of the register_busdevice instead
> of requiring it's own irq region (e.g. lasi).

You are right. IIRC, bus_devices didn't exist when I wrote the *second*
version of dino support. Technically, Alan Cox wrote the first one.

> I have started to do this, but
> it would require some modifications of the busdevice structure to add some
> support for the pci-specific part of dino. With this modification of the
> busdevice struct (and some other functions), I would be much more clean.....

hmmm...ok.

> But if you think it's clear enough, I've got other things to do :)

*I* think it's ok. more on this below.


...
> As I wrote, it's something that works on a b132/180, and I didn't check in
> the hardware database to see if it is in use in other workstations. But I
> know I'll have to make some tests to find which irq return....

No you won't. THe IRQ is hardwired in dino. It's in the ERS someplace.
You just need to verify the address for that device really
belongs to the RS232 port and isn't the PS/2 port.

...
> I agree it might not be that clear. In fact, my future plans are to use only
> one struct (bye-bye  struct dino_device !), and I didn't take care of the
> clarity of this....

ah ok. If dino_device is that similar to bus_device then the direction you
are taking makes sense.

...
> As I wrote before, it looks ugly to me because I think the driver would look
> much better if dino.c was rewritten. But again, if you don't feel like it
> is, I won't complain !

If you (or the team) feel strongly about rewriting dino, then I'd like
to encourage you to do it.  Just don't break what currently works.

...
> So, are you happy with the actual look of the driver or do you think it is
> worth rewriting it ?

PCI suport mostly works. That's what mattered to me before.
Rewriting dino.c to use bus_device instead of dino_device is probably
the "right thing" (tm) since some people will want PS/2 and RS232 to work.
I don't care to test/debug problems with dino code anymore though. I don't
mind helping someone do it.  In other words, whoever wants to rewrite dino.c
welcome to and I'll help some when needed.

> Clement MOYROUD
> Puffin ESIEE Team

grant

Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2001-06-03 21:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-05-31 15:53 [parisc-linux] Patch for dino serial port on B-class workstations Clement MOYROUD
2001-06-03  7:21 ` Grant Grundler
2001-06-03 19:20   ` Clément MOYROUD
2001-06-03 21:19     ` Grant Grundler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox