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 (3):
phy: qcom-ufs: fix build error when the driver 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
drivers/phy/phy-qcom-ufs.c | 11 +++++++++++
drivers/scsi/ufs/Kconfig | 2 +-
drivers/scsi/ufs/ufs-qcom.c | 2 +-
3 files changed, 13 insertions(+), 2 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
Export the following functions in order to avoid build errors
when the driver 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
This change fixes a compilation warning that happens if SCSI_UFS_QCOM
is compiled as a module.
Signed-off-by: Yaniv Gardi <[email protected]>
---
drivers/scsi/ufs/ufs-qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 6652a81..2e4ad21 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -885,7 +885,7 @@ out:
#define ANDROID_BOOT_DEV_MAX 30
static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
-static int get_android_boot_dev(char *str)
+static int __maybe_unused get_android_boot_dev(char *str)
{
strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
return 1;
--
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
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
On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
> This change is required in order to be able to build the component
> as a module.
>
> Signed-off-by: Yaniv Gardi <[email protected]>
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> 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
As far as I can see, in next-20150519, drivers/scsi/ufs/ufs-qcom.c lacks
the required module specific boilerplate for this to be useful. Is that
boilerplate added in another series?
Thanks,
Paul Bolle
On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
> Export the following functions in order to avoid build errors
> when the driver is compiled as a module:
Where "the driver" actually means something like ufs-qcom.c, or
SCSI_UFS_QCOM, or "QCOM specific hooks to UFS controller platform
driver", right?
> 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
Thanks,
Paul Bolle
On Wed, 2015-05-20 at 09:21 +0200, Paul Bolle wrote:
> As far as I can see, in next-20150519, drivers/scsi/ufs/ufs-qcom.c lacks
> the required module specific boilerplate for this to be useful. Is that
> boilerplate added in another series?
I need to rephrase this. Let me try again.
As far as I can see, in next-20150519, drivers/scsi/ufs/ufs-qcom.c lacks
a MODULE_LICENSE() macro. Without that macro loading the module should
trigger a warning and taint the kernel, right?
By the way, as far as I can see, this (new) module can only be loaded
manually (or via scripts). Is that what people want?
Thanks,
Paul Bolle
On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -885,7 +885,7 @@ out:
>
> #define ANDROID_BOOT_DEV_MAX 30
> static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
> -static int get_android_boot_dev(char *str)
> +static int __maybe_unused get_android_boot_dev(char *str)
> {
> strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
> return 1;
Wouldn't it be clearer to wrap these few lines (until after the
__setup() macro) with #ifndef MODULE?
And I think get_android_boot_dev() could be marked __init. Because if
it's built-in it will never be called after the kernel has finished
booting, right?
Paul Bolle
> On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
>> Export the following functions in order to avoid build errors
>> when the driver is compiled as a module:
>
> Where "the driver" actually means something like ufs-qcom.c, or
> SCSI_UFS_QCOM, or "QCOM specific hooks to UFS controller platform
> driver", right?
>
yes, Paul, you are correct.
>> 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
>
> Thanks,
>
>
> Paul Bolle
>
>
> On Tue, 2015-05-19 at 16:47 +0300, Yaniv Gardi wrote:
>> --- a/drivers/scsi/ufs/ufs-qcom.c
>> +++ b/drivers/scsi/ufs/ufs-qcom.c
>> @@ -885,7 +885,7 @@ out:
>>
>> #define ANDROID_BOOT_DEV_MAX 30
>> static char android_boot_dev[ANDROID_BOOT_DEV_MAX];
>> -static int get_android_boot_dev(char *str)
>> +static int __maybe_unused get_android_boot_dev(char *str)
>> {
>> strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX);
>> return 1;
>
> Wouldn't it be clearer to wrap these few lines (until after the
> __setup() macro) with #ifndef MODULE?
>
> And I think get_android_boot_dev() could be marked __init. Because if
> it's built-in it will never be called after the kernel has finished
> booting, right?
>
That's correct. I will upload an update to this patch.
I accept both comments
#ifndef MODULE is indeed clearer
and __init flag as well, since get_android_boot_dev() will not be called
again after booting finished.
>
> Paul Bolle
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Wed, 2015-05-20 at 10:22 +0200, Paul Bolle wrote:
> By the way, as far as I can see, this (new) module can only be loaded
> manually (or via scripts). Is that what people want?
This comment wasn't well thought through. So I hand another look at the
code of usf-qcom.
I noticed that the single thing ufs-qcom exports is "struct
ufs_hba_qcom_vops". But that's unused in next-20150520:
$ git grep -nw ufs_hba_qcom_vops
drivers/scsi/ufs/ufs-qcom.c:999: * struct ufs_hba_qcom_vops - UFS QCOM specific variant operations
drivers/scsi/ufs/ufs-qcom.c:1004:static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
drivers/scsi/ufs/ufs-qcom.c:1016:EXPORT_SYMBOL(ufs_hba_qcom_vops);
So it's not used by code outside of ufs-qcom.c. Probably because it
can't actually be used by outside code. It's not mentioned in any public
header and it's even static!
Am I missing something obvious here? Because ufs-qcom currently looks
pointless to me, and I actually see little reason to even have it in the
mainline tree.
Paul Bolle
> On Wed, 2015-05-20 at 10:22 +0200, Paul Bolle wrote:
>> By the way, as far as I can see, this (new) module can only be loaded
>> manually (or via scripts). Is that what people want?
>
> This comment wasn't well thought through. So I hand another look at the
> code of usf-qcom.
>
> I noticed that the single thing ufs-qcom exports is "struct
> ufs_hba_qcom_vops". But that's unused in next-20150520:
> $ git grep -nw ufs_hba_qcom_vops
> drivers/scsi/ufs/ufs-qcom.c:999: * struct ufs_hba_qcom_vops - UFS QCOM
> specific variant operations
> drivers/scsi/ufs/ufs-qcom.c:1004:static const struct
> ufs_hba_variant_ops ufs_hba_qcom_vops = {
> drivers/scsi/ufs/ufs-qcom.c:1016:EXPORT_SYMBOL(ufs_hba_qcom_vops);
>
> So it's not used by code outside of ufs-qcom.c. Probably because it
> can't actually be used by outside code. It's not mentioned in any public
> header and it's even static!
>
> Am I missing something obvious here? Because ufs-qcom currently looks
> pointless to me, and I actually see little reason to even have it in the
> mainline tree.
>
we haven't uploaded yet the patch that binds qcom vops to the driver, but
we will soon.
>
> Paul Bolle
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Thu, 2015-05-21 at 10:09 +0000, [email protected] wrote:
> > On Wed, 2015-05-20 at 10:22 +0200, Paul Bolle wrote:
> > Am I missing something obvious here? Because ufs-qcom currently looks
> > pointless to me, and I actually see little reason to even have it in the
> > mainline tree.
> >
>
> we haven't uploaded yet the patch that binds qcom vops to the driver, but
> we will soon.
Perhaps you could make that patch part of v2 of this series. I see
little point in this series without that patch. Perhaps someone else
still cares about it, but I'm not looking at it anymore.
Thanks,
Paul Bolle
> On Thu, 2015-05-21 at 10:09 +0000, [email protected] wrote:
>> > On Wed, 2015-05-20 at 10:22 +0200, Paul Bolle wrote:
>> > Am I missing something obvious here? Because ufs-qcom currently looks
>> > pointless to me, and I actually see little reason to even have it in
>> the
>> > mainline tree.
>> >
>>
>> we haven't uploaded yet the patch that binds qcom vops to the driver,
>> but
>> we will soon.
>
> Perhaps you could make that patch part of v2 of this series. I see
> little point in this series without that patch. Perhaps someone else
> still cares about it, but I'm not looking at it anymore.
>
fair enough. i will upload a V2 series soon. thanks for your inputs.
> Thanks,
>
>
> Paul Bolle
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>