Hi Mauro,
On 13/08/17 20:35, Mauro Carvalho Chehab wrote:
This Kaffeine's BZ:
https://bugs.kde.org/show_bug.cgi?id=374693
affects SkyStar S2 PCI DVB-S/S2 rev 3.3 device. It could be due to
a Kernel bug.
While checking the Isil 6421, comparing with its manual, available at:
http://www.intersil.com/content/dam/Intersil/documents/isl6/isl6421a.pdf
It was noticed that, if the output load is highly capacitive, a different approach
is recomended when energizing the LNBf.
Also, it is possible to detect if a current overload is happening, by checking an
special flag.
Add support for it.
Compile-tested only.
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
---
drivers/media/dvb-frontends/isl6421.c | 72 +++++++++++++++++++++++++++++++++--
1 file changed, 68 insertions(+), 4 deletions(-)
diff --git a/drivers/media/dvb-frontends/isl6421.c b/drivers/media/dvb-frontends/isl6421.c
index 838b42771a05..b04d56ad4ce8 100644
--- a/drivers/media/dvb-frontends/isl6421.c
+++ b/drivers/media/dvb-frontends/isl6421.c
@@ -38,25 +38,43 @@ struct isl6421 {
u8 override_and;
struct i2c_adapter *i2c;
u8 i2c_addr;
+ bool is_off;
};
static int isl6421_set_voltage(struct dvb_frontend *fe,
enum fe_sec_voltage voltage)
{
+ int ret;
+ u8 buf;
+ bool is_off;
struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
- struct i2c_msg msg = { .addr = isl6421->i2c_addr, .flags = 0,
- .buf = &isl6421->config,
- .len = sizeof(isl6421->config) };
+ struct i2c_msg msg[2] = {
+ {
+ .addr = isl6421->i2c_addr,
+ .flags = 0,
+ .buf = &isl6421->config,
+ .len = 1,
+ }, {
+ .addr = isl6421->i2c_addr,
+ .flags = I2C_M_RD,
+ .buf = &buf,
+ .len = 1,
+ }
+
+ };
isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
switch(voltage) {
case SEC_VOLTAGE_OFF:
+ is_off = true;
break;
case SEC_VOLTAGE_13:
+ is_off = false;
isl6421->config |= ISL6421_EN1;
break;
case SEC_VOLTAGE_18:
+ is_off = false;
isl6421->config |= (ISL6421_EN1 | ISL6421_VSEL1);
break;
default:
@@ -66,7 +84,51 @@ static int isl6421_set_voltage(struct dvb_frontend *fe,
isl6421->config |= isl6421->override_or;
isl6421->config &= isl6421->override_and;
- return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
+ /*
+ * If LNBf were not powered on, disable dynamic current limit, as,
+ * according with datasheet, highly capacitive load on the output may
+ * cause a difficult start-up.
+ */
+ if (isl6421->is_off && !is_off)
+ isl6421->config |= ISL6421_EN1;
Checking the datasheet I think we need to be setting DCL high instead. EN1 is
already set anyway.
+
+ ret = i2c_transfer(isl6421->i2c, msg, 2);
+ if (ret < 0)
+ return ret;
+ if (ret != 2)
+ return -EIO;
+
+ isl6421->is_off = is_off;
Is this in the right place?
+
+ /* On overflow, the device will try again after 900 ms (typically) */
+ if (isl6421->is_off && (buf & ISL6421_OLF1))
+ msleep(1000);
1000ms does only cover one cycle of OFF then ON - the device will keep cycling
900ms off then 20ms on until overflow is cleared so it might take longer but
adding the code to support longer is probably not worth it. Waiting one cycle
is better than the current none anyway.
+
+ if (isl6421->is_off && !is_off) {
+ isl6421->config &= ~ISL6421_EN1;
+
+ ret = i2c_transfer(isl6421->i2c, msg, 2);
+ if (ret < 0)
+ return ret;
+ if (ret != 2)
+ return -EIO;
+ }
Does this if statement ever match? isl6421->is_off and is_off are the same value
at this point. I presume this is supposed to be re-enabling DCL so would again
also need that bit instead of EN1. We might also need a little delay before
turning it on - the datasheet mentions a "chosen amount of time". I've no idea
what's appropriate here, 200ms?
A simpler if statement might be just (isl6421->config & ISL6421_DCL), we just
need to check DCL was set high above.
+
+ /* Check if overload flag is active. If so, disable power */
+ if (buf & ISL6421_OLF1) {
+ isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
+ ret = i2c_transfer(isl6421->i2c, msg, 1);
+ if (ret < 0)
+ return ret;
+ if (ret != 1)
+ return -EIO;
+ isl6421->is_off = true;
+
+ dev_warn(&isl6421->i2c->dev,
+ "Overload current detected. disabling LNBf power\n");
+ return -EINVAL;
+ }
+ return 0;
}
static int isl6421_enable_high_lnb_voltage(struct dvb_frontend *fe, long arg)
@@ -148,6 +210,8 @@ struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe, struct i2c_adapter
return NULL;
}
+ isl6421->is_off = true;
+
/* install release callback */
fe->ops.release_sec = isl6421_release;
I've just tested both your v2 patch and changes I'm suggesting above; both work
fine on my setup. Do you want me to send a v3?
Jemma.
