From: Karel Balej <[email protected]>
This patch series generalizes the Imagis touchscreen driver to support
other Imagis chips, namely IST3038B, which use a slightly different
protocol.
It also adds necessary information to the driver so that the IST3032C
touchscreen can be used with it. The motivation for this is the
samsung,coreprimevelte smartphone with which this series has been
tested. However, the support for this device is not yet in-tree, the
effort is happening at [1]. In particular, the driver for the regulator
needed by the touchscreen on this device has not been rewritten for
mainline yet.
Note that this is a prerequisite for this patch [2] which implements
support for touch keys for Imagis touchscreens that have it.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
---
v3:
- Rebase to v6.7-rc3.
- v2: https://lore.kernel.org/all/[email protected]/
v2:
- Do not rename the driver.
- Do not hardcode voltage required by the IST3032C.
- Use Markuss' series which generalizes the driver. Link to the original
series: https://lore.kernel.org/all/[email protected]/
- Separate bindings into separate patch.
- v1: https://lore.kernel.org/all/[email protected]/
---
Karel Balej (2):
dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
input/touchscreen: imagis: add support for IST3032C
Markuss Broks (3):
input/touchscreen: imagis: Correct the maximum touch area value
dt-bindings: input/touchscreen: Add compatible for IST3038B
input/touchscreen: imagis: Add support for Imagis IST3038B
.../input/touchscreen/imagis,ist3038c.yaml | 2 +
drivers/input/touchscreen/imagis.c | 70 +++++++++++++++----
2 files changed, 60 insertions(+), 12 deletions(-)
--
2.43.0
From: Markuss Broks <[email protected]>
Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
add the compatible for it to the IST3038C bindings.
Signed-off-by: Markuss Broks <[email protected]>
Signed-off-by: Karel Balej <[email protected]>
---
.../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index 0d6b033fd5fb..b5372c4eae56 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -18,6 +18,7 @@ properties:
compatible:
enum:
+ - imagis,ist3038b
- imagis,ist3038c
reg:
--
2.43.0
On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> From: Markuss Broks <[email protected]>
>
> Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> add the compatible for it to the IST3038C bindings.
This one is better, but would be well served by mentioning what
specifically is different (register addresses or firmware commands?)
Cheers,
Conor.
>
> Signed-off-by: Markuss Broks <[email protected]>
> Signed-off-by: Karel Balej <[email protected]>
> ---
> .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> index 0d6b033fd5fb..b5372c4eae56 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> @@ -18,6 +18,7 @@ properties:
>
> compatible:
> enum:
> + - imagis,ist3038b
> - imagis,ist3038c
>
> reg:
> --
> 2.43.0
>
Hi Conor,
On 12/3/23 13:20, Conor Dooley wrote:
> On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
>> From: Markuss Broks <[email protected]>
>>
>> Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
>> add the compatible for it to the IST3038C bindings.
> This one is better, but would be well served by mentioning what
> specifically is different (register addresses or firmware commands?)
I don't think anyone knows this other than Imagis itself. I would guess
it's different hardware, since register addresses are indeed different,
but on the other hand, there is a possibility that firmware on the MCU
could be responding to those commands. I suppose "... IST3038B is a
hardware variant of ... IST3038" would be more correct.
The reason why I think it could be firmware-defined is because we have a
lot of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers
usually mean feature level/completeness, e.g. some don't support the
touch pressure or touchkeys, and we don't know what A/B/C/none means.
>
> Cheers,
> Conor.
>
>> Signed-off-by: Markuss Broks <[email protected]>
>> Signed-off-by: Karel Balej <[email protected]>
>> ---
>> .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
>> index 0d6b033fd5fb..b5372c4eae56 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
>> @@ -18,6 +18,7 @@ properties:
>>
>> compatible:
>> enum:
>> + - imagis,ist3038b
>> - imagis,ist3038c
>>
>> reg:
>> --
>> 2.43.0
>>
- Markuss
On Mon, Dec 04, 2023 at 02:40:44PM +0200, Markuss Broks wrote:
> On 12/3/23 13:20, Conor Dooley wrote:
> > On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> > > From: Markuss Broks <[email protected]>
> > >
> > > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> > > add the compatible for it to the IST3038C bindings.
> > This one is better, but would be well served by mentioning what
> > specifically is different (register addresses or firmware commands?)
>
> I don't think anyone knows this other than Imagis itself. I would guess it's
> different hardware, since register addresses are indeed different, but on
> the other hand, there is a possibility that firmware on the MCU could be
> responding to those commands. I suppose "... IST3038B is a hardware variant
> of ... IST3038" would be more correct.
Only Imagis might know the specifics, but you (plural) have made driver
changes so you know what is different in terms of the programming model.
I'm just asking for you to mention how the programming model varies in
the commit message. Otherwise I can't know whether you should have added
a fallback compatible, without going and reading your driver change. The
commit message for the bindings should stand on its own merit in that
regard.
"Variant" alone does not suffice, as many variants of devices have a
compatible programming model, be that for a subset of features or
complete compatibility.
> The reason why I think it could be firmware-defined is because we have a lot
> of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers usually mean
> feature level/completeness, e.g. some don't support the touch pressure or
> touchkeys, and we don't know what A/B/C/none means.
Ultimately whether it is due to firmware or the hardware isn't
particular important, just mention what is incompatibly different.
Cheers,
Conor.
> > > Signed-off-by: Markuss Broks <[email protected]>
> > > Signed-off-by: Karel Balej <[email protected]>
> > > ---
> > > .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > index 0d6b033fd5fb..b5372c4eae56 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > > compatible:
> > > enum:
> > > + - imagis,ist3038b
> > > - imagis,ist3038c
> > > reg:
> > > --
> > > 2.43.0
> > >
> - Markuss
On Mon Dec 4, 2023 at 1:52 PM CET, Conor Dooley wrote:
> On Mon, Dec 04, 2023 at 02:40:44PM +0200, Markuss Broks wrote:
> > On 12/3/23 13:20, Conor Dooley wrote:
> > > On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> > > > From: Markuss Broks <[email protected]>
> > > >
> > > > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> > > > add the compatible for it to the IST3038C bindings.
> > > This one is better, but would be well served by mentioning what
> > > specifically is different (register addresses or firmware commands?)
> >
> > I don't think anyone knows this other than Imagis itself. I would guess it's
> > different hardware, since register addresses are indeed different, but on
> > the other hand, there is a possibility that firmware on the MCU could be
> > responding to those commands. I suppose "... IST3038B is a hardware variant
> > of ... IST3038" would be more correct.
>
> Only Imagis might know the specifics, but you (plural) have made driver
> changes so you know what is different in terms of the programming model.
> I'm just asking for you to mention how the programming model varies in
> the commit message. Otherwise I can't know whether you should have added
> a fallback compatible, without going and reading your driver change. The
> commit message for the bindings should stand on its own merit in that
> regard.
> "Variant" alone does not suffice, as many variants of devices have a
> compatible programming model, be that for a subset of features or
> complete compatibility.
>
> > The reason why I think it could be firmware-defined is because we have a lot
> > of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers usually mean
> > feature level/completeness, e.g. some don't support the touch pressure or
> > touchkeys, and we don't know what A/B/C/none means.
>
> Ultimately whether it is due to firmware or the hardware isn't
> particular important, just mention what is incompatibly different.
I propose to update the commit description as such:
Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC
differing from IST3038C in its register interface. Add the
compatible for it to the IST3038C bindings.
>
> Cheers,
> Conor.
>
>
> > > > Signed-off-by: Markuss Broks <[email protected]>
> > > > Signed-off-by: Karel Balej <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > > index 0d6b033fd5fb..b5372c4eae56 100644
> > > > --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > > @@ -18,6 +18,7 @@ properties:
> > > > compatible:
> > > > enum:
> > > > + - imagis,ist3038b
> > > > - imagis,ist3038c
> > > > reg:
> > > > --
> > > > 2.43.0
> > > >
> > - Markuss
Kind regards,
K. B.
On Sat, Dec 09, 2023 at 10:05:27AM +0100, Karel Balej wrote:
> On Mon Dec 4, 2023 at 1:52 PM CET, Conor Dooley wrote:
> > On Mon, Dec 04, 2023 at 02:40:44PM +0200, Markuss Broks wrote:
> > > On 12/3/23 13:20, Conor Dooley wrote:
> > > > On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> > > > > From: Markuss Broks <[email protected]>
> > > > >
> > > > > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> > > > > add the compatible for it to the IST3038C bindings.
> > > > This one is better, but would be well served by mentioning what
> > > > specifically is different (register addresses or firmware commands?)
> > >
> > > I don't think anyone knows this other than Imagis itself. I would guess it's
> > > different hardware, since register addresses are indeed different, but on
> > > the other hand, there is a possibility that firmware on the MCU could be
> > > responding to those commands. I suppose "... IST3038B is a hardware variant
> > > of ... IST3038" would be more correct.
> >
> > Only Imagis might know the specifics, but you (plural) have made driver
> > changes so you know what is different in terms of the programming model.
> > I'm just asking for you to mention how the programming model varies in
> > the commit message. Otherwise I can't know whether you should have added
> > a fallback compatible, without going and reading your driver change. The
> > commit message for the bindings should stand on its own merit in that
> > regard.
> > "Variant" alone does not suffice, as many variants of devices have a
> > compatible programming model, be that for a subset of features or
> > complete compatibility.
> >
> > > The reason why I think it could be firmware-defined is because we have a lot
> > > of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers usually mean
> > > feature level/completeness, e.g. some don't support the touch pressure or
> > > touchkeys, and we don't know what A/B/C/none means.
> >
> > Ultimately whether it is due to firmware or the hardware isn't
> > particular important, just mention what is incompatibly different.
>
> I propose to update the commit description as such:
>
> Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC
> differing from IST3038C in its register interface. Add the
> compatible for it to the IST3038C bindings.
SGTM. You can add
Acked-by: Conor Dooley <[email protected]>
with that commit message update.
Thanks,
Conor.
Markuss,
On Sat Dec 9, 2023 at 11:58 AM CET, Conor Dooley wrote:
> On Sat, Dec 09, 2023 at 10:05:27AM +0100, Karel Balej wrote:
> > On Mon Dec 4, 2023 at 1:52 PM CET, Conor Dooley wrote:
> > > On Mon, Dec 04, 2023 at 02:40:44PM +0200, Markuss Broks wrote:
> > > > On 12/3/23 13:20, Conor Dooley wrote:
> > > > > On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> > > > > > From: Markuss Broks <[email protected]>
> > > > > >
> > > > > > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> > > > > > add the compatible for it to the IST3038C bindings.
> > > > > This one is better, but would be well served by mentioning what
> > > > > specifically is different (register addresses or firmware commands?)
> > > >
> > > > I don't think anyone knows this other than Imagis itself. I would guess it's
> > > > different hardware, since register addresses are indeed different, but on
> > > > the other hand, there is a possibility that firmware on the MCU could be
> > > > responding to those commands. I suppose "... IST3038B is a hardware variant
> > > > of ... IST3038" would be more correct.
> > >
> > > Only Imagis might know the specifics, but you (plural) have made driver
> > > changes so you know what is different in terms of the programming model.
> > > I'm just asking for you to mention how the programming model varies in
> > > the commit message. Otherwise I can't know whether you should have added
> > > a fallback compatible, without going and reading your driver change. The
> > > commit message for the bindings should stand on its own merit in that
> > > regard.
> > > "Variant" alone does not suffice, as many variants of devices have a
> > > compatible programming model, be that for a subset of features or
> > > complete compatibility.
> > >
> > > > The reason why I think it could be firmware-defined is because we have a lot
> > > > of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers usually mean
> > > > feature level/completeness, e.g. some don't support the touch pressure or
> > > > touchkeys, and we don't know what A/B/C/none means.
> > >
> > > Ultimately whether it is due to firmware or the hardware isn't
> > > particular important, just mention what is incompatibly different.
> >
> > I propose to update the commit description as such:
> >
> > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC
> > differing from IST3038C in its register interface. Add the
> > compatible for it to the IST3038C bindings.
is this change OK with you?
>
>
> SGTM. You can add
> Acked-by: Conor Dooley <[email protected]>
> with that commit message update.
>
> Thanks,
> Conor.
Kind regards,
K. B.