From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 27 Jan 2017 12:04:36 -0800 From: Stephen Boyd To: Eric Anholt Cc: Florian Fainelli , Michael Turquette , Rob Herring , Mark Rutland , dri-devel@lists.freedesktop.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Stephen Warren , Lee Jones , bcm-kernel-feedback-list@broadcom.com, linux-clk@vger.kernel.org Subject: Re: [PATCH 2/2] drm/vc4: Add DSI driver Message-ID: <20170127200436.GI8801@codeaurora.org> References: <20170124003853.16418-1-eric@anholt.net> <20170124003853.16418-3-eric@anholt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20170124003853.16418-3-eric@anholt.net> List-ID: On 01/23, Eric Anholt wrote: > +static int > +vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) > +{ > + struct device *dev = &dsi->pdev->dev; > + const char *parent_name = __clk_get_name(dsi->pll_phy_clock); > + static const struct { > + const char *dsi0_name, *dsi1_name; > + int div; > + } phy_clocks[] = { > + { "dsi0_byte", "dsi1_byte", 8 }, > + { "dsi0_ddr2", "dsi1_ddr2", 4 }, > + { "dsi0_ddr", "dsi1_ddr", 2 }, > + }; > + int i; > + > + dsi->clk_onecell.clk_num = ARRAY_SIZE(phy_clocks); > + dsi->clk_onecell.clks = devm_kcalloc(dev, > + dsi->clk_onecell.clk_num, > + sizeof(*dsi->clk_onecell.clks), > + GFP_KERNEL); > + if (!dsi->clk_onecell.clks) > + return -ENOMEM; > + > + for (i = 0; i < ARRAY_SIZE(phy_clocks); i++) { > + struct clk_fixed_factor *fix = &dsi->phy_clocks[i]; > + struct clk_init_data init; > + struct clk *clk; > + > + /* We just use core fixed factor clock ops for the PHY > + * clocks. The clocks are actually gated by the > + * PHY_AFEC0_DDRCLK_EN bits, which we should be > + * setting if we use the DDR/DDR2 clocks. However, > + * vc4_dsi_encoder_enable() is setting up both AFEC0, > + * setting both our parent DSI PLL's rate and this > + * clock's rate, so it knows if DDR/DDR2 are going to > + * be used and could enable the gates itself. > + */ > + fix->mult = 1; > + fix->div = phy_clocks[i].div; > + fix->hw.init = &init; > + > + memset(&init, 0, sizeof(init)); > + init.parent_names = &parent_name; > + init.num_parents = 1; > + if (dsi->port == 1) > + init.name = phy_clocks[i].dsi1_name; > + else > + init.name = phy_clocks[i].dsi0_name; > + init.ops = &clk_fixed_factor_ops; > + init.flags = CLK_IS_BASIC; Please don't use this flag unless you need it for something. > + > + clk = devm_clk_register(dev, &fix->hw); Can you use devm_clk_hw_register() instead please? > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + dsi->clk_onecell.clks[i] = clk; > + } > + > + return of_clk_add_provider(dev->of_node, And the of_clk_add_hw_provider() API too. > + of_clk_src_onecell_get, > + &dsi->clk_onecell); > +} > + -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: sboyd@codeaurora.org (Stephen Boyd) Date: Fri, 27 Jan 2017 12:04:36 -0800 Subject: [PATCH 2/2] drm/vc4: Add DSI driver In-Reply-To: <20170124003853.16418-3-eric@anholt.net> References: <20170124003853.16418-1-eric@anholt.net> <20170124003853.16418-3-eric@anholt.net> Message-ID: <20170127200436.GI8801@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 01/23, Eric Anholt wrote: > +static int > +vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) > +{ > + struct device *dev = &dsi->pdev->dev; > + const char *parent_name = __clk_get_name(dsi->pll_phy_clock); > + static const struct { > + const char *dsi0_name, *dsi1_name; > + int div; > + } phy_clocks[] = { > + { "dsi0_byte", "dsi1_byte", 8 }, > + { "dsi0_ddr2", "dsi1_ddr2", 4 }, > + { "dsi0_ddr", "dsi1_ddr", 2 }, > + }; > + int i; > + > + dsi->clk_onecell.clk_num = ARRAY_SIZE(phy_clocks); > + dsi->clk_onecell.clks = devm_kcalloc(dev, > + dsi->clk_onecell.clk_num, > + sizeof(*dsi->clk_onecell.clks), > + GFP_KERNEL); > + if (!dsi->clk_onecell.clks) > + return -ENOMEM; > + > + for (i = 0; i < ARRAY_SIZE(phy_clocks); i++) { > + struct clk_fixed_factor *fix = &dsi->phy_clocks[i]; > + struct clk_init_data init; > + struct clk *clk; > + > + /* We just use core fixed factor clock ops for the PHY > + * clocks. The clocks are actually gated by the > + * PHY_AFEC0_DDRCLK_EN bits, which we should be > + * setting if we use the DDR/DDR2 clocks. However, > + * vc4_dsi_encoder_enable() is setting up both AFEC0, > + * setting both our parent DSI PLL's rate and this > + * clock's rate, so it knows if DDR/DDR2 are going to > + * be used and could enable the gates itself. > + */ > + fix->mult = 1; > + fix->div = phy_clocks[i].div; > + fix->hw.init = &init; > + > + memset(&init, 0, sizeof(init)); > + init.parent_names = &parent_name; > + init.num_parents = 1; > + if (dsi->port == 1) > + init.name = phy_clocks[i].dsi1_name; > + else > + init.name = phy_clocks[i].dsi0_name; > + init.ops = &clk_fixed_factor_ops; > + init.flags = CLK_IS_BASIC; Please don't use this flag unless you need it for something. > + > + clk = devm_clk_register(dev, &fix->hw); Can you use devm_clk_hw_register() instead please? > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + > + dsi->clk_onecell.clks[i] = clk; > + } > + > + return of_clk_add_provider(dev->of_node, And the of_clk_add_hw_provider() API too. > + of_clk_src_onecell_get, > + &dsi->clk_onecell); > +} > + -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH 2/2] drm/vc4: Add DSI driver Date: Fri, 27 Jan 2017 12:04:36 -0800 Message-ID: <20170127200436.GI8801@codeaurora.org> References: <20170124003853.16418-1-eric@anholt.net> <20170124003853.16418-3-eric@anholt.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id B1E7B6EE33 for ; Fri, 27 Jan 2017 20:04:37 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170124003853.16418-3-eric@anholt.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Eric Anholt Cc: Mark Rutland , Florian Fainelli , Stephen Warren , Michael Turquette , Lee Jones , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Rob Herring , bcm-kernel-feedback-list@broadcom.com, linux-rpi-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: dri-devel@lists.freedesktop.org T24gMDEvMjMsIEVyaWMgQW5ob2x0IHdyb3RlOgo+ICtzdGF0aWMgaW50Cj4gK3ZjNF9kc2lfaW5p dF9waHlfY2xvY2tzKHN0cnVjdCB2YzRfZHNpICpkc2kpCj4gK3sKPiArCXN0cnVjdCBkZXZpY2Ug KmRldiA9ICZkc2ktPnBkZXYtPmRldjsKPiArCWNvbnN0IGNoYXIgKnBhcmVudF9uYW1lID0gX19j bGtfZ2V0X25hbWUoZHNpLT5wbGxfcGh5X2Nsb2NrKTsKPiArCXN0YXRpYyBjb25zdCBzdHJ1Y3Qg ewo+ICsJCWNvbnN0IGNoYXIgKmRzaTBfbmFtZSwgKmRzaTFfbmFtZTsKPiArCQlpbnQgZGl2Owo+ ICsJfSBwaHlfY2xvY2tzW10gPSB7Cj4gKwkJeyAiZHNpMF9ieXRlIiwgImRzaTFfYnl0ZSIsIDgg fSwKPiArCQl7ICJkc2kwX2RkcjIiLCAiZHNpMV9kZHIyIiwgNCB9LAo+ICsJCXsgImRzaTBfZGRy IiwgImRzaTFfZGRyIiwgMiB9LAo+ICsJfTsKPiArCWludCBpOwo+ICsKPiArCWRzaS0+Y2xrX29u ZWNlbGwuY2xrX251bSA9IEFSUkFZX1NJWkUocGh5X2Nsb2Nrcyk7Cj4gKwlkc2ktPmNsa19vbmVj ZWxsLmNsa3MgPSBkZXZtX2tjYWxsb2MoZGV2LAo+ICsJCQkJCSAgICAgZHNpLT5jbGtfb25lY2Vs bC5jbGtfbnVtLAo+ICsJCQkJCSAgICAgc2l6ZW9mKCpkc2ktPmNsa19vbmVjZWxsLmNsa3MpLAo+ ICsJCQkJCSAgICAgR0ZQX0tFUk5FTCk7Cj4gKwlpZiAoIWRzaS0+Y2xrX29uZWNlbGwuY2xrcykK PiArCQlyZXR1cm4gLUVOT01FTTsKPiArCj4gKwlmb3IgKGkgPSAwOyBpIDwgQVJSQVlfU0laRShw aHlfY2xvY2tzKTsgaSsrKSB7Cj4gKwkJc3RydWN0IGNsa19maXhlZF9mYWN0b3IgKmZpeCA9ICZk c2ktPnBoeV9jbG9ja3NbaV07Cj4gKwkJc3RydWN0IGNsa19pbml0X2RhdGEgaW5pdDsKPiArCQlz dHJ1Y3QgY2xrICpjbGs7Cj4gKwo+ICsJCS8qIFdlIGp1c3QgdXNlIGNvcmUgZml4ZWQgZmFjdG9y IGNsb2NrIG9wcyBmb3IgdGhlIFBIWQo+ICsJCSAqIGNsb2Nrcy4gIFRoZSBjbG9ja3MgYXJlIGFj dHVhbGx5IGdhdGVkIGJ5IHRoZQo+ICsJCSAqIFBIWV9BRkVDMF9ERFJDTEtfRU4gYml0cywgd2hp Y2ggd2Ugc2hvdWxkIGJlCj4gKwkJICogc2V0dGluZyBpZiB3ZSB1c2UgdGhlIEREUi9ERFIyIGNs b2Nrcy4gIEhvd2V2ZXIsCj4gKwkJICogdmM0X2RzaV9lbmNvZGVyX2VuYWJsZSgpIGlzIHNldHRp bmcgdXAgYm90aCBBRkVDMCwKPiArCQkgKiBzZXR0aW5nIGJvdGggb3VyIHBhcmVudCBEU0kgUExM J3MgcmF0ZSBhbmQgdGhpcwo+ICsJCSAqIGNsb2NrJ3MgcmF0ZSwgc28gaXQga25vd3MgaWYgRERS L0REUjIgYXJlIGdvaW5nIHRvCj4gKwkJICogYmUgdXNlZCBhbmQgY291bGQgZW5hYmxlIHRoZSBn YXRlcyBpdHNlbGYuCj4gKwkJICovCj4gKwkJZml4LT5tdWx0ID0gMTsKPiArCQlmaXgtPmRpdiA9 IHBoeV9jbG9ja3NbaV0uZGl2Owo+ICsJCWZpeC0+aHcuaW5pdCA9ICZpbml0Owo+ICsKPiArCQlt ZW1zZXQoJmluaXQsIDAsIHNpemVvZihpbml0KSk7Cj4gKwkJaW5pdC5wYXJlbnRfbmFtZXMgPSAm cGFyZW50X25hbWU7Cj4gKwkJaW5pdC5udW1fcGFyZW50cyA9IDE7Cj4gKwkJaWYgKGRzaS0+cG9y dCA9PSAxKQo+ICsJCQlpbml0Lm5hbWUgPSBwaHlfY2xvY2tzW2ldLmRzaTFfbmFtZTsKPiArCQll bHNlCj4gKwkJCWluaXQubmFtZSA9IHBoeV9jbG9ja3NbaV0uZHNpMF9uYW1lOwo+ICsJCWluaXQu b3BzID0gJmNsa19maXhlZF9mYWN0b3Jfb3BzOwo+ICsJCWluaXQuZmxhZ3MgPSBDTEtfSVNfQkFT SUM7CgpQbGVhc2UgZG9uJ3QgdXNlIHRoaXMgZmxhZyB1bmxlc3MgeW91IG5lZWQgaXQgZm9yIHNv bWV0aGluZy4KCj4gKwo+ICsJCWNsayA9IGRldm1fY2xrX3JlZ2lzdGVyKGRldiwgJmZpeC0+aHcp OwoKQ2FuIHlvdSB1c2UgZGV2bV9jbGtfaHdfcmVnaXN0ZXIoKSBpbnN0ZWFkIHBsZWFzZT8KCj4g KwkJaWYgKElTX0VSUihjbGspKQo+ICsJCQlyZXR1cm4gUFRSX0VSUihjbGspOwo+ICsKPiArCQlk c2ktPmNsa19vbmVjZWxsLmNsa3NbaV0gPSBjbGs7Cj4gKwl9Cj4gKwo+ICsJcmV0dXJuIG9mX2Ns a19hZGRfcHJvdmlkZXIoZGV2LT5vZl9ub2RlLAoKQW5kIHRoZSBvZl9jbGtfYWRkX2h3X3Byb3Zp ZGVyKCkgQVBJIHRvby4KCj4gKwkJCQkgICBvZl9jbGtfc3JjX29uZWNlbGxfZ2V0LAo+ICsJCQkJ ICAgJmRzaS0+Y2xrX29uZWNlbGwpOwo+ICt9Cj4gKwoKLS0gClF1YWxjb21tIElubm92YXRpb24g Q2VudGVyLCBJbmMuIGlzIGEgbWVtYmVyIG9mIENvZGUgQXVyb3JhIEZvcnVtLAphIExpbnV4IEZv dW5kYXRpb24gQ29sbGFib3JhdGl2ZSBQcm9qZWN0Cl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxp c3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFu L2xpc3RpbmZvL2RyaS1kZXZlbAo=