| Message ID | 1439720057-9186-7-git-send-email-ygardi@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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-scsi" 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/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 */
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