Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog not in timer block

From	Guenter Roeck <linux@roeck-us.net>
Newsgroups	linux.kernel
Subject	Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog not in timer block
Date	2016-03-28 20:20 +0200
Message-ID	<rhJVD-2nJ-1@gated-at.bofh.it> (permalink)
References	<rfZ8u-1se-3@gated-at.bofh.it> <rfZ8v-1se-31@gated-at.bofh.it> <rgEbD-5hP-1@gated-at.bofh.it> <rhIGe-1ja-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Mon, Mar 28, 2016 at 11:55:28AM -0500, Matthew McClintock wrote:
> 
> > On Mar 25, 2016, at 11:23 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> > 
> >> -#define WDT_RST		0x38
> >> -#define WDT_EN		0x40
> >> -#define WDT_BITE_TIME	0x5C
> >> +enum wdt_reg {
> >> +	WDT_RST,
> >> +	WDT_EN,
> >> +	WDT_BITE_TIME,
> >> +};
> >> +
> >> +static const u32 reg_offset_data_apcs_tmr[] = {
> >> +	[WDT_RST] = 0x38,
> >> +	[WDT_EN] = 0x40,
> >> +	[WDT_BITE_TIME] = 0x5C,
> >> +};
> >> +
> >> +static const u32 reg_offset_data_kpss[] = {
> >> +	[WDT_RST] = 0x4,
> >> +	[WDT_EN] = 0x8,
> > 
> > Does this work ? In the datasheet I have in front of me (APQ8064), the watchdog
> > at this address uses different bits. At address 0x40 (eg GSS_A5_APCS_WDT0_EN),
> 
> 0x40 is acps_tmr, and looks fine.
> 
> > bit 0 is the enable bit, and bit 1 enables interrupts. At address 0x08 (eg
> > LPASS_QDSP6SS_WDOG_UNMASKED_INT_EN), bit 0 enables interrupts and bit 1 is
> > undefined.
> 
> I honestly don’t see anything at 0x8 for either blocks that looks like this. For the new block bit 0 is enabling and bit 1 enabled interrupts.
> 
That is from the APQ8064 datasheet. 

> > Or does "qcom,kpss-standalone" refer to some other watchdog ?
> 
> APQ8064 would be the apcs_tmr block variant which is unchanged. MSM8916 as well as IPQ4019 would use the new kpss variant.
> 
Unfortunately I don't have access to those datasheets.

> I went with block names I found internally here, but I will be the first to admit I am terrible at names. The old block name for APQ was CPU0_ACPS_TMR (where really the watchdog is a subset of a timer block), and on the IPQ4019 it’s called APCS_KPSS_WDT and it’s really just a watchdog block.
> 
> I kept the same driver because the register’s currently in use were compatible. By the way, I tested this on an IPQ806x and IPQ4019 both new and old blocks.
> 

The property name should probably be something like 'qcom,kpss-wdt'
(or 'qcom,kpss-watchdog' ?), possibly in addition to 'qcom,kpss-ipq4019-wdt'
and 'qcom,kpss-msm8916-wdt'.

Guenter

Thread

[PATCH 07/17] watchdog: qcom: add option for standalone watchdog not in timer block Matthew McClintock <mmcclint@codeaurora.org> - 2016-03-23 23:10 +0100
  Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog  not in timer block Stephen Boyd <sboyd@codeaurora.org> - 2016-03-23 23:50 +0100
  Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog  not in timer block Guenter Roeck <linux@roeck-us.net> - 2016-03-25 19:00 +0100
    Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog not in timer block Matthew McClintock <mmcclint@codeaurora.org> - 2016-03-28 19:00 +0200
      Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog  not in timer block Guenter Roeck <linux@roeck-us.net> - 2016-03-28 20:20 +0200
        Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog not in timer block Matthew McClintock <mmcclint@codeaurora.org> - 2016-03-28 22:50 +0200
          Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog  not in timer block Guenter Roeck <linux@roeck-us.net> - 2016-03-29 00:00 +0200
            Re: [PATCH 07/17] watchdog: qcom: add option for standalone watchdog not in timer block Matthew McClintock <mmcclint@codeaurora.org> - 2016-03-29 00:30 +0200

csiph-web