Re: [PATCH v3 15/29] acpi: Add a simple sandbox test
On Tue, Apr 07, 2020 at 08:57:19PM -0600, Simon Glass wrote:
> Hi Andy,
>
> On Fri, 3 Apr 2020 at 06:51, Andy Shevchenko
> wrote:
> >
> > On Mon, Mar 30, 2020 at 05:12:51PM -0600, Simon Glass wrote:
> > > Add a sandbox test for the basic ACPI functionality we have so far.
> >
> > > +U_BOOT_DRIVER(testacpi_drv) = {
> > > + .name = "testacpi_drv",
> > > + .of_match = testacpi_ids,
> > > + .id = UCLASS_TEST_ACPI,
> >
> > > + acpi_ops_ptr(_ops)
> >
> > I have noticed that this is not obvious why no comma here.
> > Perhaps, since apci_ops_ptr is a macro, you should upper case it.
>
> This is a bit like of_match_ptr() which is a macro used by Linux.
For ACPI there is capitalized, but...
> Putting them in upper case makes them very hard to read. Admittedly
> the lack of a comma is odd though. It is because the field doesn't
> exist until ACPI is enabled (which it is not in SPL, for example).
...and this puts them to different categories, like PM ops in Linux kernel,
where they are also capitalized, exactly to be used in struct definitions.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 15/29] acpi: Add a simple sandbox test
Hi Andy,
On Fri, 3 Apr 2020 at 06:51, Andy Shevchenko
wrote:
>
> On Mon, Mar 30, 2020 at 05:12:51PM -0600, Simon Glass wrote:
> > Add a sandbox test for the basic ACPI functionality we have so far.
>
> > +U_BOOT_DRIVER(testacpi_drv) = {
> > + .name = "testacpi_drv",
> > + .of_match = testacpi_ids,
> > + .id = UCLASS_TEST_ACPI,
>
> > + acpi_ops_ptr(_ops)
>
> I have noticed that this is not obvious why no comma here.
> Perhaps, since apci_ops_ptr is a macro, you should upper case it.
This is a bit like of_match_ptr() which is a macro used by Linux.
Putting them in upper case makes them very hard to read. Admittedly
the lack of a comma is odd though. It is because the field doesn't
exist until ACPI is enabled (which it is not in SPL, for example).
Regards,
Simon
Re: [PATCH v3 15/29] acpi: Add a simple sandbox test
On Mon, Mar 30, 2020 at 05:12:51PM -0600, Simon Glass wrote:
> Add a sandbox test for the basic ACPI functionality we have so far.
> +U_BOOT_DRIVER(testacpi_drv) = {
> + .name = "testacpi_drv",
> + .of_match = testacpi_ids,
> + .id = UCLASS_TEST_ACPI,
> + acpi_ops_ptr(_ops)
I have noticed that this is not obvious why no comma here.
Perhaps, since apci_ops_ptr is a macro, you should upper case it.
> +};
--
With Best Regards,
Andy Shevchenko
