Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 9 Mar 2021 00:35:54 +0100
From:   Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>
To:     Jim Quinlan <jim2101024@gmail.com>
Cc:     Bjorn Helgaas <helgaas@kernel.org>, linux-pci@vger.kernel.org,
        Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
        bcm-kernel-feedback-list@broadcom.com, james.quinlan@broadcom.com,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Jim Quinlan <jquinlan@broadcom.com>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-rpi-kernel@lists.infradead.org>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of
 deassert/assert
Message-ID: <YEa02utsc/thd8FV@rocinante>
References: <20210308195037.1503-1-jim2101024@gmail.com>
 <20210308195037.1503-3-jim2101024@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20210308195037.1503-3-jim2101024@gmail.com>
Precedence: bulk

Hi,

Thank you for sending the patches over!

> The Brcmstb PCIe RC uses a reset control "rescal" for certain chips.  This
[...]

A small suggestion: it would be nicer to mention "Broadcom STB" rather
than "Brcmstb" in the sentence above.

[...]
> +err1:
> +	reset_control_rearm(pcie->rescal);
> +err0:
>  	clk_disable_unprepare(pcie->clk);
>  	return ret;
[...]

A small nitpick.  Now that there are two labels on the error recovery
path, it might be better to name both of these labels a little bit
better.  Some examples from the PCI tree:

  error_clock_unprepare
  err_disable_clock
  err_disable_clk
  err_clk_disable
  
So it could be:

  err_reset:                                <-- or err_rearm or even err_reset_rearm, etc.
  	reset_control_rearm(pcie->rescal);
  err_disable_clk:
   	clk_disable_unprepare(pcie->clk);

What do you think?

Krzysztof