Hi, On 3/4/2016 9:12 PM, Arnd Bergmann wrote: > On Friday 04 March 2016 17:22:18 Joao Pinto wrote: >> This patch adds a glue platform driver for the Synopsys G210 Test Chip. >> >> Signed-off-by: Joao Pinto <jpinto@xxxxxxxxxxxx> > > Looks basically ok, but I think it can be simplified a little: > >> +/** >> + * struct ufs_hba_dwc_vops - UFS DWC specific variant operations >> + */ >> +static struct ufs_hba_variant_ops tc_dwc_g210_pltfm_hba_vops = { >> + .name = "tc-dwc-g210-pltfm", >> + .link_startup_notify = ufshcd_dwc_link_startup_notify, >> +}; >> + >> +/** >> + * tc_dwc_g210_pltfm_probe() >> + * @pdev: pointer to platform device structure >> + * >> + */ >> +static int tc_dwc_g210_pltfm_probe(struct platform_device *pdev) >> +{ >> + int err; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; >> + >> + /* Check Test Chip type and set the specific setup routine */ >> + if (of_device_is_compatible(np, "snps, g210-tc-6.00-20bit")) { >> + tc_dwc_g210_pltfm_hba_vops.custom_phy_initialization = >> + tc_dwc_g210_config_20_bit; >> + } else if (of_device_is_compatible(np, "snps, g210-tc-6.00-40bit")) { >> + tc_dwc_g210_pltfm_hba_vops.custom_phy_initialization = >> + tc_dwc_g210_config_40_bit; >> + } > > Instead of manually checking the compatible string, define two copies > of the ufs_hba_variant_ops, and put a pointer to them into the > .data field of tc_dwc_g210_pltfm_match. Sounds nice. I'll do that! > > Arnd > Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html