Re: [PATCH 2/16] gdth: split out eisa probing

2007-10-03 Thread Rolf Eike Beer
Boaz Harrosh wrote:
 Split eisa probing into it's own helper, and do proper error unwinding.
 Protect EISA probind by the proper CONFIG_EISA symbol.

 Signed-off-by: Christoph Hellwig [EMAIL PROTECTED]

 @@ -5694,6 +5582,148 @@ static int gdth_isa_probe_one(struct
 scsi_host_template *shtp, ulong32 isa_bios) }
  #endif /* CONFIG_ISA */

 +#ifdef CONFIG_EISA
 +static int gdth_eisa_probe_one(struct scsi_host_template *shtp,
 + ushort eisa_slot)
 +{
 + struct Scsi_Host *shp;
 + gdth_ha_str *ha;
 + dma_addr_t scratch_dma_handle = 0;
 + int error, hanum, i;
 + u8 b;
 +
 + if (!gdth_search_eisa(eisa_slot))
 + return -ENXIO;
 +
 + shp = scsi_register(shtp,sizeof(gdth_ext_str));
 + if (!shp)
 + return -ENOMEM;
 + ha = HADATA(shp);
 +
 + error = -ENODEV;
 + if (!gdth_init_eisa(eisa_slot,ha))
 + goto out_host_put;
 +
 + /* controller found and initialized */
 + printk(Configuring GDT-EISA HA at Slot %d IRQ %u\n,
 + eisa_slot  12, ha-irq);
 +
 + error = request_irq(ha-irq, gdth_interrupt, IRQF_DISABLED, gdth, ha);
 + if (error) {
 + printk(GDT-EISA: Unable to allocate IRQ\n);
 + goto out_host_put;
 + }
 +
 + shp-unchecked_isa_dma = 0;
 + shp-irq = ha-irq;
 + shp-dma_channel = 0xff;
 + hanum = gdth_ctr_count;
 + gdth_ctr_tab[gdth_ctr_count++] = shp;
 + gdth_ctr_vtab[gdth_ctr_vcount++] = shp;
 +
 + NUMDATA(shp)-hanum = (ushort)hanum;
 + NUMDATA(shp)-busnum= 0;
 + TRACE2((EISA detect Bus 0: hanum %d\n,
 + NUMDATA(shp)-hanum));
 +
 + ha-pccb = CMDDATA(shp);
 + ha-ccb_phys = 0L;
 +
 + error = -ENOMEM;
 +
 + ha-pdev = NULL;
 + ha-pscratch = pci_alloc_consistent(ha-pdev, GDTH_SCRATCH,
 + scratch_dma_handle);
 + if (!ha-pscratch)
 + goto out_free_irq;
 + ha-scratch_phys = scratch_dma_handle;
 +
 + ha-pmsg = pci_alloc_consistent(ha-pdev, sizeof(gdth_msg_str),
 + scratch_dma_handle);
 + if (!ha-pmsg)
 + goto out_free_pscratch;
 + ha-msg_phys = scratch_dma_handle;
 +
 +#ifdef INT_COAL
 + ha-coal_stat = pci_alloc_consistent(ha-pdev,
 + sizeof(gdth_coal_status) * MAXOFFSETS,
 + scratch_dma_handle);
 + if (!ha-coal_stat)
 + goto out_free_pmsg;
 + ha-coal_stat_phys = scratch_dma_handle;
 +#endif
 +
 + ha-ccb_phys = pci_map_single(ha-pdev,ha-pccb,
 + sizeof(gdth_cmd_str), PCI_DMA_BIDIRECTIONAL);
 + if (!ha-ccb_phys)
 + goto out_free_coal_stat;

Same thing as in ISA patch: this is not PCI and we should not hide what we're 
doing so dma_alloc_consistent(NULL, ...) is better IMHO.

Eike


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH 2/16] gdth: split out eisa probing

2007-10-03 Thread Christoph Hellwig
On Wed, Oct 03, 2007 at 07:32:24PM +0200, Rolf Eike Beer wrote:
 It's not about passing NULL there. We need to do that. But we should call it 
 with NULL as argument and not with ha-pdev to make this obvious.

Feel free to send your cleanup patches once we've finished doing the
important heavy lifting on gdth.

-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/16] gdth: split out eisa probing

2007-10-03 Thread Christoph Hellwig
On Tue, Oct 02, 2007 at 07:20:38PM +0200, Rolf Eike Beer wrote:
 Same thing as in ISA patch: this is not PCI and we should not hide what we're 
 doing so dma_alloc_consistent(NULL, ...) is better IMHO.

passing NULL is the documented way to deal with ISA/EISA devices with
the old pci dma api.  Any change to these APIs should be a different
patch, and there's about 100 more important things in gdth right now.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/16] gdth: split out eisa probing

2007-10-03 Thread Jeff Garzik

Christoph Hellwig wrote:

On Wed, Oct 03, 2007 at 07:32:24PM +0200, Rolf Eike Beer wrote:
It's not about passing NULL there. We need to do that. But we should call it 
with NULL as argument and not with ha-pdev to make this obvious.


Feel free to send your cleanup patches once we've finished doing the
important heavy lifting on gdth.


Come on.  The patches were posted for comments, and Rolf commented. 
Don't give him a hard time for a valid comment.


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/16] gdth: split out eisa probing

2007-10-03 Thread Christoph Hellwig
On Wed, Oct 03, 2007 at 01:59:23PM -0400, Jeff Garzik wrote:
 Come on.  The patches were posted for comments, and Rolf commented. 
 Don't give him a hard time for a valid comment.

Sorry, but these comments are utterly useless.  It's not like we're doing
anything related to dma mapping, but just moving some init code around.
If we actually did a major change in how dma mapping is handled the comment
would be apropinquate and a switchover should happen as part of the patch
series. 
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/16] gdth: split out eisa probing

2007-10-03 Thread Jeff Garzik

Christoph Hellwig wrote:

On Wed, Oct 03, 2007 at 01:59:23PM -0400, Jeff Garzik wrote:
Come on.  The patches were posted for comments, and Rolf commented. 
Don't give him a hard time for a valid comment.


Sorry, but these comments are utterly useless.  It's not like we're doing
anything related to dma mapping, but just moving some init code around.
If we actually did a major change in how dma mapping is handled the comment
would be apropinquate and a switchover should happen as part of the patch
series. 


A comment noting a useful improvement is always helpful, even if it 
cannot be addressed immediately.  Please don't push reviewers away. 
That severely demotivates other reviewers, when they see such treatment.


We talked about this at the Kernel Summit.

Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html