On Wed, Jan 18, 2023 at 09:38:08AM +0300, Okan Sahin wrote:
> MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
>
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
>
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
>
> They have same regmap except for ADC part of MAX77541.
...
> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.
Happy New Year!
> + * Mfd core driver for the MAX77540 and MAX77541
MFD
> + */
...
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_buck_irq_chip,
> + &max77541->irq_buck);
> + if (ret)
> + return ret;
> +
> + if (max77541->id == MAX77541) {
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_adc_irq_chip,
> + &max77541->irq_adc);
> + if (ret)
> + return ret;
> + }
> + return ret;
return 0;
...
> + return devm_mfd_add_devices(dev, -1, max77540_devs,
PLATFORM_DEVID_NONE ?
> + ARRAY_SIZE(max77540_devs),
> + NULL, 0, NULL);
...
> + return devm_mfd_add_devices(dev, -1, max77541_devs,
Ditto.
> + ARRAY_SIZE(max77541_devs),
> + NULL, 0, NULL);
...
> +static int max77541_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
No id, please. I.o.w. you should use ->probe_new().
> +{
> + struct device *dev = &client->dev;
> + struct max77541 *max77541;
> + const void *match;
> +
> + max77541 = devm_kzalloc(&client->dev, sizeof(*max77541), GFP_KERNEL);
> + if (!max77541)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, max77541);
> + max77541->i2c = client;
> +
> + match = device_get_match_data(dev);
> + if (match)
> + max77541->id = (enum max7754x_ids)match;
This is dangerous if your enum has 0 as a valid value.
Instead, use pointers in the driver_data, like
&chip_info chip[MAX77540]
> + else if (id)
> + max77541->id = id->driver_data;
> + else
It's better to check the ID range here.
Or since the change recommended above, check for NULL.
> + return -ENODEV;
> +
> + max77541->chip = &chip[max77541->id];
> +
> + max77541->regmap = devm_regmap_init_i2c(client,
> + &max77541_regmap_config);
> + if (IS_ERR(max77541->regmap))
> + return dev_err_probe(dev, PTR_ERR(max77541->regmap),
> + "Failed to allocate register map\n");
> +
> + return max77541_pmic_setup(dev);
> +}
...
> +/*BITS OF REGISTERS*/
Missing spaces.
--
With Best Regards,
Andy Shevchenko
