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

Mark Brown <broonie@xxxxxxxxxx> · Fri, 15 Mar 2024 19:03:57 +0000

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?