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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3B58AC433F5 for ; Fri, 22 Oct 2021 10:31:15 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 090FD61205 for ; Fri, 22 Oct 2021 10:31:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 090FD61205 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=27CCUq4CtEGgQTGJtH6H2zxSPjlVZJAL0HEGB+NBVN0=; b=0D43AlttdjTfGo wZFJA9S3Byx4ngjeWtrhxAbRsBxliN7mPT01fHGTLY9EhJqqiHf6lrHUHOG6p2CFLVEiafADERMgB BmlrcW9Vz1208uPT5UK+bMvmT/XOpeVwpDi28RRd/4rqHhWtr13nRXJZILyX1au/4ZlD7cuxdQX/p abH7G+nQcyPlHPgao3BD2aUna5QdUYCyLovvSqS0YWGHeULApcauR6ljvCB9fimCdtk1vHFxtCyHW T6V3GJafSHqht4SXtARrfsFSzmYlcoZWWddonx3PSX/O+Tmf/hOYBwVBeJybnkcMW+98PPm8hfjX/ nPg625Jbz6beHVpaB4IA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mdrpF-00AWJe-SD; Fri, 22 Oct 2021 10:31:01 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mdrp2-00AWGv-QR; Fri, 22 Oct 2021 10:30:50 +0000 X-UUID: e8a7c341987a40b0b6f94e7dc9037b79-20211022 X-UUID: e8a7c341987a40b0b6f94e7dc9037b79-20211022 Received: from mtkcas68.mediatek.inc [(172.29.94.19)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 948093020; Fri, 22 Oct 2021 03:30:39 -0700 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62DR.mediatek.inc (172.29.94.18) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 22 Oct 2021 03:30:37 -0700 Received: from mtkcas10.mediatek.inc (172.21.101.39) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 22 Oct 2021 18:30:36 +0800 Received: from mtksdccf07 (172.21.84.99) by mtkcas10.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Fri, 22 Oct 2021 18:30:36 +0800 Message-ID: Subject: Re: [PATCH v11 15/16] drm/mediatek: add MERGE support for mediatek-drm From: Jason-JH Lin To: AngeloGioacchino Del Regno , "Rob Herring" , Matthias Brugger , Chun-Kuang Hu , Philipp Zabel CC: Enric Balletbo i Serra , Maxime Coquelin , David Airlie , Daniel Vetter , Alexandre Torgue , , , , , Fabien Parent , "Yongqiang Niu" , , , , , , , , Date: Fri, 22 Oct 2021 18:30:36 +0800 In-Reply-To: <3e72dd1e-edf2-6d42-40e7-0c1c72749a20@collabora.com> References: <20210921155218.10387-1-jason-jh.lin@mediatek.com> <20210921155218.10387-16-jason-jh.lin@mediatek.com> <3e72dd1e-edf2-6d42-40e7-0c1c72749a20@collabora.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211022_033048_910733_37866C17 X-CRM114-Status: GOOD ( 31.24 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi Angelo, thanks for the review. On Thu, 2021-10-14 at 16:27 +0200, AngeloGioacchino Del Regno wrote: > Il 21/09/21 17:52, jason-jh.lin ha scritto: > > Add MERGE engine file: [snip] > > +int mtk_merge_clk_enable(struct device *dev) > > +{ > > + int ret = 0; > > + struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > + > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) > > + pr_err("merge clk prepare enable failed\n"); > > If you failed to enable this clock, I take it as the hardware won't > work or > won't work as expected, hence you should return a failure before > trying to > call prepare_enable for async_clk. > OK I'll fix it. > > + ret = clk_prepare_enable(priv->async_clk); > > + if (ret) > > + pr_err("async clk prepare enable failed\n"); > > + > > You should also return a failure here but, before that, you should > clean up > the state by calling clk_disable_unprepare(priv->clk), or you will > leave it > enabled, eventually getting a hardware fault later on (which may or > may not > result in a board reboot), or other sorts of unexpected states. > > At least, you will get issues with the refcount for "clk" and/or > "async_clk". > > Please fix that. > > Also, please use dev_err or, more appropriately, DRM_ERROR instead or > pr_err. > OK I'll fix it . > > + return ret; > > +} > > + > > +void mtk_merge_clk_disable(struct device *dev) > > +{ > > + struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > + > > + clk_disable_unprepare(priv->async_clk); > + clk_disable_unprepa > > re(priv->clk); > > +} > > + > > +static int mtk_disp_merge_bind(struct device *dev, struct device > > *master, > > + void *data) > > +{ > > + return 0; > > +} > > + > > +static void mtk_disp_merge_unbind(struct device *dev, struct > > device *master, > > + void *data) > > +{ > > +} > > + > > +static const struct component_ops mtk_disp_merge_component_ops = { > > + .bind = mtk_disp_merge_bind, > > + .unbind = mtk_disp_merge_unbind, > > +}; > > + > > +static int mtk_disp_merge_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct resource *res; > > + struct mtk_disp_merge *priv; > > + int ret; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->regs = devm_ioremap_resource(dev, res); > > + if (IS_ERR(priv->regs)) { > > + dev_err(dev, "failed to ioremap merge\n"); > > + return PTR_ERR(priv->regs); > > + } > > + > > + priv->clk = devm_clk_get(dev, NULL); > > + if (IS_ERR(priv->clk)) { > > + dev_err(dev, "failed to get merge clk\n"); > > + return PTR_ERR(priv->clk); > > + } > > + > > + priv->async_clk = of_clk_get(dev->of_node, 1); > > + if (IS_ERR(priv->async_clk)) { > > + ret = PTR_ERR(priv->async_clk); > > + dev_dbg(dev, "No merge async clock: %d\n", ret); > > + priv->async_clk = NULL; > > + } > > + > > You are using devm_clk_get for the first clock, of_clk_get for the > second one: > what's the reason for that? > > Also, async_clk seems to be optional... and there's the right API for > you! > If you use devm_clk_get_optional(), you won't have to manually assign > NULL > to priv->async_clk, as that's API handled... and you'll get a failure > if > the return value is an error that's not -ENOENT (so, it'll fail if > the clock > was declared in DT, but there was an error acquiring it). > > Please use devm_clk_get_optional() here. > Yes, async_clk is optional. Thanks for your suggestion. I'll try it. > Regards, > - Angelo -- Regards, Jason-JH Lin _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E9953C433EF for ; Fri, 22 Oct 2021 10:30:45 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A5A2A611F2 for ; Fri, 22 Oct 2021 10:30:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A5A2A611F2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A99316ECFC; Fri, 22 Oct 2021 10:30:44 +0000 (UTC) Received: from mailgw02.mediatek.com (unknown [210.61.82.184]) by gabe.freedesktop.org (Postfix) with ESMTPS id ABDDC6ECFC for ; Fri, 22 Oct 2021 10:30:42 +0000 (UTC) X-UUID: 5ee641a17dd94a58aa7a494abd9ee8fe-20211022 X-UUID: 5ee641a17dd94a58aa7a494abd9ee8fe-20211022 Received: from mtkcas11.mediatek.inc [(172.21.101.40)] by mailgw02.mediatek.com (envelope-from ) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 1778794933; Fri, 22 Oct 2021 18:30:37 +0800 Received: from mtkcas10.mediatek.inc (172.21.101.39) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 22 Oct 2021 18:30:36 +0800 Received: from mtksdccf07 (172.21.84.99) by mtkcas10.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Fri, 22 Oct 2021 18:30:36 +0800 Message-ID: Subject: Re: [PATCH v11 15/16] drm/mediatek: add MERGE support for mediatek-drm From: Jason-JH Lin To: AngeloGioacchino Del Regno , Rob Herring , Matthias Brugger , Chun-Kuang Hu , "Philipp Zabel" CC: Enric Balletbo i Serra , Maxime Coquelin , David Airlie , Daniel Vetter , Alexandre Torgue , , , , , Fabien Parent , "Yongqiang Niu" , , , , , , , , Date: Fri, 22 Oct 2021 18:30:36 +0800 In-Reply-To: <3e72dd1e-edf2-6d42-40e7-0c1c72749a20@collabora.com> References: <20210921155218.10387-1-jason-jh.lin@mediatek.com> <20210921155218.10387-16-jason-jh.lin@mediatek.com> <3e72dd1e-edf2-6d42-40e7-0c1c72749a20@collabora.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-MTK: N X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Angelo, thanks for the review. On Thu, 2021-10-14 at 16:27 +0200, AngeloGioacchino Del Regno wrote: > Il 21/09/21 17:52, jason-jh.lin ha scritto: > > Add MERGE engine file: [snip] > > +int mtk_merge_clk_enable(struct device *dev) > > +{ > > + int ret = 0; > > + struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > + > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) > > + pr_err("merge clk prepare enable failed\n"); > > If you failed to enable this clock, I take it as the hardware won't > work or > won't work as expected, hence you should return a failure before > trying to > call prepare_enable for async_clk. > OK I'll fix it. > > + ret = clk_prepare_enable(priv->async_clk); > > + if (ret) > > + pr_err("async clk prepare enable failed\n"); > > + > > You should also return a failure here but, before that, you should > clean up > the state by calling clk_disable_unprepare(priv->clk), or you will > leave it > enabled, eventually getting a hardware fault later on (which may or > may not > result in a board reboot), or other sorts of unexpected states. > > At least, you will get issues with the refcount for "clk" and/or > "async_clk". > > Please fix that. > > Also, please use dev_err or, more appropriately, DRM_ERROR instead or > pr_err. > OK I'll fix it . > > + return ret; > > +} > > + > > +void mtk_merge_clk_disable(struct device *dev) > > +{ > > + struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > + > > + clk_disable_unprepare(priv->async_clk); > + clk_disable_unprepa > > re(priv->clk); > > +} > > + > > +static int mtk_disp_merge_bind(struct device *dev, struct device > > *master, > > + void *data) > > +{ > > + return 0; > > +} > > + > > +static void mtk_disp_merge_unbind(struct device *dev, struct > > device *master, > > + void *data) > > +{ > > +} > > + > > +static const struct component_ops mtk_disp_merge_component_ops = { > > + .bind = mtk_disp_merge_bind, > > + .unbind = mtk_disp_merge_unbind, > > +}; > > + > > +static int mtk_disp_merge_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct resource *res; > > + struct mtk_disp_merge *priv; > > + int ret; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->regs = devm_ioremap_resource(dev, res); > > + if (IS_ERR(priv->regs)) { > > + dev_err(dev, "failed to ioremap merge\n"); > > + return PTR_ERR(priv->regs); > > + } > > + > > + priv->clk = devm_clk_get(dev, NULL); > > + if (IS_ERR(priv->clk)) { > > + dev_err(dev, "failed to get merge clk\n"); > > + return PTR_ERR(priv->clk); > > + } > > + > > + priv->async_clk = of_clk_get(dev->of_node, 1); > > + if (IS_ERR(priv->async_clk)) { > > + ret = PTR_ERR(priv->async_clk); > > + dev_dbg(dev, "No merge async clock: %d\n", ret); > > + priv->async_clk = NULL; > > + } > > + > > You are using devm_clk_get for the first clock, of_clk_get for the > second one: > what's the reason for that? > > Also, async_clk seems to be optional... and there's the right API for > you! > If you use devm_clk_get_optional(), you won't have to manually assign > NULL > to priv->async_clk, as that's API handled... and you'll get a failure > if > the return value is an error that's not -ENOENT (so, it'll fail if > the clock > was declared in DT, but there was an error acquiring it). > > Please use devm_clk_get_optional() here. > Yes, async_clk is optional. Thanks for your suggestion. I'll try it. > Regards, > - Angelo -- Regards, Jason-JH Lin 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 10D21C433EF for ; Fri, 22 Oct 2021 10:32:29 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C7A3861059 for ; Fri, 22 Oct 2021 10:32:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C7A3861059 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2Uq7XKscZooKF3cYDI40QDWRUi6KFj2E87i2L/4nFUs=; b=1mVl1ntlTbFIr/ Bn/x+/7xTmuxLKI+oS3u9GVT5JMoOj0lgsYt3804adVOHn+f6Zbyxw0WbNGb2tZVhxW1QFsQrTOBP RsC11O+XByiDLXifuH26mDGfIIZIXPKuq3SBp57TjZn34bqZrwqW0UtW6kGK+J3yBsjGwWyH8YfVt WMET6lP5dRKNkfMkC+KbTwKoiU30tD8sUiUQtDAZww2AQLSRaKqgVDaGQjxA+shvPsoQ4L/uA+QTK 3SoGBtFxL5Fnb97SJp8eqHA+Qchv2RVS8KohOqoLQqEWePUEu6QfIE/llLoCcKO8L/ZKcSnjsfi3i rDskomnJpzGvBAODecMQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mdrpH-00AWJt-Pb; Fri, 22 Oct 2021 10:31:03 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mdrp2-00AWGv-QR; Fri, 22 Oct 2021 10:30:50 +0000 X-UUID: e8a7c341987a40b0b6f94e7dc9037b79-20211022 X-UUID: e8a7c341987a40b0b6f94e7dc9037b79-20211022 Received: from mtkcas68.mediatek.inc [(172.29.94.19)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 948093020; Fri, 22 Oct 2021 03:30:39 -0700 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62DR.mediatek.inc (172.29.94.18) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 22 Oct 2021 03:30:37 -0700 Received: from mtkcas10.mediatek.inc (172.21.101.39) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 22 Oct 2021 18:30:36 +0800 Received: from mtksdccf07 (172.21.84.99) by mtkcas10.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Fri, 22 Oct 2021 18:30:36 +0800 Message-ID: Subject: Re: [PATCH v11 15/16] drm/mediatek: add MERGE support for mediatek-drm From: Jason-JH Lin To: AngeloGioacchino Del Regno , "Rob Herring" , Matthias Brugger , Chun-Kuang Hu , Philipp Zabel CC: Enric Balletbo i Serra , Maxime Coquelin , David Airlie , Daniel Vetter , Alexandre Torgue , , , , , Fabien Parent , "Yongqiang Niu" , , , , , , , , Date: Fri, 22 Oct 2021 18:30:36 +0800 In-Reply-To: <3e72dd1e-edf2-6d42-40e7-0c1c72749a20@collabora.com> References: <20210921155218.10387-1-jason-jh.lin@mediatek.com> <20210921155218.10387-16-jason-jh.lin@mediatek.com> <3e72dd1e-edf2-6d42-40e7-0c1c72749a20@collabora.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211022_033048_910733_37866C17 X-CRM114-Status: GOOD ( 31.24 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Angelo, thanks for the review. On Thu, 2021-10-14 at 16:27 +0200, AngeloGioacchino Del Regno wrote: > Il 21/09/21 17:52, jason-jh.lin ha scritto: > > Add MERGE engine file: [snip] > > +int mtk_merge_clk_enable(struct device *dev) > > +{ > > + int ret = 0; > > + struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > + > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) > > + pr_err("merge clk prepare enable failed\n"); > > If you failed to enable this clock, I take it as the hardware won't > work or > won't work as expected, hence you should return a failure before > trying to > call prepare_enable for async_clk. > OK I'll fix it. > > + ret = clk_prepare_enable(priv->async_clk); > > + if (ret) > > + pr_err("async clk prepare enable failed\n"); > > + > > You should also return a failure here but, before that, you should > clean up > the state by calling clk_disable_unprepare(priv->clk), or you will > leave it > enabled, eventually getting a hardware fault later on (which may or > may not > result in a board reboot), or other sorts of unexpected states. > > At least, you will get issues with the refcount for "clk" and/or > "async_clk". > > Please fix that. > > Also, please use dev_err or, more appropriately, DRM_ERROR instead or > pr_err. > OK I'll fix it . > > + return ret; > > +} > > + > > +void mtk_merge_clk_disable(struct device *dev) > > +{ > > + struct mtk_disp_merge *priv = dev_get_drvdata(dev); > > + > > + clk_disable_unprepare(priv->async_clk); > + clk_disable_unprepa > > re(priv->clk); > > +} > > + > > +static int mtk_disp_merge_bind(struct device *dev, struct device > > *master, > > + void *data) > > +{ > > + return 0; > > +} > > + > > +static void mtk_disp_merge_unbind(struct device *dev, struct > > device *master, > > + void *data) > > +{ > > +} > > + > > +static const struct component_ops mtk_disp_merge_component_ops = { > > + .bind = mtk_disp_merge_bind, > > + .unbind = mtk_disp_merge_unbind, > > +}; > > + > > +static int mtk_disp_merge_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct resource *res; > > + struct mtk_disp_merge *priv; > > + int ret; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->regs = devm_ioremap_resource(dev, res); > > + if (IS_ERR(priv->regs)) { > > + dev_err(dev, "failed to ioremap merge\n"); > > + return PTR_ERR(priv->regs); > > + } > > + > > + priv->clk = devm_clk_get(dev, NULL); > > + if (IS_ERR(priv->clk)) { > > + dev_err(dev, "failed to get merge clk\n"); > > + return PTR_ERR(priv->clk); > > + } > > + > > + priv->async_clk = of_clk_get(dev->of_node, 1); > > + if (IS_ERR(priv->async_clk)) { > > + ret = PTR_ERR(priv->async_clk); > > + dev_dbg(dev, "No merge async clock: %d\n", ret); > > + priv->async_clk = NULL; > > + } > > + > > You are using devm_clk_get for the first clock, of_clk_get for the > second one: > what's the reason for that? > > Also, async_clk seems to be optional... and there's the right API for > you! > If you use devm_clk_get_optional(), you won't have to manually assign > NULL > to priv->async_clk, as that's API handled... and you'll get a failure > if > the return value is an error that's not -ENOENT (so, it'll fail if > the clock > was declared in DT, but there was an error acquiring it). > > Please use devm_clk_get_optional() here. > Yes, async_clk is optional. Thanks for your suggestion. I'll try it. > Regards, > - Angelo -- Regards, Jason-JH Lin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel