Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932600AbbHXIcc (ORCPT ); Mon, 24 Aug 2015 04:32:32 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:56654 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932489AbbHXIc3 (ORCPT ); Mon, 24 Aug 2015 04:32:29 -0400 Message-ID: <72921868681be4c1fd16e6e4e636c55b.squirrel@www.codeaurora.org> In-Reply-To: References: <1440079144-26461-1-git-send-email-ygardi@codeaurora.org> <1440079144-26461-7-git-send-email-ygardi@codeaurora.org> Date: Mon, 24 Aug 2015 08:32:27 -0000 Subject: Re: [PATCH v2 6/8] scsi: ufs: make the UFS variant a platform device From: ygardi@codeaurora.org To: "Rob Herring" Cc: "Yaniv Gardi" , "linux-arm-msm" , linux-scsi@vger.kernel.org, "Dolev Raviv" , "Gilad Broner" , linux-scsi-owner@vger.kernel.org, "Jej B" , "Santosh Y" , "Subhash Jadavani" , hch@infradead.org, "Paul Bolle" , "James E.J. Bottomley" , "Vinayak Holikatti" , "linux-kernel@vger.kernel.org" User-Agent: SquirrelMail/1.4.22-4.el6 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT X-Priority: 3 (Normal) Importance: Normal Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13792 Lines: 416 > O > n Aug 20, 2015 6:59 AM, "Yaniv Gardi" wrote: >> >> 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 >> >> --- >> drivers/scsi/ufs/ufs-qcom.c | 83 >> +++++++++++++++++++++++++++++++++++- >> drivers/scsi/ufs/ufshcd-pltfrm.c | 92 >> ++++++++++------------------------------ >> drivers/scsi/ufs/ufshcd-pltfrm.h | 41 ++++++++++++++++++ >> drivers/scsi/ufs/ufshcd.c | 10 +++++ >> drivers/scsi/ufs/ufshcd.h | 1 + >> 5 files changed, 156 insertions(+), 71 deletions(-) >> create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.h >> >> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c >> index 329ac84..725cd49 100644 >> --- a/drivers/scsi/ufs/ufs-qcom.c >> +++ b/drivers/scsi/ufs/ufs-qcom.c >> @@ -19,6 +19,7 @@ >> >> #include >> #include "ufshcd.h" >> +#include "ufshcd-pltfrm.h" >> #include "unipro.h" >> #include "ufs-qcom.h" >> #include "ufshci.h" >> @@ -1036,7 +1037,7 @@ void ufs_qcom_clk_scale_notify(struct ufs_hba >> *hba) >> * The variant operations configure the necessary controller and PHY >> * handshake during initialization. >> */ >> -static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = { >> +static struct ufs_hba_variant_ops ufs_hba_qcom_vops = { >> .name = "qcom", >> .init = ufs_qcom_init, >> .exit = ufs_qcom_exit, >> @@ -1050,4 +1051,84 @@ static const 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)) { > > Shouldn't this condition be caught by init above? actually yes, it should. > >> + 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); >> + 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"}, > > Is this documented? in V3 it is documented. thanks > >> + {}, >> +}; >> + >> +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", >> + .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..91c73934 100644 >> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c >> @@ -38,20 +38,7 @@ >> #include >> >> #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; >> -} >> +#include "ufshcd-pltfrm.h" >> >> static int ufshcd_parse_clock_info(struct ufs_hba *hba) >> { >> @@ -245,10 +232,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,43 +245,47 @@ 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)); >> } >> -#else /* !CONFIG_PM */ >> -#define ufshcd_pltfrm_suspend NULL >> -#define ufshcd_pltfrm_resume NULL >> -#define ufshcd_pltfrm_runtime_suspend NULL >> -#define ufshcd_pltfrm_runtime_resume NULL >> -#define ufshcd_pltfrm_runtime_idle NULL >> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_runtime_idle); >> + >> #endif /* CONFIG_PM */ >> >> -static void ufshcd_pltfrm_shutdown(struct platform_device *pdev) >> +void ufshcd_pltfrm_shutdown(struct platform_device *pdev) >> { >> - ufshcd_shutdown((struct ufs_hba *)platform_get_drvdata(pdev)); >> + ufshcd_shutdown((struct ufs_hba *)platform_get_drvdata(pdev)); >> } >> +EXPORT_SYMBOL_GPL(ufshcd_pltfrm_shutdown); >> >> /** >> * ufshcd_pltfrm_probe - probe routine of the driver > > Needs updating. in V3 it is updated. thanks > >> * @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 +313,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 +347,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 "); >> MODULE_AUTHOR("Vinayak Holikatti "); >> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.h >> b/drivers/scsi/ufs/ufshcd-pltfrm.h >> new file mode 100644 >> index 0000000..df64c41 >> --- /dev/null >> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.h >> @@ -0,0 +1,41 @@ >> +/* 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); >> +void ufshcd_pltfrm_shutdown(struct platform_device *pdev); >> + >> +#ifdef CONFIG_PM >> + >> +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); >> + >> +#else /* !CONFIG_PM */ >> + >> +#define ufshcd_pltfrm_suspend NULL >> +#define ufshcd_pltfrm_resume NULL >> +#define ufshcd_pltfrm_runtime_suspend NULL >> +#define ufshcd_pltfrm_runtime_resume NULL >> +#define ufshcd_pltfrm_runtime_idle NULL >> + >> +#endif /* CONFIG_PM */ >> + >> +#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..f2aa47e 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 *); >> >> -- >> 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 > -- > 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/