| Message ID | 45dcc7644e2b2a04645a37e561ed7294482b0744.1727236561.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Prepare GHES driver to support error injection | expand |
On Wed, 25 Sep 2024 06:04:07 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Reduce the ident of the function and prepares it for > the next changes. > > No functional changes. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> Some of the alignment doesn't seem to match local style which is either align after ( or align 4 spaces in from line above for multiple line argument lists. Code is fine as I guess the later structure is to prepare for additions that prevent early returns being sensible. With that in mind. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > hw/acpi/ghes.c | 56 ++++++++++++++++++++++++++------------------------ > 1 file changed, 29 insertions(+), 27 deletions(-) > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > index dc217694deb9..dacbd4d0c093 100644 > --- a/hw/acpi/ghes.c > +++ b/hw/acpi/ghes.c > @@ -402,40 +402,42 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address) > > start_addr = le64_to_cpu(ags->ghes_addr_le); > > - if (physical_address) { > + if (!physical_address) { > + return -1; > + } > > - if (source_id < ACPI_GHES_ERROR_SOURCE_COUNT) { > - start_addr += source_id * sizeof(uint64_t); > - } > + if (source_id < ACPI_GHES_ERROR_SOURCE_COUNT) { > + start_addr += source_id * sizeof(uint64_t); > + } > > - cpu_physical_memory_read(start_addr, &error_block_addr, > - sizeof(error_block_addr)); > + cpu_physical_memory_read(start_addr, &error_block_addr, > + sizeof(error_block_addr)); > > - error_block_addr = le64_to_cpu(error_block_addr); > + error_block_addr = le64_to_cpu(error_block_addr); > > - read_ack_register_addr = start_addr + > - ACPI_GHES_ERROR_SOURCE_COUNT * sizeof(uint64_t); > + read_ack_register_addr = start_addr + > + ACPI_GHES_ERROR_SOURCE_COUNT * sizeof(uint64_t); > > - cpu_physical_memory_read(read_ack_register_addr, > - &read_ack_register, sizeof(read_ack_register)); > + cpu_physical_memory_read(read_ack_register_addr, > + &read_ack_register, sizeof(read_ack_register)); Alignment looks fishy plus maybe move &read_ack_register up a line. > > - /* zero means OSPM does not acknowledge the error */ > - if (!read_ack_register) { > - error_report("OSPM does not acknowledge previous error," > - " so can not record CPER for current error anymore"); > - } else if (error_block_addr) { > - read_ack_register = cpu_to_le64(0); > - /* > - * Clear the Read Ack Register, OSPM will write it to 1 when > - * it acknowledges this error. > - */ > - cpu_physical_memory_write(read_ack_register_addr, > - &read_ack_register, sizeof(uint64_t)); > + /* zero means OSPM does not acknowledge the error */ > + if (!read_ack_register) { > + error_report("OSPM does not acknowledge previous error," > + " so can not record CPER for current error anymore"); I guess later changes make it unwise to just return -1 here. > + } else if (error_block_addr) { > + read_ack_register = cpu_to_le64(0); > + /* > + * Clear the Read Ack Register, OSPM will write it to 1 when > + * it acknowledges this error. > + */ > + cpu_physical_memory_write(read_ack_register_addr, > + &read_ack_register, sizeof(uint64_t)); > > - ret = acpi_ghes_record_mem_error(error_block_addr, > - physical_address); > - } else > - error_report("can not find Generic Error Status Block"); > + ret = acpi_ghes_record_mem_error(error_block_addr, > + physical_address); or return here. > + } else { > + error_report("can not find Generic Error Status Block"); > } > > return ret;
diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c index dc217694deb9..dacbd4d0c093 100644 --- a/hw/acpi/ghes.c +++ b/hw/acpi/ghes.c @@ -402,40 +402,42 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address) start_addr = le64_to_cpu(ags->ghes_addr_le); - if (physical_address) { + if (!physical_address) { + return -1; + } - if (source_id < ACPI_GHES_ERROR_SOURCE_COUNT) { - start_addr += source_id * sizeof(uint64_t); - } + if (source_id < ACPI_GHES_ERROR_SOURCE_COUNT) { + start_addr += source_id * sizeof(uint64_t); + } - cpu_physical_memory_read(start_addr, &error_block_addr, - sizeof(error_block_addr)); + cpu_physical_memory_read(start_addr, &error_block_addr, + sizeof(error_block_addr)); - error_block_addr = le64_to_cpu(error_block_addr); + error_block_addr = le64_to_cpu(error_block_addr); - read_ack_register_addr = start_addr + - ACPI_GHES_ERROR_SOURCE_COUNT * sizeof(uint64_t); + read_ack_register_addr = start_addr + + ACPI_GHES_ERROR_SOURCE_COUNT * sizeof(uint64_t); - cpu_physical_memory_read(read_ack_register_addr, - &read_ack_register, sizeof(read_ack_register)); + cpu_physical_memory_read(read_ack_register_addr, + &read_ack_register, sizeof(read_ack_register)); - /* zero means OSPM does not acknowledge the error */ - if (!read_ack_register) { - error_report("OSPM does not acknowledge previous error," - " so can not record CPER for current error anymore"); - } else if (error_block_addr) { - read_ack_register = cpu_to_le64(0); - /* - * Clear the Read Ack Register, OSPM will write it to 1 when - * it acknowledges this error. - */ - cpu_physical_memory_write(read_ack_register_addr, - &read_ack_register, sizeof(uint64_t)); + /* zero means OSPM does not acknowledge the error */ + if (!read_ack_register) { + error_report("OSPM does not acknowledge previous error," + " so can not record CPER for current error anymore"); + } else if (error_block_addr) { + read_ack_register = cpu_to_le64(0); + /* + * Clear the Read Ack Register, OSPM will write it to 1 when + * it acknowledges this error. + */ + cpu_physical_memory_write(read_ack_register_addr, + &read_ack_register, sizeof(uint64_t)); - ret = acpi_ghes_record_mem_error(error_block_addr, - physical_address); - } else - error_report("can not find Generic Error Status Block"); + ret = acpi_ghes_record_mem_error(error_block_addr, + physical_address); + } else { + error_report("can not find Generic Error Status Block"); } return ret;