[PATCH v3 0/4] fixing building errors and warnings when components

[PATCH v3 0/4] fixing building errors and warnings when components

[Yaniv Gardi]

V3:
In this version, in ufshcd-pltfrm.c probe and remove function, added:
__module_get(ufs_variant_pdev->dev.driver->owner);
module_put(ufs_variant_pdev->dev.driver->owner);
in order to maintain a reference counter to prevent unloading
ufs-qcom component if it is the variant that ufshcd-pltfrm is using.

V2:
In this Version, comments from reviewers were addressed
and also, added another change: PATCH v2 4/4
In this change we glue the variant attributes (vops, etc)
at the time of the platform probing, so they can be used
by the driver when it doing its initialization phase.

V1:
The following combination of components, when SCSI_UFS_QCOM=y
and PHY_QCOM_UFS=m is illegal and causes build errors.
The 3rd patch in the series enables the SCSI_UFS_QCOM component to 
be compiled as a module (by changing its configuration to tristate).
So now, compiling SCSI_UFS_QCOM=m forces PHY_QCOM_UFS=m, and 
SCSI_UFS_QCOM=y forces PHY_QCOM_UFS=y.

In addition, when PHY_QCOM_UFS=m, external functions in 
phy-ufs-qcom.c should be exported. The 1st patch fixes it.

Another issue that we see when SCSI_UFS_QCOM=m is a warning that
the 2nd patch fixes.

notice:
checkpatch gives an error on the commit message of patch 1/3
in the series. Ignore as the commit message is the build errors
that this patch fixes.

Yaniv Gardi (4):
  phy: qcom-ufs: fix build error when the component is built as a module
  scsi: ufs-qcom: fix compilation warning if compiled as a module
  scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component
  scsi: ufs: probe and init of variant driver from the platform device

 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  8 ++++
 drivers/phy/phy-qcom-ufs.c                         | 11 +++++
 drivers/scsi/ufs/Kconfig                           |  2 +-
 drivers/scsi/ufs/ufs-qcom.c                        | 48 +++++++++++++++++++++-
 drivers/scsi/ufs/ufshcd-pltfrm.c                   | 45 ++++++++++++++------
 5 files changed, 98 insertions(+), 16 deletions(-)

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

[PATCH v3 1/4] phy: qcom-ufs: fix build error when the component is built as a module

[Yaniv Gardi]

Export the following functions in order to avoid build errors
when the component PHY_QCOM_UFS is compiled as a module:

ERROR: "ufs_qcom_phy_disable_ref_clk" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_enable_ref_clk" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_is_pcs_ready" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_disable_iface_clk" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_start_serdes" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_calibrate_phy" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_enable_dev_ref_clk" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_set_tx_lane_enable" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_disable_dev_ref_clk" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_save_controller_version" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
ERROR: "ufs_qcom_phy_enable_iface_clk" [drivers/scsi/ufs/ufs-qcom.ko] undefined!
make[1]: *** [__modpost] Error 1

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

---
 drivers/phy/phy-qcom-ufs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c
index f9c618f..6140a8b 100644
--- a/drivers/phy/phy-qcom-ufs.c
+++ b/drivers/phy/phy-qcom-ufs.c
@@ -432,6 +432,7 @@ out_disable_src:
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_ref_clk);
 
 static
 int ufs_qcom_phy_disable_vreg(struct phy *phy,
@@ -474,6 +475,7 @@ void ufs_qcom_phy_disable_ref_clk(struct phy *generic_phy)
 		phy->is_ref_clk_enabled = false;
 	}
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_ref_clk);
 
 #define UFS_REF_CLK_EN	(1 << 5)
 
@@ -517,11 +519,13 @@ void ufs_qcom_phy_enable_dev_ref_clk(struct phy *generic_phy)
 {
 	ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, true);
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_dev_ref_clk);
 
 void ufs_qcom_phy_disable_dev_ref_clk(struct phy *generic_phy)
 {
 	ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, false);
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_dev_ref_clk);
 
 /* Turn ON M-PHY RMMI interface clocks */
 int ufs_qcom_phy_enable_iface_clk(struct phy *generic_phy)
@@ -550,6 +554,7 @@ int ufs_qcom_phy_enable_iface_clk(struct phy *generic_phy)
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_iface_clk);
 
 /* Turn OFF M-PHY RMMI interface clocks */
 void ufs_qcom_phy_disable_iface_clk(struct phy *generic_phy)
@@ -562,6 +567,7 @@ void ufs_qcom_phy_disable_iface_clk(struct phy *generic_phy)
 		phy->is_iface_clk_enabled = false;
 	}
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_iface_clk);
 
 int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
 {
@@ -578,6 +584,7 @@ int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_start_serdes);
 
 int ufs_qcom_phy_set_tx_lane_enable(struct phy *generic_phy, u32 tx_lanes)
 {
@@ -595,6 +602,7 @@ int ufs_qcom_phy_set_tx_lane_enable(struct phy *generic_phy, u32 tx_lanes)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_set_tx_lane_enable);
 
 void ufs_qcom_phy_save_controller_version(struct phy *generic_phy,
 					  u8 major, u16 minor, u16 step)
@@ -605,6 +613,7 @@ void ufs_qcom_phy_save_controller_version(struct phy *generic_phy,
 	ufs_qcom_phy->host_ctrl_rev_minor = minor;
 	ufs_qcom_phy->host_ctrl_rev_step = step;
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_save_controller_version);
 
 int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B)
 {
@@ -625,6 +634,7 @@ int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate_phy);
 
 int ufs_qcom_phy_remove(struct phy *generic_phy,
 			struct ufs_qcom_phy *ufs_qcom_phy)
@@ -662,6 +672,7 @@ int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy)
 	return ufs_qcom_phy->phy_spec_ops->
 			is_physical_coding_sublayer_ready(ufs_qcom_phy);
 }
+EXPORT_SYMBOL_GPL(ufs_qcom_phy_is_pcs_ready);
 
 int ufs_qcom_phy_power_on(struct phy *generic_phy)
 {
-- 
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

[PATCH v3 2/4] scsi: ufs-qcom: fix compilation warning if compiled as a module

[Yaniv Gardi]

This change fixes a compilation warning that happens if SCSI_UFS_QCOM
is compiled as a module.
Also this patch fixes an error happens when insmod the module:
"ufs_qcom: module license 'unspecified' taints kernel."

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

---
 drivers/scsi/ufs/ufs-qcom.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 6652a81..6671a8e 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -885,12 +885,15 @@ out:
 
 #define	ANDROID_BOOT_DEV_MAX	30
 static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
-static int get_android_boot_dev(char *str)
+
+#ifndef MODULE
+static int __init get_android_boot_dev(char *str)
 {
 	strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
 	return 1;
 }
 __setup("androidboot.bootdevice=", get_android_boot_dev);
+#endif
 
 /**
  * ufs_qcom_init - bind phy with controller
@@ -1014,3 +1017,5 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.resume			= ufs_qcom_resume,
 };
 EXPORT_SYMBOL(ufs_hba_qcom_vops);
+
+MODULE_LICENSE("GPL v2");
-- 
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

[PATCH v3 3/4] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component

[Yaniv Gardi]

This change is required in order to be able to build the component
as a module.

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

---
 drivers/scsi/ufs/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index e945383..5f45307 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -72,7 +72,7 @@ config SCSI_UFSHCD_PLATFORM
 	  If unsure, say N.
 
 config SCSI_UFS_QCOM
-	bool "QCOM specific hooks to UFS controller platform driver"
+	tristate "QCOM specific hooks to UFS controller platform driver"
 	depends on SCSI_UFSHCD_PLATFORM && ARCH_QCOM
 	select PHY_QCOM_UFS
 	help
-- 
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

[PATCH v3 4/4] scsi: ufs: probe and init of variant driver from the platform device

[Yaniv Gardi]

It does so by adding the following changes:
1. Introducing SCSI_UFS_QCOM as a platform device. Its probe
   function registers a set of vops to its driver_data.
2. Adding an optional device tree sub-node, under SCSI_UFSHCD_PLATFORM.
   Now, the probe function of SCSI_UFSHCD_PLATFORM invokes the probe
   function of its sub-node (if it exists), by calling
   of_platform_populate(). It ensures that vops are set, and ready to
   be used, by the time the UFSHCD driver starts its initialization
   procedure.

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

---
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  8 ++++
 drivers/scsi/ufs/ufs-qcom.c                        | 41 +++++++++++++++++++-
 drivers/scsi/ufs/ufshcd-pltfrm.c                   | 45 +++++++++++++++-------
 3 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index 5357919..e2f3058 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -55,3 +55,11 @@ Example:
 		clock-names = "core_clk", "ref_clk", "iface_clk";
 		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
 	};
+
+This is an example to a variant sub-node of ufshc:
+	ufs_variant {
+		compatible = "qcom,ufs_variant";
+	};
+
+This sub-node holds various specific information that is not defined in the
+standard and that may differ between platforms.
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 6671a8e..4391f1e 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1016,6 +1016,45 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.suspend		= ufs_qcom_suspend,
 	.resume			= ufs_qcom_resume,
 };
-EXPORT_SYMBOL(ufs_hba_qcom_vops);
+
+/**
+ * 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)
+{
+	dev_set_drvdata(&pdev->dev, (void *)&ufs_hba_qcom_vops);
+	return 0;
+}
+
+/**
+ * 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)
+{
+	dev_set_drvdata(&pdev->dev, NULL);
+	return 0;
+}
+
+static const struct of_device_id ufs_qcom_of_match[] = {
+	{ .compatible = "qcom,ufs_variant"},
+	{},
+};
+
+static struct platform_driver ufs_qcom_pltform = {
+	.probe	= ufs_qcom_probe,
+	.remove	= ufs_qcom_remove,
+	.driver	= {
+		.name	= "ufs_qcom",
+		.owner	= THIS_MODULE,
+		.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..a3f0a61 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -36,22 +36,11 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 
 #include "ufshcd.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)
 {
@@ -300,6 +289,9 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
 	struct resource *mem_res;
 	int irq, err;
 	struct device *dev = &pdev->dev;
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *ufs_variant_node;
+	struct platform_device *ufs_variant_pdev;
 
 	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mmio_base = devm_ioremap_resource(dev, mem_res);
@@ -321,7 +313,23 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	hba->vops = get_variant_ops(&pdev->dev);
+	err = of_platform_populate(node, NULL, NULL, &pdev->dev);
+	if (err)
+		dev_err(&pdev->dev,
+			"%s: of_platform_populate() failed\n", __func__);
+
+	ufs_variant_node = of_get_next_available_child(node, NULL);
+
+	if (!ufs_variant_node) {
+		dev_dbg(&pdev->dev, "failed to find ufs_variant_node child\n");
+	} else {
+		ufs_variant_pdev = of_find_device_by_node(ufs_variant_node);
+
+		if (ufs_variant_pdev) {
+			__module_get(ufs_variant_pdev->dev.driver->owner);
+			hba->vops = dev_get_drvdata(&ufs_variant_pdev->dev);
+		}
+	}
 
 	err = ufshcd_parse_clock_info(hba);
 	if (err) {
@@ -365,9 +373,20 @@ out:
 static int ufshcd_pltfrm_remove(struct platform_device *pdev)
 {
 	struct ufs_hba *hba =  platform_get_drvdata(pdev);
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *ufs_variant_node;
+	struct platform_device *ufs_variant_pdev;
+
+	ufs_variant_node = of_get_next_available_child(node, NULL);
+
+	if (!ufs_variant_node)
+		dev_dbg(&pdev->dev, "no ufs_variant_node found\n");
+	else
+		ufs_variant_pdev = of_find_device_by_node(ufs_variant_node);
 
 	pm_runtime_get_sync(&(pdev)->dev);
 	ufshcd_remove(hba);
+	module_put(ufs_variant_pdev->dev.driver->owner);
 	return 0;
 }
 
-- 
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

Re: [PATCH v3 4/4] scsi: ufs: probe and init of variant driver from the platform device

[Akinobu Mita]

2015-06-08 1:27 GMT+09:00 Yaniv Gardi <ygardi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>:
>  static int ufshcd_pltfrm_remove(struct platform_device *pdev)
>  {
>         struct ufs_hba *hba =  platform_get_drvdata(pdev);
> +       struct device_node *node = pdev->dev.of_node;
> +       struct device_node *ufs_variant_node;
> +       struct platform_device *ufs_variant_pdev;
> +
> +       ufs_variant_node = of_get_next_available_child(node, NULL);
> +
> +       if (!ufs_variant_node)
> +               dev_dbg(&pdev->dev, "no ufs_variant_node found\n");
> +       else
> +               ufs_variant_pdev = of_find_device_by_node(ufs_variant_node);
>
>         pm_runtime_get_sync(&(pdev)->dev);
>         ufshcd_remove(hba);
> +       module_put(ufs_variant_pdev->dev.driver->owner);

module_put() should only be called when ufs_variant sub-node exists
and hba->vops was found.
If ufs->vops == NULL or no ufs_variant sub-node exists, this line
causes uninitialized pointer dereference.

>         return 0;
>  }
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html