[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ce9a7ea1-67bc-42b8-836d-11932dcf3790@kernel.org>
Date: Wed, 21 Aug 2024 10:58:13 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: "friday.yang" <friday.yang@...iatek.com>, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Matthias Brugger
<matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: Yong Wu <yong.wu@...iatek.com>, Philipp Zabel <p.zabel@...gutronix.de>,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH 4/4] reset: mediatek: Add reset control driver for SMI
On 21/08/2024 10:26, friday.yang wrote:
> Add a reset-controller driver for performing reset management of
> SMI LARBs on MediaTek platform. This driver uses the regmap
> frameworks to actually implement the various reset functions
> needed when SMI LARBs apply clamp operations.
How does this depend on memory controller patches? Why is this grouped
in one patchset?
>
> Signed-off-by: friday.yang <friday.yang@...iatek.com>
> ---
> drivers/reset/Kconfig | 9 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-mediatek-smi.c | 152 +++++++++++++++++++++++++++++
> 3 files changed, 162 insertions(+)
> create mode 100644 drivers/reset/reset-mediatek-smi.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 67bce340a87e..e984a5a332f1 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -154,6 +154,15 @@ config RESET_MESON_AUDIO_ARB
> This enables the reset driver for Audio Memory Arbiter of
> Amlogic's A113 based SoCs
>
> +config RESET_MTK_SMI
> + bool "MediaTek SMI Reset Driver"
> + depends on MTK_SMI
compile test
> + help
> + This option enables the reset controller driver for MediaTek SMI.
> + This reset driver is responsible for managing the reset signals
> + for SMI larbs. Say Y if you want to control reset signals for
> + MediaTek SMI larbs. Otherwise, say N.
> +
> config RESET_NPCM
> bool "NPCM BMC Reset Driver" if COMPILE_TEST
> default ARCH_NPCM
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 27b0bbdfcc04..241777485b40 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> obj-$(CONFIG_RESET_MCHP_SPARX5) += reset-microchip-sparx5.o
> obj-$(CONFIG_RESET_MESON) += reset-meson.o
> obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
> +obj-$(CONFIG_RESET_MTK_SMI) += reset-mediatek-smi.o
> obj-$(CONFIG_RESET_NPCM) += reset-npcm.o
> obj-$(CONFIG_RESET_NUVOTON_MA35D1) += reset-ma35d1.o
> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> diff --git a/drivers/reset/reset-mediatek-smi.c b/drivers/reset/reset-mediatek-smi.c
> new file mode 100644
> index 000000000000..ead747e80ad5
> --- /dev/null
> +++ b/drivers/reset/reset-mediatek-smi.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Reset driver for MediaTek SMI module
> + *
> + * Copyright (C) 2024 MediaTek Inc.
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +#include <dt-bindings/reset/mt8188-resets.h>
> +
> +#define to_mtk_smi_reset_data(_rcdev) \
> + container_of(_rcdev, struct mtk_smi_reset_data, rcdev)
> +
> +struct mtk_smi_larb_reset {
> + unsigned int offset;
> + unsigned int value;
> +};
> +
> +static const struct mtk_smi_larb_reset rst_signal_mt8188[] = {
> + [MT8188_SMI_RST_LARB10] = { 0xC, BIT(0) }, /* larb10 */
> + [MT8188_SMI_RST_LARB11A] = { 0xC, BIT(0) }, /* larb11a */
> + [MT8188_SMI_RST_LARB11C] = { 0xC, BIT(0) }, /* larb11c */
> + [MT8188_SMI_RST_LARB12] = { 0xC, BIT(8) }, /* larb12 */
> + [MT8188_SMI_RST_LARB11B] = { 0xC, BIT(0) }, /* larb11b */
> + [MT8188_SMI_RST_LARB15] = { 0xC, BIT(0) }, /* larb15 */
> + [MT8188_SMI_RST_LARB16B] = { 0xA0, BIT(4) }, /* larb16b */
> + [MT8188_SMI_RST_LARB17B] = { 0xA0, BIT(4) }, /* larb17b */
> + [MT8188_SMI_RST_LARB16A] = { 0xA0, BIT(4) }, /* larb16a */
> + [MT8188_SMI_RST_LARB17A] = { 0xA0, BIT(4) }, /* larb17a */
> +};
> +
> +struct mtk_smi_larb_plat {
> + const struct mtk_smi_larb_reset *reset_signal;
> + const unsigned int larb_reset_nr;
> +};
> +
> +struct mtk_smi_reset_data {
> + const struct mtk_smi_larb_plat *larb_plat;
> + struct reset_controller_dev rcdev;
> + struct regmap *regmap;
> +};
> +
> +static const struct mtk_smi_larb_plat mtk_smi_larb_mt8188 = {
> + .reset_signal = rst_signal_mt8188,
> + .larb_reset_nr = ARRAY_SIZE(rst_signal_mt8188),
> +};
> +
> +static int mtk_smi_larb_reset(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct mtk_smi_reset_data *data = to_mtk_smi_reset_data(rcdev);
> + const struct mtk_smi_larb_plat *larb_plat = data->larb_plat;
> + const struct mtk_smi_larb_reset *larb_rst = larb_plat->reset_signal + id;
> + int ret;
> +
> + ret = regmap_set_bits(data->regmap, larb_rst->offset, larb_rst->value);
> + if (ret)
> + return ret;
> + ret = regmap_clear_bits(data->regmap, larb_rst->offset, larb_rst->value);
> +
> + return ret;
> +}
> +
> +static int mtk_smi_larb_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct mtk_smi_reset_data *data = to_mtk_smi_reset_data(rcdev);
> + const struct mtk_smi_larb_plat *larb_plat = data->larb_plat;
> + const struct mtk_smi_larb_reset *larb_rst = larb_plat->reset_signal + id;
> + int ret;
> +
> + ret = regmap_set_bits(data->regmap, larb_rst->offset, larb_rst->value);
> + if (ret)
> + dev_err(rcdev->dev, "[%s] Failed to shutdown larb %d\n", __func__, ret);
> +
> + return ret;
> +}
> +
> +static int mtk_smi_larb_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct mtk_smi_reset_data *data = to_mtk_smi_reset_data(rcdev);
> + const struct mtk_smi_larb_plat *larb_plat = data->larb_plat;
> + const struct mtk_smi_larb_reset *larb_rst = larb_plat->reset_signal + id;
> + int ret;
> +
> + ret = regmap_clear_bits(data->regmap, larb_rst->offset, larb_rst->value);
> + if (ret)
> + dev_err(rcdev->dev, "[%s] Failed to reopen larb %d\n", __func__, ret);
> +
> + return ret;
> +}
> +
> +static const struct reset_control_ops mtk_smi_reset_ops = {
> + .reset = mtk_smi_larb_reset,
> + .assert = mtk_smi_larb_reset_assert,
> + .deassert = mtk_smi_larb_reset_deassert,
> +};
> +
> +static int mtk_smi_reset_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct mtk_smi_larb_plat *larb_plat = of_device_get_match_data(dev);
> + struct device_node *np = dev->of_node, *reset_node;
> + struct mtk_smi_reset_data *data;
> + struct regmap *regmap;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + reset_node = of_parse_phandle(np, "mediatek,larb-rst-syscon", 0);
> + if (!reset_node)
This looks just wrong. This looks like a child of whatever phandle
points here.
Why do you create MMIO-using node as not MMIO?
Best regards,
Krzysztof
Powered by blists - more mailing lists