[PATCH 2/2] regulator: rpi-panel-v2: Add regulator for 7" Raspberry Pi 720x1280

[Uwe Kleine-König]

Hello Marek,

On Mon, Jun 09, 2025 at 02:06:42AM +0200, Marek Vasut wrote:
> +static int rpi_panel_v2_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +				  const struct pwm_state *state)
> +{
> +	struct regmap *regmap = pwmchip_get_drvdata(chip);
> +	unsigned int duty;
> +
> +	if (state->polarity != PWM_POLARITY_NORMAL)
> +		return -EINVAL;
> +
> +	if (!state->enabled)
> +		return regmap_write(regmap, REG_PWM, 0);

I would swap these two if blocks to ensure that disable works even if
the wrong polarity is passed.

> +	duty = pwm_get_relative_duty_cycle(state, PWM_BL_MASK);

This is not how it works. I assume this one can only do a single period
length? Then duty should be calculated as:

	duty_cycle = state->duty_cycle > RPI_PANEL_PWM_PERIOD ?  RPI_PANEL_PWM_PERIOD : state->duty_cycle

	duty = duty_cycle * PWM_BL_MASK / RPI_PANEL_PWM_PERIOD;

> +	return regmap_write(regmap, REG_PWM, duty | PWM_BL_ENABLE);
> +}
> +
> +static const struct pwm_ops rpi_panel_v2_pwm_ops = {
> +	.apply = rpi_panel_v2_pwm_apply,
> +};

I would prefer to see new pwm drivers use the waveform stuff.

> +/*
> + * I2C driver interface functions
> + */
> +static int rpi_panel_v2_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct gpio_regmap_config gconfig = {
> +		.ngpio		= NUM_GPIO,
> +		.ngpio_per_reg	= NUM_GPIO,
> +		.parent		= &i2c->dev,
> +		.reg_set_base	= REG_POWERON,
> +	};
> +	struct regmap *regmap;
> +	struct pwm_chip *pc;
> +	int ret;
> +
> +	pc = devm_pwmchip_alloc(&i2c->dev, 1, 0);
> +	if (IS_ERR(pc))
> +		return PTR_ERR(pc);
> +
> +	pc->ops = &rpi_panel_v2_pwm_ops;
> +
> +	regmap = devm_regmap_init_i2c(i2c, &rpi_panel_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(&i2c->dev, PTR_ERR(regmap), "Failed to allocate regmap\n");
> +
> +	pwmchip_set_drvdata(pc, regmap);
> +
> +	regmap_write(regmap, REG_POWERON, 0);
> +
> +	gconfig.regmap = regmap;
> +	ret = PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&i2c->dev, &gconfig));
> +	if (ret)
> +		return dev_err_probe(&i2c->dev, ret, "Failed to create gpiochip\n");
> +
> +	return devm_pwmchip_add(&i2c->dev, pc);

	ret = devm_pwmchip_add(&i2c->dev, pc);
	if (ret < 0)
		return dev_err_probe(...);

> +}
> 

Best regards
Uwe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20250611/241d08e6/attachment.sig>