Re: [PATCH RESEND] ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for vibrator

[Linus Walleij]

On Mon, May 20, 2019 at 4:21 PM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:

> > +       vibrator@fd8c3450 {
> > +               compatible = "qcom,msm8974-vibrator";
> > +               reg = <0xfd8c3450 0x400>;
>
> This is inside the multimedia clk controller. The resource reservation
> mechanism should be complaining loudly here. Is the driver writing
> directly into clk controller registers to adjust a duty cycle of the
> camera's general purpose clk?
>
> Can you add support for duty cycle to the qcom clk driver's RCGs and
> then write a generic clk duty cycle vibrator driver that adjusts the
> duty cycle of the clk? That would be better than reaching into the clk
> controller registers to do this.

There is something ontological about this.

A clock with variable duty cycle, isn't that by definition a PWM?
I don't suppose it is normal for qcom clocks to be able to control
their duty cycle, but rather default to 50/50 as we could expect?

I would rather say that maybe the qcom drivers/clk/qcom/* file
should be exporting a PWM from the linux side of things
rather than a clock for this thingie, and adding #pwm-cells
in the DT node for the clock controller, making it possible
to obtain PWMs right out of it, if it is a single device node for
the whole thing.

Analogous to how we have GPIOs that are ortogonally interrupt
providers I don't see any big problem in a clock controller
being clock and PWM provider at the same time.

There is code in drivers/clk/clk-pwm to use a pwm as a clock
but that is kind of the reverse use case, if we implement PWMs
directly in a clock controller driver then these can be turned into
clocks using clk-pwm.c should it be needed, right?

Part of me start to question whether clk and pwm should even
be separate subsystems :/ they seem to solve an overlapping
problem space.

Yours,
Linus Walleij