Messages in this thread | | | Subject | RE: [PATCH 1/5] bfa: Brocade BFA FC SCSI driver (bfad) | Date | Sat, 21 Mar 2009 16:03:03 -0700 | From | "Jing Huang" <> |
| |
> some quick general comments: > > - Patch needs to be run through checkpatch.pl again. > - If you are not implementing a callout like issue_lip then you do not > need to implement a function that does nothing. > - Remove the OS compat stuff that just calls right into the linux > function. For example kill bfad_os_pci_probe. >
We will fix all he checkpatch issues and make necessary cleanups and resubmit soon.
> > + > > + im_port->shost->hostdata[0] = (unsigned long)im_port; > > > > I think I asked about this before. Why do you just not allocate the bfad > or im_port in the hostdata? What is the struct hierarchy? Do you have > one bfad for the entire HBA/pci_device then have a im_port for each port > on the hba? >
Thanks for review the code again. Yes, we have one bfad for each HBA port/pci device, and one im_port (im stands for initiator mode) for each scsi_host/vport. We can certainly allocate im_port in scsi_host_alloc() as you suggest, but currently we allocate all possible fc4 port (initiator/tgt/ipfc) in one function, and it is just more convenient to do it in one place.
Thanks
Jing
| |