| Message ID | 1515773982-6411-5-git-send-email-brad@nextdimension.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello And what is rationale here, is there some use case demod must be active and ts set to tristate (disabled)? Just put demod sleep when you don't use it. regards Antti On 01/12/2018 06:19 PM, Brad Love wrote: > Includes a function to set TS MODE property os si2168. The function > either disables the TS output bus, or sets mode to config option. > > When going to sleep the TS bus is turned off, this makes the driver > compatible with multiple frontend usage. > > Signed-off-by: Brad Love <brad@nextdimension.cc> > --- > drivers/media/dvb-frontends/si2168.c | 38 ++++++++++++++++++++++++++++-------- > drivers/media/dvb-frontends/si2168.h | 1 + > 2 files changed, 31 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c > index 539399d..429c03a 100644 > --- a/drivers/media/dvb-frontends/si2168.c > +++ b/drivers/media/dvb-frontends/si2168.c > @@ -409,6 +409,30 @@ static int si2168_set_frontend(struct dvb_frontend *fe) > return ret; > } > > +static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire) > +{ > + struct i2c_client *client = fe->demodulator_priv; > + struct si2168_dev *dev = i2c_get_clientdata(client); > + struct si2168_cmd cmd; > + int ret = 0; > + > + dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire); > + > + /* set TS_MODE property */ > + memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); > + if (acquire) > + cmd.args[4] |= dev->ts_mode; > + else > + cmd.args[4] |= SI2168_TS_TRISTATE; > + if (dev->ts_clock_gapped) > + cmd.args[4] |= 0x40; > + cmd.wlen = 6; > + cmd.rlen = 4; > + ret = si2168_cmd_execute(client, &cmd); > + > + return ret; > +} > + > static int si2168_init(struct dvb_frontend *fe) > { > struct i2c_client *client = fe->demodulator_priv; > @@ -540,14 +564,7 @@ static int si2168_init(struct dvb_frontend *fe) > dev->version >> 24 & 0xff, dev->version >> 16 & 0xff, > dev->version >> 8 & 0xff, dev->version >> 0 & 0xff); > > - /* set ts mode */ > - memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); > - cmd.args[4] |= dev->ts_mode; > - if (dev->ts_clock_gapped) > - cmd.args[4] |= 0x40; > - cmd.wlen = 6; > - cmd.rlen = 4; > - ret = si2168_cmd_execute(client, &cmd); > + ret = si2168_ts_bus_ctrl(fe, 1); > if (ret) > goto err; > > @@ -584,6 +601,9 @@ static int si2168_sleep(struct dvb_frontend *fe) > > dev->active = false; > > + /* tri-state data bus */ > + si2168_ts_bus_ctrl(fe, 0); > + > /* Firmware B 4.0-11 or later loses warm state during sleep */ > if (dev->version > ('B' << 24 | 4 << 16 | 0 << 8 | 11 << 0)) > dev->warm = false; > @@ -681,6 +701,8 @@ static const struct dvb_frontend_ops si2168_ops = { > .init = si2168_init, > .sleep = si2168_sleep, > > + .ts_bus_ctrl = si2168_ts_bus_ctrl, > + > .set_frontend = si2168_set_frontend, > > .read_status = si2168_read_status, > diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h > index 3225d0c..f48f0fb 100644 > --- a/drivers/media/dvb-frontends/si2168.h > +++ b/drivers/media/dvb-frontends/si2168.h > @@ -38,6 +38,7 @@ struct si2168_config { > /* TS mode */ > #define SI2168_TS_PARALLEL 0x06 > #define SI2168_TS_SERIAL 0x03 > +#define SI2168_TS_TRISTATE 0x00 > u8 ts_mode; > > /* TS clock inverted */ >
On 2018-01-15 23:07, Antti Palosaari wrote: > Hello > And what is rationale here, is there some use case demod must be > active and ts set to tristate (disabled)? Just put demod sleep when > you don't use it. > > regards > Antti Hello Antti, Perhaps the .ts_bus_ctrl callback does not need to be included in ops, but the function is necessary. The demod is already put to sleep when not in use, but it leaves the ts bus open. The ts bus has no reason to be open when the demod is put to sleep. Leaving the ts bus open during sleep affects the other connected demod and nothing is received by it. The lgdt3306a driver already tri states its ts bus when put to sleep, the si2168 should as well. Cheers, Brad > > On 01/12/2018 06:19 PM, Brad Love wrote: >> Includes a function to set TS MODE property os si2168. The function >> either disables the TS output bus, or sets mode to config option. >> >> When going to sleep the TS bus is turned off, this makes the driver >> compatible with multiple frontend usage. >> >> Signed-off-by: Brad Love <brad@nextdimension.cc> >> --- >> drivers/media/dvb-frontends/si2168.c | 38 >> ++++++++++++++++++++++++++++-------- >> drivers/media/dvb-frontends/si2168.h | 1 + >> 2 files changed, 31 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/media/dvb-frontends/si2168.c >> b/drivers/media/dvb-frontends/si2168.c >> index 539399d..429c03a 100644 >> --- a/drivers/media/dvb-frontends/si2168.c >> +++ b/drivers/media/dvb-frontends/si2168.c >> @@ -409,6 +409,30 @@ static int si2168_set_frontend(struct >> dvb_frontend *fe) >> return ret; >> } >> +static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire) >> +{ >> + struct i2c_client *client = fe->demodulator_priv; >> + struct si2168_dev *dev = i2c_get_clientdata(client); >> + struct si2168_cmd cmd; >> + int ret = 0; >> + >> + dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire); >> + >> + /* set TS_MODE property */ >> + memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); >> + if (acquire) >> + cmd.args[4] |= dev->ts_mode; >> + else >> + cmd.args[4] |= SI2168_TS_TRISTATE; >> + if (dev->ts_clock_gapped) >> + cmd.args[4] |= 0x40; >> + cmd.wlen = 6; >> + cmd.rlen = 4; >> + ret = si2168_cmd_execute(client, &cmd); >> + >> + return ret; >> +} >> + >> static int si2168_init(struct dvb_frontend *fe) >> { >> struct i2c_client *client = fe->demodulator_priv; >> @@ -540,14 +564,7 @@ static int si2168_init(struct dvb_frontend *fe) >> dev->version >> 24 & 0xff, dev->version >> 16 & 0xff, >> dev->version >> 8 & 0xff, dev->version >> 0 & 0xff); >> - /* set ts mode */ >> - memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); >> - cmd.args[4] |= dev->ts_mode; >> - if (dev->ts_clock_gapped) >> - cmd.args[4] |= 0x40; >> - cmd.wlen = 6; >> - cmd.rlen = 4; >> - ret = si2168_cmd_execute(client, &cmd); >> + ret = si2168_ts_bus_ctrl(fe, 1); >> if (ret) >> goto err; >> @@ -584,6 +601,9 @@ static int si2168_sleep(struct dvb_frontend *fe) >> dev->active = false; >> + /* tri-state data bus */ >> + si2168_ts_bus_ctrl(fe, 0); >> + >> /* Firmware B 4.0-11 or later loses warm state during sleep */ >> if (dev->version > ('B' << 24 | 4 << 16 | 0 << 8 | 11 << 0)) >> dev->warm = false; >> @@ -681,6 +701,8 @@ static const struct dvb_frontend_ops si2168_ops = { >> .init = si2168_init, >> .sleep = si2168_sleep, >> + .ts_bus_ctrl = si2168_ts_bus_ctrl, >> + >> .set_frontend = si2168_set_frontend, >> .read_status = si2168_read_status, >> diff --git a/drivers/media/dvb-frontends/si2168.h >> b/drivers/media/dvb-frontends/si2168.h >> index 3225d0c..f48f0fb 100644 >> --- a/drivers/media/dvb-frontends/si2168.h >> +++ b/drivers/media/dvb-frontends/si2168.h >> @@ -38,6 +38,7 @@ struct si2168_config { >> /* TS mode */ >> #define SI2168_TS_PARALLEL 0x06 >> #define SI2168_TS_SERIAL 0x03 >> +#define SI2168_TS_TRISTATE 0x00 >> u8 ts_mode; >> /* TS clock inverted */ >> >
On 01/16/2018 07:31 PM, Brad Love wrote: > > On 2018-01-15 23:07, Antti Palosaari wrote: >> Hello >> And what is rationale here, is there some use case demod must be >> active and ts set to tristate (disabled)? Just put demod sleep when >> you don't use it. >> >> regards >> Antti > > Hello Antti, > > Perhaps the .ts_bus_ctrl callback does not need to be included in ops, > but the function is necessary. The demod is already put to sleep when > not in use, but it leaves the ts bus open. The ts bus has no reason to > be open when the demod is put to sleep. Leaving the ts bus open during > sleep affects the other connected demod and nothing is received by it. > The lgdt3306a driver already tri states its ts bus when put to sleep, > the si2168 should as well. Sounds possible, but unlikely as chip is firmware driven. When you put chip to sleep you usually want set ts pins to tristate (also other unused pins) in order to save energy. I haven't never tested it anyway though, so it could be possible it leaves those pins to some other state like random output at given time. And if you cannot get stream from lgdt3306a, which is connected to same bus, it really sounds like ts bus pins are left some state (cannot work if same pin is driven high to other demod whilst other tries to drive it low. Setting ts pins to tri-state during sleep should resolve your issue. regards Antti
On 2018-01-16 13:32, Antti Palosaari wrote: > On 01/16/2018 07:31 PM, Brad Love wrote: >> >> On 2018-01-15 23:07, Antti Palosaari wrote: >>> Hello >>> And what is rationale here, is there some use case demod must be >>> active and ts set to tristate (disabled)? Just put demod sleep when >>> you don't use it. >>> >>> regards >>> Antti >> >> Hello Antti, >> >> Perhaps the .ts_bus_ctrl callback does not need to be included in ops, >> but the function is necessary. The demod is already put to sleep when >> not in use, but it leaves the ts bus open. The ts bus has no reason to >> be open when the demod is put to sleep. Leaving the ts bus open during >> sleep affects the other connected demod and nothing is received by it. >> The lgdt3306a driver already tri states its ts bus when put to sleep, >> the si2168 should as well. > > Sounds possible, but unlikely as chip is firmware driven. When you put > chip to sleep you usually want set ts pins to tristate (also other > unused pins) in order to save energy. I haven't never tested it anyway > though, so it could be possible it leaves those pins to some other > state like random output at given time. > > And if you cannot get stream from lgdt3306a, which is connected to > same bus, it really sounds like ts bus pins are left some state > (cannot work if same pin is driven high to other demod whilst other > tries to drive it low. > > Setting ts pins to tri-state during sleep should resolve your issue. Hello Antti, This patch fixes the issue I'm describing, hence why I submitted it. The ts bus must be tristated before putting the chip to sleep for the other demod to get a stream. Cheers, Brad > > > regards > Antti
On 01/16/2018 10:14 PM, Brad Love wrote: > > On 2018-01-16 13:32, Antti Palosaari wrote: >> On 01/16/2018 07:31 PM, Brad Love wrote: >>> >>> On 2018-01-15 23:07, Antti Palosaari wrote: >>>> Hello >>>> And what is rationale here, is there some use case demod must be >>>> active and ts set to tristate (disabled)? Just put demod sleep when >>>> you don't use it. >>>> >>>> regards >>>> Antti >>> >>> Hello Antti, >>> >>> Perhaps the .ts_bus_ctrl callback does not need to be included in ops, >>> but the function is necessary. The demod is already put to sleep when >>> not in use, but it leaves the ts bus open. The ts bus has no reason to >>> be open when the demod is put to sleep. Leaving the ts bus open during >>> sleep affects the other connected demod and nothing is received by it. >>> The lgdt3306a driver already tri states its ts bus when put to sleep, >>> the si2168 should as well. >> >> Sounds possible, but unlikely as chip is firmware driven. When you put >> chip to sleep you usually want set ts pins to tristate (also other >> unused pins) in order to save energy. I haven't never tested it anyway >> though, so it could be possible it leaves those pins to some other >> state like random output at given time. >> >> And if you cannot get stream from lgdt3306a, which is connected to >> same bus, it really sounds like ts bus pins are left some state >> (cannot work if same pin is driven high to other demod whilst other >> tries to drive it low. >> >> Setting ts pins to tri-state during sleep should resolve your issue. > > Hello Antti, > > This patch fixes the issue I'm describing, hence why I submitted it. The > ts bus must be tristated before putting the chip to sleep for the other > demod to get a stream. > I can test tri-state using power meter on some day, but it may be so small current that it cannot be seen usb power meter I use (YZXstudio, very nice small power meter). regards Antti
On 2018-01-16 14:38, Antti Palosaari wrote: > On 01/16/2018 10:14 PM, Brad Love wrote: >> >> On 2018-01-16 13:32, Antti Palosaari wrote: >>> On 01/16/2018 07:31 PM, Brad Love wrote: >>>> >>>> On 2018-01-15 23:07, Antti Palosaari wrote: >>>>> Hello >>>>> And what is rationale here, is there some use case demod must be >>>>> active and ts set to tristate (disabled)? Just put demod sleep when >>>>> you don't use it. >>>>> >>>>> regards >>>>> Antti >>>> >>>> Hello Antti, >>>> >>>> Perhaps the .ts_bus_ctrl callback does not need to be included in ops, >>>> but the function is necessary. The demod is already put to sleep when >>>> not in use, but it leaves the ts bus open. The ts bus has no reason to >>>> be open when the demod is put to sleep. Leaving the ts bus open during >>>> sleep affects the other connected demod and nothing is received by it. >>>> The lgdt3306a driver already tri states its ts bus when put to sleep, >>>> the si2168 should as well. >>> >>> Sounds possible, but unlikely as chip is firmware driven. When you put >>> chip to sleep you usually want set ts pins to tristate (also other >>> unused pins) in order to save energy. I haven't never tested it anyway >>> though, so it could be possible it leaves those pins to some other >>> state like random output at given time. >>> >>> And if you cannot get stream from lgdt3306a, which is connected to >>> same bus, it really sounds like ts bus pins are left some state >>> (cannot work if same pin is driven high to other demod whilst other >>> tries to drive it low. >>> >>> Setting ts pins to tri-state during sleep should resolve your issue. >> >> Hello Antti, >> >> This patch fixes the issue I'm describing, hence why I submitted it. The >> ts bus must be tristated before putting the chip to sleep for the other >> demod to get a stream. >> > > I can test tri-state using power meter on some day, but it may be so > small current that it cannot be seen usb power meter I use (YZXstudio, > very nice small power meter). > > regards > Antti > Nifty looking devices, one just fell into my shopping cart :) Cheers, Brad
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c index 539399d..429c03a 100644 --- a/drivers/media/dvb-frontends/si2168.c +++ b/drivers/media/dvb-frontends/si2168.c @@ -409,6 +409,30 @@ static int si2168_set_frontend(struct dvb_frontend *fe) return ret; } +static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire) +{ + struct i2c_client *client = fe->demodulator_priv; + struct si2168_dev *dev = i2c_get_clientdata(client); + struct si2168_cmd cmd; + int ret = 0; + + dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire); + + /* set TS_MODE property */ + memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); + if (acquire) + cmd.args[4] |= dev->ts_mode; + else + cmd.args[4] |= SI2168_TS_TRISTATE; + if (dev->ts_clock_gapped) + cmd.args[4] |= 0x40; + cmd.wlen = 6; + cmd.rlen = 4; + ret = si2168_cmd_execute(client, &cmd); + + return ret; +} + static int si2168_init(struct dvb_frontend *fe) { struct i2c_client *client = fe->demodulator_priv; @@ -540,14 +564,7 @@ static int si2168_init(struct dvb_frontend *fe) dev->version >> 24 & 0xff, dev->version >> 16 & 0xff, dev->version >> 8 & 0xff, dev->version >> 0 & 0xff); - /* set ts mode */ - memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6); - cmd.args[4] |= dev->ts_mode; - if (dev->ts_clock_gapped) - cmd.args[4] |= 0x40; - cmd.wlen = 6; - cmd.rlen = 4; - ret = si2168_cmd_execute(client, &cmd); + ret = si2168_ts_bus_ctrl(fe, 1); if (ret) goto err; @@ -584,6 +601,9 @@ static int si2168_sleep(struct dvb_frontend *fe) dev->active = false; + /* tri-state data bus */ + si2168_ts_bus_ctrl(fe, 0); + /* Firmware B 4.0-11 or later loses warm state during sleep */ if (dev->version > ('B' << 24 | 4 << 16 | 0 << 8 | 11 << 0)) dev->warm = false; @@ -681,6 +701,8 @@ static const struct dvb_frontend_ops si2168_ops = { .init = si2168_init, .sleep = si2168_sleep, + .ts_bus_ctrl = si2168_ts_bus_ctrl, + .set_frontend = si2168_set_frontend, .read_status = si2168_read_status, diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h index 3225d0c..f48f0fb 100644 --- a/drivers/media/dvb-frontends/si2168.h +++ b/drivers/media/dvb-frontends/si2168.h @@ -38,6 +38,7 @@ struct si2168_config { /* TS mode */ #define SI2168_TS_PARALLEL 0x06 #define SI2168_TS_SERIAL 0x03 +#define SI2168_TS_TRISTATE 0x00 u8 ts_mode; /* TS clock inverted */
Includes a function to set TS MODE property os si2168. The function either disables the TS output bus, or sets mode to config option. When going to sleep the TS bus is turned off, this makes the driver compatible with multiple frontend usage. Signed-off-by: Brad Love <brad@nextdimension.cc> --- drivers/media/dvb-frontends/si2168.c | 38 ++++++++++++++++++++++++++++-------- drivers/media/dvb-frontends/si2168.h | 1 + 2 files changed, 31 insertions(+), 8 deletions(-)