Hi, Thanks for working on this! See a few comments below.
On Fri 21 Feb 25, 17:17, Kuba Szczodrzyński wrote: > Some Allwinner chips (notably the D1s/T113 and the A100) have a "combo > MIPI DSI D-PHY" which is required when using single-link LVDS0. > > In this mode, the DSI peripheral is not used and the PHY is not > configured for DSI. Instead, the COMBO_PHY_REGx registers are set to > enable LVDS operation. > > Enable the PHY driver to work in LVDS mode on chips with a combo D-PHY. > > Signed-off-by: Kuba Szczodrzyński <[email protected]> > --- > drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 65 ++++++++++++++++++++- > 1 file changed, 63 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c > b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c > index 36eab9527..f958e34da 100644 > --- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c > +++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c > @@ -166,8 +166,8 @@ > #define SUN50I_COMBO_PHY_REG0_EN_CP BIT(0) > > #define SUN50I_COMBO_PHY_REG1 0x114 > -#define SUN50I_COMBO_PHY_REG2_REG_VREF1P6(n) (((n) & 0x7) << 4) > -#define SUN50I_COMBO_PHY_REG2_REG_VREF0P8(n) ((n) & 0x7) > +#define SUN50I_COMBO_PHY_REG1_REG_VREF1P6(n) (((n) & 0x7) << 4) > +#define SUN50I_COMBO_PHY_REG1_REG_VREF0P8(n) ((n) & 0x7) Good catch! Would be good to mention in the commit log (or split in a separate patch but that might be overdoing it since this register wasn't used so far). > #define SUN50I_COMBO_PHY_REG2 0x118 > #define SUN50I_COMBO_PHY_REG2_HS_STOP_DLY(n) ((n) & 0xff) > @@ -181,7 +181,9 @@ struct sun6i_dphy; > > struct sun6i_dphy_variant { > void (*tx_power_on)(struct sun6i_dphy *dphy); > + void (*lvds_power_on)(struct sun6i_dphy *dphy); > bool rx_supported; > + bool is_combo_dphy; > }; > > struct sun6i_dphy { > @@ -222,6 +224,18 @@ static int sun6i_dphy_configure(struct phy *phy, union > phy_configure_opts *opts) > return 0; > } > > +static int sun6i_dphy_set_mode(struct phy *phy, enum phy_mode mode, int > submode) > +{ > + struct sun6i_dphy *dphy = phy_get_drvdata(phy); > + > + if (mode == PHY_MODE_LVDS && !dphy->variant->is_combo_dphy) { > + /* Not a combo D-PHY: LVDS is not supported */ Missing a final . in the comment. > + return -EINVAL; > + } > + > + return 0; > +} > + > static void sun6i_a31_mipi_dphy_tx_power_on(struct sun6i_dphy *dphy) > { > u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0); > @@ -329,6 +343,37 @@ static void sun50i_a100_mipi_dphy_tx_power_on(struct > sun6i_dphy *dphy) > udelay(1); > } > > +static void sun50i_a100_mipi_dphy_lvds_power_on(struct sun6i_dphy *dphy) > +{ > + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG1, > + SUN50I_COMBO_PHY_REG1_REG_VREF1P6(4) | > + SUN50I_COMBO_PHY_REG1_REG_VREF0P8(3)); > + > + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG0, > + SUN50I_COMBO_PHY_REG0_EN_CP); > + udelay(5); Please add a white space here... > + regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0, > + SUN50I_COMBO_PHY_REG0_EN_LVDS, > + SUN50I_COMBO_PHY_REG0_EN_LVDS); > + udelay(5); here too... > + regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0, > + SUN50I_COMBO_PHY_REG0_EN_COMBOLDO, > + SUN50I_COMBO_PHY_REG0_EN_COMBOLDO); > + udelay(5); here too... > + regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0, > + SUN50I_COMBO_PHY_REG0_EN_MIPI, > + SUN50I_COMBO_PHY_REG0_EN_MIPI); > + > + regmap_write(dphy->regs, SUN6I_DPHY_ANA4_REG, > + SUN6I_DPHY_ANA4_REG_EN_MIPI | > + SUN6I_DPHY_ANA4_REG_IB(2)); here too... > + regmap_write(dphy->regs, SUN6I_DPHY_ANA3_REG, > + SUN6I_DPHY_ANA3_EN_LDOR | > + SUN6I_DPHY_ANA3_EN_LDOD); here too... > + regmap_write(dphy->regs, SUN6I_DPHY_ANA2_REG, 0); and here too in order to match the coding style. > + regmap_write(dphy->regs, SUN6I_DPHY_ANA1_REG, 0); > +} > + > static int sun6i_dphy_tx_power_on(struct sun6i_dphy *dphy) > { > u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0); > @@ -492,6 +537,14 @@ static int sun6i_dphy_power_on(struct phy *phy) > { > struct sun6i_dphy *dphy = phy_get_drvdata(phy); > > + if (phy->attrs.mode == PHY_MODE_LVDS && dphy->variant->is_combo_dphy) { > + if (dphy->variant->lvds_power_on) { > + dphy->variant->lvds_power_on(dphy); > + return 0; > + } > + return -EINVAL; This would look better the other way round: first check: if (!dphy->variant->lvds_power_on) return -EINVAL; and then call the function pointer and return 0 without extra indentation. > + } > + > switch (dphy->direction) { > case SUN6I_DPHY_DIRECTION_TX: > return sun6i_dphy_tx_power_on(dphy); > @@ -514,6 +567,11 @@ static int sun6i_dphy_power_off(struct phy *phy) > regmap_write(dphy->regs, SUN6I_DPHY_ANA3_REG, 0); > regmap_write(dphy->regs, SUN6I_DPHY_ANA4_REG, 0); > > + if (phy->attrs.mode == PHY_MODE_LVDS && dphy->variant->is_combo_dphy) { > + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG1, 0); > + regmap_write(dphy->regs, SUN50I_COMBO_PHY_REG0, 0); > + } > + > return 0; > } > > @@ -533,6 +591,7 @@ static const struct phy_ops sun6i_dphy_ops = { > .configure = sun6i_dphy_configure, > .power_on = sun6i_dphy_power_on, > .power_off = sun6i_dphy_power_off, > + .set_mode = sun6i_dphy_set_mode, > .init = sun6i_dphy_init, > .exit = sun6i_dphy_exit, > }; > @@ -619,6 +678,8 @@ static const struct sun6i_dphy_variant > sun6i_a31_mipi_dphy_variant = { > > static const struct sun6i_dphy_variant sun50i_a100_mipi_dphy_variant = { > .tx_power_on = sun50i_a100_mipi_dphy_tx_power_on, > + .lvds_power_on = sun50i_a100_mipi_dphy_lvds_power_on, > + .is_combo_dphy = true, > }; > > static const struct of_device_id sun6i_dphy_of_table[] = { > -- > 2.25.1 > > -- Paul Kocialkowski, Independent contractor - sys-base - https://www.sys-base.io/ Free software developer - https://www.paulk.fr/ Expert in multimedia, graphics and embedded hardware support with Linux.
signature.asc
Description: PGP signature
