| Message ID | 1432043231-31102-4-git-send-email-ygardi@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
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 <ygardi@codeaurora.org> > --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org 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. > 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 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-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2015-05-21 at 10:09 +0000, ygardi@codeaurora.org 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Thu, 2015-05-21 at 10:09 +0000, ygardi@codeaurora.org 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 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-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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(-)