Some feedback on this commit... I've not tested it at all, just read the patch.
On 08/28/2011 10:36 AM, Peter C. Wallace wrote: > Add support for Mesa 5i21, 5i25 and 4i69 FPGA cards. > > Signed-off-by: Peter C. Wallace<p...@mesanet.com> > > > http://git.linuxcnc.org/?p=emc2.git;a=commitdiff;h=9c81c5b > > --- > src/hal/drivers/mesa-hostmot2/hm2_pci.c | 130 > +++++++++++++++++++++++++----- > src/hal/drivers/mesa-hostmot2/hm2_pci.h | 14 +++- > src/hal/drivers/mesa-hostmot2/hostmot2.c | 11 +--- > src/hal/drivers/mesa-hostmot2/pins.c | 32 +++++++- > 4 files changed, 151 insertions(+), 36 deletions(-) > > diff --git a/src/hal/drivers/mesa-hostmot2/hm2_pci.c > b/src/hal/drivers/mesa-hostmot2/hm2_pci.c > index 1a4f046..9ea6de1 100644 > --- a/src/hal/drivers/mesa-hostmot2/hm2_pci.c > +++ b/src/hal/drivers/mesa-hostmot2/hm2_pci.c ... > @@ -387,6 +422,17 @@ static int hm2_pci_probe(struct pci_dev *dev, const > struct pci_device_id *id) { > break; > } > > + case HM2_PCI_SSDEV_5I21: { > + LL_PRINT("discovered 5i21 at %s\n", pci_name(dev)); > + rtapi_snprintf(board->llio.name, sizeof(board->llio.name), > "hm2_5i21.%d", num_5i21); > + num_5i21 ++; > + board->llio.num_ioport_connectors = 2; > + board->llio.ioport_connector_name[0] = "P1"; > + board->llio.ioport_connector_name[1] = "P1"; > + board->llio.fpga_part_number = "3s400pq208"; > + break; > + } > + Whoops, the 5i21 has two IO Connectors named P1, and the 5i25 has no connectors. > diff --git a/src/hal/drivers/mesa-hostmot2/pins.c > b/src/hal/drivers/mesa-hostmot2/pins.c > index 4e512f7..71f899b 100644 > --- a/src/hal/drivers/mesa-hostmot2/pins.c > +++ b/src/hal/drivers/mesa-hostmot2/pins.c > @@ -290,13 +290,39 @@ void hm2_set_pin_direction(hostmot2_t *hm2, int > pin_number, int direction) { > > void hm2_print_pin_usage(hostmot2_t *hm2) { > int i; > + int port, port_pin, mio; > > HM2_PRINT("%d I/O Pins used:\n", hm2->num_pins); > > for (i = 0; i< hm2->num_pins; i ++) { > - int port = i / hm2->idrom.port_width; > - int port_pin = ((i % 24) * 2) + 1; > - > + port_pin = i + 1; > + port = i / hm2->idrom.port_width; > + switch (hm2->idrom.port_width) { > + case 24: /* standard 50 pin 24 I/O cards, just the odd pins */ > + port_pin = ((i % hm2->idrom.port_width) * 2) + 1; > + break; > + case 17: /* 25 pin 17 I/O parallel port type cards funny DB25 > order */ > + mio = i % hm2->idrom.port_width; > + if (mio> 7){ > + port_pin = mio-3; > + } > + else { > + if (mio& 1){ > + port_pin = (mio/2)+14; > + } > + else { > + port_pin = (mio/2)+1; > + } > + } > + break; > + case 32: /* 5I21 punt on this for now */ > + port_pin = i+1; > + break; > + default: > + HM2_ERR("hm2_print_pin_usage: invalid port width %d\n", > hm2->idrom.port_width); > + } > + > + > if (hm2->pin[i].gtag == hm2->pin[i].sec_tag) { > if(hm2->pin[i].sec_unit& 0x80) > HM2_PRINT( > There are a couple of other places where the pin number is converted to a port number and a pin-within-that-port number. hm2_set_pin_source() and hm2_set_pin_direction() come to mind. The code you added to hm2_print_pin_usage() seems to do it right, maybe it'd make sense to annotate the pins with their port and pin-within-port info at init time? Add it to the hm2_pin_t maybe? Also, this commit broke the unit tests. One of the hm2-idrom test cases verifies that IOWidth is 24, and that's obviously no longer the case. You removed that requirement in hm2_read_idrom() and hm2_print_idrom(). Maybe the llio should tell us what it's expected IOWidth is? It knows how big its connectors are, after all, and it seems like a good sanity check. -- Sebastian Kuzminsky ------------------------------------------------------------------------------ EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev _______________________________________________ Emc-developers mailing list Emc-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/emc-developers
