Re: [PATCH 1/4] [SCSI] ufshcd: UFS Host controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 7, 2012 at 12:46 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> 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.
>
ok, I'll change it.

>> +         !(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.
>

ok, I'll look into it.

>> +     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
>

ok, we'll send it as a separate patch.

> --
> balbi

Thanks,
~Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux