Re: [PATCH v3 7/8] mikrobus: Add mikrobus driver

Mark Brown
2024-03-15T19:04:04Z

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

On Sat, Mar 16, 2024 at 12:19:05AM +0530, Ayush Singh wrote:

> + if (dev->regulators) {
> + if (dev->protocol == GREYBUS_PROTOCOL_SPI) {
> + snprintf(devname, sizeof(devname), "%s.%u", dev_name(&port->spi_ctrl->dev),
> + port->chip_select[dev->reg]);
> + regulator.dev_name = kmemdup(devname, MIKROBUS_NAME_SIZE, GFP_KERNEL);
> + } else if (dev->protocol == GREYBUS_PROTOCOL_RAW) {

It looks like you're trying to write a switch statement here...

> + for (i = 0; i < dev->num_regulators; i++) {
> + val = dev->regulators[i].value.u64_data;
> + regulator.supply =
> + kmemdup(dev->regulators[i].name, MIKROBUS_NAME_SIZE, GFP_KERNEL);
> + dev_info(&port->dev, "adding fixed regulator %llu uv, %s for %s", *val,
> + regulator.supply, regulator.dev_name);
> + regulator_register_always_on(0, dev->regulators[i].name, &regulator, 1,
> + *val);
> + }

The registered regualtor is ignored here which means you leak the
regualtors every time a device is unregistered...

> +static void mikrobus_device_unregister(struct mikrobus_port *port, struct board_device_info *dev,
> + char *board_name)

...an operation which does seem to be supported?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]