diff mbox series

[v2,2/3] ACPI / watchdog: Fix gas->access_width usage

Message ID 20200212145941.32914-3-mika.westerberg@linux.intel.com (mailing list archive)
State Mainlined, archived
Headers show
Series ACPI / watchdog: Fix two reported issues | expand

Commit Message

Mika Westerberg Feb. 12, 2020, 2:59 p.m. UTC 
ACPI Generic Address Structure (GAS) access_width field is not in bytes
as the driver seems to expect in few places so fix this by using the
newly introduced macro ACPI_ACCESS_BYTE_WIDTH().

Fixes: b1abf6fc4982 ("ACPI / watchdog: Fix off-by-one error at resource assignment")
Fixes: 058dfc767008 ("ACPI / watchdog: Add support for WDAT hardware watchdog")
Reported-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/acpi_watchdog.c | 3 +--
 drivers/watchdog/wdat_wdt.c  | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

Comments

Jean Delvare Feb. 12, 2020, 3:55 p.m. UTC | #1 
On Wed, 12 Feb 2020 17:59:40 +0300, Mika Westerberg wrote:
> ACPI Generic Address Structure (GAS) access_width field is not in bytes
> as the driver seems to expect in few places so fix this by using the
> newly introduced macro ACPI_ACCESS_BYTE_WIDTH().
> 
> Fixes: b1abf6fc4982 ("ACPI / watchdog: Fix off-by-one error at resource assignment")

It does not actually fix that commit, as the bug already existed prior
to it. It has to be applied on top of that commit though because they
touch the same lines, granted.

> Fixes: 058dfc767008 ("ACPI / watchdog: Add support for WDAT hardware watchdog")
> Reported-by: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/acpi/acpi_watchdog.c | 3 +--
>  drivers/watchdog/wdat_wdt.c  | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> index b5516b04ffc0..d827a4a3e946 100644
> --- a/drivers/acpi/acpi_watchdog.c
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -126,12 +126,11 @@ void __init acpi_watchdog_init(void)
>  		gas = &entries[i].register_region;
>  
>  		res.start = gas->address;
> +		res.end = res.start + ACPI_ACCESS_BYTE_WIDTH(gas->access_width) - 1;
>  		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>  			res.flags = IORESOURCE_MEM;
> -			res.end = res.start + ALIGN(gas->access_width, 4) - 1;
>  		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
>  			res.flags = IORESOURCE_IO;
> -			res.end = res.start + gas->access_width - 1;
>  		} else {
>  			pr_warn("Unsupported address space: %u\n",
>  				gas->space_id);
> diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
> index b069349b52f5..e1b1fcfc02af 100644
> --- a/drivers/watchdog/wdat_wdt.c
> +++ b/drivers/watchdog/wdat_wdt.c
> @@ -389,7 +389,7 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>  
>  		memset(&r, 0, sizeof(r));
>  		r.start = gas->address;
> -		r.end = r.start + gas->access_width - 1;
> +		r.end = r.start + ACPI_ACCESS_BYTE_WIDTH(gas->access_width) - 1;
>  		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>  			r.flags = IORESOURCE_MEM;
>  		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Mika Westerberg Feb. 12, 2020, 4:58 p.m. UTC | #2 
On Wed, Feb 12, 2020 at 04:55:37PM +0100, Jean Delvare wrote:
> On Wed, 12 Feb 2020 17:59:40 +0300, Mika Westerberg wrote:
> > ACPI Generic Address Structure (GAS) access_width field is not in bytes
> > as the driver seems to expect in few places so fix this by using the
> > newly introduced macro ACPI_ACCESS_BYTE_WIDTH().
> > 
> > Fixes: b1abf6fc4982 ("ACPI / watchdog: Fix off-by-one error at resource assignment")
> 
> It does not actually fix that commit, as the bug already existed prior
> to it. It has to be applied on top of that commit though because they
> touch the same lines, granted.

Yeah, I figured I should put the dependency commit here as well. I guess
Depends-on can be used as well:

Depends-on: b1abf6fc4982 ("ACPI / watchdog: Fix off-by-one error at resource assignment")

> > Fixes: 058dfc767008 ("ACPI / watchdog: Add support for WDAT hardware watchdog")
> > Reported-by: Jean Delvare <jdelvare@suse.de>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/acpi/acpi_watchdog.c | 3 +--
> >  drivers/watchdog/wdat_wdt.c  | 2 +-
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> > index b5516b04ffc0..d827a4a3e946 100644
> > --- a/drivers/acpi/acpi_watchdog.c
> > +++ b/drivers/acpi/acpi_watchdog.c
> > @@ -126,12 +126,11 @@ void __init acpi_watchdog_init(void)
> >  		gas = &entries[i].register_region;
> >  
> >  		res.start = gas->address;
> > +		res.end = res.start + ACPI_ACCESS_BYTE_WIDTH(gas->access_width) - 1;
> >  		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> >  			res.flags = IORESOURCE_MEM;
> > -			res.end = res.start + ALIGN(gas->access_width, 4) - 1;
> >  		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> >  			res.flags = IORESOURCE_IO;
> > -			res.end = res.start + gas->access_width - 1;
> >  		} else {
> >  			pr_warn("Unsupported address space: %u\n",
> >  				gas->space_id);
> > diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
> > index b069349b52f5..e1b1fcfc02af 100644
> > --- a/drivers/watchdog/wdat_wdt.c
> > +++ b/drivers/watchdog/wdat_wdt.c
> > @@ -389,7 +389,7 @@ static int wdat_wdt_probe(struct platform_device *pdev)
> >  
> >  		memset(&r, 0, sizeof(r));
> >  		r.start = gas->address;
> > -		r.end = r.start + gas->access_width - 1;
> > +		r.end = r.start + ACPI_ACCESS_BYTE_WIDTH(gas->access_width) - 1;
> >  		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> >  			r.flags = IORESOURCE_MEM;
> >  		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> 
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Thanks!
diff mbox series

Patch

diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index b5516b04ffc0..d827a4a3e946 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -126,12 +126,11 @@  void __init acpi_watchdog_init(void)
 		gas = &entries[i].register_region;
 
 		res.start = gas->address;
+		res.end = res.start + ACPI_ACCESS_BYTE_WIDTH(gas->access_width) - 1;
 		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
 			res.flags = IORESOURCE_MEM;
-			res.end = res.start + ALIGN(gas->access_width, 4) - 1;
 		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 			res.flags = IORESOURCE_IO;
-			res.end = res.start + gas->access_width - 1;
 		} else {
 			pr_warn("Unsupported address space: %u\n",
 				gas->space_id);
diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
index b069349b52f5..e1b1fcfc02af 100644
--- a/drivers/watchdog/wdat_wdt.c
+++ b/drivers/watchdog/wdat_wdt.c
@@ -389,7 +389,7 @@  static int wdat_wdt_probe(struct platform_device *pdev)
 
 		memset(&r, 0, sizeof(r));
 		r.start = gas->address;
-		r.end = r.start + gas->access_width - 1;
+		r.end = r.start + ACPI_ACCESS_BYTE_WIDTH(gas->access_width) - 1;
 		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
 			r.flags = IORESOURCE_MEM;
 		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {