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

Jerome Brunet
2024-07-16T12:17:30Z

On Tue 16 Jul 2024 at 01:48, Jan Dakinevich <jan.dakinevich@salutedevices.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@baylibre.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@baylibre.com>");

--
Jerome