Hi Marc, On Fri, Jun 19, 2020 at 3:15 PM Marc Gonzalez <marc.w.gonzalez@xxxxxxx> wrote: > > On 10/06/2020 17:22, Marc Gonzalez wrote: > > > FTR, on IRC, Brad pointed out this patch of his: > > https://patchwork.kernel.org/patch/10744999/ > > I suggest the following patch on top of Brad's patch: > > Author: Marc Gonzalez <marc.w.gonzalez@xxxxxxx> > Date: Fri Jun 19 22:09:26 2020 +0200 > > si2168: wait for carrier lock before next step > > diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c > index 31d3dc0216c2..e127e842f671 100644 > --- a/drivers/media/dvb-frontends/si2168.c > +++ b/drivers/media/dvb-frontends/si2168.c > @@ -152,6 +152,11 @@ static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire) > return ret; > } > > +static bool carrier_locked(struct si2168_cmd *cmd) > +{ > + return cmd->args[2] & BIT(1); > +} > + > static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status) > { > struct i2c_client *client = fe->demodulator_priv; > @@ -180,6 +185,9 @@ static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status) > if (ret) > goto err; > > + if (!carrier_locked(&cmd)) > + goto parse_response; > + My original patch has been well tested and is currently deployed in many thousands of assorted systems across Europe. Unless you can guarantee that the frontend switchover race condition will *never* happen *ever* across any system, including a large amount of architectures and array of cpu types and speeds, I don't think it's beneficial to remove it. Hence, I'm very hesitant to deploy your patch and break this auto plp detection for someone, just to save <=10ms. Regards, Brad > if ((cmd.args[3] & 0x0f) == 7) > sys = SYS_DVBT2; > } > @@ -206,27 +214,10 @@ static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status) > } > > ret = si2168_cmd_execute(client, &cmd); > - if (dvbt_auto_plp && (ret == -EREMOTEIO)) { > - /* In auto-PLP mode it is possible to read 0x8701 while > - * the frontend is in switchover transition. This causes > - * a status read failure, due to incorrect system. Check > - * the other sys if we hit this race condition. > - */ > - if (sys == SYS_DVBT) { > - memcpy(cmd.args, "\x50\x01", 2); /* DVB-T2 */ > - cmd.wlen = 2; > - cmd.rlen = 14; > - ret = si2168_cmd_execute(client, &cmd); > - } else if (sys == SYS_DVBT2) { > - memcpy(cmd.args, "\xa0\x01", 2); /* DVB-T */ > - cmd.wlen = 2; > - cmd.rlen = 13; > - ret = si2168_cmd_execute(client, &cmd); > - } > - } > if (ret) > goto err; > > +parse_response: > switch ((cmd.args[2] >> 1) & 0x03) { > case 0x01: > *status = FE_HAS_SIGNAL | FE_HAS_CARRIER; >
