Re: [PATCH v2 2/2] Input: isa1200 - new driver for Imagis ISA1200

[Linus Walleij <linusw@kernel.org>]

Hi Svyatoslav,

I just saw this thing:

On Tue, Apr 28, 2026 at 1:43 PM Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> +/* HCTRL5 controls the PWM high duty cycle of internal channel */
> +#define ISA1200_HCTRL5                 0x35
(...)
> +struct isa1200_config {
(...)
> +       u32 duty;
> +};
> +       /* Duty cycle */
> +       regmap_write(isa->map, ISA1200_HCTRL5, config->period >> 1);
(...)
> +       if (isa->clk)
> +               regmap_write(isa->map, ISA1200_HCTRL5, config->duty);
(..)
> +       if (isa->clk) {
> +               ret = device_property_read_u32(dev, "imagis,period-ns",
> +                                              &config->period);
> +               if (ret)
> +                       return dev_err_probe(dev, ret,
> +                                            "failed to get period\n");
> +
> +               config->period /= ISA1200_HCTRL6_PERIOD_SCALE;
> +               config->duty = config->period >> 1;
> +       }
> +
> +       if (isa->pwm) {
> +               struct pwm_state state;
> +
> +               pwm_init_state(isa->pwm, &state);
> +
> +               if (!state.period)
> +                       return dev_err_probe(dev, -EINVAL,
> +                                            "PWM period cannot be zero\n");
> +
> +               config->freq = div64_u64(NANO, state.period * config->clkdiv);
> +               config->duty = state.period >> 1;

So you don't need the same code writing
regmap_write(isa->map, ISA1200_HCTRL5, config->period >> 1);
above, just use confg->duty which contains config->period >> 1
(...)
> +       device_property_read_u32(dev, "imagis,duty-cycle-ns", &config->duty);

I'm not sure about this, it works for me when I set this to 100
resulting in a register write of 100 to ISA1200_HCTRL5. But are
we sure that this register is expressed in nanoseconds? I think
some calculation is needed here (and probably another value
in my device tree).

Yours,
Linus Walleij