diff mbox series

[v2,04/14] reset: starfive: Factor out common JH71X0 reset code

Message ID 20221118010627.70576-5-hal.feng@starfivetech.com (mailing list archive)
State Superseded
Headers show
Series Basic clock and reset support for StarFive JH7110 RISC-V SoC | expand

Checks

Context Check Description
conchuod/patch_count success Link
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/module_param success Was 0 now: 0
conchuod/build_rv32_defconfig success Build OK
conchuod/build_warn_rv64 success Errors and warnings before: 0 this patch: 0
conchuod/dtb_warn_rv64 success Errors and warnings before: 0 this patch: 0
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
conchuod/source_inline warning Was 1 now: 1
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Hal Feng Nov. 18, 2022, 1:06 a.m. UTC
From: Emil Renner Berthing <kernel@esmil.dk>

The StarFive JH7100 SoC has additional reset controllers for audio and
video, but the registers follow the same structure. On the JH7110 the
reset registers don't get their own memory range, but instead follow the
clock control registers. The registers still follow the same structure
though, so let's factor out the common code to handle all these cases.

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Co-developed-by: Hal Feng <hal.feng@starfivetech.com>
Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 drivers/reset/starfive/Kconfig                |   4 +
 drivers/reset/starfive/Makefile               |   2 +
 .../reset/starfive/reset-starfive-jh7100.c    | 121 ++----------------
 ...rfive-jh7100.c => reset-starfive-jh71x0.c} |  92 ++++---------
 .../reset/starfive/reset-starfive-jh71x0.h    |  14 ++
 5 files changed, 56 insertions(+), 177 deletions(-)
 copy drivers/reset/starfive/{reset-starfive-jh7100.c => reset-starfive-jh71x0.c} (50%)
 create mode 100644 drivers/reset/starfive/reset-starfive-jh71x0.h

Comments

Emil Renner Berthing Nov. 18, 2022, 4:39 p.m. UTC | #1
On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@starfivetech.com> wrote:
>
> From: Emil Renner Berthing <kernel@esmil.dk>
>
> The StarFive JH7100 SoC has additional reset controllers for audio and
> video, but the registers follow the same structure. On the JH7110 the
> reset registers don't get their own memory range, but instead follow the
> clock control registers. The registers still follow the same structure
> though, so let's factor out the common code to handle all these cases.
>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Co-developed-by: Hal Feng <hal.feng@starfivetech.com>
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  drivers/reset/starfive/Kconfig                |   4 +
>  drivers/reset/starfive/Makefile               |   2 +
>  .../reset/starfive/reset-starfive-jh7100.c    | 121 ++----------------
>  ...rfive-jh7100.c => reset-starfive-jh71x0.c} |  92 ++++---------
>  .../reset/starfive/reset-starfive-jh71x0.h    |  14 ++
>  5 files changed, 56 insertions(+), 177 deletions(-)
>  copy drivers/reset/starfive/{reset-starfive-jh7100.c => reset-starfive-jh71x0.c} (50%)
>  create mode 100644 drivers/reset/starfive/reset-starfive-jh71x0.h
>
> diff --git a/drivers/reset/starfive/Kconfig b/drivers/reset/starfive/Kconfig
> index cddebdba7177..9d15c4110e40 100644
> --- a/drivers/reset/starfive/Kconfig
> +++ b/drivers/reset/starfive/Kconfig
> @@ -1,8 +1,12 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>
> +config RESET_STARFIVE_JH71X0
> +       bool
> +
>  config RESET_STARFIVE_JH7100
>         bool "StarFive JH7100 Reset Driver"
>         depends on SOC_STARFIVE || COMPILE_TEST
> +       select RESET_STARFIVE_JH71X0
>         default SOC_STARFIVE
>         help
>           This enables the reset controller driver for the StarFive JH7100 SoC.
> diff --git a/drivers/reset/starfive/Makefile b/drivers/reset/starfive/Makefile
> index 670d049423f5..f6aa12466fad 100644
> --- a/drivers/reset/starfive/Makefile
> +++ b/drivers/reset/starfive/Makefile
> @@ -1,2 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_RESET_STARFIVE_JH71X0)            += reset-starfive-jh71x0.o
> +
>  obj-$(CONFIG_RESET_STARFIVE_JH7100)            += reset-starfive-jh7100.o
> diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh7100.c
> index fc44b2fb3e03..43248e8135fd 100644
> --- a/drivers/reset/starfive/reset-starfive-jh7100.c
> +++ b/drivers/reset/starfive/reset-starfive-jh7100.c
> @@ -5,14 +5,10 @@
>   * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
>   */
>
> -#include <linux/bitmap.h>
> -#include <linux/io.h>
> -#include <linux/io-64-nonatomic-lo-hi.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 "reset-starfive-jh71x0.h"
>
>  #include <dt-bindings/reset/starfive-jh7100.h>
>
> @@ -48,114 +44,19 @@ static const u64 jh7100_reset_asserted[2] = {
>         0,
>  };
>
> -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 = BIT_ULL_WORD(id);
> -       u64 mask = BIT_ULL_MASK(id);
> -       void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(u64);
> -       void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
> -       u64 done = jh7100_reset_asserted[offset] & mask;
> -       u64 value;
> -       unsigned long flags;
> -       int ret;
> -
> -       if (!assert)
> -               done ^= mask;
> -
> -       spin_lock_irqsave(&data->lock, flags);
> -
> -       value = readq(reg_assert);
> -       if (assert)
> -               value |= mask;
> -       else
> -               value &= ~mask;
> -       writeq(value, reg_assert);
> -
> -       /* if the associated clock is gated, deasserting might otherwise hang forever */
> -       ret = readq_poll_timeout_atomic(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 = BIT_ULL_WORD(id);
> -       u64 mask = BIT_ULL_MASK(id);
> -       void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
> -       u64 value = readq(reg_status);
> -
> -       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);
> +       void __iomem *base = devm_platform_ioremap_resource(pdev, 0);
>
> -       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);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
>
> -       return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> +       return reset_starfive_jh7100_register(&pdev->dev, pdev->dev.of_node,
> +                                             base + JH7100_RESET_ASSERT0,
> +                                             base + JH7100_RESET_STATUS0,
> +                                             jh7100_reset_asserted,
> +                                             JH7100_RSTN_END,
> +                                             true);
>  }
>
>  static const struct of_device_id jh7100_reset_dt_ids[] = {
> diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh71x0.c
> similarity index 50%
> copy from drivers/reset/starfive/reset-starfive-jh7100.c
> copy to drivers/reset/starfive/reset-starfive-jh71x0.c
> index fc44b2fb3e03..1e230f3f9841 100644
> --- a/drivers/reset/starfive/reset-starfive-jh7100.c
> +++ b/drivers/reset/starfive/reset-starfive-jh71x0.c
> @@ -6,53 +6,20 @@
>   */
>
>  #include <linux/bitmap.h>
> +#include <linux/device.h>
>  #include <linux/io.h>
>  #include <linux/io-64-nonatomic-lo-hi.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
> -
> -/*
> - * 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 u64 jh7100_reset_asserted[2] = {
> -       /* STATUS0 */
> -       BIT_ULL_MASK(JH7100_RST_U74) |
> -       BIT_ULL_MASK(JH7100_RST_VP6_DRESET) |
> -       BIT_ULL_MASK(JH7100_RST_VP6_BRESET) |
> -       /* STATUS1 */
> -       BIT_ULL_MASK(JH7100_RST_HIFI4_DRESET) |
> -       BIT_ULL_MASK(JH7100_RST_HIFI4_BRESET),
> -       /* STATUS2 */
> -       BIT_ULL_MASK(JH7100_RST_E24) |
> -       /* STATUS3 */
> -       0,
> -};
> -
>  struct jh7100_reset {
>         struct reset_controller_dev rcdev;
>         /* protect registers against concurrent read-modify-write */
>         spinlock_t lock;
> -       void __iomem *base;
> +       void __iomem *assert;
> +       void __iomem *status;
> +       const u64 *asserted;
>  };
>
>  static inline struct jh7100_reset *
> @@ -67,9 +34,9 @@ static int jh7100_reset_update(struct reset_controller_dev *rcdev,
>         struct jh7100_reset *data = jh7100_reset_from(rcdev);
>         unsigned long offset = BIT_ULL_WORD(id);
>         u64 mask = BIT_ULL_MASK(id);
> -       void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(u64);
> -       void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
> -       u64 done = jh7100_reset_asserted[offset] & mask;
> +       void __iomem *reg_assert = data->assert + offset * sizeof(u64);
> +       void __iomem *reg_status = data->status + offset * sizeof(u64);
> +       u64 done = data->asserted ? data->asserted[offset] & mask : 0;
>         u64 value;
>         unsigned long flags;
>         int ret;
> @@ -123,10 +90,10 @@ static int jh7100_reset_status(struct reset_controller_dev *rcdev,
>         struct jh7100_reset *data = jh7100_reset_from(rcdev);
>         unsigned long offset = BIT_ULL_WORD(id);
>         u64 mask = BIT_ULL_MASK(id);
> -       void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
> +       void __iomem *reg_status = data->status + offset * sizeof(u64);
>         u64 value = readq(reg_status);
>
> -       return !((value ^ jh7100_reset_asserted[offset]) & mask);
> +       return !((value ^ data->asserted[offset]) & mask);
>  }
>
>  static const struct reset_control_ops jh7100_reset_ops = {
> @@ -136,38 +103,29 @@ static const struct reset_control_ops jh7100_reset_ops = {
>         .status         = jh7100_reset_status,
>  };
>
> -static int __init jh7100_reset_probe(struct platform_device *pdev)
> +int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
> +                                  void __iomem *assert, void __iomem *status,
> +                                  const u64 *asserted, unsigned int nr_resets,
> +                                  bool is_module)
>  {
>         struct jh7100_reset *data;
>
> -       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +       data = devm_kzalloc(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;
> +       if (is_module)
> +               data->rcdev.owner = THIS_MODULE;

nit: consider just passing the owner directly, so this would just be
data->rcdev.owner = owner;

..and callers that used false can just pass NULL.

> +       data->rcdev.nr_resets = nr_resets;
> +       data->rcdev.dev = dev;
> +       data->rcdev.of_node = of_node;

Is it important to register this with the auxiliary device and not
just use the parent device?
If not you can just always pass the device that has the right of_node
and have this be

data->rcdev.of_node = dev->of_node;

> +
>         spin_lock_init(&data->lock);
> +       data->assert = assert;
> +       data->status = status;
> +       data->asserted = asserted;
>
> -       return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> +       return devm_reset_controller_register(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);
> +EXPORT_SYMBOL_GPL(reset_starfive_jh7100_register);
> diff --git a/drivers/reset/starfive/reset-starfive-jh71x0.h b/drivers/reset/starfive/reset-starfive-jh71x0.h
> new file mode 100644
> index 000000000000..10770c55ab0e
> --- /dev/null
> +++ b/drivers/reset/starfive/reset-starfive-jh71x0.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
> + */
> +
> +#ifndef __RESET_STARFIVE_JH71X0_H
> +#define __RESET_STARFIVE_JH71X0_H
> +
> +int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
> +                                  void __iomem *assert, void __iomem *status,
> +                                  const u64 *asserted, unsigned int nr_resets,
> +                                  bool is_module);
> +
> +#endif /* __RESET_STARFIVE_JH71X0_H */
> --
> 2.38.1
>
Hal Feng Nov. 21, 2022, 9:23 a.m. UTC | #2
On Sat, 19 Nov 2022 00:39:35 +0800, Emil Renner Berthing wrote:
> On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@starfivetech.com> wrote:
> > diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh71x0.c
> > similarity index 50%
> > copy from drivers/reset/starfive/reset-starfive-jh7100.c
> > copy to drivers/reset/starfive/reset-starfive-jh71x0.c
> > index fc44b2fb3e03..1e230f3f9841 100644
> > --- a/drivers/reset/starfive/reset-starfive-jh7100.c
> > +++ b/drivers/reset/starfive/reset-starfive-jh71x0.c

[...]

> > -static int __init jh7100_reset_probe(struct platform_device *pdev)
> > +int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
> > +                                  void __iomem *assert, void __iomem *status,
> > +                                  const u64 *asserted, unsigned int nr_resets,
> > +                                  bool is_module)
> >  {
> >         struct jh7100_reset *data;
> >
> > -       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > +       data = devm_kzalloc(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;
> > +       if (is_module)
> > +               data->rcdev.owner = THIS_MODULE;
> 
> nit: consider just passing the owner directly, so this would just be
> data->rcdev.owner = owner;
> 
> ..and callers that used false can just pass NULL.

Yeah, will fix it.

> 
> > +       data->rcdev.nr_resets = nr_resets;
> > +       data->rcdev.dev = dev;
> > +       data->rcdev.of_node = of_node;
> 
> Is it important to register this with the auxiliary device and not
> just use the parent device?

I'm not sure whether it still works if we use the same device, but
it's general to separate the devices of clock and reset. They have
different device names and different drivers.

Best regards,
Hal

> If not you can just always pass the device that has the right of_node
> and have this be
> 
> data->rcdev.of_node = dev->of_node;
> 
> > +
> >         spin_lock_init(&data->lock);
> > +       data->assert = assert;
> > +       data->status = status;
> > +       data->asserted = asserted;
> >
> > -       return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> > +       return devm_reset_controller_register(dev, &data->rcdev);
> >  }
Emil Renner Berthing Nov. 21, 2022, 11:26 a.m. UTC | #3
On Mon, 21 Nov 2022 at 10:24, Hal Feng <hal.feng@starfivetech.com> wrote:
>
> On Sat, 19 Nov 2022 00:39:35 +0800, Emil Renner Berthing wrote:
> > On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@starfivetech.com> wrote:
> > > diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh71x0.c
> > > similarity index 50%
> > > copy from drivers/reset/starfive/reset-starfive-jh7100.c
> > > copy to drivers/reset/starfive/reset-starfive-jh71x0.c
> > > index fc44b2fb3e03..1e230f3f9841 100644
> > > --- a/drivers/reset/starfive/reset-starfive-jh7100.c
> > > +++ b/drivers/reset/starfive/reset-starfive-jh71x0.c
>
> [...]
>
> > > -static int __init jh7100_reset_probe(struct platform_device *pdev)
> > > +int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
> > > +                                  void __iomem *assert, void __iomem *status,
> > > +                                  const u64 *asserted, unsigned int nr_resets,
> > > +                                  bool is_module)
> > >  {
> > >         struct jh7100_reset *data;
> > >
> > > -       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > > +       data = devm_kzalloc(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;
> > > +       if (is_module)
> > > +               data->rcdev.owner = THIS_MODULE;
> >
> > nit: consider just passing the owner directly, so this would just be
> > data->rcdev.owner = owner;
> >
> > ..and callers that used false can just pass NULL.
>
> Yeah, will fix it.
>
> >
> > > +       data->rcdev.nr_resets = nr_resets;
> > > +       data->rcdev.dev = dev;
> > > +       data->rcdev.of_node = of_node;
> >
> > Is it important to register this with the auxiliary device and not
> > just use the parent device?
>
> I'm not sure whether it still works if we use the same device,

Try it.

> but
> it's general to separate the devices of clock and reset. They have
> different device names and different drivers.
>
> Best regards,
> Hal
>
> > If not you can just always pass the device that has the right of_node
> > and have this be
> >
> > data->rcdev.of_node = dev->of_node;
> >
> > > +
> > >         spin_lock_init(&data->lock);
> > > +       data->assert = assert;
> > > +       data->status = status;
> > > +       data->asserted = asserted;
> > >
> > > -       return devm_reset_controller_register(&pdev->dev, &data->rcdev);
> > > +       return devm_reset_controller_register(dev, &data->rcdev);
> > >  }
>
diff mbox series

Patch

diff --git a/drivers/reset/starfive/Kconfig b/drivers/reset/starfive/Kconfig
index cddebdba7177..9d15c4110e40 100644
--- a/drivers/reset/starfive/Kconfig
+++ b/drivers/reset/starfive/Kconfig
@@ -1,8 +1,12 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 
+config RESET_STARFIVE_JH71X0
+	bool
+
 config RESET_STARFIVE_JH7100
 	bool "StarFive JH7100 Reset Driver"
 	depends on SOC_STARFIVE || COMPILE_TEST
+	select RESET_STARFIVE_JH71X0
 	default SOC_STARFIVE
 	help
 	  This enables the reset controller driver for the StarFive JH7100 SoC.
diff --git a/drivers/reset/starfive/Makefile b/drivers/reset/starfive/Makefile
index 670d049423f5..f6aa12466fad 100644
--- a/drivers/reset/starfive/Makefile
+++ b/drivers/reset/starfive/Makefile
@@ -1,2 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_RESET_STARFIVE_JH71X0)		+= reset-starfive-jh71x0.o
+
 obj-$(CONFIG_RESET_STARFIVE_JH7100)		+= reset-starfive-jh7100.o
diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh7100.c
index fc44b2fb3e03..43248e8135fd 100644
--- a/drivers/reset/starfive/reset-starfive-jh7100.c
+++ b/drivers/reset/starfive/reset-starfive-jh7100.c
@@ -5,14 +5,10 @@ 
  * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
  */
 
-#include <linux/bitmap.h>
-#include <linux/io.h>
-#include <linux/io-64-nonatomic-lo-hi.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 "reset-starfive-jh71x0.h"
 
 #include <dt-bindings/reset/starfive-jh7100.h>
 
@@ -48,114 +44,19 @@  static const u64 jh7100_reset_asserted[2] = {
 	0,
 };
 
-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 = BIT_ULL_WORD(id);
-	u64 mask = BIT_ULL_MASK(id);
-	void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(u64);
-	void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
-	u64 done = jh7100_reset_asserted[offset] & mask;
-	u64 value;
-	unsigned long flags;
-	int ret;
-
-	if (!assert)
-		done ^= mask;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	value = readq(reg_assert);
-	if (assert)
-		value |= mask;
-	else
-		value &= ~mask;
-	writeq(value, reg_assert);
-
-	/* if the associated clock is gated, deasserting might otherwise hang forever */
-	ret = readq_poll_timeout_atomic(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 = BIT_ULL_WORD(id);
-	u64 mask = BIT_ULL_MASK(id);
-	void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
-	u64 value = readq(reg_status);
-
-	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);
+	void __iomem *base = devm_platform_ioremap_resource(pdev, 0);
 
-	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);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
 
-	return devm_reset_controller_register(&pdev->dev, &data->rcdev);
+	return reset_starfive_jh7100_register(&pdev->dev, pdev->dev.of_node,
+					      base + JH7100_RESET_ASSERT0,
+					      base + JH7100_RESET_STATUS0,
+					      jh7100_reset_asserted,
+					      JH7100_RSTN_END,
+					      true);
 }
 
 static const struct of_device_id jh7100_reset_dt_ids[] = {
diff --git a/drivers/reset/starfive/reset-starfive-jh7100.c b/drivers/reset/starfive/reset-starfive-jh71x0.c
similarity index 50%
copy from drivers/reset/starfive/reset-starfive-jh7100.c
copy to drivers/reset/starfive/reset-starfive-jh71x0.c
index fc44b2fb3e03..1e230f3f9841 100644
--- a/drivers/reset/starfive/reset-starfive-jh7100.c
+++ b/drivers/reset/starfive/reset-starfive-jh71x0.c
@@ -6,53 +6,20 @@ 
  */
 
 #include <linux/bitmap.h>
+#include <linux/device.h>
 #include <linux/io.h>
 #include <linux/io-64-nonatomic-lo-hi.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
-
-/*
- * 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 u64 jh7100_reset_asserted[2] = {
-	/* STATUS0 */
-	BIT_ULL_MASK(JH7100_RST_U74) |
-	BIT_ULL_MASK(JH7100_RST_VP6_DRESET) |
-	BIT_ULL_MASK(JH7100_RST_VP6_BRESET) |
-	/* STATUS1 */
-	BIT_ULL_MASK(JH7100_RST_HIFI4_DRESET) |
-	BIT_ULL_MASK(JH7100_RST_HIFI4_BRESET),
-	/* STATUS2 */
-	BIT_ULL_MASK(JH7100_RST_E24) |
-	/* STATUS3 */
-	0,
-};
-
 struct jh7100_reset {
 	struct reset_controller_dev rcdev;
 	/* protect registers against concurrent read-modify-write */
 	spinlock_t lock;
-	void __iomem *base;
+	void __iomem *assert;
+	void __iomem *status;
+	const u64 *asserted;
 };
 
 static inline struct jh7100_reset *
@@ -67,9 +34,9 @@  static int jh7100_reset_update(struct reset_controller_dev *rcdev,
 	struct jh7100_reset *data = jh7100_reset_from(rcdev);
 	unsigned long offset = BIT_ULL_WORD(id);
 	u64 mask = BIT_ULL_MASK(id);
-	void __iomem *reg_assert = data->base + JH7100_RESET_ASSERT0 + offset * sizeof(u64);
-	void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
-	u64 done = jh7100_reset_asserted[offset] & mask;
+	void __iomem *reg_assert = data->assert + offset * sizeof(u64);
+	void __iomem *reg_status = data->status + offset * sizeof(u64);
+	u64 done = data->asserted ? data->asserted[offset] & mask : 0;
 	u64 value;
 	unsigned long flags;
 	int ret;
@@ -123,10 +90,10 @@  static int jh7100_reset_status(struct reset_controller_dev *rcdev,
 	struct jh7100_reset *data = jh7100_reset_from(rcdev);
 	unsigned long offset = BIT_ULL_WORD(id);
 	u64 mask = BIT_ULL_MASK(id);
-	void __iomem *reg_status = data->base + JH7100_RESET_STATUS0 + offset * sizeof(u64);
+	void __iomem *reg_status = data->status + offset * sizeof(u64);
 	u64 value = readq(reg_status);
 
-	return !((value ^ jh7100_reset_asserted[offset]) & mask);
+	return !((value ^ data->asserted[offset]) & mask);
 }
 
 static const struct reset_control_ops jh7100_reset_ops = {
@@ -136,38 +103,29 @@  static const struct reset_control_ops jh7100_reset_ops = {
 	.status		= jh7100_reset_status,
 };
 
-static int __init jh7100_reset_probe(struct platform_device *pdev)
+int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
+				   void __iomem *assert, void __iomem *status,
+				   const u64 *asserted, unsigned int nr_resets,
+				   bool is_module)
 {
 	struct jh7100_reset *data;
 
-	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(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;
+	if (is_module)
+		data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = nr_resets;
+	data->rcdev.dev = dev;
+	data->rcdev.of_node = of_node;
+
 	spin_lock_init(&data->lock);
+	data->assert = assert;
+	data->status = status;
+	data->asserted = asserted;
 
-	return devm_reset_controller_register(&pdev->dev, &data->rcdev);
+	return devm_reset_controller_register(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);
+EXPORT_SYMBOL_GPL(reset_starfive_jh7100_register);
diff --git a/drivers/reset/starfive/reset-starfive-jh71x0.h b/drivers/reset/starfive/reset-starfive-jh71x0.h
new file mode 100644
index 000000000000..10770c55ab0e
--- /dev/null
+++ b/drivers/reset/starfive/reset-starfive-jh71x0.h
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
+ */
+
+#ifndef __RESET_STARFIVE_JH71X0_H
+#define __RESET_STARFIVE_JH71X0_H
+
+int reset_starfive_jh7100_register(struct device *dev, struct device_node *of_node,
+				   void __iomem *assert, void __iomem *status,
+				   const u64 *asserted, unsigned int nr_resets,
+				   bool is_module);
+
+#endif /* __RESET_STARFIVE_JH71X0_H */