From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5C00C04AAA for ; Mon, 6 May 2019 09:17:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A0C502087F for ; Mon, 6 May 2019 09:17:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726403AbfEFJRt (ORCPT ); Mon, 6 May 2019 05:17:49 -0400 Received: from Mailgw01.mediatek.com ([1.203.163.78]:3051 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1725851AbfEFJRs (ORCPT ); Mon, 6 May 2019 05:17:48 -0400 X-UUID: 498fe6acfc2e44d08bf73c54b228579e-20190506 X-UUID: 498fe6acfc2e44d08bf73c54b228579e-20190506 Received: from mtkcas36.mediatek.inc [(172.27.4.253)] by mailgw01.mediatek.com (envelope-from ) (mailgw01.mediatek.com ESMTP with TLS) with ESMTP id 429816069; Mon, 06 May 2019 17:17:40 +0800 Received: from mtkcas07.mediatek.inc (172.21.101.84) by MTKMBS33N2.mediatek.inc (172.27.4.76) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 6 May 2019 17:17:39 +0800 Received: from [172.21.77.4] (172.21.77.4) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Mon, 6 May 2019 17:17:38 +0800 Message-ID: <1557134258.5345.5.camel@mtksdaap41> Subject: Re: [v2 3/3] drm/mediatek: add mipi_tx driver for mt8183 From: CK Hu To: Jitao Shi CC: Rob Herring , Pawel Moll , "Mark Rutland" , Ian Campbell , Kumar Gala , , David Airlie , "Matthias Brugger" , Thierry Reding , "Ajay Kumar" , Inki Dae , "Rahul Sharma" , Sean Paul , Vincent Palatin , Andy Yan , Philipp Zabel , "Russell King" , , , , , , , Sascha Hauer , , , , , Date: Mon, 6 May 2019 17:17:38 +0800 In-Reply-To: <20190416054217.75387-4-jitao.shi@mediatek.com> References: <20190416054217.75387-1-jitao.shi@mediatek.com> <20190416054217.75387-4-jitao.shi@mediatek.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-MTK: N Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Jitao: On Tue, 2019-04-16 at 13:42 +0800, Jitao Shi wrote: > This patch add mt8183 mipi_tx driver. > And also support other chips that use the same binding and driver. > > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/Makefile | 1 + > drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 2 + > drivers/gpu/drm/mediatek/mtk_mipi_tx.h | 1 + > drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c | 154 ++++++++++++++++++ > 4 files changed, 158 insertions(+) > create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c > [snip] > + > +static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw) > +{ > + struct mtk_mipi_tx *mipi_tx = mtk_mipi_tx_from_clk_hw(hw); > + unsigned int txdiv, txdiv0; > + u64 pcw; > + int ret; > + > + dev_dbg(mipi_tx->dev, "prepare: %u bps\n", mipi_tx->data_rate); > + > + if (mipi_tx->data_rate >= 2000000000) { > + txdiv = 1; > + txdiv0 = 0; > + } else if (mipi_tx->data_rate >= 1000000000) { > + txdiv = 2; > + txdiv0 = 1; > + } else if (mipi_tx->data_rate >= 500000000) { > + txdiv = 4; > + txdiv0 = 2; > + } else if (mipi_tx->data_rate > 250000000) { > + txdiv = 8; > + txdiv0 = 3; > + } else if (mipi_tx->data_rate >= 125000000) { > + txdiv = 16; > + txdiv0 = 4; > + } else { > + return -EINVAL; > + } > + > + ret = clk_prepare_enable(mipi_tx->ref_clk); > + if (ret < 0) { > + dev_err(mipi_tx->dev, > + "can't prepare and enable mipi_tx ref_clk %d\n", ret); > + return ret; > + } You enable the parent clock when prepare this clock here, this behavior looks strange. I think the flow should be: 1. Parent clock prepare 2. This clock prepare 3. Parent clock enable 4. This clock enable Maybe you should implement 'enable callback' so that parent clock would be already enabled. One question is, mipi_tx_pll is used by dsi driver, but I does not see dsi prepare_enable() mipi_tx_pll, how does this work? Regards, CK > + > + mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_PLL_CON4, RG_DSI_PLL_IBIAS); > + > + mtk_mipi_tx_set_bits(mipi_tx, MIPITX_PLL_PWR, AD_DSI_PLL_SDM_PWR_ON); > + usleep_range(30, 100); > + mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_PLL_PWR, AD_DSI_PLL_SDM_ISO_EN); > + pcw = div_u64(((u64)mipi_tx->data_rate * txdiv) << 24, 26000000); > + writel(pcw, mipi_tx->regs + MIPITX_PLL_CON0); > + mtk_mipi_tx_update_bits(mipi_tx, MIPITX_PLL_CON1, RG_DSI_PLL_POSDIV, > + txdiv0 << 8); > + usleep_range(1000, 2000); > + mtk_mipi_tx_set_bits(mipi_tx, MIPITX_PLL_CON1, RG_DSI_PLL_EN); > + > + return 0; > +} > + > +static void mtk_mipi_tx_pll_unprepare(struct clk_hw *hw) > +{ > + struct mtk_mipi_tx *mipi_tx = mtk_mipi_tx_from_clk_hw(hw); > + > + dev_dbg(mipi_tx->dev, "unprepare\n"); > + > + mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_PLL_CON1, RG_DSI_PLL_EN); > + > + mtk_mipi_tx_set_bits(mipi_tx, MIPITX_PLL_PWR, AD_DSI_PLL_SDM_ISO_EN); > + mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_PLL_PWR, AD_DSI_PLL_SDM_PWR_ON); > + clk_disable_unprepare(mipi_tx->ref_clk); > +} > +