Re: [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs

From: "Krzysztof Wilczyński" <kw@linux.com>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Binghui Wang <wangbinghui@hisilicon.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	Xiaowei Song <songxiaowei@hisilicon.com>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
Date: Wed, 3 Feb 2021 16:18:01 +0100	[thread overview]
Message-ID: <YBq+qaOwJdNOllQ/@rocinante> (raw)
In-Reply-To: <4fb97b1fc3fe6df9a2fea8f96bdef433e75463a6.1612335031.git.mchehab+huawei@kernel.org>

Hi Mauro,

Thank you for working on this!

> @@ -151,8 +152,10 @@ struct kirin_pcie {
>       struct clk      *phy_ref_clk;
>       struct clk      *pcie_aclk;
>       struct clk      *pcie_aux_clk;
> -     int             gpio_id_reset[4];
> +     int             n_gpio_resets;
>       int             gpio_id_clkreq[3];
> +     int             gpio_id_reset[MAX_GPIO_RESETS];
> +     const char      *reset_names[MAX_GPIO_RESETS];
>       u32             eye_param[5];
>  };
[...]

A small nit, so feel free to ignore, of course.

The "n_gpio_resets" variable might be better as "gpio_resets_num" or
"gpio_resets_count" - both are popular name suffixes for that type of
variables.  To add, other variables also start with "gpio_", thus it
would also follow the naming pattern.

[...]
> +     kirin_pcie->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");
[...]

This would then become (for example):

  kirin_pcie->gpio_resets_count = of_gpio_named_count(np, "reset-gpios");

Krzysztof