Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753915AbcDSKQp (ORCPT ); Tue, 19 Apr 2016 06:16:45 -0400 Received: from smtprelay2.synopsys.com ([198.182.60.111]:42059 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753749AbcDSKQl convert rfc822-to-8bit (ORCPT ); Tue, 19 Apr 2016 06:16:41 -0400 From: Prabu Thangamuthu To: Ludovic Desroches , Prabu Thangamuthu CC: "Ulf Hansson (ulf.hansson@linaro.org)" , "Adrian Hunter" , Bjorn Helgaas , Andrew Morton , "David S. Miller" , Greg Kroah-Hartman , "Kalle Valo" , Mauro Carvalho Chehab , Guenter Roeck , Jiri Slaby , "Chaotian Jing" , Andrei Pistirica , Ben Hutchings , Joshua Henderson , Manjunath M Bettegowda , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH v2] mmc: sdhci-pci: add Support of Synopsys DWC_MSHC IP Thread-Topic: [PATCH v2] mmc: sdhci-pci: add Support of Synopsys DWC_MSHC IP Thread-Index: AQHRmg3xKTsp2u8IJkyHg/ZPCRx7U5+Q850g Date: Tue, 19 Apr 2016 10:16:30 +0000 Message-ID: <705D14B1C7978B40A723277C067CEDE2010A4B1062@IN01WEMBXB.internal.synopsys.com> References: <705D14B1C7978B40A723277C067CEDE2010A4B0FD7@IN01WEMBXB.internal.synopsys.com> <20160419073335.GJ7225@odux.rfo.atmel.com> In-Reply-To: <20160419073335.GJ7225@odux.rfo.atmel.com> Accept-Language: en-US, en-IN Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.144.52.95] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6543 Lines: 205 Hi Ludovic, Thank you for review comments. On Tue, April 19, 2016 1:04 PM, Ludovic Desroches wrote: > Hi Prabu, > > On Tue, Apr 19, 2016 at 07:18:36AM +0000, Prabu Thangamuthu wrote: > > Synopsys DWC_MSHC is compliant with SD Host Specifications. This patch > > is to support DWC_MSHC controller on PCI interface. > > > > +static void sdhci_set_clock_snps(struct sdhci_host *host, > > + unsigned int clock) > > +{ > > + int div = 0; > > + int mul = 0; > > + int div_val = 0; > > + int mul_val = 0; > > + int mul_div_val = 0; > > + int reg = 0; > > + u16 clk = 0; > > + u32 vendor_ptr; > > + unsigned long timeout; > > + u32 tx_clk_phase_val = SDHC_DEF_TX_CLK_PH_VAL; > > + u32 rx_clk_phase_val = SDHC_DEF_RX_CLK_PH_VAL; > > + > > + /* DWC MSHC Specific clock settings */ > > + > > + /* if clock is less than 25MHz, divided clock is used. > > For multilines comments: > /* > * If clock... > Unfortunately, I didn't get any Warning/Error when I ran "scripts/checkpatch.pl" against this Patch. I will update the Patch accordingly. > > + * For divided clock, we can use the standard sdhci_set_clock(). > > + * For clock above 25MHz, DRP clock is used > > + * Here, we cannot use sdhci_set_clock(), we need to program > > + * TX RX CLOCK DCM DRP for appropriate clock > > + */ > > + > > + vendor_ptr = sdhci_readw(host, SDHCI_UHS2_VENDOR); > > + > > + if (clock <= 25000000) { > > + /* Then call generic set_clock */ > > + sdhci_ops->set_clock(host, clock); > > + } else { > > + > > + host->mmc->actual_clock = 0; > > + > > + /* Select un-phase shifted clock before reset Tx Tuning > DCM*/ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg &= ~SDHC_TX_CLK_SEL_TUNED; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + mdelay(10); > > + > > + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL); > > + > > + if (clock == 0) > > + return; > > + > > + /* Lets chose the Mulitplier value to be 0x2 */ > > + mul = 0x2; > > + for (div = 1; div <= 32; div++) { > > + if (((host->max_clk * mul) / div) > > + <= clock) > > + break; > > + } > > + /* > > + * Set Programmable Clock Mode in the Clock > > + * Control register. > > + */ > > + div_val = div - 1; > > + mul_val = mul - 1; > > + > > + host->mmc->actual_clock = (host->max_clk * mul) / div; > > + /* Program the DCM DRP */ > > + /* Step 1: Assert DCM Reset */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg = reg | SDHC_CARD_TX_CLK_DCM_RST; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + > > + /* Step 2: Program the mul and div values in DRP */ > > + mul_div_val = (mul_val << 8) | div_val; > > + sdhci_writew(host, mul_div_val, > TXRX_CLK_DCM_MUL_DIV_DRP); > > + > > + /* Step 3: issue a dummy read from DRP base 0x00 as per > > + * > www.xilinx.com/support/documentation/user_guides/ug191.pdf > > + */ > > + reg = sdhci_readw(host, TXRX_CLK_DCM_DRP_BASE_51); > > + > > + /* Step 4: De-Assert reset to DCM */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg &= ~SDHC_CARD_TX_CLK_DCM_RST; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + > > + clk |= SDHCI_PROG_CLOCK_MODE | SDHCI_CLOCK_INT_EN; > > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > > + > > + /* Wait max 20 ms */ > > + timeout = 20; > > + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL)) > > + & SDHCI_CLOCK_INT_STABLE)) { > > + if (timeout == 0) { > > + pr_err("%s: Internal clock never stabilised\n", > > + mmc_hostname(host->mmc)); > > + return; > > + } > > + timeout--; > > + mdelay(1); > > + } > > + > > + clk |= SDHCI_CLOCK_CARD_EN; > > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > > + > > + /* For some bit-files we may have to do phase shifting for > > + * Tx Clock; Let's do it here > > + */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg = reg | SDHC_TUNING_TX_CLK_DCM_RST; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + mdelay(10); > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg &= ~SDHC_TUNING_TX_CLK_DCM_RST; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + > > + /* Select working phase value if clock is <= 50MHz */ > > + if (clock <= 50000000) { > > + /*Change the Phase value here */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + > vendor_ptr)); > > + reg = reg | (SDHC_TUNING_TX_CLK_SEL_MASK & > > + (tx_clk_phase_val << > SDHC_TUNING_TX_CLK_SEL_SHIFT)); > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + > vendor_ptr)); > > + mdelay(10); > > + > > + /* Program to select phase shifted clock */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + > vendor_ptr)); > > + reg = reg | SDHC_TX_CLK_SEL_TUNED; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + > vendor_ptr)); > > + mdelay(10); > > + } > > + > > + /* This Clock might have affected the RX CLOCK DCM > > + * used for Phase control; Reset this DCM for proper clock > > + */ > > + > > + /* Program the DCM DRP */ > > + /* Step 1: Reset the DCM */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg = reg | SDHC_TUNING_RX_CLK_DCM_RST; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + mdelay(10); > > + /* Step 2: De-Assert reset to DCM */ > > + reg = sdhci_readl(host, (SDHC_GPIO_OUT + vendor_ptr)); > > + reg &= ~SDHC_TUNING_RX_CLK_DCM_RST; > > + sdhci_writew(host, reg, (SDHC_GPIO_OUT + vendor_ptr)); > > + > > + /* For 50Mhz, tuning is not possible.. Lets fix the sampling > > + * Phase of Rx Clock here > > + */ > > + if (clock <= 50000000) { > > + /*Change the Phase value here */ > > + reg = sdhci_readl(host, (SDHC_DBOUNCE + > vendor_ptr)); > > + reg &= ~SDHC_TUNING_RX_CLK_SEL_MASK; > > + reg = reg | (SDHC_TUNING_RX_CLK_SEL_MASK & > > + rx_clk_phase_val); > > + sdhci_writew(host, reg, (SDHC_DBOUNCE + > vendor_ptr)); > > + } > > + mdelay(10); > > + } > > +} > > + > > +static int sdhci_pci_enable_dma_snps(struct sdhci_host *host) { > > + /* DWC MSHC Specific Dma Enabling */ > > + > > + /* Call generic emable_dma */ > > + return sdhci_ops->enable_dma(host); > > +} > > Why doing this is if currently there is nothing specific? The same for other > sdhci ops. As we have Synopsys specific clocking module, we have to register "sdhci_set_clock_snps" with host->ops. For other functions, we registered our API's for future use only. I will revert these API's to optimize the code. Thanks & Regards, Prabu Thangamuthu.