Re: [PATCH v2 4/7] hw/misc: Allwinner AXP-209 Emulation

[Strahinja Jankovic]

Hi,

On Sun, Dec 18, 2022 at 11:23 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 18/12/22 23:12, Strahinja Jankovic wrote:
> > Hi,
> >
> > On Sun, Dec 18, 2022 at 11:07 PM Philippe Mathieu-Daudé
> > <philmd@linaro.org> wrote:
> >>
> >> On 18/12/22 22:19, Strahinja Jankovic wrote:
> >>> This patch adds minimal support for AXP-209 PMU.
> >>> Most important is chip ID since U-Boot SPL expects version 0x1. Besides
> >>> the chip ID register, reset values for two more registers used by A10
> >>> U-Boot SPL are covered.
> >>>
> >>> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> >>> ---
> >>>    hw/arm/Kconfig              |   1 +
> >>>    hw/misc/Kconfig             |   4 +
> >>>    hw/misc/allwinner-axp-209.c | 238 ++++++++++++++++++++++++++++++++++++
> >>>    hw/misc/meson.build         |   1 +
> >>>    hw/misc/trace-events        |   5 +
> >>>    5 files changed, 249 insertions(+)
> >>>    create mode 100644 hw/misc/allwinner-axp-209.c
> >>
> >>
> >>> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> >>> index 052fb54310..3855d937fd 100644
> >>> --- a/hw/misc/Kconfig
> >>> +++ b/hw/misc/Kconfig
> >>> @@ -180,4 +180,8 @@ config ALLWINNER_A10_CCM
> >>>    config ALLWINNER_A10_DRAMC
> >>>        bool
> >>>
> >>> +config ALLWINNER_AXP_209
> >>
> >> This controller is not specific to AllWinner. It can be plugged on any
> >> i2c bus. Please rename it AXP209_PMU. Otherwise LGTM!
> >
> > Do you mean only in Kconfig, or everywhere (file name, function names,
> > etc.)? Thanks.
>
> Keeping the file / functions names with 'allwinner' would give the false
> idea this is AllWinner specific; rather confusing isn't it? Besides it
> is not part of the SoC, this is an external component sitting on the
> bus. So "everywhere".

I was expecting that answer, but I wanted to check :)
When I started writing code for AXP209 it was indeed without allwinner
prefix, but then checkpatch.pl complained about missing lines in
MAINTAINERS. At that time it was easier to add the prefix (and get a
match in MAINTAINERS) than to investigate if I should update the
MAINTAINERS file or ignore the checkpatch.pl complaint.

I will update everything related to AXP209 so it does not have the
Allwinner prefix.

>
> Also, the "select ALLWINNER_AXP_209/AXP209_PMU" line in hw/arm/Kconfig
> belongs to the next patch "hw/arm: Add AXP-209 to Cubieboard".

I agree, will fix it.

Best regards,
Strahinja

>
> Regards,
>
> Phil.