Re: [PATCH v8 2/6] phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver

[Vinod Koul <vkoul@kernel.org>]

On 30-07-26, 22:13, Qiang Yu wrote:
> Some QMP PCIe PHY hardware blocks support multiple link topologies (e.g.
> x8 or x4+x4) selected via a TCSR register. The existing single-instance
> QMP PCIe PHY driver has no way to model this: it assumes a single cfg per
> DT node and instantiates exactly one PHY.
> 
> Add a dedicated driver for this class of PHY. Match data carries a
> per-mode cfg table; qmp_pcie_multiphy_probe() reads the current link
> mode from the TCSR register pointed to by "qcom,link-mode", looks up the
> corresponding cfg array, and instantiates one qmp_pcie per sub-PHY
> required by that link mode, registering the clock and #phy-cells = <1> phy
> providers so consumers can address individual sub-PHYs by index.
> 
> The driver inherits the phy setting and link-mode programmed by firmware,
> so only the no_csr reset is used and no phy setting tables are provided.
> 
> Add the first match data and compatible, qcom,glymur-qmp-gen5x8-pcie-phy,
> for the Glymur Gen5 PCIe PHY that can bifurcate into two x4 links or
> operate as a single x8 link.
> 
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/Kconfig                      |  11 +
>  drivers/phy/qualcomm/Makefile                     |   1 +
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c | 744 ++++++++++++++++++++++
>  3 files changed, 756 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 60a0ead127fa..8d45820bc84a 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -77,6 +77,17 @@ config PHY_QCOM_QMP_PCIE
>  	  Enable this to support the QMP PCIe PHY transceiver that is used
>  	  with PCIe controllers on Qualcomm chips.
>  
> +config PHY_QCOM_QMP_PCIE_MULTIPHY
> +	tristate "Qualcomm QMP PCIe Multi PHY Driver"
> +	depends on PCI || COMPILE_TEST
> +	select GENERIC_PHY
> +	default PHY_QCOM_QMP

Can you please drop this. Let the respective config select it

> +
> +static struct clk_hw *qmp_pcie_multiphy_clk_hw_get(struct of_phandle_args *clkspec,
> +						    void *data)

please align this to preceding open braces. I think this might look
neater:
static struct clk_hw *
qmp_pcie_multiphy_clk_hw_get(struct of_phandle_args *clkspec, void *data)

> +{
> +	struct qmp_pcie_multiphy *qmp_data = data;
> +	unsigned int idx = 0;
> +
> +	if (clkspec->args_count)
> +		idx = clkspec->args[0];
> +
> +	if (idx < (unsigned int)qmp_data->num_pipe_outputs)
> +		return &qmp_data->pipe_out_clks[idx].hw;
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +static int qmp_pcie_multiphy_register_clocks(struct device *dev,
> +					      struct device_node *np,
> +					      struct qmp_pcie_multiphy *qmp_data)

please align these two to preceding open braces


> +static int qmp_pcie_probe_phy(struct qmp_pcie *qmp, struct device_node *np,
> +			      struct phy **out_phy)
> +{
> +	int ret;
> +
> +	ret = qmp_pcie_get_mmio(qmp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qmp_pcie_clk_init(qmp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qmp_pcie_reset_init(qmp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qmp_pcie_vreg_init(qmp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qmp_pcie_pd_init(qmp);
> +	if (ret)
> +		return ret;
> +
> +	*out_phy = devm_phy_create(qmp->dev, np, &qmp_pcie_phy_ops);
> +	if (IS_ERR(*out_phy))
> +		return PTR_ERR(*out_phy);
> +
> +	phy_set_drvdata(*out_phy, qmp);
> +
> +	return 0;
> +}
> +
> +

Two empty lines, please drop one

-- 
~Vinod