diff mbox series

[v4,09/16] reset: starfive-jh7100: Add StarFive JH7100 reset driver

Message ID 20211116150119.2171-10-kernel@esmil.dk (mailing list archive)
State New, archived
Headers show
Series Basic StarFive JH7100 RISC-V SoC support | expand

Commit Message

Emil Renner Berthing Nov. 16, 2021, 3:01 p.m. UTC
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

Comments

Andy Shevchenko Nov. 16, 2021, 4:01 p.m. UTC | #1
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
>
Emil Renner Berthing Nov. 16, 2021, 4:06 p.m. UTC | #2
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
Andy Shevchenko Nov. 16, 2021, 4:21 p.m. UTC | #3
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.
Andy Shevchenko Nov. 16, 2021, 4:24 p.m. UTC | #4
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 mbox series

Patch

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);