lkml.org 
[lkml]   [2012]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/4] [SCSI] ufshcd: UFS Host controller driver
    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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2012-02-07 08:19    [W:0.036 / U:58.316 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site