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

[Stephen Boyd <sboyd@kernel.org>]

Quoting Jerome Brunet (2024-07-10 09:25:12)
> 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;

Nitpick: I'd just put this on the line above

> +       data->rcdev.ops = &meson_reset_ops;
> +       data->rcdev.of_node = dev->of_node;
> +
> +       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);

Just use device_get_match_data()? I don't see any need to use DT
specific APIs.

> +       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");
>