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
Attachment:
signature.asc
Description: PGP signature
