From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750978AbcBJUvM (ORCPT ); Wed, 10 Feb 2016 15:51:12 -0500 Received: from mout.kundenserver.de ([217.72.192.75]:65223 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750713AbcBJUvJ (ORCPT ); Wed, 10 Feb 2016 15:51:09 -0500 From: Arnd Bergmann To: Joao Pinto Cc: vinholikatti@gmail.com, julian.calaby@gmail.com, akinobu.mita@gmail.com, hch@infradead.org, mark.rutland@arm.com, martin.petersen@oracle.com, 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 v6 2/2] add support for DWC UFS Host Controller Date: Wed, 10 Feb 2016 21:50:28 +0100 Message-ID: <6667560.LIsUD7xT9u@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <0787e532da876f7f38b4f439ec2e16bfcbaa9cb3.1455120183.git.jpinto@synopsys.com> References: <0787e532da876f7f38b4f439ec2e16bfcbaa9cb3.1455120183.git.jpinto@synopsys.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:vFDwj6ygGGH3aNwTIa4HFNleIPXr1A/7PxjIo2L6rpBbLUXF9fX beA5t1NPLH0yxSEH9182gMaHdzxY8rFSK5gecXm+dL+7qLG6O0wXbvXwXtl0PXOWk8xRd0x WwdSOmGF/JM7zRj5Z0kMgGdkWGp6ct3bWGo3+I/TWTeQeNCFDGHckzW9H5lMxxSsUAnvOC1 Z/quJiOeTWWs0ALT4M6CA== X-UI-Out-Filterresults: notjunk:1;V01:K0:EJ047HqfBy4=:SXoJcPadHl6tgj8anreye2 E9ScPFQgzBUNpTHPhtR9lwkk+3hgnJj6oVTvHTMfQQ+zeTyC74+nSj/v9CroYq3SFUiuKMXAT OBqDQs3TqJ6NdT0oWiKTty/OZQGR3oZLoiCTpf50T//kRWb5N0Xe+Pmq48AOUA/kpmB42ZqGd OWMqy6zosT1+WdQfZiJTMyqrBXQWUQlt2dizWAkSZ7FwVfwUEyWWnWFssCyTmixnF7ZBrx6E7 i5wFxQN7bo/GjkLtCpcjlmXhISMy/LUnZ068tCeZdas4TDt2z1Chc8Zn7ZX6BDLX/lNCF2fUx YtIi4gPdyur5VZcWdyxuFPWXv/4muxkJxqgM82UmFOzHLCHCiiGXqb/f7c73r9zeqKg/CuXiq lXWU301BZzWol66fkgenOn9ZfiHNlwcF+lH2m62LTnND5RZtonGn2QqX/bGtj2TeSC9ZSJqI6 6zE0+Fo6pTZxNifyvF1J3cbthzfXIEP085RgVPjINhsVP4B2kgQ0uQdQYK0HwMhgX1Z54garq eLmj4beSF0EFS9ze195nEFt58tBKPOz8CWKayF7ccO1LpsAo+Rh2lkQNpR4q1d5JMD+QOoxQu 0yjMq/DQ3YzTZEp+F7uo1yC21s++MBGKYzavERNka8fmk6MZZqJ2RZWmvWbESxKdvt6dzB9oG OqQygODE/k2lv7F+P2dn9IQ4x6O+Sk7kDg8jTriwtCtjrO2IzPpyjmNRJwm8WDaieQsfZycQQ Xp+nXIzofy/iMBD+ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 10 February 2016 16:06:13 Joao Pinto wrote: > This patch has the goal to add support for DesignWare UFS Controller > specific operations and to add specific platform and pci drivers. > > Signed-off-by: Joao Pinto > Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 17 + > MAINTAINERS | 6 + > drivers/scsi/ufs/Kconfig | 41 ++ > drivers/scsi/ufs/Makefile | 3 + > drivers/scsi/ufs/ufs-dwc-pci.c | 180 ++++++ > drivers/scsi/ufs/ufs-dwc.c | 102 +++ > drivers/scsi/ufs/ufshcd-dwc.c | 736 ++++++++++++++++++++++ > drivers/scsi/ufs/ufshcd-dwc.h | 18 + > drivers/scsi/ufs/ufshcd.c | 50 +- > drivers/scsi/ufs/ufshcd.h | 13 + > drivers/scsi/ufs/ufshci-dwc.h | 42 ++ > drivers/scsi/ufs/ufshci.h | 1 + > drivers/scsi/ufs/unipro.h | 39 ++ Can you split this into separate patches for changes to the common code, the addition of the PCI driver and the addition of the platform driver? > diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > new file mode 100644 > index 0000000..f38a3f5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > @@ -0,0 +1,17 @@ > +* 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 string ("snps,ufshcd-1.0", "snps,ufshcd-1.1" > + or "snps,ufshcd-2.0") > +- reg : > +- interrupts : > + > +Example: > + dwc_ufshcd@0xD0000000 { Please fix the node name and address in the example. I think you want "ufs@d0000000". > +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_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. I don't think I understood you explanation why this has to be here, rather than using a proper PHY driver. Please try again. > + > +#ifdef CONFIG_PM > +/** > + * ufs_dw_pci_suspend - suspend power management function > + * @pdev: pointer to PCI device handle > + * @state: power state > + * > + * Returns 0 if successful > + * Returns non-zero otherwise > + */ > +static int ufs_dw_pci_suspend(struct device *dev) > +{ > + return ufshcd_system_suspend(dev_get_drvdata(dev)); > +} Please remove the #ifdef here. > + > +static const struct dev_pm_ops ufs_dw_pci_pm_ops = { > + .suspend = ufs_dw_pci_suspend, > + .resume = ufs_dw_pci_resume, > + .runtime_suspend = ufs_dw_pci_runtime_suspend, > + .runtime_resume = ufs_dw_pci_runtime_resume, > + .runtime_idle = ufs_dw_pci_runtime_idle, > +}; Instead, use the macros from include/linux/pm.h > +#ifdef CONFIG_SCSI_UFS_DWC_40BIT_RMMI > +/** > + * ufshcd_dwc_setup_40bit_rmmi() > + * This function configures Synopsys MPHY specific atributes (40-bit RMMI) > + * @hba: Pointer to drivers structure > + * > + * Returns 0 on success or non-zero value on failure > + */ > +static int ufshcd_dwc_setup_40bit_rmmi(struct ufs_hba *hba) > +{ > + int ret = 0; This looks like it should go into the external driver > + > +#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 > + 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 In particular, the part above cannot possibly work: When a distro ships a kernel with CONFIG_SCSI_UFS_DWC_40BIT_RMMI set, it won't ever call the ufshcd_dwc_setup_20bit_rmmi() function, regardless of what the hardware is. Arnd