Hi Johan, On Fri Nov 25, 2022 at 11:01 AM CET, Johan Hovold wrote: > On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote: > > Add the tables and config for the combo phy found on SM6350. > > > > Signed-off-by: Luca Weiss <luca.weiss@xxxxxxxxxxxxx> > > --- > > @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise > > qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a > > mistake on your side? I think this should probably be split out to > > another patch to not mix things up too much. > > Yeah, that's a difference in sc8280xp which does not have dedicated TX > registers for DP. Good to know. > > This is probably best handled explicitly when parsing the DT by using > dp_txa/b if they are set and otherwise fallback to txa/txb (e.g. > instead of hiding it in the v5 table by using the same offset in two > places). Are you thinking about something like this? if (offs->dp_txa) qmp->dp_tx = base + offs->dp_txa else qmp->dp_tx = base + offs->txa; if (offs->dp_txb) qmp->dp_tx2 = base + offs->dp_txb; else qmp->dp_tx2 = base + offs->txb; This wouldn't handle ".dp_txa = 0x0000" but I don't think this should be a problem, right? > > It can be done as part of this patch as long as you mention it in the > commit message. Ack. Regards Luca > > > I think other than that this patch is good. > > Indeed, looks good! Nice to see this working out as intended also for > the older platforms. > > > static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = { > > QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01), > > QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31), > > @@ -809,6 +873,8 @@ struct qmp_combo_offsets { > > u16 usb3_pcs; > > u16 usb3_pcs_usb; > > u16 dp_serdes; > > + u16 dp_txa; > > + u16 dp_txb; > > u16 dp_dp_phy; > > }; > > > > @@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = { > > "phy", > > }; > > > > +static const struct qmp_combo_offsets qmp_combo_offsets_v3 = { > > + .com = 0x0000, > > + .txa = 0x1200, > > + .rxa = 0x1400, > > + .txb = 0x1600, > > + .rxb = 0x1800, > > + .usb3_serdes = 0x1000, > > + .usb3_pcs_misc = 0x1a00, > > + .usb3_pcs = 0x1c00, > > + .dp_serdes = 0x2000, > > + .dp_txa = 0x2200, > > + .dp_txb = 0x2600, > > + .dp_dp_phy = 0x2c00, > > +}; > > + > > static const struct qmp_combo_offsets qmp_combo_offsets_v5 = { > > .com = 0x0000, > > .txa = 0x0400, > > > @@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp) > > qmp->pcs_usb = base + offs->usb3_pcs_usb; > > > > qmp->dp_serdes = base + offs->dp_serdes; > > - qmp->dp_tx = base + offs->txa; > > - qmp->dp_tx2 = base + offs->txb; > > + qmp->dp_tx = base + offs->dp_txa; > > + qmp->dp_tx2 = base + offs->dp_txb; > > qmp->dp_dp_phy = base + offs->dp_dp_phy; > > > > qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe"); > > Johan