2015-06-07 16:27:35

by Yaniv Gardi

[permalink] [raw]
Subject: [PATCH v3 0/4] fixing building errors and warnings when components

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


2015-06-07 16:28:26

by Yaniv Gardi

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

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 <[email protected]>

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

2015-06-07 16:27:48

by Yaniv Gardi

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

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 <[email protected]>

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

2015-06-07 16:28:10

by Yaniv Gardi

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

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

Signed-off-by: Yaniv Gardi <[email protected]>

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

2015-06-07 16:28:18

by Yaniv Gardi

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

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 <[email protected]>

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

2015-06-08 14:51:56

by Akinobu Mita

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

2015-06-08 1:27 GMT+09:00 Yaniv Gardi <[email protected]>:
> 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;
> }