linux-kernel - Re: [RESEND PATCH v10 2/6] clk: hisilicon: add CRG driver for hi3519 soc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5715DA3B.1070605@hisilicon.com>
Date:	Tue, 19 Apr 2016 15:11:55 +0800
From:	Jiancheng Xue <xuejiancheng@...ilicon.com>
To:	Stephen Boyd <sboyd@...eaurora.org>
CC:	<mturquette@...libre.com>, <p.zabel@...gutronix.de>,
	<robh+dt@...nel.org>, <linux@....linux.org.uk>,
	<khilman@...aro.org>, <arnd@...db.de>, <olof@...om.net>,
	<xuwei5@...ilicon.com>, <linux-kernel@...r.kernel.org>,
	<linux-clk@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <yanhaifeng@...ilicon.com>,
	<yanghongwei@...ilicon.com>, <suwenping@...ilicon.com>,
	<raojun@...ilicon.com>, <ml.yang@...ilicon.com>,
	<gaofei@...ilicon.com>, <zhangzhenxing@...ilicon.com>,
	Jiancheng Xue <xuejiancheng@...wei.com>
Subject: Re: [RESEND PATCH v10 2/6] clk: hisilicon: add CRG driver for hi3519
 soc

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 <http://www.gnu.org/licenses/>.
>> + */
>> +
[...]
>> +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