| Message ID | 20180313225131.GB21077@xps13.dannf |
|---|---|
| State | New |
| Headers | show |
| Series | [Bionic,SRU,Artful] watchdog: sbsa: use 32-bit read for WCV | expand |
On Tue, Mar 13, 2018 at 04:51:31PM -0600, dann frazier wrote: > From: Jayachandran C <jnair@caviumnetworks.com> > > BugLink: https://bugs.launchpad.net/bugs/1755595 > > According to SBSA spec v3.1 section 5.3: > All registers are 32 bits in size and should be accessed using > 32-bit reads and writes. If an access size other than 32 bits > is used then the results are IMPLEMENTATION DEFINED. > [...] > The Generic Watchdog is little-endian > > The current code uses readq to read the watchdog compare register > which does a 64-bit access. This fails on ThunderX2 which does not > implement 64-bit access to this register. > > Fix this by using lo_hi_readq() that does two 32-bit reads. > > Signed-off-by: Jayachandran C <jnair@caviumnetworks.com> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Wim Van Sebroeck <wim@iguana.be> > (cherry picked from commit 93ac3deb7c220cbcec032a967220a1f109d58431) > Signed-off-by: dann frazier <dann.frazier@canonical.com> Acked-by: Seth Forshee <seth.forshee@canonical.com> Applied to bionic/master-next, thanks!
On 13.03.2018 23:51, dann frazier wrote: > From: Jayachandran C <jnair@caviumnetworks.com> > > BugLink: https://bugs.launchpad.net/bugs/1755595 > > According to SBSA spec v3.1 section 5.3: > All registers are 32 bits in size and should be accessed using > 32-bit reads and writes. If an access size other than 32 bits > is used then the results are IMPLEMENTATION DEFINED. > [...] > The Generic Watchdog is little-endian > > The current code uses readq to read the watchdog compare register > which does a 64-bit access. This fails on ThunderX2 which does not > implement 64-bit access to this register. > > Fix this by using lo_hi_readq() that does two 32-bit reads. > > Signed-off-by: Jayachandran C <jnair@caviumnetworks.com> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Wim Van Sebroeck <wim@iguana.be> > (cherry picked from commit 93ac3deb7c220cbcec032a967220a1f109d58431) > Signed-off-by: dann frazier <dann.frazier@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/watchdog/sbsa_gwdt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c > index 316c2eb122d2..e8bd9887c566 100644 > --- a/drivers/watchdog/sbsa_gwdt.c > +++ b/drivers/watchdog/sbsa_gwdt.c > @@ -50,6 +50,7 @@ > */ > > #include <linux/io.h> > +#include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/moduleparam.h> > @@ -159,7 +160,7 @@ static unsigned int sbsa_gwdt_get_timeleft(struct watchdog_device *wdd) > !(readl(gwdt->control_base + SBSA_GWDT_WCS) & SBSA_GWDT_WCS_WS0)) > timeleft += readl(gwdt->control_base + SBSA_GWDT_WOR); > > - timeleft += readq(gwdt->control_base + SBSA_GWDT_WCV) - > + timeleft += lo_hi_readq(gwdt->control_base + SBSA_GWDT_WCV) - > arch_counter_get_cntvct(); > > do_div(timeleft, gwdt->clk); >
On 13.03.2018 23:51, dann frazier wrote: > From: Jayachandran C <jnair@caviumnetworks.com> > > BugLink: https://bugs.launchpad.net/bugs/1755595 > > According to SBSA spec v3.1 section 5.3: > All registers are 32 bits in size and should be accessed using > 32-bit reads and writes. If an access size other than 32 bits > is used then the results are IMPLEMENTATION DEFINED. > [...] > The Generic Watchdog is little-endian > > The current code uses readq to read the watchdog compare register > which does a 64-bit access. This fails on ThunderX2 which does not > implement 64-bit access to this register. > > Fix this by using lo_hi_readq() that does two 32-bit reads. > > Signed-off-by: Jayachandran C <jnair@caviumnetworks.com> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Wim Van Sebroeck <wim@iguana.be> > (cherry picked from commit 93ac3deb7c220cbcec032a967220a1f109d58431) > Signed-off-by: dann frazier <dann.frazier@canonical.com> > --- > drivers/watchdog/sbsa_gwdt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c > index 316c2eb122d2..e8bd9887c566 100644 > --- a/drivers/watchdog/sbsa_gwdt.c > +++ b/drivers/watchdog/sbsa_gwdt.c > @@ -50,6 +50,7 @@ > */ > > #include <linux/io.h> > +#include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/moduleparam.h> > @@ -159,7 +160,7 @@ static unsigned int sbsa_gwdt_get_timeleft(struct watchdog_device *wdd) > !(readl(gwdt->control_base + SBSA_GWDT_WCS) & SBSA_GWDT_WCS_WS0)) > timeleft += readl(gwdt->control_base + SBSA_GWDT_WOR); > > - timeleft += readq(gwdt->control_base + SBSA_GWDT_WCV) - > + timeleft += lo_hi_readq(gwdt->control_base + SBSA_GWDT_WCV) - > arch_counter_get_cntvct(); > > do_div(timeleft, gwdt->clk); > Applied to artful master-next
diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c index 316c2eb122d2..e8bd9887c566 100644 --- a/drivers/watchdog/sbsa_gwdt.c +++ b/drivers/watchdog/sbsa_gwdt.c @@ -50,6 +50,7 @@ */ #include <linux/io.h> +#include <linux/io-64-nonatomic-lo-hi.h> #include <linux/interrupt.h> #include <linux/module.h> #include <linux/moduleparam.h> @@ -159,7 +160,7 @@ static unsigned int sbsa_gwdt_get_timeleft(struct watchdog_device *wdd) !(readl(gwdt->control_base + SBSA_GWDT_WCS) & SBSA_GWDT_WCS_WS0)) timeleft += readl(gwdt->control_base + SBSA_GWDT_WOR); - timeleft += readq(gwdt->control_base + SBSA_GWDT_WCV) - + timeleft += lo_hi_readq(gwdt->control_base + SBSA_GWDT_WCV) - arch_counter_get_cntvct(); do_div(timeleft, gwdt->clk);