From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752778AbcF2Nui (ORCPT ); Wed, 29 Jun 2016 09:50:38 -0400 Received: from arroyo.ext.ti.com ([198.47.19.12]:47997 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752168AbcF2Nu3 (ORCPT ); Wed, 29 Jun 2016 09:50:29 -0400 Subject: Re: [PATCH 2/3] phy: rockchip-emmc: Be tolerant to card clock of 0 in power on To: Douglas Anderson , Heiko Stuebner , References: <1467049167-14628-1-git-send-email-dianders@chromium.org> <1467049167-14628-3-git-send-email-dianders@chromium.org> CC: , , , , , From: Kishon Vijay Abraham I Message-ID: <5773D1FD.8020601@ti.com> Date: Wed, 29 Jun 2016 19:19:49 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1467049167-14628-3-git-send-email-dianders@chromium.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Monday 27 June 2016 11:09 PM, Douglas Anderson wrote: > It's possible that there are some reasons to turn the PHY on while the > clock is 0. In this case we just won't wait for the DLL to lock. > > This is a bit of a stopgap until we figure out exactly when we're > supposed to wait for the DLL to lock and when we're supposed to power > cycle the PHY. > > Note: this patch should help with suspend/resume where the system will > try to turn the PHY back on when the clock is 0. > > Signed-off-by: Douglas Anderson > --- > drivers/phy/phy-rockchip-emmc.c | 59 ++++++++++++++++++++++++++--------------- > 1 file changed, 37 insertions(+), 22 deletions(-) > > diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c > index 9dce958233a0..a2aa6aca7dec 100644 > --- a/drivers/phy/phy-rockchip-emmc.c > +++ b/drivers/phy/phy-rockchip-emmc.c > @@ -88,15 +88,36 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > unsigned int caldone; > unsigned int dllrdy; > unsigned int freqsel = PHYCTRL_FREQSEL_200M; > + unsigned long rate; > unsigned long timeout; > > - if (rk_phy->emmcclk != NULL) { > - unsigned long rate = clk_get_rate(rk_phy->emmcclk); > + /* > + * Keep phyctrl_pdb and phyctrl_endll low to allow > + * initialization of CALIO state M/C DFFs > + */ > + regmap_write(rk_phy->reg_base, > + rk_phy->reg_offset + GRF_EMMCPHY_CON6, > + HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF, > + PHYCTRL_PDB_MASK, > + PHYCTRL_PDB_SHIFT)); > + regmap_write(rk_phy->reg_base, > + rk_phy->reg_offset + GRF_EMMCPHY_CON6, > + HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE, > + PHYCTRL_ENDLL_MASK, > + PHYCTRL_ENDLL_SHIFT)); > + > + /* Already finish power_off above */ > + if (on_off == PHYCTRL_PDB_PWR_OFF) > + return 0; > + > + rate = clk_get_rate(rk_phy->emmcclk); > + > + if (rate != 0) { > unsigned long ideal_rate; > unsigned long diff; > > switch (rate) { > - case 0 ... 74999999: > + case 1 ... 74999999: > ideal_rate = 50000000; > freqsel = PHYCTRL_FREQSEL_50M; > break; > @@ -127,25 +148,6 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > } > > /* > - * Keep phyctrl_pdb and phyctrl_endll low to allow > - * initialization of CALIO state M/C DFFs > - */ > - regmap_write(rk_phy->reg_base, > - rk_phy->reg_offset + GRF_EMMCPHY_CON6, > - HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF, > - PHYCTRL_PDB_MASK, > - PHYCTRL_PDB_SHIFT)); > - regmap_write(rk_phy->reg_base, > - rk_phy->reg_offset + GRF_EMMCPHY_CON6, > - HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE, > - PHYCTRL_ENDLL_MASK, > - PHYCTRL_ENDLL_SHIFT)); > - > - /* Already finish power_off above */ > - if (on_off == PHYCTRL_PDB_PWR_OFF) > - return 0; > - > - /* > * According to the user manual, calpad calibration > * cycle takes more than 2us without the minimal recommended > * value, so we may need a little margin here > @@ -183,6 +185,19 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > HIWORD_UPDATE(PHYCTRL_ENDLL_ENABLE, > PHYCTRL_ENDLL_MASK, > PHYCTRL_ENDLL_SHIFT)); > + > + /* > + * We turned on the DLL even though the rate was 0 because we the > + * clock might be turned on later. ...but we can't wait for the DLL > + * to lock when the rate is 0 because it will never lock with no > + * input clock. > + * > + * Technically we should be checking the lock later when the clock > + * is turned on, but for now we won't. > + */ > + if (rate == 0) > + return 0; Why not return initially from rockchip_emmc_phy_power if the clock rate is '0'. Are there other functions to lock the DLL apart from phy_power? Thanks Kishon From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kishon Vijay Abraham I Subject: Re: [PATCH 2/3] phy: rockchip-emmc: Be tolerant to card clock of 0 in power on Date: Wed, 29 Jun 2016 19:19:49 +0530 Message-ID: <5773D1FD.8020601@ti.com> References: <1467049167-14628-1-git-send-email-dianders@chromium.org> <1467049167-14628-3-git-send-email-dianders@chromium.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1467049167-14628-3-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Douglas Anderson , Heiko Stuebner , ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org Cc: shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org, briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: linux-mmc@vger.kernel.org Hi, On Monday 27 June 2016 11:09 PM, Douglas Anderson wrote: > It's possible that there are some reasons to turn the PHY on while the > clock is 0. In this case we just won't wait for the DLL to lock. > > This is a bit of a stopgap until we figure out exactly when we're > supposed to wait for the DLL to lock and when we're supposed to power > cycle the PHY. > > Note: this patch should help with suspend/resume where the system will > try to turn the PHY back on when the clock is 0. > > Signed-off-by: Douglas Anderson > --- > drivers/phy/phy-rockchip-emmc.c | 59 ++++++++++++++++++++++++++--------------- > 1 file changed, 37 insertions(+), 22 deletions(-) > > diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c > index 9dce958233a0..a2aa6aca7dec 100644 > --- a/drivers/phy/phy-rockchip-emmc.c > +++ b/drivers/phy/phy-rockchip-emmc.c > @@ -88,15 +88,36 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > unsigned int caldone; > unsigned int dllrdy; > unsigned int freqsel = PHYCTRL_FREQSEL_200M; > + unsigned long rate; > unsigned long timeout; > > - if (rk_phy->emmcclk != NULL) { > - unsigned long rate = clk_get_rate(rk_phy->emmcclk); > + /* > + * Keep phyctrl_pdb and phyctrl_endll low to allow > + * initialization of CALIO state M/C DFFs > + */ > + regmap_write(rk_phy->reg_base, > + rk_phy->reg_offset + GRF_EMMCPHY_CON6, > + HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF, > + PHYCTRL_PDB_MASK, > + PHYCTRL_PDB_SHIFT)); > + regmap_write(rk_phy->reg_base, > + rk_phy->reg_offset + GRF_EMMCPHY_CON6, > + HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE, > + PHYCTRL_ENDLL_MASK, > + PHYCTRL_ENDLL_SHIFT)); > + > + /* Already finish power_off above */ > + if (on_off == PHYCTRL_PDB_PWR_OFF) > + return 0; > + > + rate = clk_get_rate(rk_phy->emmcclk); > + > + if (rate != 0) { > unsigned long ideal_rate; > unsigned long diff; > > switch (rate) { > - case 0 ... 74999999: > + case 1 ... 74999999: > ideal_rate = 50000000; > freqsel = PHYCTRL_FREQSEL_50M; > break; > @@ -127,25 +148,6 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > } > > /* > - * Keep phyctrl_pdb and phyctrl_endll low to allow > - * initialization of CALIO state M/C DFFs > - */ > - regmap_write(rk_phy->reg_base, > - rk_phy->reg_offset + GRF_EMMCPHY_CON6, > - HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF, > - PHYCTRL_PDB_MASK, > - PHYCTRL_PDB_SHIFT)); > - regmap_write(rk_phy->reg_base, > - rk_phy->reg_offset + GRF_EMMCPHY_CON6, > - HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE, > - PHYCTRL_ENDLL_MASK, > - PHYCTRL_ENDLL_SHIFT)); > - > - /* Already finish power_off above */ > - if (on_off == PHYCTRL_PDB_PWR_OFF) > - return 0; > - > - /* > * According to the user manual, calpad calibration > * cycle takes more than 2us without the minimal recommended > * value, so we may need a little margin here > @@ -183,6 +185,19 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > HIWORD_UPDATE(PHYCTRL_ENDLL_ENABLE, > PHYCTRL_ENDLL_MASK, > PHYCTRL_ENDLL_SHIFT)); > + > + /* > + * We turned on the DLL even though the rate was 0 because we the > + * clock might be turned on later. ...but we can't wait for the DLL > + * to lock when the rate is 0 because it will never lock with no > + * input clock. > + * > + * Technically we should be checking the lock later when the clock > + * is turned on, but for now we won't. > + */ > + if (rate == 0) > + return 0; Why not return initially from rockchip_emmc_phy_power if the clock rate is '0'. Are there other functions to lock the DLL apart from phy_power? Thanks Kishon From mboxrd@z Thu Jan 1 00:00:00 1970 From: kishon@ti.com (Kishon Vijay Abraham I) Date: Wed, 29 Jun 2016 19:19:49 +0530 Subject: [PATCH 2/3] phy: rockchip-emmc: Be tolerant to card clock of 0 in power on In-Reply-To: <1467049167-14628-3-git-send-email-dianders@chromium.org> References: <1467049167-14628-1-git-send-email-dianders@chromium.org> <1467049167-14628-3-git-send-email-dianders@chromium.org> Message-ID: <5773D1FD.8020601@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On Monday 27 June 2016 11:09 PM, Douglas Anderson wrote: > It's possible that there are some reasons to turn the PHY on while the > clock is 0. In this case we just won't wait for the DLL to lock. > > This is a bit of a stopgap until we figure out exactly when we're > supposed to wait for the DLL to lock and when we're supposed to power > cycle the PHY. > > Note: this patch should help with suspend/resume where the system will > try to turn the PHY back on when the clock is 0. > > Signed-off-by: Douglas Anderson > --- > drivers/phy/phy-rockchip-emmc.c | 59 ++++++++++++++++++++++++++--------------- > 1 file changed, 37 insertions(+), 22 deletions(-) > > diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c > index 9dce958233a0..a2aa6aca7dec 100644 > --- a/drivers/phy/phy-rockchip-emmc.c > +++ b/drivers/phy/phy-rockchip-emmc.c > @@ -88,15 +88,36 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > unsigned int caldone; > unsigned int dllrdy; > unsigned int freqsel = PHYCTRL_FREQSEL_200M; > + unsigned long rate; > unsigned long timeout; > > - if (rk_phy->emmcclk != NULL) { > - unsigned long rate = clk_get_rate(rk_phy->emmcclk); > + /* > + * Keep phyctrl_pdb and phyctrl_endll low to allow > + * initialization of CALIO state M/C DFFs > + */ > + regmap_write(rk_phy->reg_base, > + rk_phy->reg_offset + GRF_EMMCPHY_CON6, > + HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF, > + PHYCTRL_PDB_MASK, > + PHYCTRL_PDB_SHIFT)); > + regmap_write(rk_phy->reg_base, > + rk_phy->reg_offset + GRF_EMMCPHY_CON6, > + HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE, > + PHYCTRL_ENDLL_MASK, > + PHYCTRL_ENDLL_SHIFT)); > + > + /* Already finish power_off above */ > + if (on_off == PHYCTRL_PDB_PWR_OFF) > + return 0; > + > + rate = clk_get_rate(rk_phy->emmcclk); > + > + if (rate != 0) { > unsigned long ideal_rate; > unsigned long diff; > > switch (rate) { > - case 0 ... 74999999: > + case 1 ... 74999999: > ideal_rate = 50000000; > freqsel = PHYCTRL_FREQSEL_50M; > break; > @@ -127,25 +148,6 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > } > > /* > - * Keep phyctrl_pdb and phyctrl_endll low to allow > - * initialization of CALIO state M/C DFFs > - */ > - regmap_write(rk_phy->reg_base, > - rk_phy->reg_offset + GRF_EMMCPHY_CON6, > - HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF, > - PHYCTRL_PDB_MASK, > - PHYCTRL_PDB_SHIFT)); > - regmap_write(rk_phy->reg_base, > - rk_phy->reg_offset + GRF_EMMCPHY_CON6, > - HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE, > - PHYCTRL_ENDLL_MASK, > - PHYCTRL_ENDLL_SHIFT)); > - > - /* Already finish power_off above */ > - if (on_off == PHYCTRL_PDB_PWR_OFF) > - return 0; > - > - /* > * According to the user manual, calpad calibration > * cycle takes more than 2us without the minimal recommended > * value, so we may need a little margin here > @@ -183,6 +185,19 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off) > HIWORD_UPDATE(PHYCTRL_ENDLL_ENABLE, > PHYCTRL_ENDLL_MASK, > PHYCTRL_ENDLL_SHIFT)); > + > + /* > + * We turned on the DLL even though the rate was 0 because we the > + * clock might be turned on later. ...but we can't wait for the DLL > + * to lock when the rate is 0 because it will never lock with no > + * input clock. > + * > + * Technically we should be checking the lock later when the clock > + * is turned on, but for now we won't. > + */ > + if (rate == 0) > + return 0; Why not return initially from rockchip_emmc_phy_power if the clock rate is '0'. Are there other functions to lock the DLL apart from phy_power? Thanks Kishon