linux-kernel - Re: [PATCH v4 3/6] clk: meson: eeclk: refactor eeclk common driver to support A1

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Dec 2019 17:36:17 +0800
From:   Jian Hu <jian.hu@...ogic.com>
To:     Jerome Brunet <jbrunet@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>
CC:     Kevin Hilman <khilman@...libre.com>, Rob Herring <robh@...nel.org>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Qiufang Dai <qiufang.dai@...ogic.com>,
        Jianxin Pan <jianxin.pan@...ogic.com>,
        Victor Wan <victor.wan@...ogic.com>,
        Chandle Zou <chandle.zou@...ogic.com>,
        <linux-clk@...r.kernel.org>, <linux-amlogic@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/6] clk: meson: eeclk: refactor eeclk common driver to
 support A1



On 2019/12/12 18:19, Jerome Brunet wrote:
> 
> On Fri 06 Dec 2019 at 08:40, Jian Hu <jian.hu@...ogic.com> wrote:
> 
>> Introduce a common probe function for A1 series, the way to get
>> regmap is different between A1 series and the previous series.
>> The register region is only for one clock driver, the function of
>> meson_eeclkc_probe is not fit for A1, So it is necessary to
>> introduce a new function.
> 
> Please drop this patch
> 
> #1 you are patching the EE driver for something that is no longer an EE
>   driver
> #2 Let's get the basics right, you can move (re)factoring afterward
> 
> Your probe function is simple enough. Just properly write it in each
> driver for now.
OK, I will realize it in each driver.
> 
>>
>> Signed-off-by: Jian Hu <jian.hu@...ogic.com>
>> ---
>>   drivers/clk/meson/meson-eeclk.c | 59 +++++++++++++++++++++++++++------
>>   drivers/clk/meson/meson-eeclk.h |  1 +
>>   2 files changed, 50 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/clk/meson/meson-eeclk.c b/drivers/clk/meson/meson-eeclk.c
>> index a7cb1e7aedc4..12ceb1caabd8 100644
>> --- a/drivers/clk/meson/meson-eeclk.c
>> +++ b/drivers/clk/meson/meson-eeclk.c
>> @@ -13,25 +13,37 @@
>>   #include "clk-regmap.h"
>>   #include "meson-eeclk.h"
>>   
>> -int meson_eeclkc_probe(struct platform_device *pdev)
>> +static struct regmap_config clkc_regmap_config = {
>> +	.reg_bits       = 32,
>> +	.val_bits       = 32,
>> +	.reg_stride     = 4,
>> +};
>> +
>> +static struct regmap *meson_regmap_resource(struct platform_device *pdev)
>> +{
>> +	struct resource *res;
>> +	void __iomem *base;
>> +	struct device *dev = &pdev->dev;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +	base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(base))
>> +		return ERR_CAST(base);
>> +
>> +	return devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
>> +}
>> +
>> +static int meson_common_probe(struct platform_device *pdev, struct regmap *map)
>>   {
>>   	const struct meson_eeclkc_data *data;
>>   	struct device *dev = &pdev->dev;
>> -	struct regmap *map;
>>   	int ret, i;
>>   
>>   	data = of_device_get_match_data(dev);
>>   	if (!data)
>>   		return -EINVAL;
>>   
>> -	/* Get the hhi system controller node */
>> -	map = syscon_node_to_regmap(of_get_parent(dev->of_node));
>> -	if (IS_ERR(map)) {
>> -		dev_err(dev,
>> -			"failed to get HHI regmap\n");
>> -		return PTR_ERR(map);
>> -	}
>> -
>>   	if (data->init_count)
>>   		regmap_multi_reg_write(map, data->init_regs, data->init_count);
>>   
>> @@ -54,3 +66,30 @@ int meson_eeclkc_probe(struct platform_device *pdev)
>>   	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
>>   					   data->hw_onecell_data);
>>   }
>> +
>> +int meson_eeclkc_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct regmap *map;
>> +
>> +	/* Get the hhi system controller node */
>> +	map = syscon_node_to_regmap(of_get_parent(dev->of_node));
>> +	if (IS_ERR(map)) {
>> +		dev_err(dev,
>> +			"failed to get HHI regmap\n");
>> +		return PTR_ERR(map);
>> +	}
>> +
>> +	return meson_common_probe(pdev, map);
>> +}
>> +
>> +int meson_clkc_probe(struct platform_device *pdev)
>> +{
>> +	struct regmap *map;
>> +
>> +	map = meson_regmap_resource(pdev);
>> +	if (IS_ERR(map))
>> +		return PTR_ERR(map);
>> +
>> +	return meson_common_probe(pdev, map);
>> +}
>> diff --git a/drivers/clk/meson/meson-eeclk.h b/drivers/clk/meson/meson-eeclk.h
>> index 77316207bde1..a2e9ab3a4f6b 100644
>> --- a/drivers/clk/meson/meson-eeclk.h
>> +++ b/drivers/clk/meson/meson-eeclk.h
>> @@ -21,5 +21,6 @@ struct meson_eeclkc_data {
>>   };
>>   
>>   int meson_eeclkc_probe(struct platform_device *pdev);
>> +int meson_clkc_probe(struct platform_device *pdev);
>>   
>>   #endif /* __MESON_CLKC_H */
> 
> .
>