| Message ID | 20221028133603.18470-9-johan+linaro@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | phy: qcom-qmp-pcie: add support for sc8280xp | expand |
On 28-10-22, 15:35, Johan Hovold wrote: > In preparation for adding a new helper function that initialises all the > registers in one place, use a shorter name for the configuration-tables > pointer so that it becomes easier to see which table is being processed. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 26 ++++++++++++------------ > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > index dd7e72424fc0..791ed7ef0eab 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > @@ -1820,17 +1820,17 @@ static void qmp_pcie_configure(void __iomem *base, > qmp_pcie_configure_lane(base, tbl, num, 0xff); > } > > -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > +static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > { > void __iomem *serdes = qmp->serdes; > > - if (!tables) > + if (!tbls) I am not sure saving 2 chars really helps either with readability or helps with shorter name, but for now am not going to nit pick this... > return; > > - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); > + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); > } > > -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > +static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > { > const struct qmp_phy_cfg *cfg = qmp->cfg; > void __iomem *tx = qmp->tx; > @@ -1838,28 +1838,28 @@ static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_t > void __iomem *tx2 = qmp->tx2; > void __iomem *rx2 = qmp->rx2; > > - if (!tables) > + if (!tbls) > return; > > - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); > - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); > + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); > + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); > > if (cfg->lanes >= 2) { > - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); > - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); > + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); > + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); > } > } > > -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > +static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > { > void __iomem *pcs = qmp->pcs; > void __iomem *pcs_misc = qmp->pcs_misc; > > - if (!tables) > + if (!tbls) > return; > > - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); > - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); > + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); > + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); > } > > static int qmp_pcie_init(struct phy *phy) > -- > 2.37.3
On Sat, Nov 05, 2022 at 05:37:27PM +0530, Vinod Koul wrote: > On 28-10-22, 15:35, Johan Hovold wrote: > > In preparation for adding a new helper function that initialises all the > > registers in one place, use a shorter name for the configuration-tables > > pointer so that it becomes easier to see which table is being processed. > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 26 ++++++++++++------------ > > 1 file changed, 13 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > > index dd7e72424fc0..791ed7ef0eab 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > > @@ -1820,17 +1820,17 @@ static void qmp_pcie_configure(void __iomem *base, > > qmp_pcie_configure_lane(base, tbl, num, 0xff); > > } > > > > -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > > +static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > > { > > void __iomem *serdes = qmp->serdes; > > > > - if (!tables) > > + if (!tbls) > > I am not sure saving 2 chars really helps either with readability or > helps with shorter name, but for now am not going to nit pick this... As I tried to explain earlier, the reason for doing using a shorter identifier is to improve readability of the initialisation sequence in the new helper function added by the next patch. It's more obvious if you compare the end result, but you can also see it some degree for the tx/tx tables below. > > > return; > > > > - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); > > + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); > > } > > > > -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) > > +static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) > > { > > const struct qmp_phy_cfg *cfg = qmp->cfg; > > void __iomem *tx = qmp->tx; > > @@ -1838,28 +1838,28 @@ static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_t > > void __iomem *tx2 = qmp->tx2; > > void __iomem *rx2 = qmp->rx2; > > > > - if (!tables) > > + if (!tbls) > > return; > > > > - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); > > - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); > > + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); > > + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); > > > > if (cfg->lanes >= 2) { > > - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); > > - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); > > + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); > > + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); Here 'tbls' is no longer significantly longer than 'tx' or 'tx_num' which I find improves readability. After this series, there will also be a function call that uses arguments from the configuration struct and this one is consistently referred to as 'cfg' rather than 'configuration' as the latter would likewise hurt readability. Also note that the driver already used 'tbl' consistently (e.g. see qmp_pcie_configure() in the context at the top of the diff) until the new tables struct was recently added. Johan
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c index dd7e72424fc0..791ed7ef0eab 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c @@ -1820,17 +1820,17 @@ static void qmp_pcie_configure(void __iomem *base, qmp_pcie_configure_lane(base, tbl, num, 0xff); } -static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) +static void qmp_pcie_serdes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) { void __iomem *serdes = qmp->serdes; - if (!tables) + if (!tbls) return; - qmp_pcie_configure(serdes, tables->serdes, tables->serdes_num); + qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num); } -static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) +static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) { const struct qmp_phy_cfg *cfg = qmp->cfg; void __iomem *tx = qmp->tx; @@ -1838,28 +1838,28 @@ static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_t void __iomem *tx2 = qmp->tx2; void __iomem *rx2 = qmp->rx2; - if (!tables) + if (!tbls) return; - qmp_pcie_configure_lane(tx, tables->tx, tables->tx_num, 1); - qmp_pcie_configure_lane(rx, tables->rx, tables->rx_num, 1); + qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1); + qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1); if (cfg->lanes >= 2) { - qmp_pcie_configure_lane(tx2, tables->tx, tables->tx_num, 2); - qmp_pcie_configure_lane(rx2, tables->rx, tables->rx_num, 2); + qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2); + qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2); } } -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tables) +static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls) { void __iomem *pcs = qmp->pcs; void __iomem *pcs_misc = qmp->pcs_misc; - if (!tables) + if (!tbls) return; - qmp_pcie_configure(pcs, tables->pcs, tables->pcs_num); - qmp_pcie_configure(pcs_misc, tables->pcs_misc, tables->pcs_misc_num); + qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num); + qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num); } static int qmp_pcie_init(struct phy *phy)
In preparation for adding a new helper function that initialises all the registers in one place, use a shorter name for the configuration-tables pointer so that it becomes easier to see which table is being processed. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 26 ++++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-)