Re: [PATCH v4 3/6] phy: qcom-qmp-pcie: support separate tables for EP mode

From: Johan Hovold <johan@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: "Andy Gross" <agross@kernel.org>,
	"Bjorn Andersson" <andersson@kernel.org>,
	"Konrad Dybcio" <konrad.dybcio@somainline.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Jingoo Han" <jingoohan1@gmail.com>,
	"Gustavo Pimentel" <gustavo.pimentel@synopsys.com>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@ti.com>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-phy@lists.infradead.org
Subject: Re: [PATCH v4 3/6] phy: qcom-qmp-pcie: support separate tables for EP mode
Date: Mon, 26 Sep 2022 08:37:55 +0200	[thread overview]
Message-ID: <YzFIw2TUGu/H0fSk@hovoldconsulting.com> (raw)
In-Reply-To: <20220924160302.285875-4-dmitry.baryshkov@linaro.org>

On Sat, Sep 24, 2022 at 07:02:59PM +0300, Dmitry Baryshkov wrote:
> The PCIe QMP PHY requires different programming sequences when being
> used for the RC (Root Complex) or for the EP (End Point) modes. Allow
> selecting the submode and thus selecting a set of PHY programming
> tables.
> 
> Since the RC and EP modes share common some common init sequence, the
> common sequence is kept in the main table and the sequence differences
> are pushed to the extra tables.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 67 ++++++++++++++++++++----
>  1 file changed, 58 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 6e8c74585670..1fc23df59454 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -1320,10 +1320,14 @@ struct qmp_phy_cfg {
>  	/* Main init sequence for PHY blocks - serdes, tx, rx, pcs */
>  	struct qmp_phy_cfg_tables common;
>  	/*
> -	 * Additional init sequence for PHY blocks, providing additional
> -	 * register programming. Unless required it can be left omitted.
> +	 * Additional init sequences for PHY blocks, providing additional
> +	 * register programming. They are used for providing separate sequences
> +	 * for the Root Complex and for the End Point usecases.
> +	 *
> +	 * If EP mode is not supported, both tables can be left empty.
>  	 */
> -	struct qmp_phy_cfg_tables *extra;
> +	struct qmp_phy_cfg_tables *extra_rc; /* for the RC only */
> +	struct qmp_phy_cfg_tables *extra_ep; /* for the EP only */

> @@ -2278,7 +2325,9 @@ static int qmp_pcie_create(struct device *dev, struct device_node *np, int id,
>  		qphy->pcs_misc = qphy->pcs + 0x400;
>  
>  	if (IS_ERR(qphy->pcs_misc)) {
> -		if (cfg->common.pcs_misc_tbl || cfg->extra->pcs_misc_tbl)
> +		if (cfg->common.pcs_misc_tbl ||
> +		    cfg->extra_rc->pcs_misc_tbl ||
> +		    cfg->extra_ep->pcs_misc_tbl)

More NULL derefs.

>  			return PTR_ERR(qphy->pcs_misc);
>  	}

Johan