| Message ID | 20230919024943.3088916-2-tylor_yang@himax.corp-partner.google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HID: touchscreen: add himax hid-over-spi driver | expand |
Hey, On Tue, Sep 19, 2023 at 10:49:42AM +0800, Tylor Yang wrote: > The Himax HID-over-SPI framework support for Himax touchscreen ICs > that report HID packet through SPI bus. The driver core need reset > pin to meet reset timing spec. of IC. An interrupt pin to disable > and enable interrupt when suspend/resume. An optional power control > pin if target board needed. Panel id pins for identify panel is also > an option. > > Additional optional arguments: > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime > conditions. Runtime conditions? Aren't thєse properties of the panel & therefore fixed? If they were runtime conditions, then setting them statically in your DT is not going to work, right? > > This patch also add maintainer of this driver. > > Signed-off-by: Tylor Yang <tylor_yang@himax.corp-partner.google.com> It looks like you deleted all of the properties from the previous submission of these changes. I don't really understand that, it kinda feels just like appeasement, as you must have needed those properties to do the firmware loading etc. How are you filling the gap those properties have left, when you still only have a single compatible string in thㄟs binding? Is there a way to do runtime detection of which chip you're dealing with that you are now using? Confused, Conor. > --- > .../bindings/input/himax,hid-over-spi.yaml | 109 ++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 115 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > diff --git a/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > new file mode 100644 > index 000000000000..3ee3a89842ac > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > @@ -0,0 +1,109 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/himax,hid-over-spi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Himax TDDI devices using SPI to send HID packets > + > +maintainers: > + - Tylor Yang <tylor_yang@himax.corp-partner.google.com> > + > +description: | > + Support the Himax TDDI devices which using SPI interface to acquire > + HID packets from the device. The device needs to be initialized using > + Himax protocol before it start sending HID packets. > + > +properties: > + compatible: > + const: himax,hid-over-spi > + > + reg: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + interrupts: > + maxItems: 1 > + > + himax,rst-gpio: > + maxItems: 1 > + description: Reset device, active low signal. > + > + himax,irq-gpio: > + maxItems: 1 > + description: Interrupt request, active low signal. > + > + himax,3v3-gpio: > + maxItems: 1 > + description: GPIO to control 3.3V power supply. > + > + himax,id-gpios: > + maxItems: 8 > + description: GPIOs to read physical Panel ID. Optional. > + > + spi-cpha: true > + spi-cpol: true > + > + himax,ic-det-delay-ms: > + description: > + Due to TDDI properties, the TPIC detection timing must after the > + display panel initialized. This property is used to specify the > + delay time when TPIC detection and display panel initialization > + timing are overlapped. How much milliseconds to delay before TPIC > + detection start. > + > + himax,ic-resume-delay-ms: > + description: > + Due to TDDI properties, the TPIC resume timing must after the > + display panel resumed. This property is used to specify the > + delay time when TPIC resume and display panel resume > + timing are overlapped. How much milliseconds to delay before TPIC > + resume start. > + > +required: > + - compatible > + - reg > + - interrupts > + - himax,rst-gpio > + - himax,irq-gpio > + > +unevaluatedProperties: false > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/gpio/gpio.h> > + > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + touchscreen@0 { > + compatible = "himax,hid-over-spi"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x0>; > + interrupt-parent = <&gpio1>; > + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; > + pinctrl-0 = <&touch_pins>; > + pinctrl-names = "default"; > + > + spi-max-frequency = <12500000>; > + spi-cpha; > + spi-cpol; > + > + himax,rst-gpio = <&gpio1 8 GPIO_ACTIVE_LOW>; > + himax,irq-gpio = <&gpio1 7 GPIO_ACTIVE_LOW>; > + himax,3v3-gpio = <&gpio1 6 GPIO_ACTIVE_HIGH>; > + himax,ic-det-delay-ms = <500>; > + himax,ic-resume-delay-ms = <100>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index bf0f54c24f81..452701261bec 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9323,6 +9323,12 @@ L: linux-kernel@vger.kernel.org > S: Maintained > F: drivers/misc/hisi_hikey_usb.c > > +HIMAX HID OVER SPI TOUCHSCREEN SUPPORT > +M: Tylor Yang <tylor_yang@himax.corp-partner.google.com> > +L: linux-input@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > + > HIMAX HX83112B TOUCHSCREEN SUPPORT > M: Job Noorman <job@noorman.info> > L: linux-input@vger.kernel.org > -- > 2.25.1 >
On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > Hi Conor, > > > > Additional optional arguments: > > > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime > > > conditions. > > > Runtime conditions? Aren't thєse properties of the panel & therefore > > fixed? If they were runtime conditions, then setting them statically in > > your DT is not going to work, right? > > Because each platform's display driver ready time is different. TP part > need to avoid this timing by measuring the waveform of LCD reset pin > low period and TP probe timing. For example, if LCD rst pin low from > timestamp 100 to 800, TP driver probe at 600. TP probe will fail. Then > user should set ic-det-delay-ms bigger than 200, to avoid LCD rst low > timing. As you can see, the timing needs to be measured at runtime to > decide how long it should be. Then, if the condition is not changed, the > value could keep the same. That sounds to me like something you would test once for a given platform and then the values are static. If you are actually changing it at *runtime*, how is doing it through DT suitable? Does your firmware do the tests & then set the values in DT dynamically? > > > It looks like you deleted all of the properties from the previous > > submission of these changes. I don't really understand that, it kinda > > feels just like appeasement, as you must have needed those properties > > to do the firmware loading etc. How are you filling the gap those > > properties have left, when you still only have a single compatible > > string in thㄟs binding? Is there a way to do runtime detection of which > > chip you're dealing with that you are now using? > > After reviewing, I found the properties could go to IC driver settings : > "himax,heatmap_16bits" because it depends on IC's ability; How do you detect the IC's abilities? > Some > could remove and use default values: "himax,fw_size", > "himax,boot_time_fw_upgrade". "himax,fw_size" has a default value in > IC settings, and likely won't change in this IC. Okay. > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > should be removed. "himax,fw_in_flash", I use the kernel config for > user to select. That seems like a bad idea, we want to be able to build one kernel that works for all hardware at the same time. > "himax,pid" could be remove and use default firmware name > "himax_i2chid.bin" to load. It was added because users may desire to > choose a special name like "himax_i2chid_{pid}.bin" instead of the default > one. > It also could be replaced with newly added "himax",id-gpios" which is still > experimental. Also, pleae don't top post, but instead reply in-line with my comments, as I have done here. > Btw, I encounter an error of patch [2/2], which says: > BOUNCE linux-input@vger.kernel.org: Message too long (>100000 chars) > and the patch didn't appear at patchwork.kernel.org. What should I do to > deal with this problem? No idea. Maybe try to split it into multiple patches? The other option is to also cc patches@lists.linux.dev as that has some higher capacities, but that's not going to be a silver bullet. Thanks, Conor. > > > Thanks, > Tylor > > > On Tue, Sep 19, 2023 at 4:41 PM Conor Dooley <conor@kernel.org> wrote: > > > Hey, > > > > > > On Tue, Sep 19, 2023 at 10:49:42AM +0800, Tylor Yang wrote: > > > The Himax HID-over-SPI framework support for Himax touchscreen ICs > > > that report HID packet through SPI bus. The driver core need reset > > > pin to meet reset timing spec. of IC. An interrupt pin to disable > > > and enable interrupt when suspend/resume. An optional power control > > > pin if target board needed. Panel id pins for identify panel is also > > > an option. > > > > > > Additional optional arguments: > > > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime > > > conditions. > > > > Runtime conditions? Aren't thєse properties of the panel & therefore > > fixed? If they were runtime conditions, then setting them statically in > > your DT is not going to work, right? > > > > > > > > This patch also add maintainer of this driver. > > > > > > Signed-off-by: Tylor Yang <tylor_yang@himax.corp-partner.google.com> > > > > It looks like you deleted all of the properties from the previous > > submission of these changes. I don't really understand that, it kinda > > feels just like appeasement, as you must have needed those properties > > to do the firmware loading etc. How are you filling the gap those > > properties have left, when you still only have a single compatible > > string in thㄟs binding? Is there a way to do runtime detection of which > > chip you're dealing with that you are now using? > > > > Confused, > > Conor. > > > > > --- > > > .../bindings/input/himax,hid-over-spi.yaml | 109 ++++++++++++++++++ > > > MAINTAINERS | 6 + > > > 2 files changed, 115 insertions(+) > > > create mode 100644 > > Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > > > > > diff --git > > a/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > > new file mode 100644 > > > index 000000000000..3ee3a89842ac > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > > @@ -0,0 +1,109 @@ > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/input/himax,hid-over-spi.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Himax TDDI devices using SPI to send HID packets > > > + > > > +maintainers: > > > + - Tylor Yang <tylor_yang@himax.corp-partner.google.com> > > > + > > > +description: | > > > + Support the Himax TDDI devices which using SPI interface to acquire > > > + HID packets from the device. The device needs to be initialized using > > > + Himax protocol before it start sending HID packets. > > > + > > > +properties: > > > + compatible: > > > + const: himax,hid-over-spi > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + '#address-cells': > > > + const: 1 > > > + > > > + '#size-cells': > > > + const: 0 > > > + > > > + interrupts: > > > + maxItems: 1 > > > + > > > + himax,rst-gpio: > > > + maxItems: 1 > > > + description: Reset device, active low signal. > > > + > > > + himax,irq-gpio: > > > + maxItems: 1 > > > + description: Interrupt request, active low signal. > > > + > > > + himax,3v3-gpio: > > > + maxItems: 1 > > > + description: GPIO to control 3.3V power supply. > > > + > > > + himax,id-gpios: > > > + maxItems: 8 > > > + description: GPIOs to read physical Panel ID. Optional. > > > + > > > + spi-cpha: true > > > + spi-cpol: true > > > + > > > + himax,ic-det-delay-ms: > > > + description: > > > + Due to TDDI properties, the TPIC detection timing must after the > > > + display panel initialized. This property is used to specify the > > > + delay time when TPIC detection and display panel initialization > > > + timing are overlapped. How much milliseconds to delay before TPIC > > > + detection start. > > > + > > > + himax,ic-resume-delay-ms: > > > + description: > > > + Due to TDDI properties, the TPIC resume timing must after the > > > + display panel resumed. This property is used to specify the > > > + delay time when TPIC resume and display panel resume > > > + timing are overlapped. How much milliseconds to delay before TPIC > > > + resume start. > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - interrupts > > > + - himax,rst-gpio > > > + - himax,irq-gpio > > > + > > > +unevaluatedProperties: false > > > + > > > +allOf: > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/interrupt-controller/irq.h> > > > + #include <dt-bindings/gpio/gpio.h> > > > + > > > + spi { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + touchscreen@0 { > > > + compatible = "himax,hid-over-spi"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + reg = <0x0>; > > > + interrupt-parent = <&gpio1>; > > > + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; > > > + pinctrl-0 = <&touch_pins>; > > > + pinctrl-names = "default"; > > > + > > > + spi-max-frequency = <12500000>; > > > + spi-cpha; > > > + spi-cpol; > > > + > > > + himax,rst-gpio = <&gpio1 8 GPIO_ACTIVE_LOW>; > > > + himax,irq-gpio = <&gpio1 7 GPIO_ACTIVE_LOW>; > > > + himax,3v3-gpio = <&gpio1 6 GPIO_ACTIVE_HIGH>; > > > + himax,ic-det-delay-ms = <500>; > > > + himax,ic-resume-delay-ms = <100>; > > > + }; > > > + }; > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index bf0f54c24f81..452701261bec 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -9323,6 +9323,12 @@ L: linux-kernel@vger.kernel.org > > > S: Maintained > > > F: drivers/misc/hisi_hikey_usb.c > > > > > > +HIMAX HID OVER SPI TOUCHSCREEN SUPPORT > > > +M: Tylor Yang <tylor_yang@himax.corp-partner.google.com> > > > +L: linux-input@vger.kernel.org > > > +S: Supported > > > +F: Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > > + > > > HIMAX HX83112B TOUCHSCREEN SUPPORT > > > M: Job Noorman <job@noorman.info> > > > L: linux-input@vger.kernel.org > > > -- > > > 2.25.1 > > > > >
On Tue, Sep 19, 2023 at 10:49:42AM +0800, Tylor Yang wrote: > The Himax HID-over-SPI framework support for Himax touchscreen ICs > that report HID packet through SPI bus. The driver core need reset > pin to meet reset timing spec. of IC. An interrupt pin to disable > and enable interrupt when suspend/resume. An optional power control > pin if target board needed. Panel id pins for identify panel is also > an option. > > Additional optional arguments: > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime > conditions. > > This patch also add maintainer of this driver. > > Signed-off-by: Tylor Yang <tylor_yang@himax.corp-partner.google.com> > --- > .../bindings/input/himax,hid-over-spi.yaml | 109 ++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 115 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > > diff --git a/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > new file mode 100644 > index 000000000000..3ee3a89842ac > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml > @@ -0,0 +1,109 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/himax,hid-over-spi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Himax TDDI devices using SPI to send HID packets > + > +maintainers: > + - Tylor Yang <tylor_yang@himax.corp-partner.google.com> > + > +description: | > + Support the Himax TDDI devices which using SPI interface to acquire > + HID packets from the device. The device needs to be initialized using > + Himax protocol before it start sending HID packets. > + > +properties: > + compatible: > + const: himax,hid-over-spi Doesn't look like a specific device. Compatibles are generally based on part numbers. 'over-spi' is redundant as the parent would be a spi controller. > + > + reg: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 These are for child nodes, but you don't have any. > + > + interrupts: > + maxItems: 1 > + > + himax,rst-gpio: > + maxItems: 1 > + description: Reset device, active low signal. Use standard reset-gpios. (-gpio is deprecated) > + > + himax,irq-gpio: > + maxItems: 1 > + description: Interrupt request, active low signal. You have the interrupt already, why do you need this? > + > + himax,3v3-gpio: > + maxItems: 1 > + description: GPIO to control 3.3V power supply. This should be a regulator supply. Then use gpio-regulator if it happens to be GPIO controlled. > + > + himax,id-gpios: > + maxItems: 8 > + description: GPIOs to read physical Panel ID. Optional. > + > + spi-cpha: true > + spi-cpol: true > + > + himax,ic-det-delay-ms: > + description: > + Due to TDDI properties, the TPIC detection timing must after the > + display panel initialized. This property is used to specify the > + delay time when TPIC detection and display panel initialization > + timing are overlapped. How much milliseconds to delay before TPIC > + detection start. > + > + himax,ic-resume-delay-ms: > + description: > + Due to TDDI properties, the TPIC resume timing must after the > + display panel resumed. This property is used to specify the > + delay time when TPIC resume and display panel resume > + timing are overlapped. How much milliseconds to delay before TPIC > + resume start. These should be implied by the compatible. Unless they are board specific and not device specific. Rob
On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > Hi Conor, > > > > > > Additional optional arguments: > > > > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime > > > > conditions. > > > > > Runtime conditions? Aren't thєse properties of the panel & therefore > > > fixed? If they were runtime conditions, then setting them statically in > > > your DT is not going to work, right? > > > > Because each platform's display driver ready time is different. TP part > > need to avoid this timing by measuring the waveform of LCD reset pin > > low period and TP probe timing. For example, if LCD rst pin low from > > timestamp 100 to 800, TP driver probe at 600. TP probe will fail. Then > > user should set ic-det-delay-ms bigger than 200, to avoid LCD rst low > > timing. As you can see, the timing needs to be measured at runtime to > > decide how long it should be. Then, if the condition is not changed, the > > value could keep the same. > > That sounds to me like something you would test once for a given > platform and then the values are static. If you are actually changing it > at *runtime*, how is doing it through DT suitable? Does your firmware do > the tests & then set the values in DT dynamically? > Yes, you are right. I'll change the description. > > > > > It looks like you deleted all of the properties from the previous > > > submission of these changes. I don't really understand that, it kinda > > > feels just like appeasement, as you must have needed those properties > > > to do the firmware loading etc. How are you filling the gap those > > > properties have left, when you still only have a single compatible > > > string in thㄟs binding? Is there a way to do runtime detection of which > > > chip you're dealing with that you are now using? > > > > After reviewing, I found the properties could go to IC driver settings : > > "himax,heatmap_16bits" because it depends on IC's ability; > > How do you detect the IC's abilities? > The driver code has a part of IC detect process, and each IC has its own driver code to define its abilities. This part moves to that position. > > Some > > could remove and use default values: "himax,fw_size", > > "himax,boot_time_fw_upgrade". "himax,fw_size" has a default value in > > IC settings, and likely won't change in this IC. > > Okay. > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > should be removed. "himax,fw_in_flash", I use the kernel config for > > user to select. > > That seems like a bad idea, we want to be able to build one kernel that > works for all hardware at the same time. > I see, so I should take that back? I'll explain more about it. > > "himax,pid" could be remove and use default firmware name > > "himax_i2chid.bin" to load. It was added because users may desire to > > choose a special name like "himax_i2chid_{pid}.bin" instead of the default > > one. > > It also could be replaced with newly added "himax",id-gpios" which is still > > experimental. > > Also, pleae don't top post, but instead reply in-line with my comments, > as I have done here. > Ok. > > Btw, I encounter an error of patch [2/2], which says: > > BOUNCE linux-input@vger.kernel.org: Message too long (>100000 chars) > > and the patch didn't appear at patchwork.kernel.org. What should I do to > > deal with this problem? > > No idea. Maybe try to split it into multiple patches? > The other option is to also cc patches@lists.linux.dev as that has some > higher capacities, but that's not going to be a silver bullet. Thanks for the reply. I'll try multiple commits to reduce the size. Thanks, Tylor
On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > user to select. > > > > That seems like a bad idea, we want to be able to build one kernel that > > works for all hardware at the same time. > > > I see, so I should take that back? > I'll explain more about it. Are there particular ICs where the firmware would always be in flash and others where it would never be? Or is this a choice made by the board or system designer? Thanks, Conor.
On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > user to select. > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > works for all hardware at the same time. > > > > > I see, so I should take that back? > > I'll explain more about it. > > Are there particular ICs where the firmware would always be in flash and > others where it would never be? Or is this a choice made by the board or > system designer? > Most cases it's about the system designer's decision. But some ICs may be forced to use flash because of its architecture(multiple IC inside, need to load firmware to multiple IC's sram by master IC). But if there is no limitation on this part, most system designers will prefer flashless. > Thanks, > Conor. Thanks, Tylor
On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > user to select. > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > works for all hardware at the same time. > > > > > > > I see, so I should take that back? > > > I'll explain more about it. > > > > Are there particular ICs where the firmware would always be in flash and > > others where it would never be? Or is this a choice made by the board or > > system designer? > > > Most cases it's about the system designer's decision. But some ICs may be forced > to use flash because of its architecture(multiple IC inside, need to > load firmware to > multiple IC's sram by master IC). But if there is no limitation on > this part, most system > designers will prefer flashless. Forgive me if I am not understanding correctly, there are some ICs that will need to load the firmware from flash and there are some where it will be a decision made by the designer of the board. Is the flash part of the IC or is it an external flash chip? Cheers, Conor.
On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > user to select. > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > works for all hardware at the same time. > > > > > > > > > I see, so I should take that back? > > > > I'll explain more about it. > > > > > > Are there particular ICs where the firmware would always be in flash and > > > others where it would never be? Or is this a choice made by the board or > > > system designer? > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > to use flash because of its architecture(multiple IC inside, need to > > load firmware to > > multiple IC's sram by master IC). But if there is no limitation on > > this part, most system > > designers will prefer flashless. > > Forgive me if I am not understanding correctly, there are some ICs that > will need to load the firmware from flash and there are some where it > will be a decision made by the designer of the board. Is the flash part > of the IC or is it an external flash chip? > Both are possible, it depends on the IC type. For TDDI, the IC is long and thin, placed on panel PCB, flash will be located at the external flash chip. For the OLED TP, IC is usually placed at FPC and its flash is embedded, thus the IC size is large compared to TDDI. But from the driver's perspective either external flash or embedded flash, the IC itself will load firmware from flash automatically when reset pin is released. Only if firmware is loading from the host storage system, the driver needs to operate the IC in detail. > Cheers, > Conor. Thanks, Tylor
On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > user to select. > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > works for all hardware at the same time. > > > > > > > > > > > I see, so I should take that back? > > > > > I'll explain more about it. > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > others where it would never be? Or is this a choice made by the board or > > > > system designer? > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > to use flash because of its architecture(multiple IC inside, need to > > > load firmware to > > > multiple IC's sram by master IC). But if there is no limitation on > > > this part, most system > > > designers will prefer flashless. > > > > Forgive me if I am not understanding correctly, there are some ICs that > > will need to load the firmware from flash and there are some where it > > will be a decision made by the designer of the board. Is the flash part > > of the IC or is it an external flash chip? > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > and thin, placed on panel PCB, flash will be located at the external > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > is embedded, thus the IC size is large compared to TDDI. But from the > driver's perspective either external flash or embedded flash, the IC > itself will load firmware from flash automatically when reset pin is > released. Only if firmware is loading from the host storage system, > the driver needs to operate the IC in detail. Since there are ICs that can use the external flash or have it loaded from the host, it sounds like you do need a property to differentiate between those cases. Is it sufficient to just set the firmware-name property for these cases? If the property exists, then you know you need to load firmware & what its name is. If it doesn't, then the firmware either isn't needed or will be automatically loaded from the external flash.
On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > > user to select. > > > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > > works for all hardware at the same time. > > > > > > > > > > > > > I see, so I should take that back? > > > > > > I'll explain more about it. > > > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > > others where it would never be? Or is this a choice made by the board or > > > > > system designer? > > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > > to use flash because of its architecture(multiple IC inside, need to > > > > load firmware to > > > > multiple IC's sram by master IC). But if there is no limitation on > > > > this part, most system > > > > designers will prefer flashless. > > > > > > Forgive me if I am not understanding correctly, there are some ICs that > > > will need to load the firmware from flash and there are some where it > > > will be a decision made by the designer of the board. Is the flash part > > > of the IC or is it an external flash chip? > > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > > and thin, placed on panel PCB, flash will be located at the external > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > > is embedded, thus the IC size is large compared to TDDI. But from the > > driver's perspective either external flash or embedded flash, the IC > > itself will load firmware from flash automatically when reset pin is > > released. Only if firmware is loading from the host storage system, > > the driver needs to operate the IC in detail. > > > Since there are ICs that can use the external flash or have it loaded > from the host, it sounds like you do need a property to differentiate > between those cases. Yep. > Is it sufficient to just set the firmware-name property for these cases? > If the property exists, then you know you need to load firmware & what > its name is. If it doesn't, then the firmware either isn't needed or > will be automatically loaded from the external flash. We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. So we'll look for it when no-flash-flag is specified. In our experience, forcing a prefix firmware name helps the user to aware what firmware they are dealing with. Thanks, Tylor
On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > > > user to select. > > > > > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > > > works for all hardware at the same time. > > > > > > > > > > > > > > > I see, so I should take that back? > > > > > > > I'll explain more about it. > > > > > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > > > others where it would never be? Or is this a choice made by the board or > > > > > > system designer? > > > > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > > > to use flash because of its architecture(multiple IC inside, need to > > > > > load firmware to > > > > > multiple IC's sram by master IC). But if there is no limitation on > > > > > this part, most system > > > > > designers will prefer flashless. > > > > > > > > Forgive me if I am not understanding correctly, there are some ICs that > > > > will need to load the firmware from flash and there are some where it > > > > will be a decision made by the designer of the board. Is the flash part > > > > of the IC or is it an external flash chip? > > > > > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > > > and thin, placed on panel PCB, flash will be located at the external > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > > > is embedded, thus the IC size is large compared to TDDI. But from the > > > driver's perspective either external flash or embedded flash, the IC > > > itself will load firmware from flash automatically when reset pin is > > > released. Only if firmware is loading from the host storage system, > > > the driver needs to operate the IC in detail. > > > > > > Since there are ICs that can use the external flash or have it loaded > > from the host, it sounds like you do need a property to differentiate > > between those cases. > Yep. > > > Is it sufficient to just set the firmware-name property for these cases? > > If the property exists, then you know you need to load firmware & what > > its name is. If it doesn't, then the firmware either isn't needed or > > will be automatically loaded from the external flash. > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. How do you intend generating the name of the firmware file? I assume the same firmware doesn't work on every IC, so you'll need to pick a different one depending on the compatible? > So we'll look for it when no-flash-flag is specified. In our experience, > forcing a prefix firmware name helps the user to aware what firmware > they are dealing with.
On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > > > > user to select. > > > > > > > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > > > > works for all hardware at the same time. > > > > > > > > > > > > > > > > > I see, so I should take that back? > > > > > > > > I'll explain more about it. > > > > > > > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > > > > others where it would never be? Or is this a choice made by the board or > > > > > > > system designer? > > > > > > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > > > > to use flash because of its architecture(multiple IC inside, need to > > > > > > load firmware to > > > > > > multiple IC's sram by master IC). But if there is no limitation on > > > > > > this part, most system > > > > > > designers will prefer flashless. > > > > > > > > > > Forgive me if I am not understanding correctly, there are some ICs that > > > > > will need to load the firmware from flash and there are some where it > > > > > will be a decision made by the designer of the board. Is the flash part > > > > > of the IC or is it an external flash chip? > > > > > > > > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > > > > and thin, placed on panel PCB, flash will be located at the external > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > > > > is embedded, thus the IC size is large compared to TDDI. But from the > > > > driver's perspective either external flash or embedded flash, the IC > > > > itself will load firmware from flash automatically when reset pin is > > > > released. Only if firmware is loading from the host storage system, > > > > the driver needs to operate the IC in detail. > > > > > > > > > Since there are ICs that can use the external flash or have it loaded > > > from the host, it sounds like you do need a property to differentiate > > > between those cases. > > Yep. > > > > > Is it sufficient to just set the firmware-name property for these cases? > > > If the property exists, then you know you need to load firmware & what > > > its name is. If it doesn't, then the firmware either isn't needed or > > > will be automatically loaded from the external flash. > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > How do you intend generating the name of the firmware file? I assume the > same firmware doesn't work on every IC, so you'll need to pick a > different one depending on the compatible? > If considering a firmware library line-up for all the incoming panels of this driver. We would use PID as part of the file name. Because all the support panels would have a unique PID associated. Which will make the firmware name like himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load at no flash condition. Thus PID information is required in dts when no-flash-flag is specified. > > So we'll look for it when no-flash-flag is specified. In our experience, > > forcing a prefix firmware name helps the user to aware what firmware > > they are dealing with. If a more simple solution for no-flash condition is needed, as you mentioned, specifying a firmware name in dts would be the best. Otherwise, a no-flash-flag and PID information needs to be added in dts. Thanks, Tylor
On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > > > > > user to select. > > > > > > > > > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > > > > > works for all hardware at the same time. > > > > > > > > > > > > > > > > > > > I see, so I should take that back? > > > > > > > > > I'll explain more about it. > > > > > > > > > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > > > > > others where it would never be? Or is this a choice made by the board or > > > > > > > > system designer? > > > > > > > > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > > > > > to use flash because of its architecture(multiple IC inside, need to > > > > > > > load firmware to > > > > > > > multiple IC's sram by master IC). But if there is no limitation on > > > > > > > this part, most system > > > > > > > designers will prefer flashless. > > > > > > > > > > > > Forgive me if I am not understanding correctly, there are some ICs that > > > > > > will need to load the firmware from flash and there are some where it > > > > > > will be a decision made by the designer of the board. Is the flash part > > > > > > of the IC or is it an external flash chip? > > > > > > > > > > > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > > > > > and thin, placed on panel PCB, flash will be located at the external > > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > > > > > is embedded, thus the IC size is large compared to TDDI. But from the > > > > > driver's perspective either external flash or embedded flash, the IC > > > > > itself will load firmware from flash automatically when reset pin is > > > > > released. Only if firmware is loading from the host storage system, > > > > > the driver needs to operate the IC in detail. > > > > > > > > > > > > Since there are ICs that can use the external flash or have it loaded > > > > from the host, it sounds like you do need a property to differentiate > > > > between those cases. > > > Yep. > > > > > > > Is it sufficient to just set the firmware-name property for these cases? > > > > If the property exists, then you know you need to load firmware & what > > > > its name is. If it doesn't, then the firmware either isn't needed or > > > > will be automatically loaded from the external flash. > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > How do you intend generating the name of the firmware file? I assume the > > same firmware doesn't work on every IC, so you'll need to pick a > > different one depending on the compatible? > > > If considering a firmware library line-up for all the incoming panels > of this driver. > We would use PID as part of the file name. Because all the support panels would > have a unique PID associated. Which will make the firmware name like > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > at no flash condition. Thus PID information is required in dts when > no-flash-flag > is specified. Firstly, where does the "xxx" come from? And you're making it sound more like having firmware-name is suitable for this use case, given you need to determine the name of the file to use based on something that is hardware specific but is not dynamically detectable. Thanks, Conor. > > > So we'll look for it when no-flash-flag is specified. In our experience, > > > forcing a prefix firmware name helps the user to aware what firmware > > > they are dealing with. > > If a more simple solution for no-flash condition is needed, as you mentioned, > specifying a firmware name in dts would be the best. Otherwise, a > no-flash-flag and > PID information needs to be added in dts.
On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > > > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > > > > > > user to select. > > > > > > > > > > > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > > > > > > works for all hardware at the same time. > > > > > > > > > > > > > > > > > > > > > I see, so I should take that back? > > > > > > > > > > I'll explain more about it. > > > > > > > > > > > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > > > > > > others where it would never be? Or is this a choice made by the board or > > > > > > > > > system designer? > > > > > > > > > > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > > > > > > to use flash because of its architecture(multiple IC inside, need to > > > > > > > > load firmware to > > > > > > > > multiple IC's sram by master IC). But if there is no limitation on > > > > > > > > this part, most system > > > > > > > > designers will prefer flashless. > > > > > > > > > > > > > > Forgive me if I am not understanding correctly, there are some ICs that > > > > > > > will need to load the firmware from flash and there are some where it > > > > > > > will be a decision made by the designer of the board. Is the flash part > > > > > > > of the IC or is it an external flash chip? > > > > > > > > > > > > > > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > > > > > > and thin, placed on panel PCB, flash will be located at the external > > > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > > > > > > is embedded, thus the IC size is large compared to TDDI. But from the > > > > > > driver's perspective either external flash or embedded flash, the IC > > > > > > itself will load firmware from flash automatically when reset pin is > > > > > > released. Only if firmware is loading from the host storage system, > > > > > > the driver needs to operate the IC in detail. > > > > > > > > > > > > > > > Since there are ICs that can use the external flash or have it loaded > > > > > from the host, it sounds like you do need a property to differentiate > > > > > between those cases. > > > > Yep. > > > > > > > > > Is it sufficient to just set the firmware-name property for these cases? > > > > > If the property exists, then you know you need to load firmware & what > > > > > its name is. If it doesn't, then the firmware either isn't needed or > > > > > will be automatically loaded from the external flash. > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > How do you intend generating the name of the firmware file? I assume the > > > same firmware doesn't work on every IC, so you'll need to pick a > > > different one depending on the compatible? > > > > > If considering a firmware library line-up for all the incoming panels > > of this driver. > > We would use PID as part of the file name. Because all the support panels would > > have a unique PID associated. Which will make the firmware name like > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > at no flash condition. Thus PID information is required in dts when > > no-flash-flag > > is specified. > > Firstly, where does the "xxx" come from? > And you're making it sound more like having firmware-name is suitable > for this use case, given you need to determine the name of the file to > use based on something that is hardware specific but is not > dynamically detectable. Current driver patch uses a prefix name "himax_i2chid" which comes from the previous project and seems not suitable for this condition, so I use "xxx" and plan to replace it in the next version. For finding firmware, I think both solutions are reasonable. - provide firmware name directly: implies no-flash and use user specified firmware, no PID info. - provide no-flash-flag and PID info: loading firmware from organized names with PID info. I prefer the 2nd solution, but it needs more properties in dts. 1st has less properties and more intuitive. I don't know which one is more acceptable by the community, as you know I'm a newbie here. Thanks, Tylor
On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote: > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote: > > > > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote: > > > > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote: > > > > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote: > > > > > > > > > > > > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and > > > > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for > > > > > > > > > > > > > user to select. > > > > > > > > > > > > > > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that > > > > > > > > > > > > works for all hardware at the same time. > > > > > > > > > > > > > > > > > > > > > > > I see, so I should take that back? > > > > > > > > > > > I'll explain more about it. > > > > > > > > > > > > > > > > > > > > Are there particular ICs where the firmware would always be in flash and > > > > > > > > > > others where it would never be? Or is this a choice made by the board or > > > > > > > > > > system designer? > > > > > > > > > > > > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced > > > > > > > > > to use flash because of its architecture(multiple IC inside, need to > > > > > > > > > load firmware to > > > > > > > > > multiple IC's sram by master IC). But if there is no limitation on > > > > > > > > > this part, most system > > > > > > > > > designers will prefer flashless. > > > > > > > > > > > > > > > > Forgive me if I am not understanding correctly, there are some ICs that > > > > > > > > will need to load the firmware from flash and there are some where it > > > > > > > > will be a decision made by the designer of the board. Is the flash part > > > > > > > > of the IC or is it an external flash chip? > > > > > > > > > > > > > > > > > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long > > > > > > > and thin, placed on panel PCB, flash will be located at the external > > > > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash > > > > > > > is embedded, thus the IC size is large compared to TDDI. But from the > > > > > > > driver's perspective either external flash or embedded flash, the IC > > > > > > > itself will load firmware from flash automatically when reset pin is > > > > > > > released. Only if firmware is loading from the host storage system, > > > > > > > the driver needs to operate the IC in detail. > > > > > > > > > > > > > > > > > > Since there are ICs that can use the external flash or have it loaded > > > > > > from the host, it sounds like you do need a property to differentiate > > > > > > between those cases. > > > > > Yep. > > > > > > > > > > > Is it sufficient to just set the firmware-name property for these cases? > > > > > > If the property exists, then you know you need to load firmware & what > > > > > > its name is. If it doesn't, then the firmware either isn't needed or > > > > > > will be automatically loaded from the external flash. > > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > > > How do you intend generating the name of the firmware file? I assume the > > > > same firmware doesn't work on every IC, so you'll need to pick a > > > > different one depending on the compatible? > > > > > > > If considering a firmware library line-up for all the incoming panels > > > of this driver. > > > We would use PID as part of the file name. Because all the support panels would > > > have a unique PID associated. Which will make the firmware name like > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > > at no flash condition. Thus PID information is required in dts when > > > no-flash-flag > > > is specified. > > > > Firstly, where does the "xxx" come from? > > And you're making it sound more like having firmware-name is suitable > > for this use case, given you need to determine the name of the file to > > use based on something that is hardware specific but is not > > dynamically detectable. > Current driver patch uses a prefix name "himax_i2chid" which comes > from the previous project > and seems not suitable for this condition, so I use "xxx" and plan to > replace it in the next version. > For finding firmware, I think both solutions are reasonable. > - provide firmware name directly: implies no-flash and use user > specified firmware, no PID info. > - provide no-flash-flag and PID info: loading firmware from organized > names with PID info. > I prefer the 2nd solution, but it needs more properties in dts. 1st > has less properties and more > intuitive. > > I don't know which one is more acceptable by the community, as you > know I'm a newbie here. To be honest, I am not all that sure either! Does the panel id have value in its own right, or is that only used to determine the firmware filename? Also, if it does have value in its own right, rather than a "pid", should the panel be a child node of this hid device with its own compatible?
On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote: > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote: > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > > > > > How do you intend generating the name of the firmware file? I assume the > > > > > same firmware doesn't work on every IC, so you'll need to pick a > > > > > different one depending on the compatible? > > > > > > > > > If considering a firmware library line-up for all the incoming panels > > > > of this driver. > > > > We would use PID as part of the file name. Because all the support panels would > > > > have a unique PID associated. Which will make the firmware name like > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > > > at no flash condition. Thus PID information is required in dts when > > > > no-flash-flag > > > > is specified. > > > > > > Firstly, where does the "xxx" come from? > > > And you're making it sound more like having firmware-name is suitable > > > for this use case, given you need to determine the name of the file to > > > use based on something that is hardware specific but is not > > > dynamically detectable. > > Current driver patch uses a prefix name "himax_i2chid" which comes > > from the previous project > > and seems not suitable for this condition, so I use "xxx" and plan to > > replace it in the next version. > > For finding firmware, I think both solutions are reasonable. > > - provide firmware name directly: implies no-flash and use user > > specified firmware, no PID info. > > - provide no-flash-flag and PID info: loading firmware from organized > > names with PID info. > > I prefer the 2nd solution, but it needs more properties in dts. 1st > > has less properties and more > > intuitive. > > > > I don't know which one is more acceptable by the community, as you > > know I'm a newbie here. > > To be honest, I am not all that sure either! Does the panel id have > value in its own right, or is that only used to determine the firmware > filename? Currently, PID stands for Panel/Project ID and is used for determining the firmware filename only. We haven't come up with any new attribute that may attach to it. The differences between panels are handled in firmware dedicated to its PID. > Also, if it does have value in its own right, rather than a "pid", > should the panel be a child node of this hid device with its own > compatible? It may need a child node if we find it necessary to add attributes to each PID. But currently we have no idea about it. Thanks, Tylor
On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote: > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote: > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > > > > > > > How do you intend generating the name of the firmware file? I assume the > > > > > > same firmware doesn't work on every IC, so you'll need to pick a > > > > > > different one depending on the compatible? > > > > > > > > > > > If considering a firmware library line-up for all the incoming panels > > > > > of this driver. > > > > > We would use PID as part of the file name. Because all the support panels would > > > > > have a unique PID associated. Which will make the firmware name like > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > > > > at no flash condition. Thus PID information is required in dts when > > > > > no-flash-flag > > > > > is specified. > > > > > > > > Firstly, where does the "xxx" come from? > > > > And you're making it sound more like having firmware-name is suitable > > > > for this use case, given you need to determine the name of the file to > > > > use based on something that is hardware specific but is not > > > > dynamically detectable. > > > Current driver patch uses a prefix name "himax_i2chid" which comes > > > from the previous project > > > and seems not suitable for this condition, so I use "xxx" and plan to > > > replace it in the next version. > > > For finding firmware, I think both solutions are reasonable. > > > - provide firmware name directly: implies no-flash and use user > > > specified firmware, no PID info. > > > - provide no-flash-flag and PID info: loading firmware from organized > > > names with PID info. > > > I prefer the 2nd solution, but it needs more properties in dts. 1st > > > has less properties and more > > > intuitive. > > > > > > I don't know which one is more acceptable by the community, as you > > > know I'm a newbie here. > > > > To be honest, I am not all that sure either! Does the panel id have > > value in its own right, or is that only used to determine the firmware > > filename? > Currently, PID stands for Panel/Project ID and is used for determining > the firmware filename only. We haven't come up with any new attribute that > may attach to it. The differences between panels are handled in firmware > dedicated to its PID. > > > Also, if it does have value in its own right, rather than a "pid", > > should the panel be a child node of this hid device with its own > > compatible? > It may need a child node if we find it necessary to add attributes to each PID. > But currently we have no idea about it. To be honest, it seems to me like you are using "PID" in place of a compatible for the panel, since it needs to be provided via DT anyway.
On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote: > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote: > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > > > > > > > > > How do you intend generating the name of the firmware file? I assume the > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a > > > > > > > different one depending on the compatible? > > > > > > > > > > > > > If considering a firmware library line-up for all the incoming panels > > > > > > of this driver. > > > > > > We would use PID as part of the file name. Because all the support panels would > > > > > > have a unique PID associated. Which will make the firmware name like > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > > > > > at no flash condition. Thus PID information is required in dts when > > > > > > no-flash-flag > > > > > > is specified. > > > > > > > > > > Firstly, where does the "xxx" come from? > > > > > And you're making it sound more like having firmware-name is suitable > > > > > for this use case, given you need to determine the name of the file to > > > > > use based on something that is hardware specific but is not > > > > > dynamically detectable. > > > > Current driver patch uses a prefix name "himax_i2chid" which comes > > > > from the previous project > > > > and seems not suitable for this condition, so I use "xxx" and plan to > > > > replace it in the next version. > > > > For finding firmware, I think both solutions are reasonable. > > > > - provide firmware name directly: implies no-flash and use user > > > > specified firmware, no PID info. > > > > - provide no-flash-flag and PID info: loading firmware from organized > > > > names with PID info. > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st > > > > has less properties and more > > > > intuitive. > > > > > > > > I don't know which one is more acceptable by the community, as you > > > > know I'm a newbie here. > > > > > > To be honest, I am not all that sure either! Does the panel id have > > > value in its own right, or is that only used to determine the firmware > > > filename? > > Currently, PID stands for Panel/Project ID and is used for determining > > the firmware filename only. We haven't come up with any new attribute that > > may attach to it. The differences between panels are handled in firmware > > dedicated to its PID. > > > > > Also, if it does have value in its own right, rather than a "pid", > > > should the panel be a child node of this hid device with its own > > > compatible? > > It may need a child node if we find it necessary to add attributes to each PID. > > But currently we have no idea about it. > > To be honest, it seems to me like you are using "PID" in place of a > compatible for the panel, since it needs to be provided via DT anyway. Hmm... So the more formal way is? If I add a sub-note inside this spi-device block, such as "panel" and add PID inside. Will it be more appropriate? ... spi { ... hx_spi@0 { ... panel { himax,pid = ... }; } } Thanks, Tylor
On Thu, Oct 12, 2023 at 10:30:03AM +0800, yang tylor wrote: > On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote: > > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote: > > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > > > > > > > > > > > How do you intend generating the name of the firmware file? I assume the > > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a > > > > > > > > different one depending on the compatible? > > > > > > > > > > > > > > > If considering a firmware library line-up for all the incoming panels > > > > > > > of this driver. > > > > > > > We would use PID as part of the file name. Because all the support panels would > > > > > > > have a unique PID associated. Which will make the firmware name like > > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > > > > > > at no flash condition. Thus PID information is required in dts when > > > > > > > no-flash-flag > > > > > > > is specified. > > > > > > > > > > > > Firstly, where does the "xxx" come from? > > > > > > And you're making it sound more like having firmware-name is suitable > > > > > > for this use case, given you need to determine the name of the file to > > > > > > use based on something that is hardware specific but is not > > > > > > dynamically detectable. > > > > > Current driver patch uses a prefix name "himax_i2chid" which comes > > > > > from the previous project > > > > > and seems not suitable for this condition, so I use "xxx" and plan to > > > > > replace it in the next version. > > > > > For finding firmware, I think both solutions are reasonable. > > > > > - provide firmware name directly: implies no-flash and use user > > > > > specified firmware, no PID info. > > > > > - provide no-flash-flag and PID info: loading firmware from organized > > > > > names with PID info. > > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st > > > > > has less properties and more > > > > > intuitive. > > > > > > > > > > I don't know which one is more acceptable by the community, as you > > > > > know I'm a newbie here. > > > > > > > > To be honest, I am not all that sure either! Does the panel id have > > > > value in its own right, or is that only used to determine the firmware > > > > filename? > > > Currently, PID stands for Panel/Project ID and is used for determining > > > the firmware filename only. We haven't come up with any new attribute that > > > may attach to it. The differences between panels are handled in firmware > > > dedicated to its PID. > > > > > > > Also, if it does have value in its own right, rather than a "pid", > > > > should the panel be a child node of this hid device with its own > > > > compatible? > > > It may need a child node if we find it necessary to add attributes to each PID. > > > But currently we have no idea about it. > > > > To be honest, it seems to me like you are using "PID" in place of a > > compatible for the panel, since it needs to be provided via DT anyway. > > Hmm... So the more formal way is? > If I add a sub-note inside this spi-device block, such as "panel" and > add PID inside. > Will it be more appropriate? > ... > spi { > ... > hx_spi@0 { > ... > panel { > himax,pid = ... And this now looks exactly like compatible = "vendor,part" now, no?
On Thu, Oct 12, 2023 at 11:24 PM Conor Dooley <conor@kernel.org> wrote: > > On Thu, Oct 12, 2023 at 10:30:03AM +0800, yang tylor wrote: > > On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <conor@kernel.org> wrote: > > > > > > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote: > > > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote: > > > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote: > > > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote: > > > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote: > > > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code. > > > > > > > > > > > > > > > > > > How do you intend generating the name of the firmware file? I assume the > > > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a > > > > > > > > > different one depending on the compatible? > > > > > > > > > > > > > > > > > If considering a firmware library line-up for all the incoming panels > > > > > > > > of this driver. > > > > > > > > We would use PID as part of the file name. Because all the support panels would > > > > > > > > have a unique PID associated. Which will make the firmware name like > > > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load > > > > > > > > at no flash condition. Thus PID information is required in dts when > > > > > > > > no-flash-flag > > > > > > > > is specified. > > > > > > > > > > > > > > Firstly, where does the "xxx" come from? > > > > > > > And you're making it sound more like having firmware-name is suitable > > > > > > > for this use case, given you need to determine the name of the file to > > > > > > > use based on something that is hardware specific but is not > > > > > > > dynamically detectable. > > > > > > Current driver patch uses a prefix name "himax_i2chid" which comes > > > > > > from the previous project > > > > > > and seems not suitable for this condition, so I use "xxx" and plan to > > > > > > replace it in the next version. > > > > > > For finding firmware, I think both solutions are reasonable. > > > > > > - provide firmware name directly: implies no-flash and use user > > > > > > specified firmware, no PID info. > > > > > > - provide no-flash-flag and PID info: loading firmware from organized > > > > > > names with PID info. > > > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st > > > > > > has less properties and more > > > > > > intuitive. > > > > > > > > > > > > I don't know which one is more acceptable by the community, as you > > > > > > know I'm a newbie here. > > > > > > > > > > To be honest, I am not all that sure either! Does the panel id have > > > > > value in its own right, or is that only used to determine the firmware > > > > > filename? > > > > Currently, PID stands for Panel/Project ID and is used for determining > > > > the firmware filename only. We haven't come up with any new attribute that > > > > may attach to it. The differences between panels are handled in firmware > > > > dedicated to its PID. > > > > > > > > > Also, if it does have value in its own right, rather than a "pid", > > > > > should the panel be a child node of this hid device with its own > > > > > compatible? > > > > It may need a child node if we find it necessary to add attributes to each PID. > > > > But currently we have no idea about it. > > > > > > To be honest, it seems to me like you are using "PID" in place of a > > > compatible for the panel, since it needs to be provided via DT anyway. > > > > Hmm... So the more formal way is? > > If I add a sub-note inside this spi-device block, such as "panel" and > > add PID inside. > > Will it be more appropriate? > > ... > > spi { > > ... > > hx_spi@0 { > > ... > > panel { > > himax,pid = ... > > And this now looks exactly like compatible = "vendor,part" now, no? I think it's not the same, I thought "compatible" is used to target from the driver side. For finding other information inside the block. But I just store PID information in this one, not used for targeting but getting infos from it. Thanks, Tylor
diff --git a/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml new file mode 100644 index 000000000000..3ee3a89842ac --- /dev/null +++ b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/himax,hid-over-spi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Himax TDDI devices using SPI to send HID packets + +maintainers: + - Tylor Yang <tylor_yang@himax.corp-partner.google.com> + +description: | + Support the Himax TDDI devices which using SPI interface to acquire + HID packets from the device. The device needs to be initialized using + Himax protocol before it start sending HID packets. + +properties: + compatible: + const: himax,hid-over-spi + + reg: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + interrupts: + maxItems: 1 + + himax,rst-gpio: + maxItems: 1 + description: Reset device, active low signal. + + himax,irq-gpio: + maxItems: 1 + description: Interrupt request, active low signal. + + himax,3v3-gpio: + maxItems: 1 + description: GPIO to control 3.3V power supply. + + himax,id-gpios: + maxItems: 8 + description: GPIOs to read physical Panel ID. Optional. + + spi-cpha: true + spi-cpol: true + + himax,ic-det-delay-ms: + description: + Due to TDDI properties, the TPIC detection timing must after the + display panel initialized. This property is used to specify the + delay time when TPIC detection and display panel initialization + timing are overlapped. How much milliseconds to delay before TPIC + detection start. + + himax,ic-resume-delay-ms: + description: + Due to TDDI properties, the TPIC resume timing must after the + display panel resumed. This property is used to specify the + delay time when TPIC resume and display panel resume + timing are overlapped. How much milliseconds to delay before TPIC + resume start. + +required: + - compatible + - reg + - interrupts + - himax,rst-gpio + - himax,irq-gpio + +unevaluatedProperties: false + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + touchscreen@0 { + compatible = "himax,hid-over-spi"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x0>; + interrupt-parent = <&gpio1>; + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; + pinctrl-0 = <&touch_pins>; + pinctrl-names = "default"; + + spi-max-frequency = <12500000>; + spi-cpha; + spi-cpol; + + himax,rst-gpio = <&gpio1 8 GPIO_ACTIVE_LOW>; + himax,irq-gpio = <&gpio1 7 GPIO_ACTIVE_LOW>; + himax,3v3-gpio = <&gpio1 6 GPIO_ACTIVE_HIGH>; + himax,ic-det-delay-ms = <500>; + himax,ic-resume-delay-ms = <100>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index bf0f54c24f81..452701261bec 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9323,6 +9323,12 @@ L: linux-kernel@vger.kernel.org S: Maintained F: drivers/misc/hisi_hikey_usb.c +HIMAX HID OVER SPI TOUCHSCREEN SUPPORT +M: Tylor Yang <tylor_yang@himax.corp-partner.google.com> +L: linux-input@vger.kernel.org +S: Supported +F: Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml + HIMAX HX83112B TOUCHSCREEN SUPPORT M: Job Noorman <job@noorman.info> L: linux-input@vger.kernel.org
The Himax HID-over-SPI framework support for Himax touchscreen ICs that report HID packet through SPI bus. The driver core need reset pin to meet reset timing spec. of IC. An interrupt pin to disable and enable interrupt when suspend/resume. An optional power control pin if target board needed. Panel id pins for identify panel is also an option. Additional optional arguments: ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime conditions. This patch also add maintainer of this driver. Signed-off-by: Tylor Yang <tylor_yang@himax.corp-partner.google.com> --- .../bindings/input/himax,hid-over-spi.yaml | 109 ++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 115 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml