Hi,
On Thu, Feb 02, 2012 at 10:27:26AM +0530, Vinayak Holikatti wrote:
> +/**
> + * ufshcd_uic_cc_handler - handle UIC command completion
> + * @work: pointer to a work queue structure
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +static void ufshcd_uic_cc_handler (struct work_struct *work)
> +{
> + struct ufs_hba *hba;
> +
> + hba = container_of(work, struct ufs_hba, uic_workq);
> +
> + if ((UIC_CMD_DME_LINK_STARTUP == hba->active_uic_cmd.command) &&
this is so annoying. Please invert all these constructs:
if ((hcd->active_uic_cmd.command == UIC_CMD_DME_LINK_STARTUP) ....
and so on.
> + !(ufshcd_get_uic_cmd_result(hba))) {
> +
> + if (ufshcd_make_hba_operational(hba))
> + dev_err(&hba->pdev->dev,
> + "cc: hba not operational state\n");
> + return;
> + }
> +}
> +
> +/**
> + * ufshcd_sl_intr - Interrupt service routine
> + * @hba: per adapter instance
> + * @intr_status: contains interrupts generated by the controller
> + */
> +static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
> +{
> + if (intr_status & UIC_COMMAND_COMPL)
> + schedule_work(&hba->uic_workq);
> +}
> +
> +/**
> + * ufshcd_intr - Main interrupt service routine
> + * @irq: irq number
> + * @__hba: pointer to adapter instance
> + *
> + * Returns IRQ_HANDLED - If interrupt is valid
> + * IRQ_NONE - If invalid interrupt
> + */
> +static irqreturn_t ufshcd_intr(int irq, void *__hba)
> +{
> + unsigned long flags;
> + u32 intr_status;
> + irqreturn_t retval = IRQ_NONE;
> + struct ufs_hba *hba = __hba;
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + intr_status = readl(UFSHCD_MMIO_BASE + REG_INTERRUPT_STATUS);
> +
> + if (intr_status) {
> + ufshcd_sl_intr(hba, intr_status);
> +
> + /* If UFSHCI 1.0 then clear interrupt status register */
> + if (UFSHCI_VERSION_10 == hba->ufs_version)
> + writel(intr_status,
> + (UFSHCD_MMIO_BASE + REG_INTERRUPT_STATUS));
> + retval = IRQ_HANDLED;
> + }
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + return retval;
> +}
> +
> +static struct scsi_host_template ufshcd_driver_template = {
> + .module = THIS_MODULE,
> + .name = UFSHCD,
> + .proc_name = UFSHCD,
> + .this_id = -1,
> +};
> +
> +/**
> + * ufshcd_shutdown - main funciton to put the controller in reset state
> + * @pdev: pointer to PCI device handle
> + */
> +static void ufshcd_shutdown(struct pci_dev *pdev)
> +{
> + ufshcd_hba_stop((struct ufs_hba *)pci_get_drvdata(pdev));
> +}
> +
> +#ifdef CONFIG_PM
> +/**
> + * ufshcd_suspend - suspend power management function
> + * @pdev: pointer to PCI device handle
> + * @state: power state
> + *
> + * Returns -ENOSYS
> + */
> +static int ufshcd_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + return -ENOSYS;
> +}
> +
> +/**
> + * ufshcd_resume - resume power management function
> + * @pdev: pointer to PCI device handle
> + *
> + * Returns -ENOSYS
> + */
> +static int ufshcd_resume(struct pci_dev *pdev)
> +{
> + return -ENOSYS;
> +}
> +#endif /* CONFIG_PM */
> +
> +/**
> + * ufshcd_hba_free - free allocated memory for
> + * host memory space data structures
> + * @hba: per adapter instance
> + */
> +static void ufshcd_hba_free(struct ufs_hba *hba)
> +{
> + iounmap(UFSHCD_MMIO_BASE);
> + ufshcd_free_hba_memory(hba);
> + pci_release_regions(hba->pdev);
> +}
> +
> +/**
> + * ufshcd_remove - deallocate PCI/SCSI host and host memory space
> + * data structure memory
> + * @pdev - pointer to PCI handle
> + */
> +static void ufshcd_remove(struct pci_dev *pdev)
> +{
> + struct ufs_hba *hba = pci_get_drvdata(pdev);
> +
> + /* disable interrupts */
> + ufshcd_int_config(hba, UFSHCD_INT_DISABLE);
> + free_irq(pdev->irq, hba);
> +
> + ufshcd_hba_stop(hba);
> + ufshcd_hba_free(hba);
> +
> + scsi_remove_host(hba->host);
> + scsi_host_put(hba->host);
> + pci_set_drvdata(pdev, NULL);
> + pci_clear_master(pdev);
> + pci_disable_device(pdev);
> +}
> +
> +/**
> + * ufshcd_set_dma_mask - Set dma addressing
> + * @pdev: PCI device struct
> + *
> + * Returns 0 for success, non-zero for failure
> + */
> +static int ufshcd_set_dma_mask(struct pci_dev *pdev)
> +{
> + int err;
> +
> + do {
> + err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> + if (!err) {
> + err = pci_set_consistent_dma_mask(pdev,
> + DMA_BIT_MASK(64));
> + break;
> + }
> + err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> + if (!err)
> + err = pci_set_consistent_dma_mask(pdev,
> + DMA_BIT_MASK(32));
> + } while (0);
> +
> + return err;
> +}
> +
> +/**
> + * ufshcd_probe - probe routine of the driver
> + * @pdev: pointer to PCI device handle
> + * @id: PCI device id
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +static int __devinit
> +ufshcd_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + struct Scsi_Host *host;
> + struct ufs_hba *hba;
> + int ufs_hba_len;
> + int err;
> +
> + ufs_hba_len = sizeof(struct ufs_hba);
> + err = pci_enable_device(pdev);
> + if (err) {
> + dev_err(&pdev->dev, "pci_enable_device failed\n");
> + goto out_error;
> + }
> +
> + pci_set_master(pdev);
> +
> + host = scsi_host_alloc(&ufshcd_driver_template, ufs_hba_len);
> + if (!host) {
> + dev_err(&pdev->dev, "scsi_host_alloc failed\n");
> + err = -ENOMEM;
> + goto out_disable;
> + }
> + hba = (struct ufs_hba *)host->hostdata;
> +
> + err = pci_request_regions(pdev, UFSHCD);
> + if (err < 0) {
> + dev_err(&pdev->dev, "request regions failed\n");
> + goto out_disable;
> + }
> +
> + hba->mmio_base = pci_ioremap_bar(pdev, 0);
> + if (!hba->mmio_base) {
> + dev_err(&pdev->dev, "memory map failed\n");
> + err = -ENOMEM;
> + goto out_release_regions;
> + }
> +
> + err = ufshcd_set_dma_mask(pdev);
> + if (err) {
> + dev_err(&pdev->dev, "set dma mask failed\n");
> + goto out_iounmap;
> + }
> +
> + hba->host = host;
> + hba->pdev = pdev;
> +
> + /* Read capabilities registers */
> + ufshcd_hba_capabilities(hba);
> +
> + /* Get UFS version supported by the controller */
> + hba->ufs_version = ufshcd_get_ufs_version(hba);
> +
> + /* Allocate memory for host memory space */
> + err = ufshcd_memory_alloc(hba);
> + if (err) {
> + dev_err(&pdev->dev, "Memory allocation failed\n");
> + goto out_iounmap;
> + }
> +
> + /* Configure LRB */
> + ufshcd_host_memory_configure(hba);
> +
> + host->can_queue = hba->nutrs;
> + host->max_id = UFSHCD_MAX_ID;
> + host->max_lun = UFSHCD_MAX_LUNS;
> + host->max_channel = UFSHCD_MAX_CHANNEL;
> + host->unique_id = host->host_no;
> +
> + /* Initialize work queues */
> + INIT_WORK(&hba->uic_workq, ufshcd_uic_cc_handler);
> +
> + /* IRQ registration */
> + err = request_irq(pdev->irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
make this a threaded IRQ and remove your workqueues and tasklets.
> + if (err) {
> + dev_err(&pdev->dev, "request irq failed\n");
> + goto out_lrb_free;
> + }
> +
> + pci_set_drvdata(pdev, hba);
would also be cool if you would abstract PCI out of this driver, so it's
easier to be re-used. See how I did it with drivers/usb/dwc3/core.c
drivers/usb/dwc3/dwc3-pci.c and drivers/usb/dwc3/dwc3-omap.c
--
balbi
signature.asc
Description: Digital signature
