Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1208227 > unrolled thread

[PATCH v1 6/8] scsi: ufs: make the UFS variant a platform device

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v1 6/8] scsi: ufs: make the UFS variant a platform device Yaniv Gardi <ygardi@codeaurora.org> - 2015-08-16 12:20 +0200
    Re: [PATCH v1 6/8] scsi: ufs: make the UFS variant a platform device Akinobu Mita <akinobu.mita@gmail.com> - 2015-08-19 14:30 +0200
      Re: [PATCH v1 6/8] scsi: ufs: make the UFS variant a platform  device ygardi@codeaurora.org - 2015-08-20 13:40 +0200

#1208227 — [PATCH v1 6/8] scsi: ufs: make the UFS variant a platform device

This change turns the UFS variant (SCSI_UFS_QCOM) into a UFS
a platform device.
In order to do so a few additional changes are required:
1. The ufshcd-pltfrm is no longer serves as a platform device.
   Now it only serves as a group of platform APIs such as PM APIs
   (runtime suspend/resume, system suspend/resume etc), parsers of
   clocks, regulators and pm_levels from DT.
2. What used to be the old platform "probe" is now "only" a pltfrm_init()
   routine, that does exactly the same, but only being called by the
   new probe function of the UFS variant.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufs-qcom.c      | 83 +++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-pltfrm.c | 84 +++++++++-------------------------------
 drivers/scsi/ufs/ufshcd-pltfrm.h | 27 +++++++++++++
 drivers/scsi/ufs/ufshcd.c        | 10 +++++
 drivers/scsi/ufs/ufshcd.h        |  6 +++
 5 files changed, 145 insertions(+), 65 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.h

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index a6b7e10..2c3d57e 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -19,6 +19,7 @@
 
 #include <linux/phy/phy-qcom-ufs.h>
 #include "ufshcd.h"
+#include "ufshcd-pltfrm.h"
 #include "unipro.h"
 #include "ufs-qcom.h"
 #include "ufshci.h"
@@ -1050,4 +1051,86 @@ static struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.resume			= ufs_qcom_resume,
 };
 
+/**
+ * ufs_qcom_probe - probe routine of the driver
+ * @pdev: pointer to Platform device handle
+ *
+ * Always return 0
+ */
+static int ufs_qcom_probe(struct platform_device *pdev)
+{
+	int err;
+	struct device *dev = &pdev->dev;
+	struct ufs_hba *hba;
+
+	/* Perform generic probe */
+	err = ufshcd_pltfrm_init(pdev, &ufs_hba_qcom_vops);
+	if (err) {
+		dev_err(dev, "ufshcd_pltfrm_init() failed %d\n", err);
+		goto out;
+	}
+
+	hba = platform_get_drvdata(pdev);
+	if (unlikely(!hba)) {
+		dev_err(dev, "no hba structure after successful probing\n");
+		goto dealloc_host;
+	}
+
+	return 0;
+
+dealloc_host:
+	/* disconnect the bind between the qcom host and the hba */
+	ufshcd_set_variant(hba, NULL);
+	ufshcd_dealloc_host(hba);
+out:
+	return err;
+}
+
+/**
+ * ufs_qcom_remove - set driver_data of the device to NULL
+ * @pdev: pointer to platform device handle
+ *
+ * Always return 0
+ */
+static int ufs_qcom_remove(struct platform_device *pdev)
+{
+	struct ufs_hba *hba =  platform_get_drvdata(pdev);
+
+	pm_runtime_get_sync(&(pdev)->dev);
+	ufshcd_remove(hba);
+	ufshcd_dealloc_host(hba);
+	return 0;
+}
+
+static void ufs_qcom_shutdown(struct platform_device *pdev)
+{
+	ufshcd_shutdown((struct ufs_hba *)platform_get_drvdata(pdev));
+}
+
+static const struct of_device_id ufs_qcom_of_match[] = {
+	{ .compatible = "qcom,ufshc"},
+	{},
+};
+
+static const struct dev_pm_ops ufs_qcom_pm_ops = {
+	.suspend	= ufshcd_pltfrm_suspend,
+	.resume		= ufshcd_pltfrm_resume,
+	.runtime_suspend = ufshcd_pltfrm_runtime_suspend,
+	.runtime_resume  = ufshcd_pltfrm_runtime_resume,
+	.runtime_idle    = ufshcd_pltfrm_runtime_idle,
+};
+
+static struct platform_driver ufs_qcom_pltform = {
+	.probe	= ufs_qcom_probe,
+	.remove	= ufs_qcom_remove,
+	.shutdown = ufs_qcom_shutdown,
+	.driver	= {
+		.name	= "ufshcd-qcom",
+		.owner	= THIS_MODULE,
+		.pm	= &ufs_qcom_pm_ops,
+		.of_match_table = of_match_ptr(ufs_qcom_of_match),
+	},
+};
+module_platform_driver(ufs_qcom_pltform);
+
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 7db9564..20009a9 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -38,20 +38,9 @@
 #include <linux/of.h>
 
 #include "ufshcd.h"
+#include "ufshcd-pltfrm.h"
 
 static const struct of_device_id ufs_of_match[];
-static struct ufs_hba_variant_ops *get_variant_ops(struct device *dev)
-{
-	if (dev->of_node) {
-		const struct of_device_id *match;
-
-		match = of_match_node(ufs_of_match, dev->of_node);
-		if (match)
-			return (struct ufs_hba_variant_ops *)match->data;
-	}
-
-	return NULL;
-}
 
 static int ufshcd_parse_clock_info(struct ufs_hba *hba)
 {
@@ -245,10 +234,11 @@ out:
  * Returns 0 if successful
  * Returns non-zero otherwise
  */
-static int ufshcd_pltfrm_suspend(struct device *dev)
+int ufshcd_pltfrm_suspend(struct device *dev)
 {
 	return ufshcd_system_suspend(dev_get_drvdata(dev));
 }
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_suspend);
 
 /**
  * ufshcd_pltfrm_resume - resume power management function
@@ -257,23 +247,30 @@ static int ufshcd_pltfrm_suspend(struct device *dev)
  * Returns 0 if successful
  * Returns non-zero otherwise
  */
-static int ufshcd_pltfrm_resume(struct device *dev)
+int ufshcd_pltfrm_resume(struct device *dev)
 {
 	return ufshcd_system_resume(dev_get_drvdata(dev));
 }
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_resume);
 
-static int ufshcd_pltfrm_runtime_suspend(struct device *dev)
+int ufshcd_pltfrm_runtime_suspend(struct device *dev)
 {
 	return ufshcd_runtime_suspend(dev_get_drvdata(dev));
 }
-static int ufshcd_pltfrm_runtime_resume(struct device *dev)
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_suspend);
+
+int ufshcd_pltfrm_runtime_resume(struct device *dev)
 {
 	return ufshcd_runtime_resume(dev_get_drvdata(dev));
 }
-static int ufshcd_pltfrm_runtime_idle(struct device *dev)
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_resume);
+
+int ufshcd_pltfrm_runtime_idle(struct device *dev)
 {
 	return ufshcd_runtime_idle(dev_get_drvdata(dev));
 }
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_idle);
+
 #else /* !CONFIG_PM */
 #define ufshcd_pltfrm_suspend	NULL
 #define ufshcd_pltfrm_resume	NULL
@@ -282,18 +279,15 @@ static int ufshcd_pltfrm_runtime_idle(struct device *dev)
 #define ufshcd_pltfrm_runtime_idle	NULL
 #endif /* CONFIG_PM */
 
-static void ufshcd_pltfrm_shutdown(struct platform_device *pdev)
-{
-	ufshcd_shutdown((struct ufs_hba *)platform_get_drvdata(pdev));
-}
-
 /**
  * ufshcd_pltfrm_probe - probe routine of the driver
  * @pdev: pointer to Platform device handle
+ * @vops: pointer to variant ops
  *
  * Returns 0 on success, non-zero value on failure
  */
-static int ufshcd_pltfrm_probe(struct platform_device *pdev)
+int ufshcd_pltfrm_init(struct platform_device *pdev,
+		       struct ufs_hba_variant_ops *vops)
 {
 	struct ufs_hba *hba;
 	void __iomem *mmio_base;
@@ -321,7 +315,7 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	hba->vops = get_variant_ops(&pdev->dev);
+	hba->vops = vops;
 
 	err = ufshcd_parse_clock_info(hba);
 	if (err) {
@@ -355,47 +349,7 @@ out_disable_rpm:
 out:
 	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 ufshcd_pltfrm_remove(struct platform_device *pdev)
-{
-	struct ufs_hba *hba =  platform_get_drvdata(pdev);
-
-	pm_runtime_get_sync(&(pdev)->dev);
-	ufshcd_remove(hba);
-	return 0;
-}
-
-static const struct of_device_id ufs_of_match[] = {
-	{ .compatible = "jedec,ufs-1.1"},
-	{},
-};
-
-static const struct dev_pm_ops ufshcd_dev_pm_ops = {
-	.suspend	= ufshcd_pltfrm_suspend,
-	.resume		= ufshcd_pltfrm_resume,
-	.runtime_suspend = ufshcd_pltfrm_runtime_suspend,
-	.runtime_resume  = ufshcd_pltfrm_runtime_resume,
-	.runtime_idle    = ufshcd_pltfrm_runtime_idle,
-};
-
-static struct platform_driver ufshcd_pltfrm_driver = {
-	.probe	= ufshcd_pltfrm_probe,
-	.remove	= ufshcd_pltfrm_remove,
-	.shutdown = ufshcd_pltfrm_shutdown,
-	.driver	= {
-		.name	= "ufshcd",
-		.pm	= &ufshcd_dev_pm_ops,
-		.of_match_table = ufs_of_match,
-	},
-};
-
-module_platform_driver(ufshcd_pltfrm_driver);
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);
 
 MODULE_AUTHOR("Santosh Yaragnavi <santosh.sy@samsung.com>");
 MODULE_AUTHOR("Vinayak Holikatti <h.vinayak@samsung.com>");
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.h b/drivers/scsi/ufs/ufshcd-pltfrm.h
new file mode 100644
index 0000000..2cec7e9
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.h
@@ -0,0 +1,27 @@
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ */
+
+#ifndef UFSHCD_PLTFRM_H_
+#define UFSHCD_PLTFRM_H_
+
+#include "ufshcd.h"
+
+int ufshcd_pltfrm_init(struct platform_device *pdev,
+		       struct ufs_hba_variant_ops *vops);
+int ufshcd_pltfrm_suspend(struct device *dev);
+int ufshcd_pltfrm_resume(struct device *dev);
+int ufshcd_pltfrm_runtime_suspend(struct device *dev);
+int ufshcd_pltfrm_runtime_resume(struct device *dev);
+int ufshcd_pltfrm_runtime_idle(struct device *dev);
+
+#endif /* UFSHCD_PLTFRM_H_ */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9e79c33..2ef9834 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5348,6 +5348,16 @@ void ufshcd_remove(struct ufs_hba *hba)
 EXPORT_SYMBOL_GPL(ufshcd_remove);
 
 /**
+ * ufshcd_dealloc_host - deallocate Host Bus Adapter (HBA)
+ * @hba: pointer to Host Bus Adapter (HBA)
+ */
+void ufshcd_dealloc_host(struct ufs_hba *hba)
+{
+	scsi_host_put(hba->host);
+}
+EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
+
+/**
  * ufshcd_set_dma_mask - Set dma mask based on the controller
  *			 addressing capability
  * @hba: per adapter instance
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index ce75626..cc65d96 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -576,6 +576,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
 }
 
 int ufshcd_alloc_host(struct device *, struct ufs_hba **);
+void ufshcd_dealloc_host(struct ufs_hba *);
 int ufshcd_init(struct ufs_hba * , void __iomem * , unsigned int);
 void ufshcd_remove(struct ufs_hba *);
 
@@ -772,4 +773,9 @@ static inline int ufshcd_vops_resume(struct ufs_hba *hba, enum ufs_pm_op op)
 	return 0;
 }
 
+/* variant specific ops structures */
+#ifdef CONFIG_SCSI_UFS_QCOM
+extern struct ufs_hba_variant_ops ufs_hba_qcom_variant;
+#endif
+
 #endif /* End of Header */
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
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/

[toc] | [next] | [standalone]

#1209815

2015-08-16 19:14 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
> +/**
> + * ufs_qcom_remove - set driver_data of the device to NULL
> + * @pdev: pointer to platform device handle
> + *
> + * Always return 0
> + */
> +static int ufs_qcom_remove(struct platform_device *pdev)
> +{
> +       struct ufs_hba *hba =  platform_get_drvdata(pdev);
> +
> +       pm_runtime_get_sync(&(pdev)->dev);
> +       ufshcd_remove(hba);
> +       ufshcd_dealloc_host(hba);

scsi_host_put (== ufshcd_dealloc_host) is already called in ufshcd_remove().
So we shouldn't call it here again.

> +static struct platform_driver ufs_qcom_pltform = {
> +       .probe  = ufs_qcom_probe,
> +       .remove = ufs_qcom_remove,
> +       .shutdown = ufs_qcom_shutdown,
> +       .driver = {
> +               .name   = "ufshcd-qcom",
> +               .owner  = THIS_MODULE,

We don't need to set .owner.  Please see commit 37b6fea57b4
("scsi: ufs: drop owner assignment from platform_drivers").

> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index 7db9564..20009a9 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -38,20 +38,9 @@
>  #include <linux/of.h>
>
>  #include "ufshcd.h"
> +#include "ufshcd-pltfrm.h"
>
>  static const struct of_device_id ufs_of_match[];

We can remove this forward declaration as ufs_of_match is removed below.

> @@ -245,10 +234,11 @@ out:
>   * Returns 0 if successful
>   * Returns non-zero otherwise
>   */
> -static int ufshcd_pltfrm_suspend(struct device *dev)
> +int ufshcd_pltfrm_suspend(struct device *dev)
>  {
>         return ufshcd_system_suspend(dev_get_drvdata(dev));
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_suspend);
>
>  /**
>   * ufshcd_pltfrm_resume - resume power management function
> @@ -257,23 +247,30 @@ static int ufshcd_pltfrm_suspend(struct device *dev)
>   * Returns 0 if successful
>   * Returns non-zero otherwise
>   */
> -static int ufshcd_pltfrm_resume(struct device *dev)
> +int ufshcd_pltfrm_resume(struct device *dev)
>  {
>         return ufshcd_system_resume(dev_get_drvdata(dev));
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_resume);
>
> -static int ufshcd_pltfrm_runtime_suspend(struct device *dev)
> +int ufshcd_pltfrm_runtime_suspend(struct device *dev)
>  {
>         return ufshcd_runtime_suspend(dev_get_drvdata(dev));
>  }
> -static int ufshcd_pltfrm_runtime_resume(struct device *dev)
> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_suspend);
> +
> +int ufshcd_pltfrm_runtime_resume(struct device *dev)
>  {
>         return ufshcd_runtime_resume(dev_get_drvdata(dev));
>  }
> -static int ufshcd_pltfrm_runtime_idle(struct device *dev)
> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_resume);
> +
> +int ufshcd_pltfrm_runtime_idle(struct device *dev)
>  {
>         return ufshcd_runtime_idle(dev_get_drvdata(dev));
>  }
> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_idle);
> +
>  #else /* !CONFIG_PM */
>  #define ufshcd_pltfrm_suspend  NULL
>  #define ufshcd_pltfrm_resume   NULL

Since ufshcd_pltfrm_suspend()/resume() and ufshcd_pltfrm_runtime_*()
are only defined when CONFIG_PM=y,  ufs-qcom.c can't be built when !CONFIG_PM.

These #ifdef should be moved to ufshcd-pltfrm.h, or we can export
ufshcd_dev_pm_ops instead of the pm functions.

> @@ -282,18 +279,15 @@ static int ufshcd_pltfrm_runtime_idle(struct device *dev)
>  #define ufshcd_pltfrm_runtime_idle     NULL
>  #endif /* CONFIG_PM */
>
> -static void ufshcd_pltfrm_shutdown(struct platform_device *pdev)
> -{
> -       ufshcd_shutdown((struct ufs_hba *)platform_get_drvdata(pdev));
> -}

How about exporting this function? ufs-qcom and other variant can use this.

> +#ifndef UFSHCD_PLTFRM_H_
> +#define UFSHCD_PLTFRM_H_
> +
> +#include "ufshcd.h"
> +
> +int ufshcd_pltfrm_init(struct platform_device *pdev,
> +                      struct ufs_hba_variant_ops *vops);

struct platform_device appears before including <linux/platform_device.h>.

> +/* variant specific ops structures */
> +#ifdef CONFIG_SCSI_UFS_QCOM
> +extern struct ufs_hba_variant_ops ufs_hba_qcom_variant;
> +#endif

What is ufs_hba_qcom_variant? I can't find in kernel source and
this patch series.
--
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/

[toc] | [prev] | [next] | [standalone]

#1210435 — Re: [PATCH v1 6/8] scsi: ufs: make the UFS variant a platform device

> 2015-08-16 19:14 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
>> +/**
>> + * ufs_qcom_remove - set driver_data of the device to NULL
>> + * @pdev: pointer to platform device handle
>> + *
>> + * Always return 0
>> + */
>> +static int ufs_qcom_remove(struct platform_device *pdev)
>> +{
>> +       struct ufs_hba *hba =  platform_get_drvdata(pdev);
>> +
>> +       pm_runtime_get_sync(&(pdev)->dev);
>> +       ufshcd_remove(hba);
>> +       ufshcd_dealloc_host(hba);
>
> scsi_host_put (== ufshcd_dealloc_host) is already called in
> ufshcd_remove().
> So we shouldn't call it here again.

good point. will be fixed.

>
>> +static struct platform_driver ufs_qcom_pltform = {
>> +       .probe  = ufs_qcom_probe,
>> +       .remove = ufs_qcom_remove,
>> +       .shutdown = ufs_qcom_shutdown,
>> +       .driver = {
>> +               .name   = "ufshcd-qcom",
>> +               .owner  = THIS_MODULE,
>
> We don't need to set .owner.  Please see commit 37b6fea57b4
> ("scsi: ufs: drop owner assignment from platform_drivers").
>

correct. will be removed.

>> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
>> b/drivers/scsi/ufs/ufshcd-pltfrm.c
>> index 7db9564..20009a9 100644
>> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
>> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
>> @@ -38,20 +38,9 @@
>>  #include <linux/of.h>
>>
>>  #include "ufshcd.h"
>> +#include "ufshcd-pltfrm.h"
>>
>>  static const struct of_device_id ufs_of_match[];
>
> We can remove this forward declaration as ufs_of_match is removed below.

done.

>
>> @@ -245,10 +234,11 @@ out:
>>   * Returns 0 if successful
>>   * Returns non-zero otherwise
>>   */
>> -static int ufshcd_pltfrm_suspend(struct device *dev)
>> +int ufshcd_pltfrm_suspend(struct device *dev)
>>  {
>>         return ufshcd_system_suspend(dev_get_drvdata(dev));
>>  }
>> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_suspend);
>>
>>  /**
>>   * ufshcd_pltfrm_resume - resume power management function
>> @@ -257,23 +247,30 @@ static int ufshcd_pltfrm_suspend(struct device
>> *dev)
>>   * Returns 0 if successful
>>   * Returns non-zero otherwise
>>   */
>> -static int ufshcd_pltfrm_resume(struct device *dev)
>> +int ufshcd_pltfrm_resume(struct device *dev)
>>  {
>>         return ufshcd_system_resume(dev_get_drvdata(dev));
>>  }
>> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_resume);
>>
>> -static int ufshcd_pltfrm_runtime_suspend(struct device *dev)
>> +int ufshcd_pltfrm_runtime_suspend(struct device *dev)
>>  {
>>         return ufshcd_runtime_suspend(dev_get_drvdata(dev));
>>  }
>> -static int ufshcd_pltfrm_runtime_resume(struct device *dev)
>> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_suspend);
>> +
>> +int ufshcd_pltfrm_runtime_resume(struct device *dev)
>>  {
>>         return ufshcd_runtime_resume(dev_get_drvdata(dev));
>>  }
>> -static int ufshcd_pltfrm_runtime_idle(struct device *dev)
>> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_resume);
>> +
>> +int ufshcd_pltfrm_runtime_idle(struct device *dev)
>>  {
>>         return ufshcd_runtime_idle(dev_get_drvdata(dev));
>>  }
>> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_idle);
>> +
>>  #else /* !CONFIG_PM */
>>  #define ufshcd_pltfrm_suspend  NULL
>>  #define ufshcd_pltfrm_resume   NULL
>
> Since ufshcd_pltfrm_suspend()/resume() and ufshcd_pltfrm_runtime_*()
> are only defined when CONFIG_PM=y,  ufs-qcom.c can't be built when
> !CONFIG_PM.
>
> These #ifdef should be moved to ufshcd-pltfrm.h, or we can export
> ufshcd_dev_pm_ops instead of the pm functions.
>

I wasn't sure what you mean by "ufs-qcom.c can't be built when !CONFIG_PM",
but anyhow, you had a point here:
in case !CONFIG_PM:
#define ufshcd_pltfrm_*
are not familiar in ufs_qcom.c
so in V2 i will take care of this, moving it to ufshcd_pltfrm.h file

>> @@ -282,18 +279,15 @@ static int ufshcd_pltfrm_runtime_idle(struct
>> device *dev)
>>  #define ufshcd_pltfrm_runtime_idle     NULL
>>  #endif /* CONFIG_PM */
>>
>> -static void ufshcd_pltfrm_shutdown(struct platform_device *pdev)
>> -{
>> -       ufshcd_shutdown((struct ufs_hba *)platform_get_drvdata(pdev));
>> -}
>
> How about exporting this function? ufs-qcom and other variant can use
> this.
>

you got a point here. will undo the deletion of this routine.

>> +#ifndef UFSHCD_PLTFRM_H_
>> +#define UFSHCD_PLTFRM_H_
>> +
>> +#include "ufshcd.h"
>> +
>> +int ufshcd_pltfrm_init(struct platform_device *pdev,
>> +                      struct ufs_hba_variant_ops *vops);
>
> struct platform_device appears before including <linux/platform_device.h>.
>

as there are no build errors, i assume that ufshcd.h include some
other headers that contain include of <linux/platform_device.h>.

>> +/* variant specific ops structures */
>> +#ifdef CONFIG_SCSI_UFS_QCOM
>> +extern struct ufs_hba_variant_ops ufs_hba_qcom_variant;
>> +#endif
>
> What is ufs_hba_qcom_variant? I can't find in kernel source and
> this patch series.

will be removed. (leftovers from different version).

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


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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web