Hi,
Thank you for submitting the patch set for R40. I think Niek and Peter
covered the most important comments in the cover e-mail that you sent.
On Thu, Mar 2, 2023 at 12:41 PM <qianfangui...@163.com> wrote:
>
> From: qianfan Zhao <qianfangui...@163.com>
>
> This patch adds minimal support for AXP-221 PMU and connect it to
> bananapi M2U board.
>
> Signed-off-by: qianfan Zhao <qianfangui...@163.com>
I would just like to suggest merging the AXP-221 PMU implementation
with the AXP-209 (perhaps into axp2xx_pmu.c), since behavior and
functionality of both devices is the same.
>From what I can see, the difference is only in the register map and
reset values, so the functions can be shared.
Other than that, great work!
I hope I will have more time when you submit the patches without the
RFC tag, so I can help with the review.
Best regards,
Strahinja Jankovic
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/bananapi_m2u.c | 5 ++
> hw/misc/Kconfig | 4 +
> hw/misc/axp221.c | 196 ++++++++++++++++++++++++++++++++++++++++++
> hw/misc/meson.build | 1 +
> hw/misc/trace-events | 5 ++
> 6 files changed, 212 insertions(+)
> create mode 100644 hw/misc/axp221.c
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 8eabedce43..5d2c10cb18 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -347,6 +347,7 @@ config ALLWINNER_H3
> config ALLWINNER_R40
> bool
> select ALLWINNER_A10_PIT
> + select AXP221_PMU
> select SERIAL
> select ARM_TIMER
> select ARM_GIC
> diff --git a/hw/arm/bananapi_m2u.c b/hw/arm/bananapi_m2u.c
> index 1b6241719d..bdee12efd3 100644
> --- a/hw/arm/bananapi_m2u.c
> +++ b/hw/arm/bananapi_m2u.c
> @@ -22,6 +22,7 @@
> #include "exec/address-spaces.h"
> #include "qapi/error.h"
> #include "hw/boards.h"
> +#include "hw/i2c/i2c.h"
> #include "hw/qdev-properties.h"
> #include "hw/arm/allwinner-r40.h"
>
> @@ -91,6 +92,10 @@ static void bpim2u_init(MachineState *machine)
> &bootroom_loaded);
> mmc_attach_drive(r40, &r40->mmc3, 3, false, NULL);
>
> + /* Connect AXP221 */
> + i2c = I2C_BUS(qdev_get_child_bus(DEVICE(&r40->i2c0), "i2c"));
> + i2c_slave_create_simple(i2c, "axp221_pmu", 0x34);
> +
> /* SDRAM */
> memory_region_add_subregion(get_system_memory(),
> r40->memmap[AW_R40_DEV_SDRAM],
> machine->ram);
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 2ef5781ef8..f66ac390b1 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -180,4 +180,8 @@ config AXP209_PMU
> bool
> depends on I2C
>
> +config AXP221_PMU
> + bool
> + depends on I2C
> +
> source macio/Kconfig
> diff --git a/hw/misc/axp221.c b/hw/misc/axp221.c
> new file mode 100644
> index 0000000000..47784bb085
> --- /dev/null
> +++ b/hw/misc/axp221.c
> @@ -0,0 +1,196 @@
> +/*
> + * AXP-221/221s PMU Emulation
> + *
> + * Copyright (C) 2023 qianfan Zhao <qianfangui...@163.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/bitops.h"
> +#include "trace.h"
> +#include "hw/i2c/i2c.h"
> +#include "migration/vmstate.h"
> +
> +#define TYPE_AXP221_PMU "axp221_pmu"
> +
> +#define AXP221(obj) \
> + OBJECT_CHECK(AXP221I2CState, (obj), TYPE_AXP221_PMU)
> +
> +#define NR_REGS 0xff
> +
> +/* A simple I2C slave which returns values of ID or CNT register. */
> +typedef struct AXP221I2CState {
> + /*< private >*/
> + I2CSlave i2c;
> + /*< public >*/
> + uint8_t regs[NR_REGS]; /* peripheral registers */
> + uint8_t ptr; /* current register index */
> + uint8_t count; /* counter used for tx/rx */
> +} AXP221I2CState;
> +
> +#define AXP221_PWR_STATUS_ACIN_PRESENT BIT(7)
> +#define AXP221_PWR_STATUS_ACIN_AVAIL BIT(6)
> +#define AXP221_PWR_STATUS_VBUS_PRESENT BIT(5)
> +#define AXP221_PWR_STATUS_VBUS_USED BIT(4)
> +#define AXP221_PWR_STATUS_BAT_CHARGING BIT(2)
> +#define AXP221_PWR_STATUS_ACIN_VBUS_POWERED BIT(1)
> +
> +/* Reset all counters and load ID register */
> +static void axp221_reset_enter(Object *obj, ResetType type)
> +{
> + AXP221I2CState *s = AXP221(obj);
> +
> + memset(s->regs, 0, NR_REGS);
> + s->ptr = 0;
> + s->count = 0;
> +
> + /* input power status register */
> + s->regs[0x00] = AXP221_PWR_STATUS_ACIN_PRESENT
> + | AXP221_PWR_STATUS_ACIN_AVAIL
> + | AXP221_PWR_STATUS_ACIN_VBUS_POWERED;
> +
> + s->regs[0x01] = 0x00; /* no battery is connected */
> +
> + /* CHIPID register, no documented on datasheet, but it is checked in
> + * u-boot spl. I had read it from AXP221s and got 0x06 value.
> + * So leave 06h here.
> + */
> + s->regs[0x03] = 0x06;
> +
> + s->regs[0x10] = 0xbf;
> + s->regs[0x13] = 0x01;
> + s->regs[0x30] = 0x60;
> + s->regs[0x31] = 0x03;
> + s->regs[0x32] = 0x43;
> + s->regs[0x33] = 0xc6;
> + s->regs[0x34] = 0x45;
> + s->regs[0x35] = 0x0e;
> + s->regs[0x36] = 0x5d;
> + s->regs[0x37] = 0x08;
> + s->regs[0x38] = 0xa5;
> + s->regs[0x39] = 0x1f;
> + s->regs[0x3c] = 0xfc;
> + s->regs[0x3d] = 0x16;
> + s->regs[0x80] = 0x80;
> + s->regs[0x82] = 0xe0;
> + s->regs[0x84] = 0x32;
> + s->regs[0x8f] = 0x01;
> +
> + s->regs[0x90] = 0x07;
> + s->regs[0x91] = 0x1f;
> + s->regs[0x92] = 0x07;
> + s->regs[0x93] = 0x1f;
> +
> + s->regs[0x40] = 0xd8;
> + s->regs[0x41] = 0xff;
> + s->regs[0x42] = 0x03;
> + s->regs[0x43] = 0x03;
> +
> + s->regs[0xb8] = 0xc0;
> + s->regs[0xb9] = 0x64;
> + s->regs[0xe6] = 0xa0;
> +}
> +
> +/* Handle events from master. */
> +static int axp221_event(I2CSlave *i2c, enum i2c_event event)
> +{
> + AXP221I2CState *s = AXP221(i2c);
> +
> + s->count = 0;
> +
> + return 0;
> +}
> +
> +/* Called when master requests read */
> +static uint8_t axp221_rx(I2CSlave *i2c)
> +{
> + AXP221I2CState *s = AXP221(i2c);
> + uint8_t ret = 0xff;
> +
> + if (s->ptr < NR_REGS) {
> + ret = s->regs[s->ptr];
> + trace_axp221_rx(s->ptr, ret);
> + s->ptr++;
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Called when master sends write.
> + * Update ptr with byte 0, then perform write with second byte.
> + */
> +static int axp221_tx(I2CSlave *i2c, uint8_t data)
> +{
> + AXP221I2CState *s = AXP221(i2c);
> +
> + if (s->count == 0) {
> + /* Store register address */
> + s->ptr = data;
> + s->count++;
> + trace_axp221_select(data);
> + } else {
> + trace_axp221_tx(s->ptr, data);
> + s->regs[s->ptr++] = data;
> + }
> +
> + return 0;
> +}
> +
> +static const VMStateDescription vmstate_axp221 = {
> + .name = TYPE_AXP221_PMU,
> + .version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT8_ARRAY(regs, AXP221I2CState, NR_REGS),
> + VMSTATE_UINT8(count, AXP221I2CState),
> + VMSTATE_UINT8(ptr, AXP221I2CState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void axp221_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> + I2CSlaveClass *isc = I2C_SLAVE_CLASS(oc);
> + ResettableClass *rc = RESETTABLE_CLASS(oc);
> +
> + rc->phases.enter = axp221_reset_enter;
> + dc->vmsd = &vmstate_axp221;
> + isc->event = axp221_event;
> + isc->recv = axp221_rx;
> + isc->send = axp221_tx;
> +}
> +
> +static const TypeInfo axp221_info = {
> + .name = TYPE_AXP221_PMU,
> + .parent = TYPE_I2C_SLAVE,
> + .instance_size = sizeof(AXP221I2CState),
> + .class_init = axp221_class_init
> +};
> +
> +static void axp221_register_devices(void)
> +{
> + type_register_static(&axp221_info);
> +}
> +
> +type_init(axp221_register_devices);
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index b19b44841a..6b81670641 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -46,6 +46,7 @@ softmmu_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true:
> files('allwinner-h3-sysctrl
> softmmu_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true:
> files('allwinner-sid.c'))
> softmmu_ss.add(when: 'CONFIG_ALLWINNER_R40', if_true:
> files('allwinner-r40-ccu.c'))
> softmmu_ss.add(when: 'CONFIG_AXP209_PMU', if_true: files('axp209.c'))
> +softmmu_ss.add(when: 'CONFIG_AXP221_PMU', if_true: files('axp221.c'))
> softmmu_ss.add(when: 'CONFIG_REALVIEW', if_true: files('arm_sysctl.c'))
> softmmu_ss.add(when: 'CONFIG_NSERIES', if_true: files('cbus.c'))
> softmmu_ss.add(when: 'CONFIG_ECCMEMCTL', if_true: files('eccmemctl.c'))
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index c47876a902..63b072d2d8 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -28,6 +28,11 @@ axp209_rx(uint8_t reg, uint8_t data) "Read reg 0x%" PRIx8
> " : 0x%" PRIx8
> axp209_select(uint8_t reg) "Accessing reg 0x%" PRIx8
> axp209_tx(uint8_t reg, uint8_t data) "Write reg 0x%" PRIx8 " : 0x%" PRIx8
>
> +# axp221.c
> +axp221_rx(uint8_t reg, uint8_t data) "Read reg 0x%" PRIx8 " : 0x%" PRIx8
> +axp221_select(uint8_t reg) "Accessing reg 0x%" PRIx8
> +axp221_tx(uint8_t reg, uint8_t data) "Write reg 0x%" PRIx8 " : 0x%" PRIx8
> +
> # eccmemctl.c
> ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
> ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"
> --
> 2.25.1
>
