On 6/11/26 6:40 PM, Lee Jones wrote:
> /* Sashiko Automation: Reviewed (0 Findings) */
>
> On Wed, 03 Jun 2026, Ronald Claveau via B4 Relay wrote:
>
>> From: Ronald Claveau <[email protected]>
>>
>> Refactor probe() to use per-variant values
>> instead of hardcoded globals.
>>
>> Add dedicated regmap configuration 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 <[email protected]>
>> Signed-off-by: Ronald Claveau <[email protected]>
>> ---
>> drivers/mfd/khadas-mcu.c | 115 ++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 99 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
>> index ba981a7886921..1bf260729d73b 100644
>> --- a/drivers/mfd/khadas-mcu.c
>> +++ b/drivers/mfd/khadas-mcu.c
>> @@ -75,47 +75,129 @@ 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 */
>
> Instead of this comment, you could always just define the value(s)?
>
I will do that with define.
>> +};
>> +
>> +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),
>
> No need to attempt to align the '=' like this, please.
>
I will remove these extra spaces.
>> + },
>> };
>>
>> -static struct mfd_cell khadas_mcu_cells[] = {
>> +static const struct mfd_cell khadas_mcu_cells[] = {
>> { .name = "khadas-mcu-user-mem", },
>> };
>>
>> +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 int khadas_mcu_probe(struct i2c_client *client)
>> {
>> + const struct mfd_cell *cells, *fan_cells;
>> + const struct regmap_config *regmap_cfg;
>> struct device *dev = &client->dev;
>> + int ncells, nfan_cells, ret;
>> struct khadas_mcu *ddata;
>> - int ret;
>>
>> ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
>> if (!ddata)
>> return -ENOMEM;
>>
>> + switch ((uintptr_t)i2c_get_match_data(client)) {
>> + case KHADAS_MCU_GENERIC:
>> + regmap_cfg = &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);
>> + break;
>> + case KHADAS_MCU_VIM4:
>> + regmap_cfg = &khadas_mcu_vim4_regmap_config;
>> + cells = NULL;
>> + ncells = 0;
>> + fan_cells = khadas_mcu_vim4_cells;
>> + nfan_cells = ARRAY_SIZE(khadas_mcu_vim4_cells);
>
> I'm not as offended by this as I thought I would be!
>
>> + 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, regmap_cfg);
>> 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 (cells && ncells) {
>> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> + cells,
>> + 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),
>> + fan_cells,
>> + nfan_cells,
>> NULL, 0, NULL);
>>
>> return 0;
>> @@ -123,7 +205,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
_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.