Message-Id: <CVVJR34G5A55.2LYQW8Z5PEEDA@gimli.ms.mff.cuni.cz>
Date:   Fri, 29 Sep 2023 18:37:20 +0200
From:   "Karel Balej" <karelb@...li.ms.mff.cuni.cz>
To:     "Markuss Broks" <markuss.broks@...il.com>,
        "Karel Balej" <balejk@...fyz.cz>
Cc:     "Dmitry Torokhov" <dmitry.torokhov@...il.com>,
        "Rob Herring" <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        "Conor Dooley" <conor+dt@...nel.org>,
        <linux-input@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Duje Mihanović <duje.mihanovic@...le.hr>,
        <~postmarketos/upstreaming@...ts.sr.ht>,
        "Jeff LaBundy" <jeff@...undy.com>, <linmengbo0689@...tonmail.com>
Subject: Re: [PATCH 1/2] input: generalize the Imagis touchscreen driver

Hello, Markuss,

> If you don't mind the extra hassle, I'm all in for my generalization 
> thing going together with your series.
>
> Alternatively, I can resend it myself, but I believe it would be better 
> if they go in bulk since they need to be applied together.

great. Do you wish to make any changes to your original series? If not,
please let me know and I will use the v2 [1] as it is.

> >> As for the voltage set, I believe this does not belong in a kernel
> >> driver. You should set it in device-tree with `regulator-max-microvolt`
> >> and `regulator-min-microvolt`.
> > Please see my response to Jeff regarding this. I will be happy to hear
> > your thoughts on what I propose.
>
> Actually, the regulator values belong to the device-tree, because the 
> device-tree for the board is what describes the board's regulators, and 
> since you know what components are installed on that specific board you 
> can know which regulator values are supposed to be set for it.
>
> [...]
>
> The actual min/max values for regulators or its voltage table is 
> provided by the regulator driver itself, so there's not much point in 
> specifying those again in the device-tree.

I see. I think the reason why I thought what I wrote before is that
downstream the regulator DTS lives separately from the board as a .dtsi
file which made me think that it can be used universally. So if I
understand correctly now, the hardware specifications of the regulator,
such as the minimal and maximal voltage should be part of the driver,
while the DT should contain requirements for the given use of the
regulator (with a specific board) - is this correct?

> This manual voltage setting can cause conflicts with other drivers for
> example. Also some device can use a variable wide voltage range, and
> some only specific narrow one, and e.g. the driver with wide range
> would set it to voltage that isn't suitable for the narrow range
> device, so it's much better to just specify it manually than have it
> resolved.

I would expect that in the case you describe, the kernel would set the
voltage to a value which would satisfy both the ranges. I don't know
what would happen if that was not possible (i. e. there was no
intersection in the two requested voltage ranges), though. Or does a
call to regulator_set_voltage set the voltage immediately taking notice
only of the hardware contraints? I think I am having trouble
understanding what this quote from the regulator_set_voltage
documentation means:

> If the regulator is shared between several devices then the lowest
> request voltage that meets the system constraints will be used.

But actually, it probably doesn't make sense that the kernel would try
to resolve a range suitable for all calls to this function as, I assume,
a single driver could call it multiple times with disjoint voltage
intervals.

Thank you for your patience.

Kind regards,
K. B.

Powered by blists - more mailing lists