diff mbox series

[RESEND,v27,2/3] reset: npcm: register npcm8xx clock auxiliary bus device

Message ID 20240815150255.3996258-3-tmaimon77@gmail.com (mailing list archive)
State Under Review
Headers show
Series Introduce Nuvoton Arbel NPCM8XX BMC SoC | expand

Commit Message

Tomer Maimon Aug. 15, 2024, 3:02 p.m. UTC 
Add NPCM8xx clock controller auxiliary bus device registration.

The NPCM8xx clock controller is registered as an aux device because the
reset and the clock controller share the same register region.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Tested-by: Benjamin Fair <benjaminfair@google.com>
---
 drivers/reset/Kconfig               |  1 +
 drivers/reset/reset-npcm.c          | 74 ++++++++++++++++++++++++++++-
 include/soc/nuvoton/clock-npcm8xx.h | 16 +++++++
 3 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100755 include/soc/nuvoton/clock-npcm8xx.h

Comments

Philipp Zabel Sept. 12, 2024, 3:18 p.m. UTC | #1 
On Do, 2024-08-15 at 18:02 +0300, Tomer Maimon wrote:
> Add NPCM8xx clock controller auxiliary bus device registration.
> 
> The NPCM8xx clock controller is registered as an aux device because the
> reset and the clock controller share the same register region.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> Tested-by: Benjamin Fair <benjaminfair@google.com>
> ---
>  drivers/reset/Kconfig               |  1 +
>  drivers/reset/reset-npcm.c          | 74 ++++++++++++++++++++++++++++-
>  include/soc/nuvoton/clock-npcm8xx.h | 16 +++++++
>  3 files changed, 90 insertions(+), 1 deletion(-)
>  create mode 100755 include/soc/nuvoton/clock-npcm8xx.h
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 67bce340a87e..c6bf5275cca2 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -157,6 +157,7 @@ config RESET_MESON_AUDIO_ARB
>  config RESET_NPCM
>  	bool "NPCM BMC Reset Driver" if COMPILE_TEST
>  	default ARCH_NPCM
> +	select AUXILIARY_BUS
>  	help
>  	  This enables the reset controller driver for Nuvoton NPCM
>  	  BMC SoCs.
> diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
> index 8935ef95a2d1..aa68b947226a 100644
> --- a/drivers/reset/reset-npcm.c
> +++ b/drivers/reset/reset-npcm.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2019 Nuvoton Technology corporation.
>  
> +#include <linux/auxiliary_bus.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/io.h>
> @@ -10,11 +11,14 @@
>  #include <linux/property.h>
>  #include <linux/reboot.h>
>  #include <linux/reset-controller.h>
> +#include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
>  #include <linux/of_address.h>
>  
> +#include <soc/nuvoton/clock-npcm8xx.h>
> +
>  /* NPCM7xx GCR registers */
>  #define NPCM_MDLR_OFFSET	0x7C
>  #define NPCM7XX_MDLR_USBD0	BIT(9)
> @@ -89,6 +93,7 @@ struct npcm_rc_data {
>  	const struct npcm_reset_info *info;
>  	struct regmap *gcr_regmap;
>  	u32 sw_reset_number;
> +	struct device *dev;
>  	void __iomem *base;
>  	spinlock_t lock;
>  };
> @@ -372,6 +377,67 @@ static const struct reset_control_ops npcm_rc_ops = {
>  	.status		= npcm_rc_status,
>  };
>  
> +static void npcm_clock_unregister_adev(void *_adev)
> +{
> +	struct auxiliary_device *adev = _adev;
> +
> +	auxiliary_device_delete(adev);
> +	auxiliary_device_uninit(adev);
> +}
> +
> +static void npcm_clock_adev_release(struct device *dev)
> +{
> +	struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +	struct npcm_clock_adev *rdev = to_npcm_clock_adev(adev);
> +
> +	kfree(rdev);
> +}
> +
> +static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
> +{
> +	struct npcm_clock_adev *rdev;
> +	struct auxiliary_device *adev;
> +	int ret;
> +
> +	rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
> +	if (!rdev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rdev->base = rst_data->base;
> +
> +	adev = &rdev->adev;
> +	adev->name = clk_name;
> +	adev->dev.parent = rst_data->dev;
> +	adev->dev.release = npcm_clock_adev_release;
> +	adev->id = 555u;
> +
> +	ret = auxiliary_device_init(adev);
> +	if (ret) {
> +		kfree(rdev);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return adev;
> +}
> +
> +static int npcm8xx_clock_controller_register(struct npcm_rc_data *rst_data, char *clk_name)
> +{
> +	struct auxiliary_device *adev;
> +	int ret;
> +
> +	adev = npcm_clock_adev_alloc(rst_data, clk_name);
> +	if (IS_ERR(adev))
> +		return PTR_ERR(adev);
> +
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(rst_data->dev, npcm_clock_unregister_adev, adev);
> +}
> +
>  static int npcm_rc_probe(struct platform_device *pdev)
>  {
>  	struct npcm_rc_data *rc;
> @@ -392,6 +458,7 @@ static int npcm_rc_probe(struct platform_device *pdev)
>  	rc->rcdev.of_node = pdev->dev.of_node;
>  	rc->rcdev.of_reset_n_cells = 2;
>  	rc->rcdev.of_xlate = npcm_reset_xlate;
> +	rc->dev = &pdev->dev;
>  
>  	ret = devm_reset_controller_register(&pdev->dev, &rc->rcdev);
>  	if (ret) {
> @@ -413,7 +480,12 @@ static int npcm_rc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	return ret;
> +	switch (rc->info->bmc_id) {
> +	case BMC_NPCM8XX:

Here ret is ignored, which may be the return value from
register_restart_handler() above.

> +		return npcm8xx_clock_controller_register(rc, "clk-npcm8xx");
> +	default:
> +		return ret;
> +	}
>  }
>  
>  static struct platform_driver npcm_rc_driver = {
> diff --git a/include/soc/nuvoton/clock-npcm8xx.h b/include/soc/nuvoton/clock-npcm8xx.h
> new file mode 100755
> index 000000000000..139130e98c51
> --- /dev/null
> +++ b/include/soc/nuvoton/clock-npcm8xx.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __SOC_NPCM8XX_CLOCK_H
> +#define __SOC_NPCM8XX_CLOCK_H
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/container_of.h>
> +
> +struct npcm_clock_adev {
> +	void __iomem *base;
> +	struct auxiliary_device adev;
> +};
> +
> +#define to_npcm_clock_adev(_adev) \
> +	container_of((_adev), struct npcm_clock_adev, adev)

Could you make this an inline function instead?

With those two issues addressed,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Tomer Maimon Sept. 12, 2024, 7:12 p.m. UTC | #2 
Hi Philipp,

Thanks for your review.

Done in Version 28 :-)

Best regards,

Tomer


On Thu, 12 Sept 2024 at 18:18, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Do, 2024-08-15 at 18:02 +0300, Tomer Maimon wrote:
> > Add NPCM8xx clock controller auxiliary bus device registration.
> >
> > The NPCM8xx clock controller is registered as an aux device because the
> > reset and the clock controller share the same register region.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > Tested-by: Benjamin Fair <benjaminfair@google.com>
> > ---
> >  drivers/reset/Kconfig               |  1 +
> >  drivers/reset/reset-npcm.c          | 74 ++++++++++++++++++++++++++++-
> >  include/soc/nuvoton/clock-npcm8xx.h | 16 +++++++
> >  3 files changed, 90 insertions(+), 1 deletion(-)
> >  create mode 100755 include/soc/nuvoton/clock-npcm8xx.h
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index 67bce340a87e..c6bf5275cca2 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -157,6 +157,7 @@ config RESET_MESON_AUDIO_ARB
> >  config RESET_NPCM
> >       bool "NPCM BMC Reset Driver" if COMPILE_TEST
> >       default ARCH_NPCM
> > +     select AUXILIARY_BUS
> >       help
> >         This enables the reset controller driver for Nuvoton NPCM
> >         BMC SoCs.
> > diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
> > index 8935ef95a2d1..aa68b947226a 100644
> > --- a/drivers/reset/reset-npcm.c
> > +++ b/drivers/reset/reset-npcm.c
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  // Copyright (c) 2019 Nuvoton Technology corporation.
> >
> > +#include <linux/auxiliary_bus.h>
> >  #include <linux/delay.h>
> >  #include <linux/err.h>
> >  #include <linux/io.h>
> > @@ -10,11 +11,14 @@
> >  #include <linux/property.h>
> >  #include <linux/reboot.h>
> >  #include <linux/reset-controller.h>
> > +#include <linux/slab.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/regmap.h>
> >  #include <linux/of_address.h>
> >
> > +#include <soc/nuvoton/clock-npcm8xx.h>
> > +
> >  /* NPCM7xx GCR registers */
> >  #define NPCM_MDLR_OFFSET     0x7C
> >  #define NPCM7XX_MDLR_USBD0   BIT(9)
> > @@ -89,6 +93,7 @@ struct npcm_rc_data {
> >       const struct npcm_reset_info *info;
> >       struct regmap *gcr_regmap;
> >       u32 sw_reset_number;
> > +     struct device *dev;
> >       void __iomem *base;
> >       spinlock_t lock;
> >  };
> > @@ -372,6 +377,67 @@ static const struct reset_control_ops npcm_rc_ops = {
> >       .status         = npcm_rc_status,
> >  };
> >
> > +static void npcm_clock_unregister_adev(void *_adev)
> > +{
> > +     struct auxiliary_device *adev = _adev;
> > +
> > +     auxiliary_device_delete(adev);
> > +     auxiliary_device_uninit(adev);
> > +}
> > +
> > +static void npcm_clock_adev_release(struct device *dev)
> > +{
> > +     struct auxiliary_device *adev = to_auxiliary_dev(dev);
> > +     struct npcm_clock_adev *rdev = to_npcm_clock_adev(adev);
> > +
> > +     kfree(rdev);
> > +}
> > +
> > +static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
> > +{
> > +     struct npcm_clock_adev *rdev;
> > +     struct auxiliary_device *adev;
> > +     int ret;
> > +
> > +     rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
> > +     if (!rdev)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     rdev->base = rst_data->base;
> > +
> > +     adev = &rdev->adev;
> > +     adev->name = clk_name;
> > +     adev->dev.parent = rst_data->dev;
> > +     adev->dev.release = npcm_clock_adev_release;
> > +     adev->id = 555u;
> > +
> > +     ret = auxiliary_device_init(adev);
> > +     if (ret) {
> > +             kfree(rdev);
> > +             return ERR_PTR(ret);
> > +     }
> > +
> > +     return adev;
> > +}
> > +
> > +static int npcm8xx_clock_controller_register(struct npcm_rc_data *rst_data, char *clk_name)
> > +{
> > +     struct auxiliary_device *adev;
> > +     int ret;
> > +
> > +     adev = npcm_clock_adev_alloc(rst_data, clk_name);
> > +     if (IS_ERR(adev))
> > +             return PTR_ERR(adev);
> > +
> > +     ret = auxiliary_device_add(adev);
> > +     if (ret) {
> > +             auxiliary_device_uninit(adev);
> > +             return ret;
> > +     }
> > +
> > +     return devm_add_action_or_reset(rst_data->dev, npcm_clock_unregister_adev, adev);
> > +}
> > +
> >  static int npcm_rc_probe(struct platform_device *pdev)
> >  {
> >       struct npcm_rc_data *rc;
> > @@ -392,6 +458,7 @@ static int npcm_rc_probe(struct platform_device *pdev)
> >       rc->rcdev.of_node = pdev->dev.of_node;
> >       rc->rcdev.of_reset_n_cells = 2;
> >       rc->rcdev.of_xlate = npcm_reset_xlate;
> > +     rc->dev = &pdev->dev;
> >
> >       ret = devm_reset_controller_register(&pdev->dev, &rc->rcdev);
> >       if (ret) {
> > @@ -413,7 +480,12 @@ static int npcm_rc_probe(struct platform_device *pdev)
> >               }
> >       }
> >
> > -     return ret;
> > +     switch (rc->info->bmc_id) {
> > +     case BMC_NPCM8XX:
>
> Here ret is ignored, which may be the return value from
> register_restart_handler() above.
>
> > +             return npcm8xx_clock_controller_register(rc, "clk-npcm8xx");
> > +     default:
> > +             return ret;
> > +     }
> >  }
> >
> >  static struct platform_driver npcm_rc_driver = {
> > diff --git a/include/soc/nuvoton/clock-npcm8xx.h b/include/soc/nuvoton/clock-npcm8xx.h
> > new file mode 100755
> > index 000000000000..139130e98c51
> > --- /dev/null
> > +++ b/include/soc/nuvoton/clock-npcm8xx.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __SOC_NPCM8XX_CLOCK_H
> > +#define __SOC_NPCM8XX_CLOCK_H
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/container_of.h>
> > +
> > +struct npcm_clock_adev {
> > +     void __iomem *base;
> > +     struct auxiliary_device adev;
> > +};
> > +
> > +#define to_npcm_clock_adev(_adev) \
> > +     container_of((_adev), struct npcm_clock_adev, adev)
>
> Could you make this an inline function instead?
>
> With those two issues addressed,
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> regards
> Philipp
diff mbox series

Patch

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 67bce340a87e..c6bf5275cca2 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -157,6 +157,7 @@  config RESET_MESON_AUDIO_ARB
 config RESET_NPCM
 	bool "NPCM BMC Reset Driver" if COMPILE_TEST
 	default ARCH_NPCM
+	select AUXILIARY_BUS
 	help
 	  This enables the reset controller driver for Nuvoton NPCM
 	  BMC SoCs.
diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
index 8935ef95a2d1..aa68b947226a 100644
--- a/drivers/reset/reset-npcm.c
+++ b/drivers/reset/reset-npcm.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2019 Nuvoton Technology corporation.
 
+#include <linux/auxiliary_bus.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
@@ -10,11 +11,14 @@ 
 #include <linux/property.h>
 #include <linux/reboot.h>
 #include <linux/reset-controller.h>
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 #include <linux/of_address.h>
 
+#include <soc/nuvoton/clock-npcm8xx.h>
+
 /* NPCM7xx GCR registers */
 #define NPCM_MDLR_OFFSET	0x7C
 #define NPCM7XX_MDLR_USBD0	BIT(9)
@@ -89,6 +93,7 @@  struct npcm_rc_data {
 	const struct npcm_reset_info *info;
 	struct regmap *gcr_regmap;
 	u32 sw_reset_number;
+	struct device *dev;
 	void __iomem *base;
 	spinlock_t lock;
 };
@@ -372,6 +377,67 @@  static const struct reset_control_ops npcm_rc_ops = {
 	.status		= npcm_rc_status,
 };
 
+static void npcm_clock_unregister_adev(void *_adev)
+{
+	struct auxiliary_device *adev = _adev;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+}
+
+static void npcm_clock_adev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+	struct npcm_clock_adev *rdev = to_npcm_clock_adev(adev);
+
+	kfree(rdev);
+}
+
+static struct auxiliary_device *npcm_clock_adev_alloc(struct npcm_rc_data *rst_data, char *clk_name)
+{
+	struct npcm_clock_adev *rdev;
+	struct auxiliary_device *adev;
+	int ret;
+
+	rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
+	if (!rdev)
+		return ERR_PTR(-ENOMEM);
+
+	rdev->base = rst_data->base;
+
+	adev = &rdev->adev;
+	adev->name = clk_name;
+	adev->dev.parent = rst_data->dev;
+	adev->dev.release = npcm_clock_adev_release;
+	adev->id = 555u;
+
+	ret = auxiliary_device_init(adev);
+	if (ret) {
+		kfree(rdev);
+		return ERR_PTR(ret);
+	}
+
+	return adev;
+}
+
+static int npcm8xx_clock_controller_register(struct npcm_rc_data *rst_data, char *clk_name)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = npcm_clock_adev_alloc(rst_data, clk_name);
+	if (IS_ERR(adev))
+		return PTR_ERR(adev);
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(rst_data->dev, npcm_clock_unregister_adev, adev);
+}
+
 static int npcm_rc_probe(struct platform_device *pdev)
 {
 	struct npcm_rc_data *rc;
@@ -392,6 +458,7 @@  static int npcm_rc_probe(struct platform_device *pdev)
 	rc->rcdev.of_node = pdev->dev.of_node;
 	rc->rcdev.of_reset_n_cells = 2;
 	rc->rcdev.of_xlate = npcm_reset_xlate;
+	rc->dev = &pdev->dev;
 
 	ret = devm_reset_controller_register(&pdev->dev, &rc->rcdev);
 	if (ret) {
@@ -413,7 +480,12 @@  static int npcm_rc_probe(struct platform_device *pdev)
 		}
 	}
 
-	return ret;
+	switch (rc->info->bmc_id) {
+	case BMC_NPCM8XX:
+		return npcm8xx_clock_controller_register(rc, "clk-npcm8xx");
+	default:
+		return ret;
+	}
 }
 
 static struct platform_driver npcm_rc_driver = {
diff --git a/include/soc/nuvoton/clock-npcm8xx.h b/include/soc/nuvoton/clock-npcm8xx.h
new file mode 100755
index 000000000000..139130e98c51
--- /dev/null
+++ b/include/soc/nuvoton/clock-npcm8xx.h
@@ -0,0 +1,16 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __SOC_NPCM8XX_CLOCK_H
+#define __SOC_NPCM8XX_CLOCK_H
+
+#include <linux/auxiliary_bus.h>
+#include <linux/container_of.h>
+
+struct npcm_clock_adev {
+	void __iomem *base;
+	struct auxiliary_device adev;
+};
+
+#define to_npcm_clock_adev(_adev) \
+	container_of((_adev), struct npcm_clock_adev, adev)
+
+#endif