Hi Tony,
it looks as if something with this patch is broken on GTA04. For v5.6 and v5.7-rc1.
HDQ battery access times out after ca. 15 seconds and I get temperature of -273.1°C...
Reverting this patch and everything is ok again.
What is "ti,mode" about? Do we have that (indirectly) in gta04.dtsi?
Or does this patch need some CONFIGs we do not happen to have?
BR and thanks,
Nikolaus
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=0
POWER_SUPPLY_CURRENT_NOW=0
POWER_SUPPLY_CAPACITY=0
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=-2731
POWER_SUPPLY_TIME_TO_EMPTY_NOW=0
POWER_SUPPLY_TIME_TO_EMPTY_AVG=0
POWER_SUPPLY_TIME_TO_FULL_NOW=0
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=0
POWER_SUPPLY_CHARGE_NOW=0
POWER_SUPPLY_CHARGE_FULL_DESIGN=0
POWER_SUPPLY_CYCLE_COUNT=0
POWER_SUPPLY_ENERGY_NOW=0
POWER_SUPPLY_POWER_AVG=0
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real 0m15.761s
user 0m0.001s
sys 0m0.025s
root@letux:~#
root@letux:~# dmesg|fgrep -i hdq
[ 20.252136] omap_hdq 480b2000.1w: OMAP HDQ Hardware Rev 0.5. Driver in Interrupt mode
root@letux:~#
> Am 17.12.2019 um 01:40 schrieb Tony Lindgren <[email protected]>:
>
> We've had generic code handling module sysconfig and OCP reset registers
> for omap variants for many years now and all the drivers really needs to
> do is just call runtime PM functions.
>
> Looks like the omap-hdq driver got only partially updated over the years
> to use runtime PM, and still has lots of custom PM code left.
>
> We can replace all the custom code for sysconfig, OCP reset, and PM with
> just a few lines of runtime PM autosuspend code.
>
> In order to set the device mode properly when pm_runtime_get_sync() is
> called during probe, we need to also move parsing of "ti,mode" to happen
> earlier before we call pm_runtime_enable().
>
> Since we now disable interrupts lazily in omap_hdq_runtime_suspend(), we
> must remove the call to hdq_disable_interrupt() in omap_w1_read_byte().
> And we must clear irqstatus calling wait_event_timeout() on it, so let's
> add hdq_reset_irqstatus() for that.
>
> Note that the earlier driver specific usage count limit of four seems
> completely artificial and should not be an issue in normal use.
>
> Cc: Adam Ford <[email protected]>
> Cc: Andrew F. Davis <[email protected]>
> Cc: Andreas Kemnade <[email protected]>
> Cc: H. Nikolaus Schaller <[email protected]>
> Cc: Vignesh R <[email protected]>
> Tested-by: Andreas Kemnade <[email protected]> # gta04
> Tested-by: Adam Ford <[email protected]> #logicpd-torpedo-37xx-devkit
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
>
> Changes since v2:
>
> - Added hdq_reset_irqstatus() and fixed up the calls so ti,mode = "1w"
> works
>
> - Kept earlier Tested-by from Adam and Andreas as the hdq_reset_irqstatus()
> fix only affected "1w" mode and not "hdq" mode
>
> Changes since v1:
>
> - Drop call to hdq_disable_interrupt() as we now disable interrupts
> lazily un runtime_suspend
>
> - Parse "ti,mode" earlier before first pm_runtime_get_sync() so the
> mode gets set properly also during the probe
>
>
> drivers/w1/masters/omap_hdq.c | 348 +++++++++++-----------------------
> 1 file changed, 113 insertions(+), 235 deletions(-)
>
> diff --git a/drivers/w1/masters/omap_hdq.c b/drivers/w1/masters/omap_hdq.c
> --- a/drivers/w1/masters/omap_hdq.c
> +++ b/drivers/w1/masters/omap_hdq.c
> @@ -38,12 +38,6 @@
> #define OMAP_HDQ_INT_STATUS_TXCOMPLETE BIT(2)
> #define OMAP_HDQ_INT_STATUS_RXCOMPLETE BIT(1)
> #define OMAP_HDQ_INT_STATUS_TIMEOUT BIT(0)
> -#define OMAP_HDQ_SYSCONFIG 0x14
> -#define OMAP_HDQ_SYSCONFIG_SOFTRESET BIT(1)
> -#define OMAP_HDQ_SYSCONFIG_AUTOIDLE BIT(0)
> -#define OMAP_HDQ_SYSCONFIG_NOIDLE 0x0
> -#define OMAP_HDQ_SYSSTATUS 0x18
> -#define OMAP_HDQ_SYSSTATUS_RESETDONE BIT(0)
>
> #define OMAP_HDQ_FLAG_CLEAR 0
> #define OMAP_HDQ_FLAG_SET 1
> @@ -62,17 +56,9 @@ struct hdq_data {
> void __iomem *hdq_base;
> /* lock status update */
> struct mutex hdq_mutex;
> - int hdq_usecount;
> u8 hdq_irqstatus;
> /* device lock */
> spinlock_t hdq_spinlock;
> - /*
> - * Used to control the call to omap_hdq_get and omap_hdq_put.
> - * HDQ Protocol: Write the CMD|REG_address first, followed by
> - * the data wrire or read.
> - */
> - int init_trans;
> - int rrw;
> /* mode: 0-HDQ 1-W1 */
> int mode;
>
> @@ -99,15 +85,6 @@ static inline u8 hdq_reg_merge(struct hdq_data *hdq_data, u32 offset,
> return new_val;
> }
>
> -static void hdq_disable_interrupt(struct hdq_data *hdq_data, u32 offset,
> - u32 mask)
> -{
> - u32 ie;
> -
> - ie = readl(hdq_data->hdq_base + offset);
> - writel(ie & mask, hdq_data->hdq_base + offset);
> -}
> -
> /*
> * Wait for one or more bits in flag change.
> * HDQ_FLAG_SET: wait until any bit in the flag is set.
> @@ -142,22 +119,24 @@ static int hdq_wait_for_flag(struct hdq_data *hdq_data, u32 offset,
> return ret;
> }
>
> +/* Clear saved irqstatus after using an interrupt */
> +static void hdq_reset_irqstatus(struct hdq_data *hdq_data)
> +{
> + unsigned long irqflags;
> +
> + spin_lock_irqsave(&hdq_data->hdq_spinlock, irqflags);
> + hdq_data->hdq_irqstatus = 0;
> + spin_unlock_irqrestore(&hdq_data->hdq_spinlock, irqflags);
> +}
> +
> /* write out a byte and fill *status with HDQ_INT_STATUS */
> static int hdq_write_byte(struct hdq_data *hdq_data, u8 val, u8 *status)
> {
> int ret;
> u8 tmp_status;
> - unsigned long irqflags;
>
> *status = 0;
>
> - spin_lock_irqsave(&hdq_data->hdq_spinlock, irqflags);
> - /* clear interrupt flags via a dummy read */
> - hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
> - /* ISR loads it with new INT_STATUS */
> - hdq_data->hdq_irqstatus = 0;
> - spin_unlock_irqrestore(&hdq_data->hdq_spinlock, irqflags);
> -
> hdq_reg_out(hdq_data, OMAP_HDQ_TX_DATA, val);
>
> /* set the GO bit */
> @@ -191,6 +170,7 @@ static int hdq_write_byte(struct hdq_data *hdq_data, u8 val, u8 *status)
> }
>
> out:
> + hdq_reset_irqstatus(hdq_data);
> return ret;
> }
>
> @@ -237,47 +217,11 @@ static void omap_w1_search_bus(void *_hdq, struct w1_master *master_dev,
> slave_found(master_dev, id);
> }
>
> -static int _omap_hdq_reset(struct hdq_data *hdq_data)
> -{
> - int ret;
> - u8 tmp_status;
> -
> - hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG,
> - OMAP_HDQ_SYSCONFIG_SOFTRESET);
> - /*
> - * Select HDQ/1W mode & enable clocks.
> - * It is observed that INT flags can't be cleared via a read and GO/INIT
> - * won't return to zero if interrupt is disabled. So we always enable
> - * interrupt.
> - */
> - hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
> - OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
> - OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK);
> -
> - /* wait for reset to complete */
> - ret = hdq_wait_for_flag(hdq_data, OMAP_HDQ_SYSSTATUS,
> - OMAP_HDQ_SYSSTATUS_RESETDONE, OMAP_HDQ_FLAG_SET, &tmp_status);
> - if (ret)
> - dev_dbg(hdq_data->dev, "timeout waiting HDQ reset, %x",
> - tmp_status);
> - else {
> - hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
> - OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
> - OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK |
> - hdq_data->mode);
> - hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG,
> - OMAP_HDQ_SYSCONFIG_AUTOIDLE);
> - }
> -
> - return ret;
> -}
> -
> /* Issue break pulse to the device */
> static int omap_hdq_break(struct hdq_data *hdq_data)
> {
> int ret = 0;
> u8 tmp_status;
> - unsigned long irqflags;
>
> ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> if (ret < 0) {
> @@ -286,13 +230,6 @@ static int omap_hdq_break(struct hdq_data *hdq_data)
> goto rtn;
> }
>
> - spin_lock_irqsave(&hdq_data->hdq_spinlock, irqflags);
> - /* clear interrupt flags via a dummy read */
> - hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
> - /* ISR loads it with new INT_STATUS */
> - hdq_data->hdq_irqstatus = 0;
> - spin_unlock_irqrestore(&hdq_data->hdq_spinlock, irqflags);
> -
> /* set the INIT and GO bit */
> hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS,
> OMAP_HDQ_CTRL_STATUS_INITIALIZATION | OMAP_HDQ_CTRL_STATUS_GO,
> @@ -341,6 +278,7 @@ static int omap_hdq_break(struct hdq_data *hdq_data)
> " return to zero, %x", tmp_status);
>
> out:
> + hdq_reset_irqstatus(hdq_data);
> mutex_unlock(&hdq_data->hdq_mutex);
> rtn:
> return ret;
> @@ -357,7 +295,7 @@ static int hdq_read_byte(struct hdq_data *hdq_data, u8 *val)
> goto rtn;
> }
>
> - if (!hdq_data->hdq_usecount) {
> + if (pm_runtime_suspended(hdq_data->dev)) {
> ret = -EINVAL;
> goto out;
> }
> @@ -388,86 +326,13 @@ static int hdq_read_byte(struct hdq_data *hdq_data, u8 *val)
> /* the data is ready. Read it in! */
> *val = hdq_reg_in(hdq_data, OMAP_HDQ_RX_DATA);
> out:
> + hdq_reset_irqstatus(hdq_data);
> mutex_unlock(&hdq_data->hdq_mutex);
> rtn:
> return ret;
>
> }
>
> -/* Enable clocks and set the controller to HDQ/1W mode */
> -static int omap_hdq_get(struct hdq_data *hdq_data)
> -{
> - int ret = 0;
> -
> - ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> - if (ret < 0) {
> - ret = -EINTR;
> - goto rtn;
> - }
> -
> - if (OMAP_HDQ_MAX_USER == hdq_data->hdq_usecount) {
> - dev_dbg(hdq_data->dev, "attempt to exceed the max use count");
> - ret = -EINVAL;
> - goto out;
> - } else {
> - hdq_data->hdq_usecount++;
> - try_module_get(THIS_MODULE);
> - if (1 == hdq_data->hdq_usecount) {
> -
> - pm_runtime_get_sync(hdq_data->dev);
> -
> - /* make sure HDQ/1W is out of reset */
> - if (!(hdq_reg_in(hdq_data, OMAP_HDQ_SYSSTATUS) &
> - OMAP_HDQ_SYSSTATUS_RESETDONE)) {
> - ret = _omap_hdq_reset(hdq_data);
> - if (ret)
> - /* back up the count */
> - hdq_data->hdq_usecount--;
> - } else {
> - /* select HDQ/1W mode & enable clocks */
> - hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
> - OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
> - OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK |
> - hdq_data->mode);
> - hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG,
> - OMAP_HDQ_SYSCONFIG_NOIDLE);
> - hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
> - }
> - }
> - }
> -
> -out:
> - mutex_unlock(&hdq_data->hdq_mutex);
> -rtn:
> - return ret;
> -}
> -
> -/* Disable clocks to the module */
> -static int omap_hdq_put(struct hdq_data *hdq_data)
> -{
> - int ret = 0;
> -
> - ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> - if (ret < 0)
> - return -EINTR;
> -
> - hdq_reg_out(hdq_data, OMAP_HDQ_SYSCONFIG,
> - OMAP_HDQ_SYSCONFIG_AUTOIDLE);
> - if (0 == hdq_data->hdq_usecount) {
> - dev_dbg(hdq_data->dev, "attempt to decrement use count"
> - " when it is zero");
> - ret = -EINVAL;
> - } else {
> - hdq_data->hdq_usecount--;
> - module_put(THIS_MODULE);
> - if (0 == hdq_data->hdq_usecount)
> - pm_runtime_put_sync(hdq_data->dev);
> - }
> - mutex_unlock(&hdq_data->hdq_mutex);
> -
> - return ret;
> -}
> -
> /*
> * W1 triplet callback function - used for searching ROM addresses.
> * Registered only when controller is in 1-wire mode.
> @@ -482,7 +347,12 @@ static u8 omap_w1_triplet(void *_hdq, u8 bdir)
> OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK;
> u8 mask = ctrl | OMAP_HDQ_CTRL_STATUS_DIR;
>
> - omap_hdq_get(_hdq);
> + err = pm_runtime_get_sync(hdq_data->dev);
> + if (err < 0) {
> + pm_runtime_put_noidle(hdq_data->dev);
> +
> + return err;
> + }
>
> err = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> if (err < 0) {
> @@ -490,7 +360,6 @@ static u8 omap_w1_triplet(void *_hdq, u8 bdir)
> goto rtn;
> }
>
> - hdq_data->hdq_irqstatus = 0;
> /* read id_bit */
> hdq_reg_merge(_hdq, OMAP_HDQ_CTRL_STATUS,
> ctrl | OMAP_HDQ_CTRL_STATUS_DIR, mask);
> @@ -504,7 +373,9 @@ static u8 omap_w1_triplet(void *_hdq, u8 bdir)
> }
> id_bit = (hdq_reg_in(_hdq, OMAP_HDQ_RX_DATA) & 0x01);
>
> - hdq_data->hdq_irqstatus = 0;
> + /* Must clear irqstatus for another RXCOMPLETE interrupt */
> + hdq_reset_irqstatus(hdq_data);
> +
> /* read comp_bit */
> hdq_reg_merge(_hdq, OMAP_HDQ_CTRL_STATUS,
> ctrl | OMAP_HDQ_CTRL_STATUS_DIR, mask);
> @@ -547,18 +418,33 @@ static u8 omap_w1_triplet(void *_hdq, u8 bdir)
> OMAP_HDQ_CTRL_STATUS_SINGLE);
>
> out:
> + hdq_reset_irqstatus(hdq_data);
> mutex_unlock(&hdq_data->hdq_mutex);
> rtn:
> - omap_hdq_put(_hdq);
> + pm_runtime_mark_last_busy(hdq_data->dev);
> + pm_runtime_put_autosuspend(hdq_data->dev);
> +
> return ret;
> }
>
> /* reset callback */
> static u8 omap_w1_reset_bus(void *_hdq)
> {
> - omap_hdq_get(_hdq);
> - omap_hdq_break(_hdq);
> - omap_hdq_put(_hdq);
> + struct hdq_data *hdq_data = _hdq;
> + int err;
> +
> + err = pm_runtime_get_sync(hdq_data->dev);
> + if (err < 0) {
> + pm_runtime_put_noidle(hdq_data->dev);
> +
> + return err;
> + }
> +
> + omap_hdq_break(hdq_data);
> +
> + pm_runtime_mark_last_busy(hdq_data->dev);
> + pm_runtime_put_autosuspend(hdq_data->dev);
> +
> return 0;
> }
>
> @@ -569,37 +455,19 @@ static u8 omap_w1_read_byte(void *_hdq)
> u8 val = 0;
> int ret;
>
> - /* First write to initialize the transfer */
> - if (hdq_data->init_trans == 0)
> - omap_hdq_get(hdq_data);
> + ret = pm_runtime_get_sync(hdq_data->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(hdq_data->dev);
>
> - ret = hdq_read_byte(hdq_data, &val);
> - if (ret) {
> - ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> - if (ret < 0) {
> - dev_dbg(hdq_data->dev, "Could not acquire mutex\n");
> - return -EINTR;
> - }
> - hdq_data->init_trans = 0;
> - mutex_unlock(&hdq_data->hdq_mutex);
> - omap_hdq_put(hdq_data);
> return -1;
> }
>
> - hdq_disable_interrupt(hdq_data, OMAP_HDQ_CTRL_STATUS,
> - ~OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK);
> + ret = hdq_read_byte(hdq_data, &val);
> + if (ret)
> + ret = -1;
>
> - /* Write followed by a read, release the module */
> - if (hdq_data->init_trans) {
> - ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> - if (ret < 0) {
> - dev_dbg(hdq_data->dev, "Could not acquire mutex\n");
> - return -EINTR;
> - }
> - hdq_data->init_trans = 0;
> - mutex_unlock(&hdq_data->hdq_mutex);
> - omap_hdq_put(hdq_data);
> - }
> + pm_runtime_mark_last_busy(hdq_data->dev);
> + pm_runtime_put_autosuspend(hdq_data->dev);
>
> return val;
> }
> @@ -611,9 +479,12 @@ static void omap_w1_write_byte(void *_hdq, u8 byte)
> int ret;
> u8 status;
>
> - /* First write to initialize the transfer */
> - if (hdq_data->init_trans == 0)
> - omap_hdq_get(hdq_data);
> + ret = pm_runtime_get_sync(hdq_data->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(hdq_data->dev);
> +
> + return;
> + }
>
> /*
> * We need to reset the slave before
> @@ -623,31 +494,15 @@ static void omap_w1_write_byte(void *_hdq, u8 byte)
> if (byte == W1_SKIP_ROM)
> omap_hdq_break(hdq_data);
>
> - ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> - if (ret < 0) {
> - dev_dbg(hdq_data->dev, "Could not acquire mutex\n");
> - return;
> - }
> - hdq_data->init_trans++;
> - mutex_unlock(&hdq_data->hdq_mutex);
> -
> ret = hdq_write_byte(hdq_data, byte, &status);
> if (ret < 0) {
> dev_dbg(hdq_data->dev, "TX failure:Ctrl status %x\n", status);
> - return;
> + goto out_err;
> }
>
> - /* Second write, data transferred. Release the module */
> - if (hdq_data->init_trans > 1) {
> - omap_hdq_put(hdq_data);
> - ret = mutex_lock_interruptible(&hdq_data->hdq_mutex);
> - if (ret < 0) {
> - dev_dbg(hdq_data->dev, "Could not acquire mutex\n");
> - return;
> - }
> - hdq_data->init_trans = 0;
> - mutex_unlock(&hdq_data->hdq_mutex);
> - }
> +out_err:
> + pm_runtime_mark_last_busy(hdq_data->dev);
> + pm_runtime_put_autosuspend(hdq_data->dev);
> }
>
> static struct w1_bus_master omap_w1_master = {
> @@ -656,6 +511,35 @@ static struct w1_bus_master omap_w1_master = {
> .reset_bus = omap_w1_reset_bus,
> };
>
> +static int __maybe_unused omap_hdq_runtime_suspend(struct device *dev)
> +{
> + struct hdq_data *hdq_data = dev_get_drvdata(dev);
> +
> + hdq_reg_out(hdq_data, 0, hdq_data->mode);
> + hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused omap_hdq_runtime_resume(struct device *dev)
> +{
> + struct hdq_data *hdq_data = dev_get_drvdata(dev);
> +
> + /* select HDQ/1W mode & enable clocks */
> + hdq_reg_out(hdq_data, OMAP_HDQ_CTRL_STATUS,
> + OMAP_HDQ_CTRL_STATUS_CLOCKENABLE |
> + OMAP_HDQ_CTRL_STATUS_INTERRUPTMASK |
> + hdq_data->mode);
> + hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops omap_hdq_pm_ops = {
> + SET_RUNTIME_PM_OPS(omap_hdq_runtime_suspend,
> + omap_hdq_runtime_resume, NULL)
> +};
> +
> static int omap_hdq_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -677,23 +561,27 @@ static int omap_hdq_probe(struct platform_device *pdev)
> if (IS_ERR(hdq_data->hdq_base))
> return PTR_ERR(hdq_data->hdq_base);
>
> - hdq_data->hdq_usecount = 0;
> - hdq_data->rrw = 0;
> mutex_init(&hdq_data->hdq_mutex);
>
> + ret = of_property_read_string(pdev->dev.of_node, "ti,mode", &mode);
> + if (ret < 0 || !strcmp(mode, "hdq")) {
> + hdq_data->mode = 0;
> + omap_w1_master.search = omap_w1_search_bus;
> + } else {
> + hdq_data->mode = 1;
> + omap_w1_master.triplet = omap_w1_triplet;
> + }
> +
> pm_runtime_enable(&pdev->dev);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 300);
> ret = pm_runtime_get_sync(&pdev->dev);
> if (ret < 0) {
> + pm_runtime_put_noidle(&pdev->dev);
> dev_dbg(&pdev->dev, "pm_runtime_get_sync failed\n");
> goto err_w1;
> }
>
> - ret = _omap_hdq_reset(hdq_data);
> - if (ret) {
> - dev_dbg(&pdev->dev, "reset failed\n");
> - goto err_irq;
> - }
> -
> rev = hdq_reg_in(hdq_data, OMAP_HDQ_REVISION);
> dev_info(&pdev->dev, "OMAP HDQ Hardware Rev %c.%c. Driver in %s mode\n",
> (rev >> 4) + '0', (rev & 0x0f) + '0', "Interrupt");
> @@ -715,16 +603,8 @@ static int omap_hdq_probe(struct platform_device *pdev)
>
> omap_hdq_break(hdq_data);
>
> - pm_runtime_put_sync(&pdev->dev);
> -
> - ret = of_property_read_string(pdev->dev.of_node, "ti,mode", &mode);
> - if (ret < 0 || !strcmp(mode, "hdq")) {
> - hdq_data->mode = 0;
> - omap_w1_master.search = omap_w1_search_bus;
> - } else {
> - hdq_data->mode = 1;
> - omap_w1_master.triplet = omap_w1_triplet;
> - }
> + pm_runtime_mark_last_busy(&pdev->dev);
> + pm_runtime_put_autosuspend(&pdev->dev);
>
> omap_w1_master.data = hdq_data;
>
> @@ -739,6 +619,7 @@ static int omap_hdq_probe(struct platform_device *pdev)
> err_irq:
> pm_runtime_put_sync(&pdev->dev);
> err_w1:
> + pm_runtime_dont_use_autosuspend(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> return ret;
> @@ -746,23 +627,19 @@ static int omap_hdq_probe(struct platform_device *pdev)
>
> static int omap_hdq_remove(struct platform_device *pdev)
> {
> - struct hdq_data *hdq_data = platform_get_drvdata(pdev);
> + int active;
>
> - mutex_lock(&hdq_data->hdq_mutex);
> -
> - if (hdq_data->hdq_usecount) {
> - dev_dbg(&pdev->dev, "removed when use count is not zero\n");
> - mutex_unlock(&hdq_data->hdq_mutex);
> - return -EBUSY;
> - }
> + active = pm_runtime_get_sync(&pdev->dev);
> + if (active < 0)
> + pm_runtime_put_noidle(&pdev->dev);
>
> - mutex_unlock(&hdq_data->hdq_mutex);
> + w1_remove_master_device(&omap_w1_master);
>
> - /* remove module dependency */
> + pm_runtime_dont_use_autosuspend(&pdev->dev);
> + if (active >= 0)
> + pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> - w1_remove_master_device(&omap_w1_master);
> -
> return 0;
> }
>
> @@ -779,6 +656,7 @@ static struct platform_driver omap_hdq_driver = {
> .driver = {
> .name = "omap_hdq",
> .of_match_table = omap_hdq_dt_ids,
> + .pm = &omap_hdq_pm_ops,
> },
> };
> module_platform_driver(omap_hdq_driver);
> --
> 2.24.1
* H. Nikolaus Schaller <[email protected]> [200416 15:04]:
> Hi Tony,
> it looks as if something with this patch is broken on GTA04. For v5.6 and v5.7-rc1.
>
> HDQ battery access times out after ca. 15 seconds and I get temperature of -273.1°C...
>
> Reverting this patch and everything is ok again.
Hmm OK interesting.
> What is "ti,mode" about? Do we have that (indirectly) in gta04.dtsi?
> Or does this patch need some CONFIGs we do not happen to have?
Sounds like you have things working though so there should be no
need for having ti,mode = "1w" in the dts.
> > pm_runtime_enable(&pdev->dev);
> > + pm_runtime_use_autosuspend(&pdev->dev);
> > + pm_runtime_set_autosuspend_delay(&pdev->dev, 300);
Care to check if changing pm_runtime_set_autosuspend_delay value
to -1 in probe makes the issue go away? Or change it manually
to -1 via sysfs.
If that helps, likely we have a missing pm_runtime_get_sync()
somewhere in the driver.
Regards,
Tony
On Thu, 16 Apr 2020 11:46:38 -0700
Tony Lindgren <[email protected]> wrote:
> * H. Nikolaus Schaller <[email protected]> [200416 15:04]:
> > Hi Tony,
> > it looks as if something with this patch is broken on GTA04. For v5.6 and v5.7-rc1.
> >
> > HDQ battery access times out after ca. 15 seconds and I get temperature of -273.1°C...
> >
> > Reverting this patch and everything is ok again.
>
> Hmm OK interesting.
>
> > What is "ti,mode" about? Do we have that (indirectly) in gta04.dtsi?
> > Or does this patch need some CONFIGs we do not happen to have?
>
> Sounds like you have things working though so there should be no
> need for having ti,mode = "1w" in the dts.
>
> > > pm_runtime_enable(&pdev->dev);
> > > + pm_runtime_use_autosuspend(&pdev->dev);
> > > + pm_runtime_set_autosuspend_delay(&pdev->dev, 300);
>
> Care to check if changing pm_runtime_set_autosuspend_delay value
> to -1 in probe makes the issue go away? Or change it manually
> to -1 via sysfs.
>
> If that helps, likely we have a missing pm_runtime_get_sync()
> somewhere in the driver.
>
I have not tested yet with v5.7-rc1 (it is compiling right now),
but I have not seen any problems with init=/bin/bash on v5.6
and only a minimal set of modules loaded on gta04. I have seen that
42 for IDLEST
So might be something a bit more weird.
Regards,
Andreas
* Andreas Kemnade <[email protected]> [200416 20:05]:
> On Thu, 16 Apr 2020 11:46:38 -0700
> Tony Lindgren <[email protected]> wrote:
> > If that helps, likely we have a missing pm_runtime_get_sync()
> > somewhere in the driver.
> >
> I have not tested yet with v5.7-rc1 (it is compiling right now),
> but I have not seen any problems with init=/bin/bash on v5.6
> and only a minimal set of modules loaded on gta04. I have seen that
> 42 for IDLEST
Yeah I think you did test this change on gta04 earlier based
on the Tested-by from you :)
> So might be something a bit more weird.
OK please let us know what you find out.
Regards,
Tony
> Am 16.04.2020 um 22:33 schrieb Tony Lindgren <[email protected]>:
>
> * Andreas Kemnade <[email protected]> [200416 20:05]:
>> On Thu, 16 Apr 2020 11:46:38 -0700
>> Tony Lindgren <[email protected]> wrote:
>>> If that helps, likely we have a missing pm_runtime_get_sync()
>>> somewhere in the driver.
>>>
>> I have not tested yet with v5.7-rc1 (it is compiling right now),
>> but I have not seen any problems with init=/bin/bash on v5.6
>> and only a minimal set of modules loaded on gta04. I have seen that
>> 42 for IDLEST
>
> Yeah I think you did test this change on gta04 earlier based
> on the Tested-by from you :)
I think Andreas may have simply missed my test situation.
>
>> So might be something a bit more weird.
>
> OK please let us know what you find out.
>
> Regards,
>
> Tony
> Am 16.04.2020 um 20:46 schrieb Tony Lindgren <[email protected]>:
>
> * H. Nikolaus Schaller <[email protected]> [200416 15:04]:
>> Hi Tony,
>> it looks as if something with this patch is broken on GTA04. For v5.6 and v5.7-rc1.
>>
>> HDQ battery access times out after ca. 15 seconds and I get temperature of -273.1°C...
>>
>> Reverting this patch and everything is ok again.
>
> Hmm OK interesting.
>
>> What is "ti,mode" about? Do we have that (indirectly) in gta04.dtsi?
>> Or does this patch need some CONFIGs we do not happen to have?
>
> Sounds like you have things working though so there should be no
> need for having ti,mode = "1w" in the dts.
>
>>> pm_runtime_enable(&pdev->dev);
>>> + pm_runtime_use_autosuspend(&pdev->dev);
>>> + pm_runtime_set_autosuspend_delay(&pdev->dev, 300);
>
> Care to check if changing pm_runtime_set_autosuspend_delay value
> to -1 in probe makes the issue go away? Or change it manually
> to -1 via sysfs.
>
> If that helps, likely we have a missing pm_runtime_get_sync()
> somewhere in the driver.
Yes, it does! It suffices to set it to -1 for one readout.
Aything else I can test?
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=4049000
POWER_SUPPLY_CURRENT_NOW=233478
POWER_SUPPLY_CAPACITY=0
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=-2731
POWER_SUPPLY_TIME_TO_EMPTY_NOW=0
POWER_SUPPLY_TIME_TO_EMPTY_AVG=0
POWER_SUPPLY_TIME_TO_FULL_NOW=0
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=0
POWER_SUPPLY_CHARGE_NOW=755788
POWER_SUPPLY_CHARGE_FULL_DESIGN=1233792
POWER_SUPPLY_CYCLE_COUNT=80
POWER_SUPPLY_ENERGY_NOW=0
POWER_SUPPLY_POWER_AVG=941700
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real 0m8.910s
user 0m0.001s
sys 0m0.028s
root@letux:~# echo -1 >/sys/bus/platform/drivers/omap_hdq/480b2000.1w/power/autosuspend_delay_ms
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=3985000
POWER_SUPPLY_CURRENT_NOW=231871
POWER_SUPPLY_CAPACITY=78
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=354
POWER_SUPPLY_TIME_TO_EMPTY_NOW=10440
POWER_SUPPLY_TIME_TO_EMPTY_AVG=9900
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=858138
POWER_SUPPLY_CHARGE_NOW=670170
POWER_SUPPLY_CHARGE_FULL_DESIGN=1233792
POWER_SUPPLY_CYCLE_COUNT=80
POWER_SUPPLY_ENERGY_NOW=2544780
POWER_SUPPLY_POWER_AVG=922720
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real 0m0.232s
user 0m0.001s
sys 0m0.023s
root@letux:~# echo 300 >/sys/bus/platform/drivers/omap_hdq/480b2000.1w/power/autosuspend_delay_ms
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=3985000
POWER_SUPPLY_CURRENT_NOW=234727
POWER_SUPPLY_CAPACITY=78
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=354
POWER_SUPPLY_TIME_TO_EMPTY_NOW=10620
POWER_SUPPLY_TIME_TO_EMPTY_AVG=10140
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=858138
POWER_SUPPLY_CHARGE_NOW=669102
POWER_SUPPLY_CHARGE_FULL_DESIGN=1233792
POWER_SUPPLY_CYCLE_COUNT=80
POWER_SUPPLY_ENERGY_NOW=2541860
POWER_SUPPLY_POWER_AVG=903740
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real 0m0.178s
user 0m0.000s
sys 0m0.029s
root@letux:~#
BR and thanks,
Nikolaus
On Fri, 17 Apr 2020 16:22:47 +0200
"H. Nikolaus Schaller" <[email protected]> wrote:
> > Am 16.04.2020 um 20:46 schrieb Tony Lindgren <[email protected]>:
> >
> > * H. Nikolaus Schaller <[email protected]> [200416 15:04]:
> >> Hi Tony,
> >> it looks as if something with this patch is broken on GTA04. For v5.6 and v5.7-rc1.
> >>
> >> HDQ battery access times out after ca. 15 seconds and I get temperature of -273.1°C...
> >>
> >> Reverting this patch and everything is ok again.
> >
> > Hmm OK interesting.
> >
> >> What is "ti,mode" about? Do we have that (indirectly) in gta04.dtsi?
> >> Or does this patch need some CONFIGs we do not happen to have?
> >
> > Sounds like you have things working though so there should be no
> > need for having ti,mode = "1w" in the dts.
> >
> >>> pm_runtime_enable(&pdev->dev);
> >>> + pm_runtime_use_autosuspend(&pdev->dev);
> >>> + pm_runtime_set_autosuspend_delay(&pdev->dev, 300);
> >
> > Care to check if changing pm_runtime_set_autosuspend_delay value
> > to -1 in probe makes the issue go away? Or change it manually
> > to -1 via sysfs.
> >
> > If that helps, likely we have a missing pm_runtime_get_sync()
> > somewhere in the driver.
>
> Yes, it does! It suffices to set it to -1 for one readout.
> Aything else I can test?
>
How does it depend on loaded drivers?
Is it really mainline kernel + config + devicetree or something else?
Can you reproduce the problem with init=/bin/bash
and then mount sysfs and modprobe omap_hdq?
Regarding pm_runtime stuff I thought I have the worst case scenario.
Regards,
Andreas
> Am 17.04.2020 um 16:43 schrieb Andreas Kemnade <[email protected]>:
>
> On Fri, 17 Apr 2020 16:22:47 +0200
> "H. Nikolaus Schaller" <[email protected]> wrote:
>
>>> Am 16.04.2020 um 20:46 schrieb Tony Lindgren <[email protected]>:
>>>
>>> * H. Nikolaus Schaller <[email protected]> [200416 15:04]:
>>>> Hi Tony,
>>>> it looks as if something with this patch is broken on GTA04. For v5.6 and v5.7-rc1.
>>>>
>>>> HDQ battery access times out after ca. 15 seconds and I get temperature of -273.1°C...
>>>>
>>>> Reverting this patch and everything is ok again.
>>>
>>> Hmm OK interesting.
>>>
>>>> What is "ti,mode" about? Do we have that (indirectly) in gta04.dtsi?
>>>> Or does this patch need some CONFIGs we do not happen to have?
>>>
>>> Sounds like you have things working though so there should be no
>>> need for having ti,mode = "1w" in the dts.
>>>
>>>>> pm_runtime_enable(&pdev->dev);
>>>>> + pm_runtime_use_autosuspend(&pdev->dev);
>>>>> + pm_runtime_set_autosuspend_delay(&pdev->dev, 300);
>>>
>>> Care to check if changing pm_runtime_set_autosuspend_delay value
>>> to -1 in probe makes the issue go away? Or change it manually
>>> to -1 via sysfs.
>>>
>>> If that helps, likely we have a missing pm_runtime_get_sync()
>>> somewhere in the driver.
>>
>> Yes, it does! It suffices to set it to -1 for one readout.
>> Aything else I can test?
>>
> How does it depend on loaded drivers?
> Is it really mainline kernel + config + devicetree or something else?
Well, I can revert the patch on the same
kernel (5.6 or 5.7-rc1) + config + devicetree + user-space
and the problem is gone.
This means that something is different between the old and the new
version which makes the hdq access delayed and failing. Of course I
don't know the reason for it and what does influence it.
>
> Can you reproduce the problem with init=/bin/bash
> and then mount sysfs and modprobe omap_hdq?
I am not sure how quickly I can test such a setup.
> Regarding pm_runtime stuff I thought I have the worst case scenario.
What may make a difference is the sequence in which drivers are loaded.
BR,
Nikolaus
* H. Nikolaus Schaller <[email protected]> [200417 14:53]:
>
> > Am 17.04.2020 um 16:43 schrieb Andreas Kemnade <[email protected]>:
> >
> > On Fri, 17 Apr 2020 16:22:47 +0200
> > "H. Nikolaus Schaller" <[email protected]> wrote:
> >
> >>> Am 16.04.2020 um 20:46 schrieb Tony Lindgren <[email protected]>:
> >>> Care to check if changing pm_runtime_set_autosuspend_delay value
> >>> to -1 in probe makes the issue go away? Or change it manually
> >>> to -1 via sysfs.
> >>>
> >>> If that helps, likely we have a missing pm_runtime_get_sync()
> >>> somewhere in the driver.
> >>
> >> Yes, it does! It suffices to set it to -1 for one readout.
> >> Aything else I can test?
You could sprinkle dev_info(dev, "%s\n", __func__) to the
omap_hdq_runtime_suspend() and omap_hdq_runtime_resume()
functions.
> > How does it depend on loaded drivers?
> > Is it really mainline kernel + config + devicetree or something else?
>
> Well, I can revert the patch on the same
> kernel (5.6 or 5.7-rc1) + config + devicetree + user-space
> and the problem is gone.
>
> This means that something is different between the old and the new
> version which makes the hdq access delayed and failing. Of course I
> don't know the reason for it and what does influence it.
>
> >
> > Can you reproduce the problem with init=/bin/bash
> > and then mount sysfs and modprobe omap_hdq?
>
> I am not sure how quickly I can test such a setup.
>
> > Regarding pm_runtime stuff I thought I have the worst case scenario.
>
> What may make a difference is the sequence in which drivers are loaded.
Well to me it seems that we have PM runtime handling properly
implemented for all the functions in w1_bus_master omap_w1_master,
so we should not have any consumers calling into the driver
bypassing PM runtime.
Maybe the PM runtime usecounts get unbalanced somewhere in the
driver where we end up with driver permanently in disabled state?
Regards,
Tony
* Tony Lindgren <[email protected]> [200417 15:08]:
> Maybe the PM runtime usecounts get unbalanced somewhere in the
> driver where we end up with driver permanently in disabled state?
Or it could be that with omap_hdq.c no longer blocking SoC
deeper idle states, omap_hdq.c now loses context if you have
omap3 off mode enabled during idle?
If so, this can easily be fixed by adding a cpu_pm notifier
along the lines of what we already have for few drivers:
$ git grep -e CLUSTER_PM_ENTER -e CLUSTER_PM_EXIT
Regards,
Tony
On Fri, 17 Apr 2020 08:14:47 -0700
Tony Lindgren <[email protected]> wrote:
> * Tony Lindgren <[email protected]> [200417 15:08]:
> > Maybe the PM runtime usecounts get unbalanced somewhere in the
> > driver where we end up with driver permanently in disabled state?
>
> Or it could be that with omap_hdq.c no longer blocking SoC
> deeper idle states, omap_hdq.c now loses context if you have
> omap3 off mode enabled during idle?
>
I run twice (to get mmc access cached):
root@(none):/# sleep 15 ; /usr/local/bin/idledump >/run/idledump ; cat /sys/cla
ss/power_supply/bq27000-battery/current_now ; cat /run/idledump
32665
CM_IDLEST1_CORE 00000042
CM_IDLEST3_CORE 00000000
CM_FCLKEN1_CORE 00000000
CM_FCLKEN3_CORE 00000002
CM_CLKSTST_CORE 00000003
CM_IDLEST_CKGEN 00000001
CM_IDLEST2_CKGEN 00000000
CM_FCLKEN_DSS 00000000
CM_IDLEST_DSS 00000000
CM_FCLKEN_CAM 00000000
CM_IDLEST_CAM 00000000
CM_FCLKEN_PER 00000000
CM_IDLEST_PER 00030000
root@(none):/#
root@(none):/# cat /sys/kernel/debug/pm_debug/count
usbhost_pwrdm (ON),OFF:754,RET:9148,INA:0,ON:9903,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
sgx_pwrdm (OFF),OFF:1,RET:0,INA:1,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
per_pwrdm (ON),OFF:237,RET:1202,INA:0,ON:1440,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
dss_pwrdm (ON),OFF:754,RET:9148,INA:0,ON:9903,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
cam_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
neon_pwrdm (ON),OFF:638,RET:9015,INA:250,ON:9904,RET-LOGIC-OFF:0
mpu_pwrdm (ON),OFF:638,RET:9014,INA:250,ON:9903,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
iva2_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0
usbhost_clkdm->usbhost_pwrdm (1)
sgx_clkdm->sgx_pwrdm (0)
per_clkdm->per_pwrdm (16)
cam_clkdm->cam_pwrdm (0)
dss_clkdm->dss_pwrdm (1)
d2d_clkdm->core_pwrdm (0)
iva2_clkdm->iva2_pwrdm (0)
mpu_clkdm->mpu_pwrdm (0)
core_l4_clkdm->core_pwrdm (21)
core_l3_clkdm->core_pwrdm (1)
neon_clkdm->neon_pwrdm (0)
Regards,
Andreas
Hi Tony,
> Am 17.04.2020 um 17:07 schrieb Tony Lindgren <[email protected]>:
>
> * H. Nikolaus Schaller <[email protected]> [200417 14:53]:
>>
>>> Am 17.04.2020 um 16:43 schrieb Andreas Kemnade <[email protected]>:
>>>
>>> On Fri, 17 Apr 2020 16:22:47 +0200
>>> "H. Nikolaus Schaller" <[email protected]> wrote:
>>>
>>>>> Am 16.04.2020 um 20:46 schrieb Tony Lindgren <[email protected]>:
>>>>> Care to check if changing pm_runtime_set_autosuspend_delay value
>>>>> to -1 in probe makes the issue go away? Or change it manually
>>>>> to -1 via sysfs.
>>>>>
>>>>> If that helps, likely we have a missing pm_runtime_get_sync()
>>>>> somewhere in the driver.
>>>>
>>>> Yes, it does! It suffices to set it to -1 for one readout.
>>>> Aything else I can test?
>
> You could sprinkle dev_info(dev, "%s\n", __func__) to the
> omap_hdq_runtime_suspend() and omap_hdq_runtime_resume()
> functions.
I have done it incl. adding dev_info to hdq_read_byte(). It
looks as if the read attempts already time out during boot,
but after omap_hdq_runtime_resume(). And omap_hdq_runtime_resume()
is done well after the last one. This looks ok.
echo -1 >autosuspend_delay_ms
Does omap_hdq_runtime_resume() once.
To me it looks as if reading hqd too quickly after omap_hdq_runtime_resume()
may be part of the problem, although it is 0.4 seconds between [ 18.355163]
and [ 18.745269]. So I am not sure about my interpretation.
A different attempt for interpretation may be that trying to read the
slave triggers omap_hdq_runtime_resume() just before doing the
first hdq_read_byte().
This may be different context from separating these steps into different
processes/threads (echo + cat).
I.e.
[ 18.355163] omap_hdq 480b2000.1w: omap_hdq_runtime_resume
[ 18.432403] omap_hdq 480b2000.1w: OMAP HDQ Hardware Rev 0.5. Driver in Interrupt mode
[ 18.745269] omap_hdq 480b2000.1w: hdq_read_byte
somehow differs from
root@letux:~# echo -1 >/sys/bus/platform/drivers/omap_hdq/480b2000.1w/power/autosuspend_delay_ms
[ 544.714904] omap_hdq 480b2000.1w: omap_hdq_runtime_resume
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
[ 551.704498] omap_hdq 480b2000.1w: hdq_read_byte
BR,
Nikolaus
root@letux:~# dmesg|fgrep hdq
[ 18.355163] omap_hdq 480b2000.1w: omap_hdq_runtime_resume
[ 18.432403] omap_hdq 480b2000.1w: OMAP HDQ Hardware Rev 0.5. Driver in Interrupt mode
[ 18.745269] omap_hdq 480b2000.1w: hdq_read_byte
[ 19.163055] omap_hdq 480b2000.1w: hdq_read_byte
[ 19.583099] omap_hdq 480b2000.1w: hdq_read_byte
[ 20.003051] omap_hdq 480b2000.1w: hdq_read_byte
[ 20.422973] omap_hdq 480b2000.1w: hdq_read_byte
[ 20.843109] omap_hdq 480b2000.1w: hdq_read_byte
[ 21.262908] omap_hdq 480b2000.1w: hdq_read_byte
[ 21.682922] omap_hdq 480b2000.1w: hdq_read_byte
[ 22.103149] omap_hdq 480b2000.1w: hdq_read_byte
[ 22.523040] omap_hdq 480b2000.1w: hdq_read_byte
[ 22.963012] omap_hdq 480b2000.1w: hdq_read_byte
[ 23.390197] omap_hdq 480b2000.1w: hdq_read_byte
[ 23.812988] omap_hdq 480b2000.1w: hdq_read_byte
[ 24.232971] omap_hdq 480b2000.1w: hdq_read_byte
[ 24.653167] omap_hdq 480b2000.1w: hdq_read_byte
[ 25.073028] omap_hdq 480b2000.1w: hdq_read_byte
[ 25.493011] omap_hdq 480b2000.1w: hdq_read_byte
[ 25.923095] omap_hdq 480b2000.1w: hdq_read_byte
[ 26.133392] omap_hdq 480b2000.1w: hdq_read_byte
[ 26.553009] omap_hdq 480b2000.1w: hdq_read_byte
[ 26.973083] omap_hdq 480b2000.1w: hdq_read_byte
[ 27.393035] omap_hdq 480b2000.1w: hdq_read_byte
[ 27.813354] omap_hdq 480b2000.1w: hdq_read_byte
[ 28.233367] omap_hdq 480b2000.1w: hdq_read_byte
[ 28.673309] omap_hdq 480b2000.1w: hdq_read_byte
[ 29.093322] omap_hdq 480b2000.1w: hdq_read_byte
[ 29.513366] omap_hdq 480b2000.1w: hdq_read_byte
[ 29.948089] omap_hdq 480b2000.1w: hdq_read_byte
[ 30.403076] omap_hdq 480b2000.1w: hdq_read_byte
[ 30.823303] omap_hdq 480b2000.1w: hdq_read_byte
[ 31.243408] omap_hdq 480b2000.1w: hdq_read_byte
[ 31.663085] omap_hdq 480b2000.1w: hdq_read_byte
[ 32.083312] omap_hdq 480b2000.1w: hdq_read_byte
[ 32.503326] omap_hdq 480b2000.1w: hdq_read_byte
[ 32.923309] omap_hdq 480b2000.1w: hdq_read_byte
[ 33.343353] omap_hdq 480b2000.1w: hdq_read_byte
[ 33.763305] omap_hdq 480b2000.1w: hdq_read_byte
[ 33.814392] Modules linked in: wl18xx wlcore mac80211 cfg80211 libarc4 omapdrm cmac bnep panel_tpo_td028ttec1 pps_gpio snd_soc_simple_card snd_soc_simple_card_utils pps_core simple_bridge wwan_on_off snd_soc_omap_twl4030 snd_soc_w2cbw003_bt snd_soc_gtm601 display_connector generic_adc_battery pwm_bl pwm_omap_dmtimer omap_aes_driver crypto_engine omap_crypto wlcore_sdio omap3_isp videobuf2_dma_contig omap_sham videobuf2_memops videobuf2_v4l2 bq27xxx_battery_hdq bq27xxx_battery videobuf2_common omap2430 omap_hdq bmp280_spi snd_soc_omap_mcbsp snd_soc_ti_sdma ov9655 bmp280_i2c v4l2_fwnode bmp280 bmg160_i2c bmg160_core at24 tsc2007 videodev leds_tca6507 mc bno055 bmc150_magn_i2c bmc150_accel_i2c bmc150_magn bmc150_accel_core industrialio_triggered_buffer snd_soc_si47xx kfifo_buf phy_twl4030_usb musb_hdrc twl4030_pwrbutton twl4030_vibra snd_soc_twl4030 hci_uart btbcm twl4030_charger twl4030_madc industrialio input_polldev gnss_sirf bluetooth gnss ecdh_generic ecc ehci_omap omapdss omapdss_base
[ 34.363281] omap_hdq 480b2000.1w: hdq_read_byte
[ 34.783142] omap_hdq 480b2000.1w: hdq_read_byte
[ 35.203124] omap_hdq 480b2000.1w: hdq_read_byte
[ 35.623382] omap_hdq 480b2000.1w: hdq_read_byte
[ 36.043273] omap_hdq 480b2000.1w: hdq_read_byte
[ 36.463317] omap_hdq 480b2000.1w: hdq_read_byte
[ 36.883392] omap_hdq 480b2000.1w: hdq_read_byte
[ 37.303314] omap_hdq 480b2000.1w: hdq_read_byte
[ 37.723327] omap_hdq 480b2000.1w: hdq_read_byte
[ 38.143341] omap_hdq 480b2000.1w: hdq_read_byte
[ 38.563323] omap_hdq 480b2000.1w: hdq_read_byte
[ 38.983306] omap_hdq 480b2000.1w: hdq_read_byte
[ 39.403350] omap_hdq 480b2000.1w: hdq_read_byte
[ 39.823303] omap_hdq 480b2000.1w: hdq_read_byte
[ 40.243347] omap_hdq 480b2000.1w: hdq_read_byte
[ 40.663299] omap_hdq 480b2000.1w: hdq_read_byte
[ 41.083374] omap_hdq 480b2000.1w: hdq_read_byte
[ 41.503295] omap_hdq 480b2000.1w: hdq_read_byte
[ 41.923400] omap_hdq 480b2000.1w: hdq_read_byte
[ 42.343292] omap_hdq 480b2000.1w: hdq_read_byte
[ 42.763305] omap_hdq 480b2000.1w: hdq_read_byte
[ 43.183319] omap_hdq 480b2000.1w: hdq_read_byte
[ 43.603332] omap_hdq 480b2000.1w: hdq_read_byte
[ 44.188964] omap_hdq 480b2000.1w: omap_hdq_runtime_suspend
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
[ 383.654083] omap_hdq 480b2000.1w: omap_hdq_runtime_resume
[ 383.868103] omap_hdq 480b2000.1w: hdq_read_byte
[ 384.288055] omap_hdq 480b2000.1w: hdq_read_byte
[ 384.708129] omap_hdq 480b2000.1w: hdq_read_byte
[ 385.128173] omap_hdq 480b2000.1w: hdq_read_byte
[ 385.548156] omap_hdq 480b2000.1w: hdq_read_byte
[ 385.968139] omap_hdq 480b2000.1w: hdq_read_byte
[ 385.973052] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.200683] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.215606] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.223358] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.241729] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.251129] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.260986] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.272460] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.281402] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.286865] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.510711] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.526153] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.533905] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.551818] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.561065] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.570922] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.582641] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.591339] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.602508] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.610900] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.620941] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.632476] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.641387] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.652709] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.660949] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.671051] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.680908] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.691986] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.701324] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.709045] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.720886] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.731872] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.741424] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.749053] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.761413] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.768798] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.780853] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.791839] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.801635] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.808746] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.820831] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.828124] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.840362] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.847534] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.859680] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.866882] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.882263] omap_hdq 480b2000.1w: hdq_read_byte
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRE[ 386.897064] omap_hdq 480b2000.1w: hdq_read_byte
SENT=1
POWER_SUPPLY_VOLTAGE_NOW=4017000
POWER_SUPPLY_CURRENT_NOW=226873
POWER_SUPPLY_CAPACITY=83
POWER_SUPPL[ 386.911407] omap_hdq 480b2000.1w: hdq_read_byte
Y_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=-2731
POWER_SUPPLY_T[ 386.922393] omap_hdq 480b2000.1w: hdq_read_byte
IME_TO_EMPTY_NOW=900
POWER_SUPPLY_TIME_TO_EMPTY_AVG=10860
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=858138
POW[ 386.937408] omap_hdq 480b2000.1w: hdq_read_byte
ER_SUPPLY_CHARGE_NOW=716806
POWER_SUPPLY_CHARGE_FULL_DESIGN=1233792
POWER_SUPPLY_CYCLE_COUNT=80
POWER_SUPPLY_[ 386.951812] omap_hdq 480b2000.1w: hdq_read_byte
ENERGY_NOW=2718520
POWER_SUPPLY_POWER_AVG=900820
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas Instruments
real[ 386.968322] omap_hdq 480b2000.1w: hdq_read_byte
0m3.267s
user 0m0.002s
sys 0m0.230s
[ 386.981445] omap_hdq 480b2000.1w: hdq_read_byte
[ 386.991394] omap_hdq 480b2000.1w: hdq_read_byte
root@letux:~# [ 387.001861] omap_hdq 480b2000.1w: hdq_read_byte
[ 387.012512] omap_hdq 480b2000.1w: hdq_read_byte
[ 387.020996] omap_hdq 480b2000.1w: hdq_read_byte
[ 387.378234] omap_hdq 480b2000.1w: omap_hdq_runtime_suspend
root@letux:~#
root@letux:~# echo -1 >/sys/bus/platform/drivers/omap_hdq/480b2000.1w/power/autosuspend_delay_ms
[ 544.714904] omap_hdq 480b2000.1w: omap_hdq_runtime_resume
root@letux:~# time cat /sys/class/power_supply/bq27000-battery/uevent
[ 551.704498] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.711944] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.727844] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.735015] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.748260] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.755432] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.768615] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.775787] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.788604] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.795776] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.808715] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.815887] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.828735] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.839477] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.849578] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.859802] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.869720] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.879913] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.889739] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.899780] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.909667] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.919677] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.929748] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.939727] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.949768] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.959716] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.969787] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.979888] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.987060] omap_hdq 480b2000.1w: hdq_read_byte
[ 551.999176] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.007049] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.019042] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.026977] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.039184] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.046844] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.058288] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.065521] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.078613] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.085815] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.098785] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.106170] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.118682] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.126037] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.138702] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.146667] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.159271] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.166900] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.181365] omap_hdq 480b2000.1w: hdq_read_byte
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_S[ 552.199462] omap_hdq 480b2000.1w: hdq_read_byte
UPPLY_VOLTAGE_NOW=4012000
POWER_SUPPLY_CURRENT_NOW=220447
POWE[ 552.210998] omap_hdq 480b2000.1w: hdq_read_byte
R_SUPPLY_CAPACITY=82
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=289
POWER_SUPPLY_TIME_TO_EMPTY_NOW=11580
POWER_SUPPLY_TIME_TO_EMPT[ 552.227203] omap_hdq 480b2000.1w: hdq_read_byte
Y_AVG=11040
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL=858138
POWER_SUPPLY_CHARGE_NOW=706660
PO[ 552.241851] omap_hdq 480b2000.1w: hdq_read_byte
WER_SUPPLY_CHARGE_FULL_DESIGN=1233792
POWER_SUPPLY_CYCLE_COUNT=80
POWER_SUPPLY_ENERGY_NOW=2680560
POWER_SUPPLY_POWER_AVG=8760[ 552.257995] omap_hdq 480b2000.1w: hdq_read_byte
00
POWER_SUPPLY_HEALTH=Good
POWER_SUPPLY_MANUFACTURER=Texas In[ 552.269348] omap_hdq 480b2000.1w: hdq_read_byte
struments
real 0m0.510s
user 0m0.001s
sys 0m0.192s
root@letux:~# [ 552.286468] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.294067] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.305786] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.317443] omap_hdq 480b2000.1w: hdq_read_byte
[ 552.324645] omap_hdq 480b2000.1w: hdq_read_byte
* H. Nikolaus Schaller <[email protected]> [200417 21:04]:
> To me it looks as if reading hqd too quickly after omap_hdq_runtime_resume()
> may be part of the problem, although it is 0.4 seconds between [ 18.355163]
> and [ 18.745269]. So I am not sure about my interpretation.
>
> A different attempt for interpretation may be that trying to read the
> slave triggers omap_hdq_runtime_resume() just before doing the
> first hdq_read_byte().
Hmm so I wonder if adding msleep(100) at the end of
omap_hdq_runtime_resume() might help?
Regards,
Tony
Hi Tony,
> Am 20.04.2020 um 17:08 schrieb Tony Lindgren <[email protected]>:
>
> * H. Nikolaus Schaller <[email protected]> [200417 21:04]:
>> To me it looks as if reading hqd too quickly after omap_hdq_runtime_resume()
>> may be part of the problem, although it is 0.4 seconds between [ 18.355163]
>> and [ 18.745269]. So I am not sure about my interpretation.
>>
>> A different attempt for interpretation may be that trying to read the
>> slave triggers omap_hdq_runtime_resume() just before doing the
>> first hdq_read_byte().
>
> Hmm so I wonder if adding msleep(100) at the end of
> omap_hdq_runtime_resume() might help?
I have tried and initially it did boot and work once.
But after the second boot/reboot the effect was back.
This is something I have observed previously, that the issue
is there in ca. 9 or 10 boot attempts. So I would assume
some race condition with udev reading the uevent file of the
bq27xxx bus client and hence through hdq.
I already had noticed some hqd_read activity right after probing
success.
I had also tried to change pm_runtime_set_autosuspend_delay(, 1000)
with no success. And I tried to call omap_hdq_runtime_resume() at the
end of probe.
The only maybe important observation was when I disabled all
kernel modules except *hdq*.ko and *bq27*.ko. Then I did only
get an emergency shell so that it is quite similar to the
scenario Andreas has tested. With this setup it did work.
I then tried to reenable other kernel modules but the result
wasn't convincing that it gives a reliable result.
So I have still no clear indication when the problem occurs and
when not.
BR and thanks,
Nikolaus
On Mon, 20 Apr 2020 23:11:18 +0200
"H. Nikolaus Schaller" <[email protected]> wrote:
> Hi Tony,
>
> > Am 20.04.2020 um 17:08 schrieb Tony Lindgren <[email protected]>:
> >
> > * H. Nikolaus Schaller <[email protected]> [200417 21:04]:
> >> To me it looks as if reading hqd too quickly after omap_hdq_runtime_resume()
> >> may be part of the problem, although it is 0.4 seconds between [ 18.355163]
> >> and [ 18.745269]. So I am not sure about my interpretation.
> >>
> >> A different attempt for interpretation may be that trying to read the
> >> slave triggers omap_hdq_runtime_resume() just before doing the
> >> first hdq_read_byte().
> >
> > Hmm so I wonder if adding msleep(100) at the end of
> > omap_hdq_runtime_resume() might help?
>
> I have tried and initially it did boot and work once.
> But after the second boot/reboot the effect was back.
>
> This is something I have observed previously, that the issue
> is there in ca. 9 or 10 boot attempts. So I would assume
> some race condition with udev reading the uevent file of the
> bq27xxx bus client and hence through hdq.
>
> I already had noticed some hqd_read activity right after probing
> success.
>
> I had also tried to change pm_runtime_set_autosuspend_delay(, 1000)
> with no success. And I tried to call omap_hdq_runtime_resume() at the
> end of probe.
>
> The only maybe important observation was when I disabled all
> kernel modules except *hdq*.ko and *bq27*.ko. Then I did only
> get an emergency shell so that it is quite similar to the
> scenario Andreas has tested. With this setup it did work.
>
So I guess without idling uarts?
> I then tried to reenable other kernel modules but the result
> wasn't convincing that it gives a reliable result.
>
> So I have still no clear indication when the problem occurs and
> when not.
>
Hmm, last summer I had problems even without that patch reading
temperature while doing umts transfers. Maybe there are some
connections,
maybe not. For that scenario we might have emc issues, thermal problems
or a real kernel problem.
Regards,
Andreas
* Andreas Kemnade <[email protected]> [200421 06:54]:
> On Mon, 20 Apr 2020 23:11:18 +0200
> "H. Nikolaus Schaller" <[email protected]> wrote:
> > The only maybe important observation was when I disabled all
> > kernel modules except *hdq*.ko and *bq27*.ko. Then I did only
> > get an emergency shell so that it is quite similar to the
> > scenario Andreas has tested. With this setup it did work.
> >
> So I guess without idling uarts?
>
> > I then tried to reenable other kernel modules but the result
> > wasn't convincing that it gives a reliable result.
> >
> > So I have still no clear indication when the problem occurs and
> > when not.
> >
> Hmm, last summer I had problems even without that patch reading
> temperature while doing umts transfers. Maybe there are some
> connections,
> maybe not. For that scenario we might have emc issues, thermal problems
> or a real kernel problem.
I have confimed here that logicpd torpedo devkit with battery
works just fine while entering off mode during idle. I can see
the temperature just fine with:
# cat /sys/class/power_supply/bq27000-battery/uevent
POWER_SUPPLY_NAME=bq27000-battery
POWER_SUPPLY_STATUS=Charging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_VOLTAGE_NOW=3829000
POWER_SUPPLY_CURRENT_NOW=-592084
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_TEMP=306
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CHARGE_FULL_DESIGN=2056320
POWER_SUPPLY_CYCLE_COUNT=0
POWER_SUPPLY_POWER_AVG=0
POWER_SUPPLY_MANUFACTURER=Texas Instruments
This is 37xx though, maybe you have 35xx and there's some errata
that we're not handling?
I'm only seeing "2.7. HDQTM/1-Wire® Communication Constraints"
for external pull-up resitor in 34xx errata at [0].
I wonder if wrong external pull could cause flakyeness after
enabling the hdq module?
If nothing else helps, you could try to block idle for hdq
module, but I have a feeling that's a workaround for something
else.
Regards,
Tony
[0] https://www.ti.com/pdfs/wtbu/SWPZ009A_OMAP4430_Errata_Public_vA.pdf
Hi Tony,
> Am 21.04.2020 um 20:02 schrieb Tony Lindgren <[email protected]>:
>
> * Andreas Kemnade <[email protected]> [200421 06:54]:
>> On Mon, 20 Apr 2020 23:11:18 +0200
>> "H. Nikolaus Schaller" <[email protected]> wrote:
>>> The only maybe important observation was when I disabled all
>>> kernel modules except *hdq*.ko and *bq27*.ko. Then I did only
>>> get an emergency shell so that it is quite similar to the
>>> scenario Andreas has tested. With this setup it did work.
>>>
>> So I guess without idling uarts?
>>
>>> I then tried to reenable other kernel modules but the result
>>> wasn't convincing that it gives a reliable result.
>>>
>>> So I have still no clear indication when the problem occurs and
>>> when not.
>>>
>> Hmm, last summer I had problems even without that patch reading
>> temperature while doing umts transfers. Maybe there are some
>> connections,
>> maybe not. For that scenario we might have emc issues, thermal problems
>> or a real kernel problem.
>
> I have confimed here that logicpd torpedo devkit with battery
> works just fine while entering off mode during idle. I can see
> the temperature just fine with:
>
> # cat /sys/class/power_supply/bq27000-battery/uevent
> POWER_SUPPLY_NAME=bq27000-battery
> POWER_SUPPLY_STATUS=Charging
> POWER_SUPPLY_PRESENT=1
> POWER_SUPPLY_VOLTAGE_NOW=3829000
> POWER_SUPPLY_CURRENT_NOW=-592084
> POWER_SUPPLY_CAPACITY_LEVEL=Normal
> POWER_SUPPLY_TEMP=306
> POWER_SUPPLY_TECHNOLOGY=Li-ion
> POWER_SUPPLY_CHARGE_FULL_DESIGN=2056320
> POWER_SUPPLY_CYCLE_COUNT=0
> POWER_SUPPLY_POWER_AVG=0
> POWER_SUPPLY_MANUFACTURER=Texas Instruments
>
> This is 37xx though, maybe you have 35xx and there's some errata
> that we're not handling?
No, it is dm3730 on three different units I have tried.
> I'm only seeing "2.7. HDQTM/1-Wire® Communication Constraints"
> for external pull-up resitor in 34xx errata at [0].
>
> I wonder if wrong external pull could cause flakyeness after
> enabling the hdq module?
I have checked and we have 10 kOhm pullup to 1.8 V and a 470 Ohm
series resistor.
>
> If nothing else helps, you could try to block idle for hdq
> module, but I have a feeling that's a workaround for something
> else.
Well, what helps is reverting the patch and using the old driver
(which did work for several years). So I would not assume that
there is a hardware influence. It seems to be something the new
driver is doing differently.
I need more time to understand and trace this issue on what it
depends... It may depend on the sequence some other modules are
loaded and what the user-space (udevd) is doing in the meantime.
>
> Regards,
>
> Tony
>
> [0] https://www.ti.com/pdfs/wtbu/SWPZ009A_OMAP4430_Errata_Public_vA.pdf
>
BR and thanks,
Nikolaus
* H. Nikolaus Schaller <[email protected]> [200421 18:14]:
> > Am 21.04.2020 um 20:02 schrieb Tony Lindgren <[email protected]>:
> > This is 37xx though, maybe you have 35xx and there's some errata
> > that we're not handling?
>
> No, it is dm3730 on three different units I have tried.
>
> > I'm only seeing "2.7. HDQTM/1-Wire® Communication Constraints"
> > for external pull-up resitor in 34xx errata at [0].
> >
> > I wonder if wrong external pull could cause flakyeness after
> > enabling the hdq module?
>
> I have checked and we have 10 kOhm pullup to 1.8 V and a 470 Ohm
> series resistor.
OK
> > If nothing else helps, you could try to block idle for hdq
> > module, but I have a feeling that's a workaround for something
> > else.
>
> Well, what helps is reverting the patch and using the old driver
> (which did work for several years). So I would not assume that
> there is a hardware influence. It seems to be something the new
> driver is doing differently.
Well earlier hdq1w.c did not idle, now it does. If you just want
to keep it enabled like earlier, you can just add something like:
&hdqw1w {
ti,no-idle;
};
> I need more time to understand and trace this issue on what it
> depends... It may depend on the sequence some other modules are
> loaded and what the user-space (udevd) is doing in the meantime.
Yes would be good to understand what goes wrong here before we
apply the ti,no-idle as that will block SoC deeper idle states.
Regards,
Tony
On Tue, 21 Apr 2020 11:20:17 -0700
Tony Lindgren <[email protected]> wrote:
> * H. Nikolaus Schaller <[email protected]> [200421 18:14]:
> > > Am 21.04.2020 um 20:02 schrieb Tony Lindgren <[email protected]>:
> > > This is 37xx though, maybe you have 35xx and there's some errata
> > > that we're not handling?
> >
> > No, it is dm3730 on three different units I have tried.
> >
> > > I'm only seeing "2.7. HDQTM/1-Wire® Communication Constraints"
> > > for external pull-up resitor in 34xx errata at [0].
> > >
> > > I wonder if wrong external pull could cause flakyeness after
> > > enabling the hdq module?
> >
> > I have checked and we have 10 kOhm pullup to 1.8 V and a 470 Ohm
> > series resistor.
>
> OK
>
> > > If nothing else helps, you could try to block idle for hdq
> > > module, but I have a feeling that's a workaround for something
> > > else.
> >
> > Well, what helps is reverting the patch and using the old driver
> > (which did work for several years). So I would not assume that
> > there is a hardware influence. It seems to be something the new
> > driver is doing differently.
>
> Well earlier hdq1w.c did not idle, now it does. If you just want
> to keep it enabled like earlier, you can just add something like:
>
> &hdqw1w {
> ti,no-idle;
> };
>
> > I need more time to understand and trace this issue on what it
> > depends... It may depend on the sequence some other modules are
> > loaded and what the user-space (udevd) is doing in the meantime.
>
> Yes would be good to understand what goes wrong here before we
> apply the ti,no-idle as that will block SoC deeper idle states.
>
hmm, he is testing without idling uarts, so I am a bit confused here,
the problem only seems to occur when more things are *active*.
Is something not handled in time.
Regards,
Andreas
Hi Tony,
> Am 21.04.2020 um 20:20 schrieb Tony Lindgren <[email protected]>:
>
>> Well, what helps is reverting the patch and using the old driver
>> (which did work for several years). So I would not assume that
>> there is a hardware influence. It seems to be something the new
>> driver is doing differently.
>
> Well earlier hdq1w.c did not idle, now it does.
Ah, I see!
> If you just want to keep it enabled like earlier, you can just add something like:
Well, I don't want it enabled, it just should work as before.
Ideally including all improvements :)
>
> &hdqw1w {
> ti,no-idle;
> };
I have added that and there might be a slightly different pattern
(unless that is just by luck): the first two or three attempts to
read the bq27xx/uevent did still time out. But then the next ones
worked fine (with a response time of ca. 65 .. 230 ms).
So as if something needs to be shaken into the right position after
boot until it works.
Interestingly, after reinstalling the version without ti,no-idle;
it did work well on the first boot but not afterwards. Like there
is some memory surviving powerdown and battery removal... But again,
it started to work after 6 or 7 failed attempts.
If only it weren't so time-consuming to perform such tests...
>> I need more time to understand and trace this issue on what it
>> depends... It may depend on the sequence some other modules are
>> loaded and what the user-space (udevd) is doing in the meantime.
>
> Yes would be good to understand what goes wrong here before we
> apply the ti,no-idle as that will block SoC deeper idle states.
>
> Regards,
>
> Tony
BR and thanks,
Nikolaus
On Tue, 21 Apr 2020 22:40:39 +0200
"H. Nikolaus Schaller" <[email protected]> wrote:
> Hi Tony,
>
> > Am 21.04.2020 um 20:20 schrieb Tony Lindgren <[email protected]>:
> >
> >> Well, what helps is reverting the patch and using the old driver
> >> (which did work for several years). So I would not assume that
> >> there is a hardware influence. It seems to be something the new
> >> driver is doing differently.
> >
> > Well earlier hdq1w.c did not idle, now it does.
>
> Ah, I see!
>
> > If you just want to keep it enabled like earlier, you can just add something like:
>
> Well, I don't want it enabled, it just should work as before.
> Ideally including all improvements :)
>
> >
> > &hdqw1w {
> > ti,no-idle;
> > };
>
> I have added that and there might be a slightly different pattern
> (unless that is just by luck): the first two or three attempts to
> read the bq27xx/uevent did still time out. But then the next ones
> worked fine (with a response time of ca. 65 .. 230 ms).
>
> So as if something needs to be shaken into the right position after
> boot until it works.
>
What about reading battery with just omap_hdq loaded and then continue
booting as usual, e.g. something like:
have script init-bat.sh
#!/bin/sh
modprobe omap_hdq
cat /sys/class/power_supply/bq27000_battery/uevent
exec /sbin/init "$@"
and then append to kernel parameters init=/init-bat.sh
Regards,
Andreas
Hi,
> Am 22.04.2020 um 12:04 schrieb Andreas Kemnade <[email protected]>:
>
> On Tue, 21 Apr 2020 22:40:39 +0200
> "H. Nikolaus Schaller" <[email protected]> wrote:
>
>> Hi Tony,
>>
>>> Am 21.04.2020 um 20:20 schrieb Tony Lindgren <[email protected]>:
>>>
>>>> Well, what helps is reverting the patch and using the old driver
>>>> (which did work for several years). So I would not assume that
>>>> there is a hardware influence. It seems to be something the new
>>>> driver is doing differently.
>>>
>>> Well earlier hdq1w.c did not idle, now it does.
>>
>> Ah, I see!
>>
>>> If you just want to keep it enabled like earlier, you can just add something like:
>>
>> Well, I don't want it enabled, it just should work as before.
>> Ideally including all improvements :)
>>
>>>
>>> &hdqw1w {
>>> ti,no-idle;
>>> };
>>
>> I have added that and there might be a slightly different pattern
>> (unless that is just by luck): the first two or three attempts to
>> read the bq27xx/uevent did still time out. But then the next ones
>> worked fine (with a response time of ca. 65 .. 230 ms).
>>
>> So as if something needs to be shaken into the right position after
>> boot until it works.
>>
>
> What about reading battery with just omap_hdq loaded and then continue
> booting as usual, e.g. something like:
>
> have script init-bat.sh
> #!/bin/sh
> modprobe omap_hdq
> cat /sys/class/power_supply/bq27000_battery/uevent
> exec /sbin/init "$@"
>
> and then append to kernel parameters init=/init-bat.sh
Interesting idea. But how would it help to identify the
issue?
I can confirm that after a while of attempting to read
the uevent it suddenly starts to work.
Then I can even swap the battery (and verify by looking
at the cycle count which changes).
I have some unconfirmed impression that it helps to run
two cat /sys/class/power_supply/bq27000_battery/uevent
in quick enough succession to get it out of the hickup mode.
This needs more test.
It would indicate that if a new omap_hdq_runtime_resume()
is called by the second open+read+close sequence before
the autosuspend (300ms) of the previous omap_hdq_runtime_suspend()
happens, something gets back in place for the second
omap_hdq_runtime_suspend().
BR and thanks,
Nikolaus