Re: [PATCH RFC 1/4] ACPI/AEST: Initial AEST driver

[Tyler Baicar OS]

Hello Shiju,

Thank you for the feedback!

On Thu, Jul 4, 2019 at 12:03 PM Shiju Jose <shiju.jose@xxxxxxxxxx> wrote:
> >+struct ras_ext_regs {
> >+      u64 err_fr;
> >+      u64 err_ctlr;
> >+      u64 err_status;
> >+      u64 err_addr;
> >+      u64 err_misc0;
> >+      u64 err_misc1;
> >+      u64 err_misc2;
> >+      u64 err_misc3;
> err_misc2 and err_misc3 are not used. Are they for the future purpose?

Yes, these will be for future purpose once ARMv8.4 support is added. I'd like
to keep them in this structure define since that makes iterating through the
memory mapped error records easier. Regardless of ARMv8.2 or ARMv8.4 support,
each error record in memory mapped nodes are 64 bytes apart. Having these in
the structure make the structure 64 bytes long making it possible for me to
increment through error records with the increment ++ operation.

If folks don't agree with that, I can change this structure to just have a
reserved field at the end such as:

+ u64 res0[2];

or

+ u8 res0[8];


> >+      ppi_data = kcalloc(num_ppi, sizeof(struct aest_node_data *),
> >+                         GFP_KERNEL);
> >+
> >+      for (i = 0; i < num_ppi; i++) {
> >+              ppi_data[i] = alloc_percpu(struct aest_node_data);
> >+              if (!ppi_data[i]) {
> >+                      ret = -ENOMEM;
> >+                      break;
> >+              }
> >+      }
> >+
> >+      if (ret) {
> >+              pr_err("Failed percpu allocation\n");
> >+              for (i = 0; i < num_ppi; i++)
> >+                      free_percpu(ppi_data[i]);
> I think 'ppi_data' to be freed here?

Yes it should be! I'll add that in the next version.

Thanks,
Tyler