diff mbox series

[v1,1/4] dt-bindings: input: Add Himax HX83102J touchscreen

Message ID TY0PR06MB56116F0902017225C78EDDDD9E312@TY0PR06MB5611.apcprd06.prod.outlook.com (mailing list archive)
State Superseded
Headers show
Series HID: Add support for Himax HX83102j touchscreen | expand

Commit Message

Allen_Lin March 22, 2024, 8:56 a.m. UTC 
Add the HX83102j touchscreen device tree bindings documents.
HX83102j is a Himax TDDI touchscreen controller.
It's power sequence should be bound with a lcm driver, thus it
needs to be a panel follower. Others are the same as normal SPI
touchscreen controller.

Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
---
 .../input/touchscreen/himax,hx83102j.yaml     | 73 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml

Comments

Conor Dooley March 22, 2024, 5:54 p.m. UTC | #1 
On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> Add the HX83102j touchscreen device tree bindings documents.
> HX83102j is a Himax TDDI touchscreen controller.
> It's power sequence should be bound with a lcm driver, thus it
> needs to be a panel follower. Others are the same as normal SPI
> touchscreen controller.
> 
> Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>

note to self/Krzysztof/Rob:
There was a previous attempt at this kind of device. This version looks
better but might be incomplete given there's a bunch more properties in
that patchset:
https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/

> ---
>  .../input/touchscreen/himax,hx83102j.yaml     | 73 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> new file mode 100644
> index 000000000000..6c0a1ebf8d91
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/himax,hx83102j.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Himax hx83102j touchscreen
> +
> +maintainers:
> +  - Allen Lin <allencl_lin@hotmail.com>
> +
> +description:
> +  This Himax hx83102j touchscreen uses the spi protocol.
> +
> +allOf:
> +  - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: himax,hx83102j
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  spi-max-frequency: true
> +
> +  panel: true
> +
> +  himax,pid:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      PID for HID device, used to load correct firmware.

One thing I did comment on that old patchset is why you cannot just use
firmware-name here?

Cheers,
Conor.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - reset-gpios
> +  - panel
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      ap_ts: touchscreen@0 {
> +        compatible = "himax,hx83102j";
> +        reg = <0>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&touch_int0 &touch_reset>;
> +        reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
> +        spi-cpha;
> +        spi-cpol;
> +        interrupt-parent = <&gpio1>;
> +        interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +        panel = <&panel>;
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 43b39956694a..aa51c60fd66d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9669,6 +9669,12 @@ L:	linux-kernel@vger.kernel.org
>  S:	Maintained
>  F:	drivers/misc/hisi_hikey_usb.c
>  
> +HIMAX HID HX83102J TOUCHSCREEN
> +M:	Allen Lin <allencl_lin@hotmail.com>
> +L:	linux-input@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> +
>  HIMAX HX83112B TOUCHSCREEN SUPPORT
>  M:	Job Noorman <job@noorman.info>
>  L:	linux-input@vger.kernel.org
> -- 
> 2.34.1
>
Rob Herring (Arm) March 22, 2024, 6:30 p.m. UTC | #2 
On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > Add the HX83102j touchscreen device tree bindings documents.
> > HX83102j is a Himax TDDI touchscreen controller.
> > It's power sequence should be bound with a lcm driver, thus it
> > needs to be a panel follower. Others are the same as normal SPI
> > touchscreen controller.
> > 
> > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> 
> note to self/Krzysztof/Rob:
> There was a previous attempt at this kind of device. This version looks
> better but might be incomplete given there's a bunch more properties in
> that patchset:
> https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/

Those don't look like properties we want coming back.

Rob
Rob Herring (Arm) March 22, 2024, 6:31 p.m. UTC | #3 
On Fri, 22 Mar 2024 16:56:03 +0800, Allen_Lin wrote:
> Add the HX83102j touchscreen device tree bindings documents.
> HX83102j is a Himax TDDI touchscreen controller.
> It's power sequence should be bound with a lcm driver, thus it
> needs to be a panel follower. Others are the same as normal SPI
> touchscreen controller.
> 
> Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> ---
>  .../input/touchscreen/himax,hx83102j.yaml     | 73 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Conor Dooley March 22, 2024, 6:34 p.m. UTC | #4 
On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > Add the HX83102j touchscreen device tree bindings documents.
> > > HX83102j is a Himax TDDI touchscreen controller.
> > > It's power sequence should be bound with a lcm driver, thus it
> > > needs to be a panel follower. Others are the same as normal SPI
> > > touchscreen controller.
> > > 
> > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > 
> > note to self/Krzysztof/Rob:
> > There was a previous attempt at this kind of device. This version looks
> > better but might be incomplete given there's a bunch more properties in
> > that patchset:
> > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> 
> Those don't look like properties we want coming back.

Oh, I don't want most of them coming back either. There are some
supplies in there though that I think we would like to come back, no?
Maybe this particular device doesn't have any supplies, but that doesn't
really seem credible.
Allen_Lin March 26, 2024, 5:46 a.m. UTC | #5 
Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
>
> On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > It's power sequence should be bound with a lcm driver, thus it
> > > > needs to be a panel follower. Others are the same as normal SPI
> > > > touchscreen controller.
> > > >
> > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > >
> > > note to self/Krzysztof/Rob:
> > > There was a previous attempt at this kind of device. This version looks
> > > better but might be incomplete given there's a bunch more properties in
> > > that patchset:
> > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> >
> > Those don't look like properties we want coming back.
>
> Oh, I don't want most of them coming back either. There are some
> supplies in there though that I think we would like to come back, no?
> Maybe this particular device doesn't have any supplies, but that doesn't
> really seem credible.

We will use Firmware-name in Device Tree.
For power supply settings, because there may be other device using
same regulator.
We plan to define it as an optional property for user to control in
next release.
Conor Dooley March 26, 2024, 8:48 a.m. UTC | #6 
On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> >
> > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > touchscreen controller.
> > > > >
> > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > >
> > > > note to self/Krzysztof/Rob:
> > > > There was a previous attempt at this kind of device. This version looks
> > > > better but might be incomplete given there's a bunch more properties in
> > > > that patchset:
> > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > >
> > > Those don't look like properties we want coming back.
> >
> > Oh, I don't want most of them coming back either. There are some
> > supplies in there though that I think we would like to come back, no?
> > Maybe this particular device doesn't have any supplies, but that doesn't
> > really seem credible.
> 
> We will use Firmware-name in Device Tree.

> For power supply settings, because there may be other device using
> same regulator.

If there are other devices using the same regulator is it more
important that you document it so that it doesn't get disabled by the
other users.

> We plan to define it as an optional property for user to control in
> next release.

I don't see how the regulator would not be required, the device doesn't
function without power.

Thanks,
Conor.
Allen_Lin March 26, 2024, 10:40 a.m. UTC | #7 
Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
>
> On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > >
> > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > touchscreen controller.
> > > > > >
> > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > >
> > > > > note to self/Krzysztof/Rob:
> > > > > There was a previous attempt at this kind of device. This version looks
> > > > > better but might be incomplete given there's a bunch more properties in
> > > > > that patchset:
> > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > >
> > > > Those don't look like properties we want coming back.
> > >
> > > Oh, I don't want most of them coming back either. There are some
> > > supplies in there though that I think we would like to come back, no?
> > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > really seem credible.
> >
> > We will use Firmware-name in Device Tree.
>
> > For power supply settings, because there may be other device using
> > same regulator.
>
> If there are other devices using the same regulator is it more
> important that you document it so that it doesn't get disabled by the
> other users.
>
> > We plan to define it as an optional property for user to control in
> > next release.
>
> I don't see how the regulator would not be required, the device doesn't
> function without power.
>
> Thanks,
> Conor.

I will set power supply as required.
The description of power supply as below,

properties:
  vccd-supply:
    description: A phandle for the regualtor supplying IO power. Should be own
                 by TPIC only. This works for TP digital IO only, main power is
                 given by display part VSP/VSN power source which is controlled
                 by lcm driver.

required:
  - compatible
  - reg
  - interrupts
  - reset-gpios
  - panel
  - vccd-supply

Thanks
Allen
Conor Dooley March 26, 2024, 7:28 p.m. UTC | #8 
On Tue, Mar 26, 2024 at 06:40:28PM +0800, Allen Lin wrote:
> Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
> >
> > On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > > >
> > > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > > touchscreen controller.
> > > > > > >
> > > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > > >
> > > > > > note to self/Krzysztof/Rob:
> > > > > > There was a previous attempt at this kind of device. This version looks
> > > > > > better but might be incomplete given there's a bunch more properties in
> > > > > > that patchset:
> > > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > > >
> > > > > Those don't look like properties we want coming back.
> > > >
> > > > Oh, I don't want most of them coming back either. There are some
> > > > supplies in there though that I think we would like to come back, no?
> > > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > > really seem credible.
> > >
> > > We will use Firmware-name in Device Tree.
> >
> > > For power supply settings, because there may be other device using
> > > same regulator.
> >
> > If there are other devices using the same regulator is it more
> > important that you document it so that it doesn't get disabled by the
> > other users.
> >
> > > We plan to define it as an optional property for user to control in
> > > next release.
> >
> > I don't see how the regulator would not be required, the device doesn't
> > function without power.
> >
> > Thanks,
> > Conor.
> 
> I will set power supply as required.
> The description of power supply as below,
> 
> properties:
>   vccd-supply:
>     description: A phandle for the regualtor supplying IO power. Should be own
>                  by TPIC only.

What does "owned by TPIC" only mean? Why would the vccd supply not be
allowed to be shared with another device?

> This works for TP digital IO only, main power is
>                  given by display part VSP/VSN power source which is controlled
>                  by lcm driver.

What drivers control things doesn't really matter here, we're just
describing the hardware. If there's another supply to the controller,
then document it too please.

Thanks,
Conor.
Allen_Lin March 27, 2024, 7:48 a.m. UTC | #9 
Conor Dooley <conor@kernel.org> 於 2024年3月27日 週三 上午3:28寫道：
>
> On Tue, Mar 26, 2024 at 06:40:28PM +0800, Allen Lin wrote:
> > Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
> > >
> > > On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > > > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > > > >
> > > > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > > > touchscreen controller.
> > > > > > > >
> > > > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > > > >
> > > > > > > note to self/Krzysztof/Rob:
> > > > > > > There was a previous attempt at this kind of device. This version looks
> > > > > > > better but might be incomplete given there's a bunch more properties in
> > > > > > > that patchset:
> > > > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > > > >
> > > > > > Those don't look like properties we want coming back.
> > > > >
> > > > > Oh, I don't want most of them coming back either. There are some
> > > > > supplies in there though that I think we would like to come back, no?
> > > > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > > > really seem credible.
> > > >
> > > > We will use Firmware-name in Device Tree.
> > >
> > > > For power supply settings, because there may be other device using
> > > > same regulator.
> > >
> > > If there are other devices using the same regulator is it more
> > > important that you document it so that it doesn't get disabled by the
> > > other users.
> > >
> > > > We plan to define it as an optional property for user to control in
> > > > next release.
> > >
> > > I don't see how the regulator would not be required, the device doesn't
> > > function without power.
> > >
> > > Thanks,
> > > Conor.
> >
> > I will set power supply as required.
> > The description of power supply as below,
> >
> > properties:
> >   vccd-supply:
> >     description: A phandle for the regualtor supplying IO power. Should be own
> >                  by TPIC only.
>
> What does "owned by TPIC" only mean? Why would the vccd supply not be
> allowed to be shared with another device?
>
> > This works for TP digital IO only, main power is
> >                  given by display part VSP/VSN power source which is controlled
> >                  by lcm driver.
>
> What drivers control things doesn't really matter here, we're just
> describing the hardware. If there's another supply to the controller,
> then document it too please.
>

Below is IC power sequence introduction.
https://github.com/HimaxSoftware/Doc/tree/main/Himax_Chipset_Power_Sequence

TDDI IC, which means Touch and Display Driver is integrated in one IC,
So some power supplies will be controlled by Display driver.

In yaml Document, can we just list power supplies controlled by touch driver?

Thanks,
Allen
Conor Dooley March 27, 2024, 4:44 p.m. UTC | #10 
On Wed, Mar 27, 2024 at 03:48:48PM +0800, Allen Lin wrote:
> Conor Dooley <conor@kernel.org> 於 2024年3月27日 週三 上午3:28寫道：
> >
> > On Tue, Mar 26, 2024 at 06:40:28PM +0800, Allen Lin wrote:
> > > Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
> > > >
> > > > On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > > > > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > > > > >
> > > > > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > > > > touchscreen controller.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > > > > >
> > > > > > > > note to self/Krzysztof/Rob:
> > > > > > > > There was a previous attempt at this kind of device. This version looks
> > > > > > > > better but might be incomplete given there's a bunch more properties in
> > > > > > > > that patchset:
> > > > > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > > > > >
> > > > > > > Those don't look like properties we want coming back.
> > > > > >
> > > > > > Oh, I don't want most of them coming back either. There are some
> > > > > > supplies in there though that I think we would like to come back, no?
> > > > > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > > > > really seem credible.
> > > > >
> > > > > We will use Firmware-name in Device Tree.
> > > >
> > > > > For power supply settings, because there may be other device using
> > > > > same regulator.
> > > >
> > > > If there are other devices using the same regulator is it more
> > > > important that you document it so that it doesn't get disabled by the
> > > > other users.
> > > >
> > > > > We plan to define it as an optional property for user to control in
> > > > > next release.
> > > >
> > > > I don't see how the regulator would not be required, the device doesn't
> > > > function without power.
> > > >
> > > > Thanks,
> > > > Conor.
> > >
> > > I will set power supply as required.
> > > The description of power supply as below,
> > >
> > > properties:
> > >   vccd-supply:
> > >     description: A phandle for the regualtor supplying IO power. Should be own
> > >                  by TPIC only.
> >
> > What does "owned by TPIC" only mean? Why would the vccd supply not be
> > allowed to be shared with another device?
> >
> > > This works for TP digital IO only, main power is
> > >                  given by display part VSP/VSN power source which is controlled
> > >                  by lcm driver.
> >
> > What drivers control things doesn't really matter here, we're just
> > describing the hardware. If there's another supply to the controller,
> > then document it too please.
> >
> 
> Below is IC power sequence introduction.
> https://github.com/HimaxSoftware/Doc/tree/main/Himax_Chipset_Power_Sequence
> 
> TDDI IC, which means Touch and Display Driver is integrated in one IC,
> So some power supplies will be controlled by Display driver.

If someone was to turn off the supplies, would the touch component stop
working? The document says that these supplies must be turned on before
the touch supplies, so I'm going to assume that both are needed for the
device to function.

> In yaml Document, can we just list power supplies controlled by touch driver?

If the touch part of this device needs the supplies to function, then
you need to mention them, regardless of where they're controlled. All we
are doing in the binding is describing the hardware. What drivers do
what depends entirely on what software you're using.

Cheers,
Conor.
Allen_Lin March 28, 2024, 6:05 a.m. UTC | #11 
Conor Dooley <conor@kernel.org> 於 2024年3月28日 週四 上午12:44寫道：
>
> On Wed, Mar 27, 2024 at 03:48:48PM +0800, Allen Lin wrote:
> > Conor Dooley <conor@kernel.org> 於 2024年3月27日 週三 上午3:28寫道：
> > >
> > > On Tue, Mar 26, 2024 at 06:40:28PM +0800, Allen Lin wrote:
> > > > Conor Dooley <conor.dooley@microchip.com> 於 2024年3月26日 週二 下午4:48寫道：
> > > > >
> > > > > On Tue, Mar 26, 2024 at 01:46:56PM +0800, Allen Lin wrote:
> > > > > > Conor Dooley <conor@kernel.org> 於 2024年3月23日 週六 上午2:34寫道：
> > > > > > >
> > > > > > > On Fri, Mar 22, 2024 at 01:30:09PM -0500, Rob Herring wrote:
> > > > > > > > On Fri, Mar 22, 2024 at 05:54:08PM +0000, Conor Dooley wrote:
> > > > > > > > > On Fri, Mar 22, 2024 at 04:56:03PM +0800, Allen_Lin wrote:
> > > > > > > > > > Add the HX83102j touchscreen device tree bindings documents.
> > > > > > > > > > HX83102j is a Himax TDDI touchscreen controller.
> > > > > > > > > > It's power sequence should be bound with a lcm driver, thus it
> > > > > > > > > > needs to be a panel follower. Others are the same as normal SPI
> > > > > > > > > > touchscreen controller.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> > > > > > > > >
> > > > > > > > > note to self/Krzysztof/Rob:
> > > > > > > > > There was a previous attempt at this kind of device. This version looks
> > > > > > > > > better but might be incomplete given there's a bunch more properties in
> > > > > > > > > that patchset:
> > > > > > > > > https://lore.kernel.org/all/20231017091900.801989-1-tylor_yang@himax.corp-partner.google.com/
> > > > > > > >
> > > > > > > > Those don't look like properties we want coming back.
> > > > > > >
> > > > > > > Oh, I don't want most of them coming back either. There are some
> > > > > > > supplies in there though that I think we would like to come back, no?
> > > > > > > Maybe this particular device doesn't have any supplies, but that doesn't
> > > > > > > really seem credible.
> > > > > >
> > > > > > We will use Firmware-name in Device Tree.
> > > > >
> > > > > > For power supply settings, because there may be other device using
> > > > > > same regulator.
> > > > >
> > > > > If there are other devices using the same regulator is it more
> > > > > important that you document it so that it doesn't get disabled by the
> > > > > other users.
> > > > >
> > > > > > We plan to define it as an optional property for user to control in
> > > > > > next release.
> > > > >
> > > > > I don't see how the regulator would not be required, the device doesn't
> > > > > function without power.
> > > > >
> > > > > Thanks,
> > > > > Conor.
> > > >
> > > > I will set power supply as required.
> > > > The description of power supply as below,
> > > >
> > > > properties:
> > > >   vccd-supply:
> > > >     description: A phandle for the regualtor supplying IO power. Should be own
> > > >                  by TPIC only.
> > >
> > > What does "owned by TPIC" only mean? Why would the vccd supply not be
> > > allowed to be shared with another device?
> > >
> > > > This works for TP digital IO only, main power is
> > > >                  given by display part VSP/VSN power source which is controlled
> > > >                  by lcm driver.
> > >
> > > What drivers control things doesn't really matter here, we're just
> > > describing the hardware. If there's another supply to the controller,
> > > then document it too please.
> > >
> >
> > Below is IC power sequence introduction.
> > https://github.com/HimaxSoftware/Doc/tree/main/Himax_Chipset_Power_Sequence
> >
> > TDDI IC, which means Touch and Display Driver is integrated in one IC,
> > So some power supplies will be controlled by Display driver.
>
> If someone was to turn off the supplies, would the touch component stop
> working? The document says that these supplies must be turned on before
> the touch supplies, so I'm going to assume that both are needed for the
> device to function.
>
> > In yaml Document, can we just list power supplies controlled by touch driver?
>
> If the touch part of this device needs the supplies to function, then
> you need to mention them, regardless of where they're controlled. All we
> are doing in the binding is describing the hardware. What drivers do
> what depends entirely on what software you're using.
>

OK, I will list all supplies required by the driver in the Yaml
document as shown below,

properties:
  compatible:
    const: himax,hx83102j

  reg:
    maxItems: 1

  interrupts:
    maxItems: 1

  reset-gpios:
    maxItems: 1

  vccd-supply:
    description: A phandle for the regualtor supplying IO power.

  vsn-supply:
    description: Negative supply regulator.

  vsp-supply:
    description: Positive supply regulator.

  ddreset-gpios:
    description: A phandle of gpio for display reset controlled by the
LCD driver.

required:
  - compatible
  - reg
  - interrupts
  - reset-gpios
  - panel
  - vccd-supply
  - vsn-supply
  - vsp-supply
  - ddreset-gpios


Thanks,
Allen
Conor Dooley March 28, 2024, 6:05 p.m. UTC | #12 
On Thu, Mar 28, 2024 at 02:05:17PM +0800, Allen Lin wrote:

> OK, I will list all supplies required by the driver in the Yaml
> document as shown below,
> 
> properties:
>   compatible:
>     const: himax,hx83102j
> 
>   reg:
>     maxItems: 1
> 
>   interrupts:
>     maxItems: 1
> 
>   reset-gpios:
>     maxItems: 1
> 
>   vccd-supply:
>     description: A phandle for the regualtor supplying IO power.
> 
>   vsn-supply:
>     description: Negative supply regulator.
> 
>   vsp-supply:
>     description: Positive supply regulator.
> 
>   ddreset-gpios:

I think this should be s/dd// with the description explaining how this
display reset is related to this device.

Otherwise, this looks okay to me.

On another note - every time you reply to me I get it 3 times. Can you
fix that please?

Thanks,
Conor.

>     description: A phandle of gpio for display reset controlled by the
> LCD driver.
> 
> required:
>   - compatible
>   - reg
>   - interrupts
>   - reset-gpios
>   - panel
>   - vccd-supply
>   - vsn-supply
>   - vsp-supply
>   - ddreset-gpios
> 
> 
> Thanks,
> Allen
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
new file mode 100644
index 000000000000..6c0a1ebf8d91
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
@@ -0,0 +1,73 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/himax,hx83102j.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Himax hx83102j touchscreen
+
+maintainers:
+  - Allen Lin <allencl_lin@hotmail.com>
+
+description:
+  This Himax hx83102j touchscreen uses the spi protocol.
+
+allOf:
+  - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: himax,hx83102j
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  spi-max-frequency: true
+
+  panel: true
+
+  himax,pid:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      PID for HID device, used to load correct firmware.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - panel
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      ap_ts: touchscreen@0 {
+        compatible = "himax,hx83102j";
+        reg = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&touch_int0 &touch_reset>;
+        reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
+        spi-cpha;
+        spi-cpol;
+        interrupt-parent = <&gpio1>;
+        interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+        panel = <&panel>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 43b39956694a..aa51c60fd66d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9669,6 +9669,12 @@  L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	drivers/misc/hisi_hikey_usb.c
 
+HIMAX HID HX83102J TOUCHSCREEN
+M:	Allen Lin <allencl_lin@hotmail.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
+
 HIMAX HX83112B TOUCHSCREEN SUPPORT
 M:	Job Noorman <job@noorman.info>
 L:	linux-input@vger.kernel.org