On Wednesday 03 February 2016 11:28:26 Joao Pinto wrote: > > Signed-off-by: Joao Pinto <jpinto@xxxxxxxxxxxx> 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 <linux/pci.h> > #include <linux/pm_runtime.h> > > @@ -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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html