public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: linux-kernel-dev@aliel•fr
To: Lee Jones <lee@kernel•org>
Cc: Neil Armstrong <neil.armstrong@linaro•org>,
	Rob Herring <robh@kernel•org>,
	Krzysztof Kozlowski <krzk+dt@kernel•org>,
	Conor Dooley <conor+dt@kernel•org>,
	Andi Shyti <andi.shyti@kernel•org>,
	Kevin Hilman <khilman@baylibre•com>,
	Jerome Brunet <jbrunet@baylibre•com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail•com>,
	Beniamino Galvani <b.galvani@gmail•com>,
	"Rafael J. Wysocki" <rafael@kernel•org>,
	Daniel Lezcano <daniel.lezcano@kernel•org>,
	Zhang Rui <rui.zhang@intel•com>,
	Lukasz Luba <lukasz.luba@arm•com>,
	Liam Girdwood <lgirdwood@gmail•com>,
	Mark Brown <broonie@kernel•org>,
	linux-amlogic@lists•infradead.org, devicetree@vger•kernel.org,
	linux-kernel@vger•kernel.org, linux-i2c@vger•kernel.org,
	linux-arm-kernel@lists•infradead.org, linux-pm@vger•kernel.org
Subject: Re: [PATCH v6 4/8] mfd: khadas-mcu: Add support for VIM4 MCU variant
Date: Wed, 3 Jun 2026 19:50:22 +0200	[thread overview]
Message-ID: <178b09d3-4e05-4f0c-b598-fa37c7a2503c@aliel.fr> (raw)
In-Reply-To: <20260527145322.GB671544@google.com>

On 5/27/26 4:53 PM, Lee Jones wrote:
> On Sat, 16 May 2026, Ronald Claveau via B4 Relay wrote:
> 
>> From: Ronald Claveau <linux-kernel-dev@aliel•fr>
>>
>> Refactor probe() to use per-variant khadas_mcu_data
>> instead of hardcoded globals.
>>
>> Add dedicated regmap configuration and device data for the VIM4 MCU,
>> with its own volatile/writeable registers.
>>
>> Add the fan control register
>> (0–100 levels vs 0–3 for previous supported boards).
>>
>> Add a new compatible string "khadas,vim4-mcu".
>>
>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro•org>
>> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel•fr>
>> ---
>>  drivers/mfd/khadas-mcu.c | 119 +++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 104 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
>> index ba981a7886921..7bc538232a445 100644
>> --- a/drivers/mfd/khadas-mcu.c
>> +++ b/drivers/mfd/khadas-mcu.c
>> @@ -75,15 +75,91 @@ static const struct regmap_config khadas_mcu_regmap_config = {
>>  	.cache_type	= REGCACHE_MAPLE,
>>  };
>>  
>> -static struct mfd_cell khadas_mcu_fan_cells[] = {
>> +static const struct khadas_mcu_fan_pdata khadas_mcu_fan_pdata = {
>> +	.fan_reg	= KHADAS_MCU_CMD_FAN_STATUS_CTRL_REG,
>> +	.max_level	= 3, /* Fan speed: 0 = off, 1 = low, 2 = medium, 3 = high */
>> +};
>> +
>> +static const struct mfd_cell khadas_mcu_fan_cells[] = {
>>  	/* VIM1/2 Rev13+ and VIM3 only */
>> -	{ .name = "khadas-mcu-fan-ctrl", },
>> +	{
>> +		.name = "khadas-mcu-fan-ctrl",
>> +		.platform_data = &khadas_mcu_fan_pdata,
>> +		.pdata_size    = sizeof(khadas_mcu_fan_pdata),
>> +	},
>>  };
>>  
>> -static struct mfd_cell khadas_mcu_cells[] = {
>> +static const struct mfd_cell khadas_mcu_cells[] = {
>>  	{ .name = "khadas-mcu-user-mem", },
>>  };
>>  
>> +static const struct khadas_mcu_data khadas_mcu_data = {
>> +	.regmap_config	= &khadas_mcu_regmap_config,
>> +	.cells		= khadas_mcu_cells,
>> +	.ncells		= ARRAY_SIZE(khadas_mcu_cells),
>> +	.fan_cells	= khadas_mcu_fan_cells,
>> +	.nfan_cells	= ARRAY_SIZE(khadas_mcu_fan_cells),
>> +};
> 
> Let's not over-complicate things please.
> 
> You can do all of this with local variables inside khadas_mcu_probe()
> and omit the terribly named khadas_mcu_data structure entirely.
> 

Ok thanks, I will send a new version according to your feedback.

>> +static bool khadas_mcu_vim4_reg_volatile(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	case KHADAS_MCU_PWR_OFF_CMD_REG:
>> +	case KHADAS_MCU_VIM4_REST_CONF_REG:
>> +	case KHADAS_MCU_WOL_INIT_START_REG:
>> +	case KHADAS_MCU_VIM4_LED_ON_RAM_REG:
>> +	case KHADAS_MCU_VIM4_FAN_CTRL_REG:
>> +	case KHADAS_MCU_VIM4_WDT_EN_REG:
>> +	case KHADAS_MCU_VIM4_SYS_RST_REG:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static bool khadas_mcu_vim4_reg_writeable(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	case KHADAS_MCU_VERSION_0_REG:
>> +	case KHADAS_MCU_VERSION_1_REG:
>> +	case KHADAS_MCU_SHUTDOWN_NORMAL_STATUS_REG:
>> +		return false;
>> +	default:
>> +		return true;
>> +	}
>> +}
>> +
>> +static const struct regmap_config khadas_mcu_vim4_regmap_config = {
>> +	.reg_bits	= 8,
>> +	.reg_stride	= 1,
>> +	.val_bits	= 8,
>> +	.max_register	= KHADAS_MCU_VIM4_SYS_RST_REG,
>> +	.volatile_reg	= khadas_mcu_vim4_reg_volatile,
>> +	.writeable_reg	= khadas_mcu_vim4_reg_writeable,
>> +	.cache_type	= REGCACHE_MAPLE,
>> +};
>> +
>> +static const struct khadas_mcu_fan_pdata khadas_vim4_fan_pdata = {
>> +	.fan_reg	= KHADAS_MCU_VIM4_FAN_CTRL_REG,
>> +	.max_level	= 0x64,
>> +};
>> +
>> +static const struct mfd_cell khadas_mcu_vim4_cells[] = {
>> +	{
>> +		.name		= "khadas-mcu-fan-ctrl",
>> +		.platform_data	= &khadas_vim4_fan_pdata,
>> +		.pdata_size	= sizeof(khadas_vim4_fan_pdata),
>> +	},
>> +};
>> +
>> +static const struct khadas_mcu_data khadas_vim4_mcu_data = {
>> +	.regmap_config	= &khadas_mcu_vim4_regmap_config,
>> +	.cells		= NULL,
>> +	.ncells		= 0,
>> +	.fan_cells	= khadas_mcu_vim4_cells,
>> +	.nfan_cells	= ARRAY_SIZE(khadas_mcu_vim4_cells),
>> +};
>> +
>>  static int khadas_mcu_probe(struct i2c_client *client)
>>  {
>>  	struct device *dev = &client->dev;
>> @@ -94,28 +170,40 @@ static int khadas_mcu_probe(struct i2c_client *client)
>>  	if (!ddata)
>>  		return -ENOMEM;
>>  
>> +	switch ((uintptr_t)i2c_get_match_data(client)) {
>> +	case KHADAS_MCU_GENERIC:
>> +		ddata->data = &khadas_mcu_data;
>> +		break;
>> +	case KHADAS_MCU_VIM4:
>> +		ddata->data = &khadas_vim4_mcu_data;
>> +		break;
>> +	default:
>> +		return -ENODEV;
>> +	}
>> +
>>  	i2c_set_clientdata(client, ddata);
>>  
>>  	ddata->dev = dev;
>>  
>> -	ddata->regmap = devm_regmap_init_i2c(client, &khadas_mcu_regmap_config);
>> +	ddata->regmap = devm_regmap_init_i2c(client, ddata->data->regmap_config);
>>  	if (IS_ERR(ddata->regmap)) {
>>  		ret = PTR_ERR(ddata->regmap);
>> -		dev_err(dev, "Failed to allocate register map: %d\n", ret);
>> -		return ret;
>> +		return dev_err_probe(dev, ret, "Failed to allocate register map\n");
>>  	}
>>  
>> -	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> -				   khadas_mcu_cells,
>> -				   ARRAY_SIZE(khadas_mcu_cells),
>> -				   NULL, 0, NULL);
>> -	if (ret)
>> -		return ret;
>> +	if (ddata->data->cells && ddata->data->ncells) {
>> +		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> +					   ddata->data->cells,
>> +					   ddata->data->ncells,
>> +					   NULL, 0, NULL);
>> +		if (ret)
>> +			return ret;
>> +	}
>>  
>>  	if (of_property_present(dev->of_node, "#cooling-cells"))
>>  		return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> -					    khadas_mcu_fan_cells,
>> -					    ARRAY_SIZE(khadas_mcu_fan_cells),
>> +					    ddata->data->fan_cells,
>> +					    ddata->data->nfan_cells,
>>  					    NULL, 0, NULL);
>>  
>>  	return 0;
>> @@ -123,7 +211,8 @@ static int khadas_mcu_probe(struct i2c_client *client)
>>  
>>  #ifdef CONFIG_OF
>>  static const struct of_device_id khadas_mcu_of_match[] = {
>> -	{ .compatible = "khadas,mcu", },
>> +	{ .compatible = "khadas,mcu", .data = (void *)KHADAS_MCU_GENERIC },
>> +	{ .compatible = "khadas,vim4-mcu", .data = (void *)KHADAS_MCU_VIM4 },
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
>>
>> -- 
>> 2.49.0
>>
>>
> 


-- 
Best regards,
Ronald


  reply	other threads:[~2026-06-03 17:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-16 17:17 [PATCH v6 0/8] Add VIM4 MCU/FAN support Ronald Claveau via B4 Relay
2026-05-16 17:17 ` [PATCH v6 1/8] dt-bindings: mfd: khadas: Add new compatible for Khadas VIM4 MCU Ronald Claveau via B4 Relay
2026-05-16 17:17 ` [PATCH v6 2/8] dt-bindings: i2c: amlogic: Add compatible for T7 SOC Ronald Claveau via B4 Relay
2026-05-16 17:17 ` [PATCH v6 3/8] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support Ronald Claveau via B4 Relay
2026-05-27 14:54   ` Lee Jones
2026-05-16 17:17 ` [PATCH v6 4/8] mfd: khadas-mcu: Add support for VIM4 MCU variant Ronald Claveau via B4 Relay
2026-05-27 14:53   ` Lee Jones
2026-06-03 17:50     ` linux-kernel-dev [this message]
2026-05-16 17:17 ` [PATCH v6 5/8] thermal: khadas-mcu-fan: Add fan config from platform data Add regulator support Ronald Claveau via B4 Relay
2026-05-16 17:17 ` [PATCH v6 6/8] arm64: dts: amlogic: t7: Add i2c pinctrl node Ronald Claveau via B4 Relay
2026-05-16 17:17 ` [PATCH v6 7/8] arm64: dts: amlogic: t7: Add i2c controller node Ronald Claveau via B4 Relay
2026-05-16 17:17 ` [PATCH v6 8/8] arm64: dts: amlogic: t7: khadas-vim4: Add i2c MCU fan node Ronald Claveau via B4 Relay
2026-05-18 14:17 ` [PATCH v6 0/8] Add VIM4 MCU/FAN support Neil Armstrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=178b09d3-4e05-4f0c-b598-fa37c7a2503c@aliel.fr \
    --to=linux-kernel-dev@aliel$(echo .)fr \
    --cc=andi.shyti@kernel$(echo .)org \
    --cc=b.galvani@gmail$(echo .)com \
    --cc=broonie@kernel$(echo .)org \
    --cc=conor+dt@kernel$(echo .)org \
    --cc=daniel.lezcano@kernel$(echo .)org \
    --cc=devicetree@vger$(echo .)kernel.org \
    --cc=jbrunet@baylibre$(echo .)com \
    --cc=khilman@baylibre$(echo .)com \
    --cc=krzk+dt@kernel$(echo .)org \
    --cc=lee@kernel$(echo .)org \
    --cc=lgirdwood@gmail$(echo .)com \
    --cc=linux-amlogic@lists$(echo .)infradead.org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-i2c@vger$(echo .)kernel.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=linux-pm@vger$(echo .)kernel.org \
    --cc=lukasz.luba@arm$(echo .)com \
    --cc=martin.blumenstingl@googlemail$(echo .)com \
    --cc=neil.armstrong@linaro$(echo .)org \
    --cc=rafael@kernel$(echo .)org \
    --cc=robh@kernel$(echo .)org \
    --cc=rui.zhang@intel$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox