From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757823AbcBCMzH (ORCPT ); Wed, 3 Feb 2016 07:55:07 -0500 Received: from mout.kundenserver.de ([217.72.192.75]:61218 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757693AbcBCMzB (ORCPT ); Wed, 3 Feb 2016 07:55:01 -0500 From: Arnd Bergmann To: Joao Pinto Cc: santosh.sy@samsung.com, h.vinayak@samsung.com, julian.calaby@gmail.com, akinobu.mita@gmail.com, hch@infradead.org, gbroner@codeaurora.org, subhashj@codeaurora.org, CARLOS.PALMINHA@synopsys.com, ijc+devicetree@hellion.org.uk, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH 3/3] add support for DWC UFS Host Controller Date: Wed, 03 Feb 2016 13:54:25 +0100 Message-ID: <2072510.CA47OHQsUN@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:tnNlyiirBPHynYQYryrDKxJ71HQWBhwip5jfD27xbz+7eKWUQ3J g5tXoCntGvy4oEYN/I2boy6GZFzyKRWHazX7bhI9wV56lG/88ovB1V5dtmnsCF8eeot673N 3u6/dKfeBfBkq/BQffzcLzzo+VzFqlwqf8HXbD2mJHcToAZH5qiI/JIWdqh4RQfDVHeRUD8 1GOX9zwNCNaWLksAWdRXQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:Yf+lUFLkRek=:Y0L/Yjf43bV71gqppCu498 MDyzrEp7hJRWC6ZwV32xT+94qT9yG7oydyz2njmrarRE1/0sf1kAloJY53khPcfFff9Bv8x29 Pr1IUksi86QuernPPs1+JxRUcPkESnO/YaYnuyCQhmm6UWRovezgaUoH2JZYA+AUCxYqgc2pL wBBIoIiEvsP7Ne9h2KyFNbEa4bn0Bt2WbKrNj8B+ASAVlkGdk6YvXpjLkB0fuPPrT2dsyLNCn SVwJT+or22RltCvWVkCYNTLL3VyJcLFtI7WVsxmOGYMbm6tZnp/1z0AnxPHQgELalDEaJAU5Y uh+xOox0COMoNFaUgSdZ9nB9NH8HVRF8TV5lRuvloCOaUoP0sQw0dHpdmMeIbheymGVWUHDB3 4sXgi6doZQY1c9BQLOOJYNiLEr97aZiQvWCsBVd129vKU10+sA/bq4YyZml4By8PORxL+7vwC rIyPhweWWvGYKfB3suelKfjVnL5VtKWnm4G5mohIYebgslo09aQVz7sqRRMl0XivwSYR4JhTd TnFEeQmkBRTIqY2FIrkYGK/HQuh7FJpmnGkmVckK15wigAfCM/1q/dQLm63NOLLo7AqPjEPFe Pt8wYTDDO6UC8BSPXFmWYjy0b4+uwNSKRe8EPpHoEpEBrpTmOSLlP6sop67Y0o/hdXBLRrBwt fkLzjyhe+OXFJOMaaEux2J8JpZjcQRrVSMeXAxWDRY+cxS5daoRz/UQ6QPJfTGIHPKQI+X+KU WavxZuGHrxAayQwz Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 03 February 2016 11:28:26 Joao Pinto wrote: > > Signed-off-by: Joao Pinto This needs a changelog comment, like every patch. > @@ -0,0 +1,16 @@ > +* Universal Flash Storage (UFS) DesignWare Host Controller > + > +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers. > +Each UFS controller instance should have its own node. > + > +Required properties: > +- compatible : compatible list, contains "snps,ufshcd" Are there multiple versions of this controller? Usually for designware parts the version is known, so we should document which versions exist > + > +config SCSI_UFS_DWC_HOOKS > + bool "DesignWare hooks to UFS controller" > + depends on SCSI_UFSHCD > + ---help--- > + This selects the DesignWare hooks for the UFS host controller. > + > + Select this if you have a DesignWare UFS controller. > + If unsure, say N. This could be a silent symbol that gets selected by SCSI_UFS_DWC_PLAT > +config SCSI_UFS_DWC_MPHY_TC > + bool "Support for the Synopsys MPHY Test Chip" > + depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) > + ---help--- > + This selects the support for the Synopsys MPHY Test Chip. > + > + Select this if you have a Synopsys MPHY Test Chip. > + If unsure, say N. > + > +config SCSI_UFS_DWC_20BIT_RMMI > + bool "20-bit RMMI MPHY" > + depends on SCSI_UFS_DWC_MPHY_TC > + ---help--- > + This specifies that the Synopsys MPHY supports 40-bit RMMI operations. > + > + Select this if you are using a 40-bit RMMI Synopsys MPHY. > + If unsure, say N. > + > +config SCSI_UFS_DWC_40BIT_RMMI > + bool "40-bit RMMI MPHY" > + depends on SCSI_UFS_DWC_MPHY_TC > + ---help--- > + This specifies that the Synopsys MPHY supports 40-bit RMMI operations. > + > + Select this if you are using a 40-bit RMMI Synopsys MPHY. > + If unsure, say N. Move the PHY config options to drivers/phy/ along with the respective drivers, the device using the PHY should not need to be aware which one is being used. > +/** > + * ufshcd_dwc_setup_mphy() > + * This function configures Local (host) Synopsys MPHY specific attributes > + * > + * @hba: Pointer to drivers structure > + * > + * Returns 0 on success non-zero value on failure > + */ > +int ufshcd_dwc_setup_mphy(struct ufs_hba *hba) > +{ > + int ret = 0; > + > +#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI > + dev_info(hba->dev, "Configuring MPHY 40-bit RMMI"); > + ret = ufshcd_dwc_setup_40bit_rmmi(hba); > + if (ret) { > + dev_err(hba->dev, "40-bit RMMI configuration failed"); > + goto out; > + } > +#else > +#ifdef CONFIG_SCSI_UFS_DWC_20BIT_RMMI > + dev_info(hba->dev, "Configuring MPHY 20-bit RMMI"); > + ret = ufshcd_dwc_setup_20bit_rmmi(hba); > + if (ret) { > + dev_err(hba->dev, "20-bit RMMI configuration failed"); > + goto out; > + } > +#endif > +#endif > + /* To write Shadow register bank to effective configuration block */ > + ret = ufshcd_dme_set(hba, UIC_ARG_MIB(VS_MPHYCFGUPDT), 0x01); > + if (ret) > + goto out; > + > + /* To configure Debug OMC */ > + ret = ufshcd_dme_set(hba, UIC_ARG_MIB(VS_DEBUGOMC), 0x01); > + > +out: > + return ret; > +} Try to use the generic PHY abstraction here and remove all the #ifdef etc. > diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c > index d15eaa4..66518a1 100644 > --- a/drivers/scsi/ufs/ufshcd-pci.c > +++ b/drivers/scsi/ufs/ufshcd-pci.c > @@ -34,6 +34,9 @@ > */ > > #include "ufshcd.h" > +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS > +#include "ufshcd-dwc.h" > +#endif > #include > #include > > @@ -83,6 +86,16 @@ static int ufshcd_pci_runtime_idle(struct device *dev) > #define ufshcd_pci_runtime_idle NULL > #endif /* CONFIG_PM */ > > +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS > +/** > + * struct ufs_hba_dwc_vops - UFS DWC specific variant operations > + */ > +static struct ufs_hba_variant_ops ufs_dwc_pci_hba_vops = { > + .name = "dwc-pci", > + .custom_probe_hba = ufshcd_dwc_configuration, > +}; > +#endif > + > /** > * ufshcd_pci_shutdown - main function to put the controller in reset state > * @pdev: pointer to PCI device handle > @@ -144,6 +157,9 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > INIT_LIST_HEAD(&hba->clk_list_head); > > +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS > + hba->vops = &ufs_dwc_pci_hba_vops; > +#endif > err = ufshcd_init(hba, mmio_base, pdev->irq); > if (err) { > dev_err(&pdev->dev, "Initialization failed\n"); > @@ -167,6 +183,10 @@ static const struct dev_pm_ops ufshcd_pci_pm_ops = { > > static const struct pci_device_id ufshcd_pci_tbl[] = { > { PCI_VENDOR_ID_SAMSUNG, 0xC00C, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, > +#ifdef CONFIG_SCSI_UFS_DWC_HOOKS > + { PCI_VENDOR_ID_SYNOPSYS, 0xB101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, > + { PCI_VENDOR_ID_SYNOPSYS, 0xB102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, > +#endif > { } /* terminate list */ > }; I think you're better off with a separate PCI driver for this. Remove all the #ifdef mess here, put whatever is dwc specific into a new file, and perhaps move the common parts into a shared file that can be used by both the samsung and designware drivers. Arnd