On 11:16-20220622, Laurent Pinchart wrote:
> Hi Rahul,
>
> Thank you for the patch.
>
> On Wed, Jun 22, 2022 at 01:23:39PM +0530, Rahul T R wrote:
> > Add support for band ctrl config for dphy tx.
> >
> > Signed-off-by: Rahul T R <r-ravikumar@xxxxxx>
> > ---
> > drivers/phy/cadence/cdns-dphy.c | 52 ++++++++++++++++++++++++++++++++-
> > 1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> > index ba042e39cfaf..ddfa524d8ce7 100644
> > --- a/drivers/phy/cadence/cdns-dphy.c
> > +++ b/drivers/phy/cadence/cdns-dphy.c
> > @@ -4,6 +4,7 @@
> > */
> >
> > #include <linux/bitops.h>
> > +#include <linux/bitfield.h>
>
> Nitpicking, bitfield goes before bitops :-)
>
> > #include <linux/clk.h>
> > #include <linux/io.h>
> > #include <linux/module.h>
> > @@ -45,6 +46,10 @@
> > #define DPHY_CMN_OPDIV_FROM_REG BIT(6)
> > #define DPHY_CMN_OPDIV(x) ((x) << 7)
> >
> > +#define DPHY_BAND_CFG DPHY_PCS(0x0)
> > +#define DPHY_BAND_CFG_LEFT_BAND GENMASK(4, 0)
> > +#define DPHY_BAND_CFG_RIGHT_BAND GENMASK(9, 5)
> > +
> > #define DPHY_PSM_CFG DPHY_PCS(0x4)
> > #define DPHY_PSM_CFG_FROM_REG BIT(0)
> > #define DPHY_PSM_CLK_DIV(x) ((x) << 1)
> > @@ -92,6 +97,22 @@ struct cdns_dphy {
> > struct phy *phy;
> > };
> >
> > +struct cdns_dphy_band {
> > + unsigned int min_rate;
> > + unsigned int max_rate;
> > +};
> > +
> > +/* Order of bands is important since the index is the band number. */
> > +static struct cdns_dphy_band tx_bands[] = {
>
> static const
>
> > + {80, 100}, {100, 120}, {120, 160}, {160, 200}, {200, 240},
> > + {240, 320}, {320, 390}, {390, 450}, {450, 510}, {510, 560},
> > + {560, 640}, {640, 690}, {690, 770}, {770, 870}, {870, 950},
> > + {950, 1000}, {1000, 1200}, {1200, 1400}, {1400, 1600}, {1600, 1800},
> > + {1800, 2000}, {2000, 2200}, {2200, 2500}
>
> The max_rate value of band N is always equal to the min_rate value of
> band N+1. Could we store one only ?
>
> > +};
> > +
> > +static int num_tx_bands = ARRAY_SIZE(tx_bands);
>
> You can use ARRAY_SIZE(tx_bands) directly below and drop this.
>
> > +
> > static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
> > struct cdns_dphy_cfg *cfg,
> > struct phy_configure_opts_mipi_dphy *opts,
> > @@ -232,6 +253,26 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
> > return 0;
> > }
> >
> > +static int cdns_dphy_tx_get_band_ctrl(unsigned long hs_clk_rate)
> > +{
> > + unsigned int rate;
> > + int i;
> > +
> > + rate = hs_clk_rate / 1000000UL;
> > +
> > + if (rate < tx_bands[0].min_rate || rate >= tx_bands[num_tx_bands - 1].max_rate)
> > + return -EOPNOTSUPP;
> > +
> > + for (i = 0; i < num_tx_bands; i++) {
> > + if (rate >= tx_bands[i].min_rate && rate < tx_bands[i].max_rate)
> > + return i;
> > + }
> > +
> > + /* Unreachable. */
> > + WARN(1, "Reached unreachable code.");
>
> I'd drop the WARN() if it's really unreachable.
>
Hi Laurent,
Thanks for the review!
I have sent a v3, addressing
all the comments
Please review
Regards
Rahul T R
> > + return -EINVAL;
> > +}
> > +
> > static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> > union phy_configure_opts *opts)
> > {
> > @@ -247,7 +288,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> > {
> > struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > struct cdns_dphy_cfg cfg = { 0 };
> > - int ret;
> > + int ret, band_ctrl;
> > + unsigned int reg;
> >
> > ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> > if (ret)
> > @@ -276,6 +318,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> > */
> > cdns_dphy_set_pll_cfg(dphy, &cfg);
> >
> > + band_ctrl = cdns_dphy_tx_get_band_ctrl(opts->mipi_dphy.hs_clk_rate);
> > + if (band_ctrl < 0)
> > + return band_ctrl;
> > +
> > + reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
> > + FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
> > + writel(reg, dphy->regs + DPHY_BAND_CFG);
> > +
> > return 0;
> > }
> >
>
> --
> Regards,
>
> Laurent Pinchart
