Received-SPF: pass (google.com: domain of linux-kernel+bounces-182736-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <da32106a-5768-4ee2-bf96-6dcf4d9aed0f@salutedevices.com>
Date: Sat, 18 May 2024 04:28:15 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH 7/9] reset: amlogic: add auxiliary reset driver
 support
Content-Language: en-US
To: Jerome Brunet <jbrunet@baylibre.com>, Philipp Zabel
	<p.zabel@pengutronix.de>, Stephen Boyd <sboyd@kernel.org>, Neil Armstrong
	<neil.armstrong@linaro.org>
CC: <linux-kernel@vger.kernel.org>, <linux-amlogic@lists.infradead.org>,
	<linux-clk@vger.kernel.org>
References: <20240516150842.705844-1-jbrunet@baylibre.com>
 <20240516150842.705844-8-jbrunet@baylibre.com>
From: Jan Dakinevich <jan.dakinevich@salutedevices.com>
In-Reply-To: <20240516150842.705844-8-jbrunet@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit


On 5/16/24 18:08, Jerome Brunet wrote:
> Add support for the reset controller present in the audio clock
> controller of the g12 and sm1 SoC families, using the auxiliary bus.
> 
> This is expected to replace the driver currently present directly
> within the related clock driver.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/reset/Kconfig                         |  1 +
>  drivers/reset/reset-meson.c                   | 46 ++++++++++++++++++-
>  include/soc/amlogic/meson8b-auxiliary-reset.h | 17 +++++++
>  3 files changed, 63 insertions(+), 1 deletion(-)
>  create mode 100644 include/soc/amlogic/meson8b-auxiliary-reset.h
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 85b27c42cf65..4ceb4dc48fbc 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -134,6 +134,7 @@ config RESET_MCHP_SPARX5
>  config RESET_MESON
>  	tristate "Meson Reset Driver"
>  	depends on ARCH_MESON || COMPILE_TEST
> +	depends on AUXILIARY_BUS

I don't understand, who enables AUXILIARY_BUS. If I'm not mistaken,
AUXILIARY_BUS should be selected by something that is going to use it,
and it is not intended for defconfig.

>  	default ARCH_MESON
>  	help
>  	  This enables the reset driver for Amlogic Meson SoCs.
> diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
> index e34a10b15593..b5ddb85296ec 100644
> --- a/drivers/reset/reset-meson.c
> +++ b/drivers/reset/reset-meson.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2016 BayLibre, SAS.
>   * Author: Neil Armstrong <narmstrong@baylibre.com>
>   */
> +#include <linux/auxiliary_bus.h>
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> @@ -16,6 +17,8 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> +#include <soc/amlogic/meson8b-auxiliary-reset.h>
> +
>  struct meson_reset_param {
>  	const struct reset_control_ops *reset_ops;
>  	unsigned int reset_num;
> @@ -218,6 +221,47 @@ static struct platform_driver meson_reset_pltf_driver = {
>  };
>  module_platform_driver(meson_reset_pltf_driver);
>  
> -MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
> +static const struct meson_reset_param meson_g12a_audio_param = {
> +	.reset_ops	= &meson_reset_toggle_ops,
> +	.reset_num	= 26,
> +	.level_offset	= 0x24,
> +};
> +
> +static const struct meson_reset_param meson_sm1_audio_param = {
> +	.reset_ops	= &meson_reset_toggle_ops,
> +	.reset_num	= 39,
> +	.level_offset	= 0x28,
> +};
> +
> +static const struct auxiliary_device_id meson_reset_aux_ids[] = {
> +	{
> +		.name = "axg-audio-clkc.rst-g12a",
> +		.driver_data = (kernel_ulong_t)&meson_g12a_audio_param,
> +	}, {
> +		.name = "axg-audio-clkc.rst-sm1",
> +		.driver_data = (kernel_ulong_t)&meson_sm1_audio_param,
> +	},
> +};
> +MODULE_DEVICE_TABLE(auxiliary, meson_reset_aux_ids);
> +
> +static int meson_reset_aux_probe(struct auxiliary_device *adev,
> +				 const struct auxiliary_device_id *id)
> +{
> +	const struct meson_reset_param *param =
> +		(const struct meson_reset_param *)(id->driver_data);
> +	struct meson8b_reset_adev *raux =
> +		to_meson8b_reset_adev(adev);
> +
> +	return meson_reset_probe(&adev->dev, raux->map, param);
> +}
> +
> +static struct auxiliary_driver meson_reset_aux_driver = {
> +	.probe		= meson_reset_aux_probe,
> +	.id_table	= meson_reset_aux_ids,
> +};
> +module_auxiliary_driver(meson_reset_aux_driver);
> +
> +MODULE_DESCRIPTION("Amlogic Meson Reset driver");
>  MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
>  MODULE_LICENSE("Dual BSD/GPL");
> diff --git a/include/soc/amlogic/meson8b-auxiliary-reset.h b/include/soc/amlogic/meson8b-auxiliary-reset.h
> new file mode 100644
> index 000000000000..0a465deb4440
> --- /dev/null
> +++ b/include/soc/amlogic/meson8b-auxiliary-reset.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __SOC_AMLOGIC_MESON8B_AUX_RESET_H
> +#define __SOC_AMLOGIC_MESON8B_AUX_RESET_H
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/container_of.h>
> +#include <linux/regmap.h>
> +
> +struct meson8b_reset_adev {
> +	struct auxiliary_device adev;
> +	struct regmap *map;
> +};
> +
> +#define to_meson8b_reset_adev(_adev) \
> +	container_of((_adev), struct meson8b_reset_adev, adev)
> +
> +#endif /* __SOC_AMLOGIC_MESON8B_AUX_RESET_H */

-- 
Best regards
Jan Dakinevich