On Mon, Sep 26, 2022 at 08:34:32PM +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@xxxxxxxxxx>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 47 +++++++++++++++++++++---
> 1 file changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index dc8f0f236212..dd7911879b10 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -14,6 +14,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/of_address.h>
> +#include <linux/phy/pcie.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> @@ -1320,10 +1321,14 @@ struct qmp_phy_cfg {
> /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */
> const struct qmp_phy_cfg_tables tables;
> /*
> - * 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.
"use cases", drop the second "for the".
> + *
> + * If EP mode is not supported, both tables can be left empty.
s/empty/unset/
> */
> const struct qmp_phy_cfg_tables *tables_rc;
> + const struct qmp_phy_cfg_tables *tables_ep;
>
> /* clock ids to be requested */
> const char * const *clk_list;
> +static int qmp_pcie_set_mode(struct phy *phy,
> + enum phy_mode mode, int submode)
No need for line break.
> +{
> + struct qmp_phy *qphy = phy_get_drvdata(phy);
> +
> + switch (submode) {
> + case PHY_MODE_PCIE_RC:
> + case PHY_MODE_PCIE_EP:
> + qphy->mode = submode;
> + break;
> + default:
> + dev_err(&phy->dev, "Unuspported submode %d\n", submode);
You forgot to fix the "unsupported" typo.
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
Looks good otherwise:
Reviewed-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
Johan
