On Tue, Aug 20, 2019 at 4:58 PM Dinh Nguyen <dingu...@kernel.org> wrote:

> @@ -401,6 +402,26 @@ static int amba_device_try_add(struct amba_device *dev, 
> struct resource *parent)
>         ret = amba_get_enable_pclk(dev);
>         if (ret == 0) {
>                 u32 pid, cid;
> +               int count;
> +               struct reset_control *rstc;
> +
> +               /*
> +                * Find reset control(s) of the amba bus and de-assert them.
> +                */
> +               count = reset_control_get_count(&dev->dev);
> +               while (count > 0) {
> +                       rstc = 
> of_reset_control_get_shared_by_index(dev->dev.of_node, count - 1);
> +                       if (IS_ERR(rstc)) {
> +                               if (PTR_ERR(rstc) == -EPROBE_DEFER)
> +                                       ret = -EPROBE_DEFER;
> +                               else
> +                                       dev_err(&dev->dev, "Can't get amba 
> reset!\n");
> +                               break;
> +                       }
> +                       reset_control_deassert(rstc);
> +                       reset_control_put(rstc);
> +                       count--;
> +               }

I'm not normally a footprint person, but the looks of the stubs in
<linux/reset.h> makes me suspicious whether this will have zero impact
in size on platforms without reset controllers.

Can you just ls -al on the kernel without CONFIG_RESET_CONTROLLER
before and after this patch and ascertain that it has zero footprint effect?

If it doesn't I'd sure like to break this into its own function and
stick a if (!IS_ENABLED(CONFIG_RESET_CONTROLLER)) return 0;
in there to make sure the compiler drops it.

Also it'd be nice to get Philipp's ACK on the semantics, though they
look correct to me.

Yours,
Linus Walleij

Reply via email to