From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752724AbcCGKGh (ORCPT ); Mon, 7 Mar 2016 05:06:37 -0500 Received: from us01smtprelay-2.synopsys.com ([198.182.47.9]:45920 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752611AbcCGKGX (ORCPT ); Mon, 7 Mar 2016 05:06:23 -0500 Subject: Re: [PATCH v10 5/6] add TC G210 platform driver To: Arnd Bergmann , Joao Pinto References: <2253265.cRVI7LPIKe@wuerfel> CC: , , , , , , , , , , , From: Joao Pinto Message-ID: <56DD51C0.30900@synopsys.com> Date: Mon, 7 Mar 2016 10:02:40 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <2253265.cRVI7LPIKe@wuerfel> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.13.184.19] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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