| Message ID | 20211116150119.2171-10-kernel@esmil.dk (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Basic StarFive JH7100 RISC-V SoC support | expand |
On Tue, Nov 16, 2021 at 5:06 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > Add a driver for the StarFive JH7100 reset controller. Thanks! Couple of minor remarks below and take my Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > --- > MAINTAINERS | 7 + > drivers/reset/Kconfig | 7 + > drivers/reset/Makefile | 1 + > drivers/reset/reset-starfive-jh7100.c | 176 ++++++++++++++++++++++++++ > 4 files changed, 191 insertions(+) > create mode 100644 drivers/reset/reset-starfive-jh7100.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 2548ef1f8246..1152f5d94690 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -18137,6 +18137,13 @@ F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml > F: drivers/clk/starfive/clk-starfive-jh7100.c > F: include/dt-bindings/clock/starfive-jh7100.h > > +STARFIVE JH7100 RESET CONTROLLER DRIVER > +M: Emil Renner Berthing <kernel@esmil.dk> > +S: Maintained > +F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml > +F: drivers/reset/reset-starfive-jh7100.c > +F: include/dt-bindings/reset/starfive-jh7100.h > + > STATIC BRANCH/CALL > M: Peter Zijlstra <peterz@infradead.org> > M: Josh Poimboeuf <jpoimboe@redhat.com> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > index 85024eb1d2ea..6f8ba0ddc05f 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -224,6 +224,13 @@ config RESET_SOCFPGA > This enables the reset driver for the SoCFPGA ARMv7 platforms. This > driver gets initialized early during platform init calls. > > +config RESET_STARFIVE_JH7100 > + bool "StarFive JH7100 Reset Driver" > + depends on SOC_STARFIVE || COMPILE_TEST > + default SOC_STARFIVE > + help > + This enables the reset controller driver for the StarFive JH7100 SoC. > + > config RESET_SUNXI > bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI > default ARCH_SUNXI > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile > index 21d46d8869ff..bd0a97be18b5 100644 > --- a/drivers/reset/Makefile > +++ b/drivers/reset/Makefile > @@ -29,6 +29,7 @@ obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o > obj-$(CONFIG_RESET_SCMI) += reset-scmi.o > obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o > obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o > +obj-$(CONFIG_RESET_STARFIVE_JH7100) += reset-starfive-jh7100.o > obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o > obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o > obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o > diff --git a/drivers/reset/reset-starfive-jh7100.c b/drivers/reset/reset-starfive-jh7100.c > new file mode 100644 > index 000000000000..2f7615cbf1dc > --- /dev/null > +++ b/drivers/reset/reset-starfive-jh7100.c > @@ -0,0 +1,176 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Reset driver for the StarFive JH7100 SoC > + * > + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> > + */ > + > +#include <linux/bitmap.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/mod_devicetable.h> > +#include <linux/platform_device.h> > +#include <linux/reset-controller.h> > +#include <linux/spinlock.h> > + > +#include <dt-bindings/reset/starfive-jh7100.h> > + > +/* register offsets */ > +#define JH7100_RESET_ASSERT0 0x00 > +#define JH7100_RESET_ASSERT1 0x04 > +#define JH7100_RESET_ASSERT2 0x08 > +#define JH7100_RESET_ASSERT3 0x0c > +#define JH7100_RESET_STATUS0 0x10 > +#define JH7100_RESET_STATUS1 0x14 > +#define JH7100_RESET_STATUS2 0x18 > +#define JH7100_RESET_STATUS3 0x1c > +#if BITS_PER_LONG == 64 > +#define jh7100_reset_read readq > +#define jh7100_reset_write writeq > +#else > +#define jh7100_reset_read readl > +#define jh7100_reset_write writel > +#endif No need, just use always readq() / writeq() and include io-64-nonatomic-lo-hi > +/* > + * Writing a 1 to the n'th bit of the m'th ASSERT register asserts > + * line 32m + n, and writing a 0 deasserts the same line. > + * Most reset lines have their status inverted so a 0 bit in the STATUS > + * register means the line is asserted and a 1 means it's deasserted. A few > + * lines don't though, so store the expected value of the status registers when > + * all lines are asserted. > + */ > +static const DECLARE_BITMAP(jh7100_reset_asserted, 4 * 32) = { > + BITMAP_FROM_U64(BIT_ULL_MASK(JH7100_RST_U74) | > + BIT_ULL_MASK(JH7100_RST_VP6_DRESET) | > + BIT_ULL_MASK(JH7100_RST_VP6_BRESET) | > + BIT_ULL_MASK(JH7100_RST_HIFI4_DRESET) | > + BIT_ULL_MASK(JH7100_RST_HIFI4_BRESET)), > + BITMAP_FROM_U64(BIT_ULL_MASK(JH7100_RST_E24)), > +}; > + > +struct jh7100_reset { > + struct reset_controller_dev rcdev; > + /* protect registers against concurrent read-modify-write */ > + spinlock_t lock; > + void __iomem *base; > +}; > + > +static inline struct jh7100_reset * > +jh7100_reset_from(struct reset_controller_dev *rcdev) > +{ > + return container_of(rcdev, struct jh7100_reset, rcdev); > +} > + > +static int jh7100_reset_update(struct reset_controller_dev *rcdev, > + unsigned long id, bool assert) > +{ > + struct jh7100_reset *data = jh7100_reset_from(rcdev); > + unsigned long offset = id / BITS_PER_LONG; > + void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(long); > + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); > + unsigned long mask = BIT_MASK(id); It's better to group offset and mask, so it will give a hint to human and compiler that '/' + '%' can be optimized to one assembly instructions on some architectures (in case of compile-test or new RISC-V types, I dunno if they have it now). And use BIT_WORD() for offset calculation. Same for the above function. > + unsigned long done = jh7100_reset_asserted[offset] & mask; > + unsigned long value; > + unsigned long flags; > + int ret; > + > + if (!assert) > + done ^= mask; > + > + spin_lock_irqsave(&data->lock, flags); > + > + value = jh7100_reset_read(reg_assert); > + if (assert) > + value |= mask; > + else > + value &= ~mask; > + jh7100_reset_write(value, reg_assert); > + > + /* if the associated clock is gated, deasserting might otherwise hang forever */ > + ret = readx_poll_timeout_atomic(jh7100_reset_read, reg_status, value, > + (value & mask) == done, 0, 1000); > + > + spin_unlock_irqrestore(&data->lock, flags); > + return ret; > +} > + > +static int jh7100_reset_assert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + return jh7100_reset_update(rcdev, id, true); > +} > + > +static int jh7100_reset_deassert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + return jh7100_reset_update(rcdev, id, false); > +} > + > +static int jh7100_reset_reset(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + int ret; > + > + ret = jh7100_reset_assert(rcdev, id); > + if (ret) > + return ret; > + > + return jh7100_reset_deassert(rcdev, id); > +} > + > +static int jh7100_reset_status(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct jh7100_reset *data = jh7100_reset_from(rcdev); > + unsigned long offset = id / BITS_PER_LONG; > + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); > + unsigned long value = jh7100_reset_read(reg_status); > + unsigned long mask = BIT_MASK(id); > + > + return !((value ^ jh7100_reset_asserted[offset]) & mask); > +} > + > +static const struct reset_control_ops jh7100_reset_ops = { > + .assert = jh7100_reset_assert, > + .deassert = jh7100_reset_deassert, > + .reset = jh7100_reset_reset, > + .status = jh7100_reset_status, > +}; > + > +static int __init jh7100_reset_probe(struct platform_device *pdev) > +{ > + struct jh7100_reset *data; > + > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(data->base)) > + return PTR_ERR(data->base); > + > + data->rcdev.ops = &jh7100_reset_ops; > + data->rcdev.owner = THIS_MODULE; > + data->rcdev.nr_resets = JH7100_RSTN_END; > + data->rcdev.dev = &pdev->dev; > + data->rcdev.of_node = pdev->dev.of_node; > + spin_lock_init(&data->lock); > + > + return devm_reset_controller_register(&pdev->dev, &data->rcdev); > +} > + > +static const struct of_device_id jh7100_reset_dt_ids[] = { > + { .compatible = "starfive,jh7100-reset" }, > + { /* sentinel */ } > +}; > + > +static struct platform_driver jh7100_reset_driver = { > + .driver = { > + .name = "jh7100-reset", > + .of_match_table = jh7100_reset_dt_ids, > + .suppress_bind_attrs = true, > + }, > +}; > +builtin_platform_driver_probe(jh7100_reset_driver, jh7100_reset_probe); > -- > 2.33.1 >
On Tue, 16 Nov 2021 at 17:01, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Nov 16, 2021 at 5:06 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > > > Add a driver for the StarFive JH7100 reset controller. > > Thanks! > Couple of minor remarks below and take my > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Thank you. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > --- > > MAINTAINERS | 7 + > > drivers/reset/Kconfig | 7 + > > drivers/reset/Makefile | 1 + > > drivers/reset/reset-starfive-jh7100.c | 176 ++++++++++++++++++++++++++ > > 4 files changed, 191 insertions(+) > > create mode 100644 drivers/reset/reset-starfive-jh7100.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 2548ef1f8246..1152f5d94690 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -18137,6 +18137,13 @@ F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml > > F: drivers/clk/starfive/clk-starfive-jh7100.c > > F: include/dt-bindings/clock/starfive-jh7100.h > > > > +STARFIVE JH7100 RESET CONTROLLER DRIVER > > +M: Emil Renner Berthing <kernel@esmil.dk> > > +S: Maintained > > +F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml > > +F: drivers/reset/reset-starfive-jh7100.c > > +F: include/dt-bindings/reset/starfive-jh7100.h > > + > > STATIC BRANCH/CALL > > M: Peter Zijlstra <peterz@infradead.org> > > M: Josh Poimboeuf <jpoimboe@redhat.com> > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > > index 85024eb1d2ea..6f8ba0ddc05f 100644 > > --- a/drivers/reset/Kconfig > > +++ b/drivers/reset/Kconfig > > @@ -224,6 +224,13 @@ config RESET_SOCFPGA > > This enables the reset driver for the SoCFPGA ARMv7 platforms. This > > driver gets initialized early during platform init calls. > > > > +config RESET_STARFIVE_JH7100 > > + bool "StarFive JH7100 Reset Driver" > > + depends on SOC_STARFIVE || COMPILE_TEST > > + default SOC_STARFIVE > > + help > > + This enables the reset controller driver for the StarFive JH7100 SoC. > > + > > config RESET_SUNXI > > bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI > > default ARCH_SUNXI > > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile > > index 21d46d8869ff..bd0a97be18b5 100644 > > --- a/drivers/reset/Makefile > > +++ b/drivers/reset/Makefile > > @@ -29,6 +29,7 @@ obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o > > obj-$(CONFIG_RESET_SCMI) += reset-scmi.o > > obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o > > obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o > > +obj-$(CONFIG_RESET_STARFIVE_JH7100) += reset-starfive-jh7100.o > > obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o > > obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o > > obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o > > diff --git a/drivers/reset/reset-starfive-jh7100.c b/drivers/reset/reset-starfive-jh7100.c > > new file mode 100644 > > index 000000000000..2f7615cbf1dc > > --- /dev/null > > +++ b/drivers/reset/reset-starfive-jh7100.c > > @@ -0,0 +1,176 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Reset driver for the StarFive JH7100 SoC > > + * > > + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> > > + */ > > + > > +#include <linux/bitmap.h> > > +#include <linux/io.h> > > +#include <linux/iopoll.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/platform_device.h> > > +#include <linux/reset-controller.h> > > +#include <linux/spinlock.h> > > + > > +#include <dt-bindings/reset/starfive-jh7100.h> > > + > > +/* register offsets */ > > +#define JH7100_RESET_ASSERT0 0x00 > > +#define JH7100_RESET_ASSERT1 0x04 > > +#define JH7100_RESET_ASSERT2 0x08 > > +#define JH7100_RESET_ASSERT3 0x0c > > +#define JH7100_RESET_STATUS0 0x10 > > +#define JH7100_RESET_STATUS1 0x14 > > +#define JH7100_RESET_STATUS2 0x18 > > +#define JH7100_RESET_STATUS3 0x1c > > > +#if BITS_PER_LONG == 64 > > +#define jh7100_reset_read readq > > +#define jh7100_reset_write writeq > > +#else > > +#define jh7100_reset_read readl > > +#define jh7100_reset_write writel > > +#endif > > No need, just use always readq() / writeq() and include io-64-nonatomic-lo-hi Hmm.. how would that work on a 32bit architecture where sizeof(long) == 4? The whole point of this is to read chunks in sizes matching the words in the bitmap later on. > > +/* > > + * Writing a 1 to the n'th bit of the m'th ASSERT register asserts > > + * line 32m + n, and writing a 0 deasserts the same line. > > + * Most reset lines have their status inverted so a 0 bit in the STATUS > > + * register means the line is asserted and a 1 means it's deasserted. A few > > + * lines don't though, so store the expected value of the status registers when > > + * all lines are asserted. > > + */ > > +static const DECLARE_BITMAP(jh7100_reset_asserted, 4 * 32) = { > > + BITMAP_FROM_U64(BIT_ULL_MASK(JH7100_RST_U74) | > > + BIT_ULL_MASK(JH7100_RST_VP6_DRESET) | > > + BIT_ULL_MASK(JH7100_RST_VP6_BRESET) | > > + BIT_ULL_MASK(JH7100_RST_HIFI4_DRESET) | > > + BIT_ULL_MASK(JH7100_RST_HIFI4_BRESET)), > > + BITMAP_FROM_U64(BIT_ULL_MASK(JH7100_RST_E24)), > > +}; > > + > > +struct jh7100_reset { > > + struct reset_controller_dev rcdev; > > + /* protect registers against concurrent read-modify-write */ > > + spinlock_t lock; > > + void __iomem *base; > > +}; > > + > > +static inline struct jh7100_reset * > > +jh7100_reset_from(struct reset_controller_dev *rcdev) > > +{ > > + return container_of(rcdev, struct jh7100_reset, rcdev); > > +} > > + > > +static int jh7100_reset_update(struct reset_controller_dev *rcdev, > > + unsigned long id, bool assert) > > +{ > > + struct jh7100_reset *data = jh7100_reset_from(rcdev); > > + unsigned long offset = id / BITS_PER_LONG; > > + void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(long); > > + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); > > + unsigned long mask = BIT_MASK(id); > > It's better to group offset and mask, so it will give a hint to human > and compiler that > '/' + '%' can be optimized to one assembly instructions on some > architectures (in case of compile-test or new RISC-V types, I dunno if > they have it now). > > And use BIT_WORD() for offset calculation. Same for the above function. Makes sense. Thanks. > > + unsigned long done = jh7100_reset_asserted[offset] & mask; > > + unsigned long value; > > + unsigned long flags; > > + int ret; > > + > > + if (!assert) > > + done ^= mask; > > + > > + spin_lock_irqsave(&data->lock, flags); > > + > > + value = jh7100_reset_read(reg_assert); > > + if (assert) > > + value |= mask; > > + else > > + value &= ~mask; > > + jh7100_reset_write(value, reg_assert); > > + > > + /* if the associated clock is gated, deasserting might otherwise hang forever */ > > + ret = readx_poll_timeout_atomic(jh7100_reset_read, reg_status, value, > > + (value & mask) == done, 0, 1000); > > + > > + spin_unlock_irqrestore(&data->lock, flags); > > + return ret; > > +} > > + > > +static int jh7100_reset_assert(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + return jh7100_reset_update(rcdev, id, true); > > +} > > + > > +static int jh7100_reset_deassert(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + return jh7100_reset_update(rcdev, id, false); > > +} > > + > > +static int jh7100_reset_reset(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + int ret; > > + > > + ret = jh7100_reset_assert(rcdev, id); > > + if (ret) > > + return ret; > > + > > + return jh7100_reset_deassert(rcdev, id); > > +} > > + > > +static int jh7100_reset_status(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + struct jh7100_reset *data = jh7100_reset_from(rcdev); > > + unsigned long offset = id / BITS_PER_LONG; > > + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); > > + unsigned long value = jh7100_reset_read(reg_status); > > + unsigned long mask = BIT_MASK(id); > > + > > + return !((value ^ jh7100_reset_asserted[offset]) & mask); > > +} > > + > > +static const struct reset_control_ops jh7100_reset_ops = { > > + .assert = jh7100_reset_assert, > > + .deassert = jh7100_reset_deassert, > > + .reset = jh7100_reset_reset, > > + .status = jh7100_reset_status, > > +}; > > + > > +static int __init jh7100_reset_probe(struct platform_device *pdev) > > +{ > > + struct jh7100_reset *data; > > + > > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + data->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(data->base)) > > + return PTR_ERR(data->base); > > + > > + data->rcdev.ops = &jh7100_reset_ops; > > + data->rcdev.owner = THIS_MODULE; > > + data->rcdev.nr_resets = JH7100_RSTN_END; > > + data->rcdev.dev = &pdev->dev; > > + data->rcdev.of_node = pdev->dev.of_node; > > + spin_lock_init(&data->lock); > > + > > + return devm_reset_controller_register(&pdev->dev, &data->rcdev); > > +} > > + > > +static const struct of_device_id jh7100_reset_dt_ids[] = { > > + { .compatible = "starfive,jh7100-reset" }, > > + { /* sentinel */ } > > +}; > > + > > +static struct platform_driver jh7100_reset_driver = { > > + .driver = { > > + .name = "jh7100-reset", > > + .of_match_table = jh7100_reset_dt_ids, > > + .suppress_bind_attrs = true, > > + }, > > +}; > > +builtin_platform_driver_probe(jh7100_reset_driver, jh7100_reset_probe); > > -- > > 2.33.1 > > > > > -- > With Best Regards, > Andy Shevchenko
On Tue, Nov 16, 2021 at 6:06 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > On Tue, 16 Nov 2021 at 17:01, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Nov 16, 2021 at 5:06 PM Emil Renner Berthing <kernel@esmil.dk> wrote: ... > > > +#if BITS_PER_LONG == 64 > > > +#define jh7100_reset_read readq > > > +#define jh7100_reset_write writeq > > > +#else > > > +#define jh7100_reset_read readl > > > +#define jh7100_reset_write writel > > > +#endif > > > > No need, just use always readq() / writeq() and include io-64-nonatomic-lo-hi > > Hmm.. how would that work on a 32bit architecture where sizeof(long) > == 4? The whole point of this is to read chunks in sizes matching the > words in the bitmap later on. Why not? You just put some amendments, see below an example. ... > > > + void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(long); > > > + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); > > > + unsigned long value; void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + off64; void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + off64; u64 value; > > > + value = jh7100_reset_read(reg_assert); > > > + if (assert) > > > + value |= mask; > > > + else > > > + value &= ~mask; > > > + jh7100_reset_write(value, reg_assert); value = readq(...); ...update... writeq(value, ...); > > > + /* if the associated clock is gated, deasserting might otherwise hang forever */ > > > + ret = readx_poll_timeout_atomic(jh7100_reset_read, reg_status, value, > > > + (value & mask) == done, 0, 1000); readq_poll_timeout_atomic() Maybe at the end of the day simple use of u64 will be better than bitmap. Up to you, you have got the idea. Either way you may put my tag.
On Tue, Nov 16, 2021 at 6:21 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Nov 16, 2021 at 6:06 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > On Tue, 16 Nov 2021 at 17:01, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > On Tue, Nov 16, 2021 at 5:06 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > Maybe at the end of the day simple use of u64 will be better than bitmap. > Up to you, you have got the idea. Either way you may put my tag. At least I see pros of the current solution that it won't read more registers than needed. P.S. And do not add that guy to the Cc list, I think people are already annoyed by bounces.
diff --git a/MAINTAINERS b/MAINTAINERS index 2548ef1f8246..1152f5d94690 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -18137,6 +18137,13 @@ F: Documentation/devicetree/bindings/clock/starfive,jh7100-clkgen.yaml F: drivers/clk/starfive/clk-starfive-jh7100.c F: include/dt-bindings/clock/starfive-jh7100.h +STARFIVE JH7100 RESET CONTROLLER DRIVER +M: Emil Renner Berthing <kernel@esmil.dk> +S: Maintained +F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml +F: drivers/reset/reset-starfive-jh7100.c +F: include/dt-bindings/reset/starfive-jh7100.h + STATIC BRANCH/CALL M: Peter Zijlstra <peterz@infradead.org> M: Josh Poimboeuf <jpoimboe@redhat.com> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index 85024eb1d2ea..6f8ba0ddc05f 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -224,6 +224,13 @@ config RESET_SOCFPGA This enables the reset driver for the SoCFPGA ARMv7 platforms. This driver gets initialized early during platform init calls. +config RESET_STARFIVE_JH7100 + bool "StarFive JH7100 Reset Driver" + depends on SOC_STARFIVE || COMPILE_TEST + default SOC_STARFIVE + help + This enables the reset controller driver for the StarFive JH7100 SoC. + config RESET_SUNXI bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI default ARCH_SUNXI diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 21d46d8869ff..bd0a97be18b5 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o obj-$(CONFIG_RESET_SCMI) += reset-scmi.o obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o +obj-$(CONFIG_RESET_STARFIVE_JH7100) += reset-starfive-jh7100.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o diff --git a/drivers/reset/reset-starfive-jh7100.c b/drivers/reset/reset-starfive-jh7100.c new file mode 100644 index 000000000000..2f7615cbf1dc --- /dev/null +++ b/drivers/reset/reset-starfive-jh7100.c @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Reset driver for the StarFive JH7100 SoC + * + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> + */ + +#include <linux/bitmap.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/mod_devicetable.h> +#include <linux/platform_device.h> +#include <linux/reset-controller.h> +#include <linux/spinlock.h> + +#include <dt-bindings/reset/starfive-jh7100.h> + +/* register offsets */ +#define JH7100_RESET_ASSERT0 0x00 +#define JH7100_RESET_ASSERT1 0x04 +#define JH7100_RESET_ASSERT2 0x08 +#define JH7100_RESET_ASSERT3 0x0c +#define JH7100_RESET_STATUS0 0x10 +#define JH7100_RESET_STATUS1 0x14 +#define JH7100_RESET_STATUS2 0x18 +#define JH7100_RESET_STATUS3 0x1c + +#if BITS_PER_LONG == 64 +#define jh7100_reset_read readq +#define jh7100_reset_write writeq +#else +#define jh7100_reset_read readl +#define jh7100_reset_write writel +#endif + +/* + * Writing a 1 to the n'th bit of the m'th ASSERT register asserts + * line 32m + n, and writing a 0 deasserts the same line. + * Most reset lines have their status inverted so a 0 bit in the STATUS + * register means the line is asserted and a 1 means it's deasserted. A few + * lines don't though, so store the expected value of the status registers when + * all lines are asserted. + */ +static const DECLARE_BITMAP(jh7100_reset_asserted, 4 * 32) = { + BITMAP_FROM_U64(BIT_ULL_MASK(JH7100_RST_U74) | + BIT_ULL_MASK(JH7100_RST_VP6_DRESET) | + BIT_ULL_MASK(JH7100_RST_VP6_BRESET) | + BIT_ULL_MASK(JH7100_RST_HIFI4_DRESET) | + BIT_ULL_MASK(JH7100_RST_HIFI4_BRESET)), + BITMAP_FROM_U64(BIT_ULL_MASK(JH7100_RST_E24)), +}; + +struct jh7100_reset { + struct reset_controller_dev rcdev; + /* protect registers against concurrent read-modify-write */ + spinlock_t lock; + void __iomem *base; +}; + +static inline struct jh7100_reset * +jh7100_reset_from(struct reset_controller_dev *rcdev) +{ + return container_of(rcdev, struct jh7100_reset, rcdev); +} + +static int jh7100_reset_update(struct reset_controller_dev *rcdev, + unsigned long id, bool assert) +{ + struct jh7100_reset *data = jh7100_reset_from(rcdev); + unsigned long offset = id / BITS_PER_LONG; + void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(long); + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); + unsigned long mask = BIT_MASK(id); + unsigned long done = jh7100_reset_asserted[offset] & mask; + unsigned long value; + unsigned long flags; + int ret; + + if (!assert) + done ^= mask; + + spin_lock_irqsave(&data->lock, flags); + + value = jh7100_reset_read(reg_assert); + if (assert) + value |= mask; + else + value &= ~mask; + jh7100_reset_write(value, reg_assert); + + /* if the associated clock is gated, deasserting might otherwise hang forever */ + ret = readx_poll_timeout_atomic(jh7100_reset_read, reg_status, value, + (value & mask) == done, 0, 1000); + + spin_unlock_irqrestore(&data->lock, flags); + return ret; +} + +static int jh7100_reset_assert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + return jh7100_reset_update(rcdev, id, true); +} + +static int jh7100_reset_deassert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + return jh7100_reset_update(rcdev, id, false); +} + +static int jh7100_reset_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + int ret; + + ret = jh7100_reset_assert(rcdev, id); + if (ret) + return ret; + + return jh7100_reset_deassert(rcdev, id); +} + +static int jh7100_reset_status(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct jh7100_reset *data = jh7100_reset_from(rcdev); + unsigned long offset = id / BITS_PER_LONG; + void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(long); + unsigned long value = jh7100_reset_read(reg_status); + unsigned long mask = BIT_MASK(id); + + return !((value ^ jh7100_reset_asserted[offset]) & mask); +} + +static const struct reset_control_ops jh7100_reset_ops = { + .assert = jh7100_reset_assert, + .deassert = jh7100_reset_deassert, + .reset = jh7100_reset_reset, + .status = jh7100_reset_status, +}; + +static int __init jh7100_reset_probe(struct platform_device *pdev) +{ + struct jh7100_reset *data; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(data->base)) + return PTR_ERR(data->base); + + data->rcdev.ops = &jh7100_reset_ops; + data->rcdev.owner = THIS_MODULE; + data->rcdev.nr_resets = JH7100_RSTN_END; + data->rcdev.dev = &pdev->dev; + data->rcdev.of_node = pdev->dev.of_node; + spin_lock_init(&data->lock); + + return devm_reset_controller_register(&pdev->dev, &data->rcdev); +} + +static const struct of_device_id jh7100_reset_dt_ids[] = { + { .compatible = "starfive,jh7100-reset" }, + { /* sentinel */ } +}; + +static struct platform_driver jh7100_reset_driver = { + .driver = { + .name = "jh7100-reset", + .of_match_table = jh7100_reset_dt_ids, + .suppress_bind_attrs = true, + }, +}; +builtin_platform_driver_probe(jh7100_reset_driver, jh7100_reset_probe);
Add a driver for the StarFive JH7100 reset controller. Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> --- MAINTAINERS | 7 + drivers/reset/Kconfig | 7 + drivers/reset/Makefile | 1 + drivers/reset/reset-starfive-jh7100.c | 176 ++++++++++++++++++++++++++ 4 files changed, 191 insertions(+) create mode 100644 drivers/reset/reset-starfive-jh7100.c