From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752547AbcDSHQH (ORCPT ); Tue, 19 Apr 2016 03:16:07 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:8723 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751925AbcDSHQE (ORCPT ); Tue, 19 Apr 2016 03:16:04 -0400 Subject: Re: [RESEND PATCH v10 2/6] clk: hisilicon: add CRG driver for hi3519 soc To: Stephen Boyd References: <1459411811-12390-1-git-send-email-xuejiancheng@hisilicon.com> <1459411811-12390-3-git-send-email-xuejiancheng@hisilicon.com> <20160416004055.GN26353@codeaurora.org> CC: , , , , , , , , , , , , , , , , , , , Jiancheng Xue From: Jiancheng Xue Message-ID: <5715DA3B.1070605@hisilicon.com> Date: Tue, 19 Apr 2016 15:11:55 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160416004055.GN26353@codeaurora.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.217.211] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.5715DA33.010A,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: aec2acebc57597397685bdc60315fd25 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Stephen, On 2016/4/16 8:40, Stephen Boyd wrote: > On 03/31, Jiancheng Xue wrote: >> diff --git a/drivers/clk/hisilicon/clk-hi3519.c b/drivers/clk/hisilicon/clk-hi3519.c >> new file mode 100644 >> index 0000000..ee9df82 >> --- /dev/null >> +++ b/drivers/clk/hisilicon/clk-hi3519.c >> @@ -0,0 +1,129 @@ >> +/* >> + * Hi3519 Clock Driver >> + * >> + * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see . >> + */ >> + [...] >> +static int hi3519_clk_probe(struct platform_device *pdev) >> +{ >> + struct device_node *np = pdev->dev.of_node; >> + struct hisi_clock_data *clk_data; >> + >> + clk_data = hisi_clk_init(np, HI3519_NR_CLKS); >> + if (!clk_data) >> + return -ENODEV; >> + >> + hisi_clk_register_fixed_rate(hi3519_fixed_rate_clks, >> + ARRAY_SIZE(hi3519_fixed_rate_clks), >> + clk_data); >> + hisi_clk_register_mux(hi3519_mux_clks, ARRAY_SIZE(hi3519_mux_clks), >> + clk_data); >> + hisi_clk_register_gate(hi3519_gate_clks, >> + ARRAY_SIZE(hi3519_gate_clks), clk_data); >> + >> + return hisi_reset_init(np); > > Now that this is a platform driver we need to do lots of cleanup > in error cases. I mean we need to unregister clks, OF clk > providers, and reset controllers. Please add all that code too. > Practically, clock driver is always needed during the whole system running. It must be probed and never be removed. Is it a must for clock driver being a platform driver? If it's not, I'd prefer to use CLK_OF_DECLARE instead. Could you help me to understand the advantage of platform driver here? If it's really a must, I will continue to fix these bugs as you pointed. It indeed needs to be changed a lot including drivers/clk/hisilicon/clk.c. Thank you! Regards, Jiancheng From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiancheng Xue Subject: Re: [RESEND PATCH v10 2/6] clk: hisilicon: add CRG driver for hi3519 soc Date: Tue, 19 Apr 2016 15:11:55 +0800 Message-ID: <5715DA3B.1070605@hisilicon.com> References: <1459411811-12390-1-git-send-email-xuejiancheng@hisilicon.com> <1459411811-12390-3-git-send-email-xuejiancheng@hisilicon.com> <20160416004055.GN26353@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160416004055.GN26353-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Stephen Boyd Cc: mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org, khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, arnd-r2nGTMty4D4@public.gmane.org, olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, yanghongwei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, suwenping-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, raojun-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, ml.yang-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, gaofei-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, zhangzhenxing-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, Jiancheng Xue List-Id: devicetree@vger.kernel.org Hi Stephen, On 2016/4/16 8:40, Stephen Boyd wrote: > On 03/31, Jiancheng Xue wrote: >> diff --git a/drivers/clk/hisilicon/clk-hi3519.c b/drivers/clk/hisilicon/clk-hi3519.c >> new file mode 100644 >> index 0000000..ee9df82 >> --- /dev/null >> +++ b/drivers/clk/hisilicon/clk-hi3519.c >> @@ -0,0 +1,129 @@ >> +/* >> + * Hi3519 Clock Driver >> + * >> + * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see . >> + */ >> + [...] >> +static int hi3519_clk_probe(struct platform_device *pdev) >> +{ >> + struct device_node *np = pdev->dev.of_node; >> + struct hisi_clock_data *clk_data; >> + >> + clk_data = hisi_clk_init(np, HI3519_NR_CLKS); >> + if (!clk_data) >> + return -ENODEV; >> + >> + hisi_clk_register_fixed_rate(hi3519_fixed_rate_clks, >> + ARRAY_SIZE(hi3519_fixed_rate_clks), >> + clk_data); >> + hisi_clk_register_mux(hi3519_mux_clks, ARRAY_SIZE(hi3519_mux_clks), >> + clk_data); >> + hisi_clk_register_gate(hi3519_gate_clks, >> + ARRAY_SIZE(hi3519_gate_clks), clk_data); >> + >> + return hisi_reset_init(np); > > Now that this is a platform driver we need to do lots of cleanup > in error cases. I mean we need to unregister clks, OF clk > providers, and reset controllers. Please add all that code too. > Practically, clock driver is always needed during the whole system running. It must be probed and never be removed. Is it a must for clock driver being a platform driver? If it's not, I'd prefer to use CLK_OF_DECLARE instead. Could you help me to understand the advantage of platform driver here? If it's really a must, I will continue to fix these bugs as you pointed. It indeed needs to be changed a lot including drivers/clk/hisilicon/clk.c. Thank you! Regards, Jiancheng -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: xuejiancheng@hisilicon.com (Jiancheng Xue) Date: Tue, 19 Apr 2016 15:11:55 +0800 Subject: [RESEND PATCH v10 2/6] clk: hisilicon: add CRG driver for hi3519 soc In-Reply-To: <20160416004055.GN26353@codeaurora.org> References: <1459411811-12390-1-git-send-email-xuejiancheng@hisilicon.com> <1459411811-12390-3-git-send-email-xuejiancheng@hisilicon.com> <20160416004055.GN26353@codeaurora.org> Message-ID: <5715DA3B.1070605@hisilicon.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Stephen, On 2016/4/16 8:40, Stephen Boyd wrote: > On 03/31, Jiancheng Xue wrote: >> diff --git a/drivers/clk/hisilicon/clk-hi3519.c b/drivers/clk/hisilicon/clk-hi3519.c >> new file mode 100644 >> index 0000000..ee9df82 >> --- /dev/null >> +++ b/drivers/clk/hisilicon/clk-hi3519.c >> @@ -0,0 +1,129 @@ >> +/* >> + * Hi3519 Clock Driver >> + * >> + * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see . >> + */ >> + [...] >> +static int hi3519_clk_probe(struct platform_device *pdev) >> +{ >> + struct device_node *np = pdev->dev.of_node; >> + struct hisi_clock_data *clk_data; >> + >> + clk_data = hisi_clk_init(np, HI3519_NR_CLKS); >> + if (!clk_data) >> + return -ENODEV; >> + >> + hisi_clk_register_fixed_rate(hi3519_fixed_rate_clks, >> + ARRAY_SIZE(hi3519_fixed_rate_clks), >> + clk_data); >> + hisi_clk_register_mux(hi3519_mux_clks, ARRAY_SIZE(hi3519_mux_clks), >> + clk_data); >> + hisi_clk_register_gate(hi3519_gate_clks, >> + ARRAY_SIZE(hi3519_gate_clks), clk_data); >> + >> + return hisi_reset_init(np); > > Now that this is a platform driver we need to do lots of cleanup > in error cases. I mean we need to unregister clks, OF clk > providers, and reset controllers. Please add all that code too. > Practically, clock driver is always needed during the whole system running. It must be probed and never be removed. Is it a must for clock driver being a platform driver? If it's not, I'd prefer to use CLK_OF_DECLARE instead. Could you help me to understand the advantage of platform driver here? If it's really a must, I will continue to fix these bugs as you pointed. It indeed needs to be changed a lot including drivers/clk/hisilicon/clk.c. Thank you! Regards, Jiancheng