Re: [PATCH v4 01/13] hw/arm/virt-acpi-build.c: Migrate fw_cfg creation to common location

[Sunil V L <sunilvl@ventanamicro.com>]

On Thu, Oct 26, 2023 at 10:15:00AM +0200, Andrew Jones wrote:
> On Thu, Oct 26, 2023 at 01:37:01AM +0530, Sunil V L wrote:
> ...
> > diff --git a/hw/nvram/fw_cfg-acpi.c b/hw/nvram/fw_cfg-acpi.c
> > new file mode 100644
> > index 0000000000..eddaffc09b
> > --- /dev/null
> > +++ b/hw/nvram/fw_cfg-acpi.c
> > @@ -0,0 +1,44 @@
> > +/*
> > + * Add fw_cfg device in DSDT
> > + *
> > + * Copyright (C) 2008-2010  Kevin O'Connor <kevin@koconnor.net>
> > + * Copyright (C) 2006 Fabrice Bellard
> > + * Copyright (C) 2013 Red Hat Inc
> > + *
> > + * Author: Michael S. Tsirkin <mst@redhat.com>
> > + *
> > + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
> > + *
> > + * Author: Shannon Zhao <zhaoshenglong@huawei.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > +
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > +
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> 
> I don't recommend creating new files with the long form GPL instead of an
> SPDX. I can't find a QEMU SPDX policy to point at, but pretty much every
> project I work on has been moving towards SPDX, and usually with a format
> policy. I presume QEMU will either slowly work its way there too or
> someday do a mass change. New files can participate in an unofficial slow
> transition now, rather than have to be touched again in a mass change.
> 
Sure. Let me update this in the next revision.

> ...
> > diff --git a/include/hw/nvram/fw_cfg_acpi.h b/include/hw/nvram/fw_cfg_acpi.h
> > new file mode 100644
> > index 0000000000..1c863df329
> > --- /dev/null
> > +++ b/include/hw/nvram/fw_cfg_acpi.h
> > @@ -0,0 +1,15 @@
> > +/*
> > + * ACPI support for fw_cfg
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> 
> While QEMU doesn't appear to have an SPDX policy with formatting rules,
> I would format this as
> 
> /* SPDX-License-Identifier: GPL-2.0-or-later */
> /*
>  * ACPI support for fw_cfg
>  */
> 
> And the source file above as
> 
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
>  * Add fw_cfg device in DSDT
>  *
>  * ...
>  */
> 
> as that is the recommended format for many projects (I think starting
> with Linux which documents[1] it) and tools have already learned that
> formatting. QEMU's checkpatch will accept the C99 comment style[2].
> 
> [1] https://www.kernel.org/doc/html/latest/process/license-rules.html#license-identifier-syntax
> [2] commit 8d061278d385 ("checkpatch: allow SPDX-License-Identifier")
> 
Thanks for the pointers. Let me update according to this.

Thanks,
Sunil