On Sat, 19 Nov 2022 01:14:50 +0800, Emil Renner Berthing wrote: > On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote: >> diff --git a/drivers/reset/starfive/reset-starfive-jh7110.c b/drivers/reset/starfive/reset-starfive-jh7110.c >> new file mode 100644 >> index 000000000000..00f3b4ecfb02 >> --- /dev/null >> +++ b/drivers/reset/starfive/reset-starfive-jh7110.c >> @@ -0,0 +1,67 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Reset driver for the StarFive JH7110 SoC >> + * >> + * Copyright (C) 2022 StarFive Technology Co., Ltd. >> + * Copyright (C) 2022 Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> >> + */ >> + >> +#include <linux/auxiliary_bus.h> >> + >> +#include "reset-starfive-jh71x0.h" >> + >> +#include <dt-bindings/reset/starfive-jh7110.h> >> + >> +static int jh7110_reset_probe(struct auxiliary_device *adev, >> + const struct auxiliary_device_id *id) >> +{ >> + struct reset_info *info = (struct reset_info *)(id->driver_data); >> + void __iomem *base = dev_get_drvdata(adev->dev.parent); >> + >> + if (!info || !base) >> + return -ENODEV; >> + >> + return reset_starfive_jh71x0_register(&adev->dev, adev->dev.parent->of_node, >> + base + info->assert_offset, >> + base + info->status_offset, >> + info->asserted, >> + info->nr_resets, >> + false); >> +} >> + >> +static const struct reset_info jh7110_sys_info = { >> + .nr_resets = JH7110_SYSRST_END, >> + .assert_offset = 0x2F8, >> + .status_offset = 0x308, >> + .asserted = NULL, >> +}; >> + >> +static const struct reset_info jh7110_aon_info = { >> + .nr_resets = JH7110_AONRST_END, >> + .assert_offset = 0x38, >> + .status_offset = 0x3C, >> + .asserted = NULL, >> +}; > > It doesn't seem like syscrg, aoncrg or stgcrg have any inverted lines. > Do you know if any other CRGs do? If not you can just leave out the > .asserted member and always pass NULL. All JH7110 CRGs don't have inverted lines, and it could be modified as what you said. > >> +static const struct auxiliary_device_id jh7110_reset_ids[] = { >> + { >> + .name = "clk_starfive_jh71x0.reset-sys", >> + .driver_data = (kernel_ulong_t)&jh7110_sys_info, >> + }, >> + { >> + .name = "clk_starfive_jh71x0.reset-aon", >> + .driver_data = (kernel_ulong_t)&jh7110_aon_info, >> + }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(auxiliary, jh7110_reset_ids); >> + >> +static struct auxiliary_driver jh7110_reset_driver = { >> + .probe = jh7110_reset_probe, >> + .id_table = jh7110_reset_ids, >> +}; >> +module_auxiliary_driver(jh7110_reset_driver); >> + >> +MODULE_DESCRIPTION("StarFive JH7110 Reset Driver"); >> +MODULE_AUTHOR("Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>"); >> +MODULE_LICENSE("GPL"); >> diff --git a/drivers/reset/starfive/reset-starfive-jh71x0.h b/drivers/reset/starfive/reset-starfive-jh71x0.h >> index e6b27110de48..63a94ee1b395 100644 >> --- a/drivers/reset/starfive/reset-starfive-jh71x0.h >> +++ b/drivers/reset/starfive/reset-starfive-jh71x0.h >> @@ -6,6 +6,13 @@ >> #ifndef __RESET_STARFIVE_JH71X0_H >> #define __RESET_STARFIVE_JH71X0_H >> >> +struct reset_info { >> + unsigned int nr_resets; >> + unsigned int assert_offset; >> + unsigned int status_offset; >> + const u32 *asserted; >> +}; > > This struct isn't used outside of reset-starfive-jh7110.c, so no need > to define it in this header. > Also consider calling it jh7110_reset_info so it blends in with the > functions defined in that file. Maybe it can be used in JH7100 audio reset? Best regards, Hal > >> int reset_starfive_jh71x0_register(struct device *dev, struct device_node *of_node, >> void __iomem *assert, void __iomem *status, >> const u32 *asserted, unsigned int nr_resets,