Messages in this thread | | | Date | Tue, 17 Jan 2006 08:20:37 -0500 | From | Jeff Garzik <> | Subject | Re: PATCH: pata_oldpiix rev 0.2 |
| |
Alan Cox wrote: > static void oldpiix_pata_phy_reset(struct ata_port *ap) > { > struct pci_dev *pdev = to_pci_dev(ap->host_set->dev); > static struct pci_bits oldpiix_enable_bits[] = { > { 0x41U, 1U, 0x80UL, 0x80UL }, /* port 0 */ > { 0x43U, 1U, 0x80UL, 0x80UL }, /* port 1 */ > };
const
> if (!pci_test_config_bits(pdev, &oldpiix_enable_bits[ap->hard_port_no])) { > ata_port_disable(ap); > printk(KERN_INFO "ata%u: port disabled. ignoring.\n", ap->id); > return; > } > ap->cbl = ATA_CBL_PATA40; > ata_port_probe(ap); > ata_bus_reset(ap); > } > > /** > * oldpiix_set_piomode - Initialize host controller PATA PIO timings > * @ap: Port whose timings we are configuring > * @adev: um > * > * Set PIO mode for device, in host controller PCI config space. > * > * LOCKING: > * None (inherited from caller). > */ > > static void oldpiix_set_piomode (struct ata_port *ap, struct ata_device *adev) > { > unsigned int pio = adev->pio_mode - XFER_PIO_0; > struct pci_dev *dev = to_pci_dev(ap->host_set->dev); > unsigned int idetm_port= ap->hard_port_no ? 0x42 : 0x40;
space after '='?
> u16 idetm_data; > int control = 0; > > /* > * See Intel Document 298600-004 for the timing programing rules > * for PIIX/ICH. Note that the early PIIX does not have the slave > * timing port at 0x44. > */ > > static const /* ISP RTC */ > u8 timings[][2] = { { 0, 0 }, > { 0, 0 }, > { 1, 0 }, > { 2, 1 }, > { 2, 3 }, }; > if (pio > 2) > control |= 1; /* TIME1 enable */ > if (ata_pio_need_iordy(adev)) > control |= 2; /* IE IORDY */ > > /* Intel specifies that the PPE functionality is for disk only */ > if (adev->class == ATA_DEV_ATA) > control |= 4; /* PPE enable */ > > pci_read_config_word(dev, idetm_port, &idetm_data); > > /* Enable PPE, IE and TIME as appropriate. Clear the other > drive timing bits */ > if (adev->devno == 0) { > idetm_data &= 0xCCE0; > idetm_data |= control; > } else { > idetm_data &= 0xCC0E; > idetm_data |= (control << 4); > } > idetm_data |= (timings[pio][0] << 12) | > (timings[pio][1] << 8); > pci_write_config_word(dev, idetm_port, idetm_data); > > /* Track which port is configured */ > ap->private_data = adev; > } > > /** > * oldpiix_set_dmamode - Initialize host controller PATA DMA timings > * @ap: Port whose timings we are configuring > * @adev: Device to program > * @isich: True if the device is an ICH and has IOCFG registers > * > * Set MWDMA mode for device, in host controller PCI config space. > * > * LOCKING: > * None (inherited from caller). > */ > > /** > * oldpiix_qc_issue_prot - command issue > * @qc: command pending > * > * Called when the libata layer is about to issue a command. We wrap > * this interface so that we can load the correct ATA timings if > * neccessary. Our logic also clears TIME0/TIME1 for the other device so > * that, even if we get this wrong, cycles to the other device will > * be made PIO0. > */ > > static int oldpiix_qc_issue_prot(struct ata_queued_cmd *qc) > { > struct ata_port *ap = qc->ap; > struct ata_device *adev = qc->dev; > > if (adev != ap->private_data) { > if (adev->dma_mode) > oldpiix_set_dmamode(ap, adev); > else if (adev->pio_mode) > oldpiix_set_piomode(ap, adev); > } > return ata_qc_issue_prot(qc); > } > > > static struct scsi_host_template oldpiix_sht = { > .module = THIS_MODULE, > .name = DRV_NAME, > .ioctl = ata_scsi_ioctl, > .queuecommand = ata_scsi_queuecmd, > .eh_strategy_handler = ata_scsi_error, > .can_queue = ATA_DEF_QUEUE, > .this_id = ATA_SHT_THIS_ID, > .sg_tablesize = LIBATA_MAX_PRD, > .max_sectors = ATA_MAX_SECTORS, > .cmd_per_lun = ATA_SHT_CMD_PER_LUN, > .emulated = ATA_SHT_EMULATED, > .use_clustering = ATA_SHT_USE_CLUSTERING, > .proc_name = DRV_NAME, > .dma_boundary = ATA_DMA_BOUNDARY, > .slave_configure = ata_scsi_slave_config, > .bios_param = ata_std_bios_param, > .ordered_flush = 1, > }; > > static const struct ata_port_operations oldpiix_pata_ops = { > .port_disable = ata_port_disable, > .set_piomode = oldpiix_set_piomode, > .set_dmamode = oldpiix_set_dmamode, > > .tf_load = ata_tf_load, > .tf_read = ata_tf_read, > .check_status = ata_check_status, > .exec_command = ata_exec_command, > .dev_select = ata_std_dev_select, > > .phy_reset = oldpiix_pata_phy_reset, > > .bmdma_setup = ata_bmdma_setup, > .bmdma_start = ata_bmdma_start, > .bmdma_stop = ata_bmdma_stop, > .bmdma_status = ata_bmdma_status, > .qc_prep = ata_qc_prep, > .qc_issue = oldpiix_qc_issue_prot, > > .eng_timeout = ata_eng_timeout, > > .irq_handler = ata_interrupt, > .irq_clear = ata_bmdma_irq_clear, > > .port_start = ata_port_start, > .port_stop = ata_port_stop, > .host_stop = ata_host_stop, > }; > > > /** > * oldpiix_init_one - Register PIIX ATA PCI device with kernel services > * @pdev: PCI device to register > * @ent: Entry in oldpiix_pci_tbl matching with @pdev > * > * Called from kernel PCI layer. > * > * LOCKING: > * Inherited from PCI layer (may sleep). > * > * RETURNS: > * Zero on success, or -ERRNO value. > */ > > static int oldpiix_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) > { > static int printed_version; > static struct ata_port_info info = { > .sht = &oldpiix_sht, > .host_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST, > .pio_mask = 0x1f, /* pio0-4 */ > .mwdma_mask = 0x07, /* mwdma1-2 */ > .port_ops = &oldpiix_pata_ops, > }; > static struct ata_port_info *port_info[2] = { &info, &info }; > > if (!printed_version++) > dev_printk(KERN_DEBUG, &pdev->dev, > "version " DRV_VERSION "\n"); > > return ata_pci_init_one(pdev, port_info, 2); > } > > static const struct pci_device_id oldpiix_pci_tbl[] = { > { 0x8086, 0x1230, PCI_ANY_ID, PCI_ANY_ID, }, > { } /* terminate list */ > }; > > static struct pci_driver oldpiix_pci_driver = { > .name = DRV_NAME, > .id_table = oldpiix_pci_tbl, > .probe = oldpiix_init_one, > .remove = ata_pci_remove_one, > }; > > static int __init oldpiix_init(void) > { > int rc; > > DPRINTK("pci_module_init\n"); > rc = pci_module_init(&oldpiix_pci_driver); > if (rc) > return rc; > > DPRINTK("done\n"); > return 0; > }
same comment as before:
reduce this function to a single line, a call to pci_register_driver().
otherwise OK.
Jeff
- 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/
| |