linux-kernel - Re: [PATCH 3/8] reset: amlogic: split the device and platform probe

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]

Message-ID: <1j34o9pm3s.fsf@starbuckisacylon.baylibre.com>
Date: Tue, 16 Jul 2024 14:17:27 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Jan Dakinevich <jan.dakinevich@...utedevices.com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>,  Stephen Boyd
 <sboyd@...nel.org>,  Neil Armstrong <neil.armstrong@...aro.org>,
  <linux-kernel@...r.kernel.org>,  <linux-amlogic@...ts.infradead.org>,
  <linux-clk@...r.kernel.org>
Subject: Re: [PATCH 3/8] reset: amlogic: split the device and platform probe

On Tue 16 Jul 2024 at 01:48, Jan Dakinevich <jan.dakinevich@...utedevices.com> wrote:

> On 7/10/24 19:25, Jerome Brunet wrote:
>> To prepare the addition of the auxiliary device support, split
>> out the device probe from the probe of the platform device.
>> 
>> The device probe will be common to both the platform and auxiliary
>> driver.
>> 
>> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
>> ---
>>  drivers/reset/reset-meson.c | 55 +++++++++++++++++++++++--------------
>>  1 file changed, 34 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
>> index 59126c9f194a..fec55321b52b 100644
>> --- a/drivers/reset/reset-meson.c
>> +++ b/drivers/reset/reset-meson.c
>> @@ -87,6 +87,27 @@ static const struct reset_control_ops meson_reset_ops = {
>>  	.deassert	= meson_reset_deassert,
>>  };
>>  
>> +static int meson_reset_probe(struct device *dev, struct regmap *map,
>> +			     const struct meson_reset_param *param)
>> +{
>> +	unsigned int stride = regmap_get_reg_stride(map);
>> +	struct meson_reset *data;
>> +
>> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->param = param;
>> +	data->map = map;
>> +	data->rcdev.owner = dev->driver->owner;
>> +	data->rcdev.nr_resets = param->reg_count * BITS_PER_BYTE
>> +		* stride;
>> +	data->rcdev.ops = &meson_reset_ops;
>> +	data->rcdev.of_node = dev->of_node;
>
> It will be good to add here something like this. Later it would help in
> reset debugging.
>
> data->rcdev.dev = dev;

That is not the purpose of this change.
I'm merely re-organizing exiting code, not changing it.

Plus, if you refering to rcdev_name(), we already populate
rcdev->of_node, so a name is provided.

>
>> +
>> +	return devm_reset_controller_register(dev, &data->rcdev);
>> +}
>> +
>>  static const struct meson_reset_param meson8b_param = {
>>  	.reg_count	= 8,
>>  	.reset_offset	= 0x0,
>> @@ -125,46 +146,38 @@ static const struct regmap_config regmap_config = {
>>  	.reg_stride = 4,
>>  };
>>  
>> -static int meson_reset_probe(struct platform_device *pdev)
>> +static int meson_reset_pltf_probe(struct platform_device *pdev)
>>  {
>> +
>> +	const struct meson_reset_param *param;
>>  	struct device *dev = &pdev->dev;
>> -	struct meson_reset *data;
>> +	struct regmap *map;
>>  	void __iomem *base;
>>  
>> -	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> -	if (!data)
>> -		return -ENOMEM;
>> -
>>  	base = devm_platform_ioremap_resource(pdev, 0);
>>  	if (IS_ERR(base))
>>  		return PTR_ERR(base);
>>  
>> -	data->param = of_device_get_match_data(dev);
>> -	if (!data->param)
>> +	param = of_device_get_match_data(dev);
>> +	if (!param)
>>  		return -ENODEV;
>>  
>> -	data->map = devm_regmap_init_mmio(dev, base, &regmap_config);
>> -	if (IS_ERR(data->map))
>> -		return dev_err_probe(dev, PTR_ERR(data->map),
>> +	map = devm_regmap_init_mmio(dev, base, &regmap_config);
>> +	if (IS_ERR(map))
>> +		return dev_err_probe(dev, PTR_ERR(map),
>>  				     "can't init regmap mmio region\n");
>>  
>> -	data->rcdev.owner = THIS_MODULE;
>> -	data->rcdev.nr_resets = data->param->reg_count * BITS_PER_BYTE
>> -		* regmap_config.reg_stride;
>> -	data->rcdev.ops = &meson_reset_ops;
>> -	data->rcdev.of_node = dev->of_node;
>> -
>> -	return devm_reset_controller_register(dev, &data->rcdev);
>> +	return meson_reset_probe(dev, map, param);
>>  }
>>  
>> -static struct platform_driver meson_reset_driver = {
>> -	.probe	= meson_reset_probe,
>> +static struct platform_driver meson_reset_pltf_driver = {
>> +	.probe	= meson_reset_pltf_probe,
>>  	.driver = {
>>  		.name		= "meson_reset",
>>  		.of_match_table	= meson_reset_dt_ids,
>>  	},
>>  };
>> -module_platform_driver(meson_reset_driver);
>> +module_platform_driver(meson_reset_pltf_driver);
>>  
>>  MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
>>  MODULE_AUTHOR("Neil Armstrong <narmstrong@...libre.com>");

-- 
Jerome