Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support

From: Philipp Zabel <p.zabel@pengutronix.de>
To: Richard Zhu <hongxing.zhu@nxp.com>,
	l.stach@pengutronix.de, bhelgaas@google.com,
	lorenzo.pieralisi@arm.com, robh@kernel.org, shawnguo@kernel.org,
	vkoul@kernel.org, alexander.stein@ew.tq-group.com
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kernel@pengutronix.de,
	linux-imx@nxp.com
Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
Date: Mon, 04 Apr 2022 11:34:10 +0200	[thread overview]
Message-ID: <c967e6bdb586c273c187e4892b03aa82064af4ab.camel@pengutronix.de> (raw)
In-Reply-To: <1646644054-24421-2-git-send-email-hongxing.zhu@nxp.com>

Hi Richard,

On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote:
> Add the i.MX8MP PCIe PHY PERST support.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/reset/reset-imx7.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 185a333df66c..d2408725eb2c 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct
> reset_controller_dev *rcdev,
>                 break;
>  
>         case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
> +       case IMX8MP_RESET_PCIEPHY_PERST:
>                 value = assert ? 0 : bit;
>                 break;
>         }

This doesn't do what the commit description says.

The PCIEPHY_PERST bit is already supported by the driver (albeit
incorrectly?) - this patch just inverts the bit.

Since this bit is not inverted on the other platforms, and the i.MX8MP
reference manual says nothing about this, please explicitly state why
this needs to be inverted and call it a fix in the commit description.

regards
Philipp