Re: [PATCH V5 3/4] [SCSI] ufs: Add Platform glue driver for ufshcd

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

 



On 1/4/2013 1:07 PM, vinayak holikatti wrote:
On Thu, Dec 27, 2012 at 8:28 PM, Subhash Jadavani
<subhashj@xxxxxxxxxxxxxx> wrote:
On 12/27/2012 1:45 AM, vinholikatti@xxxxxxxxx wrote:
From: Vinayak Holikatti <vinholikatti@xxxxxxxxx>

This patch adds Platform glue driver for ufshcd.

Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx>
Reviewed-by: Namjae Jeon <linkinjeon@xxxxxxxxx>
Signed-off-by: Vinayak Holikatti <vinholikatti@xxxxxxxxx>
Signed-off-by: Santosh Yaraganavi <santoshsy@xxxxxxxxx>
---
   drivers/scsi/ufs/Kconfig         |   11 ++
   drivers/scsi/ufs/Makefile        |    1 +
   drivers/scsi/ufs/ufshcd-pltfrm.c |  205
++++++++++++++++++++++++++++++++++++++
   3 files changed, 217 insertions(+), 0 deletions(-)
   create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.c

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index d77ae97..96e1721 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -57,3 +57,14 @@ config SCSI_UFSHCD_PCI
           If you have a controller with this interface, say Y or M here.
           If unsure, say N.
+
+config SCSI_UFSHCD_PLATFORM
+       tristate "Platform based UFS Controller support"
This may sound more explicit: s/"Platform based"/"Platform bus based"
+       depends on SCSI_UFSHCD
+       ---help---
+       This selects the UFS host controller support. If you have a
+          platform with UFS controller, say Y or M here.
s/"If you have a platform with UFS controller,"/"If you have UFS controller
on platform bus,"
+
+          If you have a controller with this interface, say Y or M here.
Why do we need this line? we already one comment above.
+
+         If unsure, say N.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 9eda0df..1e5bd48 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -1,3 +1,4 @@
   # UFSHCD makefile
   obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
   obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
+obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
b/drivers/scsi/ufs/ufshcd-pltfrm.c
new file mode 100644
index 0000000..94acfdc
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -0,0 +1,205 @@
+/*
+ * Universal Flash Storage Host controller driver
Please add some comment to indicate that this is platform bus UFS host
controller driver.
The File name itself would identify the nature of driver. can add for more info.
+ *
+ * This code is based on drivers/scsi/ufs/ufshcd-pltfrm.c
+ * Copyright (C) 2011-2012 Samsung India Software Operations
+ *
+ * Authors:
+ *     Santosh Yaraganavi <santosh.sy@xxxxxxxxxxx>
+ *     Vinayak Holikatti <h.vinayak@xxxxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * See the COPYING file in the top-level directory or visit
+ * <http://www.gnu.org/licenses/gpl-2.0.html>
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This program is provided "AS IS" and "WITH ALL FAULTS" and
+ * without warranty of any kind. You are solely responsible for
+ * determining the appropriateness of using and distributing
+ * the program and assume all risks associated with your exercise
+ * of rights with respect to the program, including but not limited
+ * to infringement of third party rights, the risks and costs of
+ * program errors, damage to or loss of data, programs or equipment,
+ * and unavailability or interruption of operations. Under no
+ * circumstances will the contributor of this Program be liable for
+ * any damages of any kind arising from your use or distribution of
+ * this program.
+ */
+
+#include "ufshcd.h"
+#include <linux/platform_device.h>
+
+#ifdef CONFIG_PM
+/**
+ * ufshcd_pltfrm_suspend - suspend power management function
+ * @pdev: pointer to Platform device handle
+ * @mesg: power state
+ *
+ * Returns -ENOSYS
+ */
+static int ufshcd_pltfrm_suspend(struct platform_device *pdev,
+                                pm_message_t mesg)
+{
+       /*
+        * TODO:
+        * 1. Call ufshcd_suspend
+        * 2. Do bus specific power management
+        */
+
+       return -ENOSYS;
+}
+
+/**
+ * ufshcd_pltfrm_resume - resume power management function
+ * @pdev: pointer to Platform device handle
+ *
+ * Returns -ENOSYS
+ */
+static int ufshcd_pltfrm_resume(struct platform_device *pdev)
+{
+       /*
+        * TODO:
+        * 1. Call ufshcd_resume.
+        * 2. Do bus specific wake up
+        */
+
+       return -ENOSYS;
+}
+#endif
+
+/**
+ * ufshcd_pltfrm_probe - probe routine of the driver
+ * @pdev: pointer to Platform device handle
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int __devinit
+ufshcd_pltfrm_probe(struct platform_device *pdev)
+{
+       struct ufs_hba *hba;
+       void __iomem *mmio_base;
+       struct resource *mem_res;
+       struct resource *irq_res;
+       resource_size_t mem_size;
+       int err;
+       struct device *dev = &pdev->dev;
+
+       mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!mem_res) {
+               dev_err(&pdev->dev,
+                       "%s: Memory resource not available\n", __FILE__);
Why would we print the file name? Won't the device name prefix enough in
debug message?
File name would be necessary for faster debugging
+               err = -ENODEV;
+               goto out_error;
+       }
+
+       mem_size = resource_size(mem_res);
+       if (!request_mem_region(mem_res->start, mem_size, "ufshcd")) {
you may want to use the UFSHCD macro:   s/"ufshcd"/UFSHCD

+               dev_err(&pdev->dev,
+                       "ufshcd: Cannot reserve the memory resource\n");

"ufshcd:" prefix may not be required here as the device name prefix should
be enough to know the context of the error.
"ufshcd:" Can be removed.
+               err = -EBUSY;
+               goto out_error;
+       }
+
+       mmio_base = ioremap_nocache(mem_res->start, mem_size);
+       if (!mmio_base) {
+               dev_err(&pdev->dev, "memory map failed\n");
+               err = -ENOMEM;
+               goto out_release_regions;
+       }
+
+       irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+       if (!irq_res) {
+               dev_err(&pdev->dev, "ufshcd: IRQ resource not
available\n");

Same as above. "ufshcd:" prefix may not be required here as the device name
prefix should be enough to know the context of the error.
can be removed.
+               err = -ENODEV;
+               goto out_iounmap;
+       }
+
+       err = dma_set_coherent_mask(dev, dev->coherent_dma_mask);
+       if (err) {
+               dev_err(&pdev->dev, "set dma mask failed\n");
+               goto out_iounmap;
+       }
+
+       err = ufshcd_init(&pdev->dev, &hba, mmio_base, irq_res->start);
+       if (err) {
+               dev_err(&pdev->dev, "%s: Intialization failed\n",
+                       __FILE__);

Again, not sure why we would need to prefix the file name here.
__FILE__ is necessary for faster debugging
+               goto out_iounmap;
+       }
+
+       platform_set_drvdata(pdev, hba);
+
+       return 0;
+
+out_iounmap:
+       iounmap(mmio_base);
+out_release_regions:
+       release_mem_region(mem_res->start, mem_size);
+out_error:
+       return err;
+}
+
+/**
+ * ufshcd_pltfrm_remove - remove platform driver routine
+ * @pdev: pointer to platform device handle
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int __devexit ufshcd_pltfrm_remove(struct platform_device *pdev)
+{
+       struct resource *mem_res;
+       struct resource *irq_res;
+       resource_size_t mem_size;
+       struct ufs_hba *hba =  platform_get_drvdata(pdev);
+
+       irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
It would be better to save the irq number under "struct ufs_hba" during
probe. So here during remove you just need simply need to call the
"free_irq(irq_res->start, hba)"
Will modify the code accordingly in the next patchset.
+
+       if (!irq_res)
+               dev_err(&pdev->dev, "ufshcd: IRQ resource not
available\n");
+       else
+               free_irq(irq_res->start, hba);
+
+       ufshcd_remove(hba);
Remove should be exactly opposite of probe(). So shouldn't you call the
ufshcd_remove() first and then free_irq() after that.
Some bugging controllers might raise the interrupt after resources are removed.
this sequence will prevent it.

Could you please add the same as comment in above code sequence?

+       mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
You might want to save the pointer to mem_res in "struct ufs_hba" during
probe and may use the same here.
+       if (!mem_res)
+               dev_err(&pdev->dev, "ufshcd: Memory resource not
available\n");
+       else {
+               mem_size = resource_size(mem_res);
+               release_mem_region(mem_res->start, mem_size);
+       }
+       platform_set_drvdata(pdev, NULL);
+       return 0;
+}
+
+static const struct of_device_id ufs_of_match[] = {
+       { .compatible = "jedec,ufs-1.1"},
+};
+
+static struct platform_driver ufshcd_pltfrm_driver = {
+       .probe  = ufshcd_pltfrm_probe,
+       .remove = __devexit_p(ufshcd_pltfrm_remove),
+#ifdef CONFIG_PM
+       .suspend = ufshcd_pltfrm_suspend,
+       .resume = ufshcd_pltfrm_resume,
+#endif
+       .driver = {
+               .name   = "ufshcd",
+               .owner  = THIS_MODULE,
+               .of_match_table = ufs_of_match,
+       },
+};
+
+module_platform_driver(ufshcd_pltfrm_driver);
+
+MODULE_AUTHOR("Santosh Yaragnavi <santosh.sy@xxxxxxxxxxx>");
+MODULE_AUTHOR("Vinayak Holikatti <h.vinayak@xxxxxxxxxxx>");
+MODULE_DESCRIPTION("Platform based UFS host controller driver");

s/"Platform based"/"Platform Bus based"

+MODULE_LICENSE("GPL");
+MODULE_VERSION(UFSHCD_DRIVER_VERSION);
You want to keep the same driver version as the UFS core driver?


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