All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Andrej Valek <andrej.valek@siemens.com>,
	nick@shmanahar.org, hadess@hadess.net,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/3] Input: goodix - add option to disable firmware loading
Date: Mon, 23 Nov 2020 09:46:03 +0100	[thread overview]
Message-ID: <07a7e9e2-0c82-fdc0-2db0-5011b94816e2@redhat.com> (raw)
In-Reply-To: <20201123065336.GC2034289@dtor-ws>

Hi,

On 11/23/20 7:53 AM, Dmitry Torokhov wrote:
> On Fri, Oct 30, 2020 at 10:56:20AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 10/29/20 9:36 PM, Dmitry Torokhov wrote:
>>> Hi Andrej,
>>>
>>> On Thu, Oct 29, 2020 at 06:03:11PM +0100, Andrej Valek wrote:
>>>> Firmware file loadind for GT911 controller takes too much time (~60s).
>>>> There is no check that configuration is the same which is already present.
>>>> This happens always during boot, which makes touchscreen unusable.
>>>>
>>>> Add there an option to prevent firmware file loading, but keep it enabled
>>>> by default.
>>>
>>> I thought that Goodix was losing firmware loading at poweroff. Is this
>>> not the case with this model?
>>
>> So first of all there are 2 sorts of firmware involved with the
>> Goodix touchscreen controllers, the actual firmware and a block
>> of config data for that firmware which I presume adjusts it for
>> the specific (model of) the digitizer which is attached.
>>
>> ATM the mainline Linux driver does not support models where
>> the actual firmware itself needs to be loaded (because they
>> only have RAM, so they come up without firmware).
>>
>> I do have one model tablet with a ROM-less goodix touchpad
>> controller, so if I ever find the time I might add support
>> for loading the actual firmware.
>>
>> So what we are talking about here is just loading the config
>> data and I'm a bit surprised that this take so long.
> 
> So I am still confused about this: is the config stored in RAM or NVRAM?
> I.e. do we actually need to re-load it every time on boot, or it
> supposed to be flashed only when it is changed (or lost)?

I only know about these touchscreens on x86, where we have a BIOS
muddling the waters.

We have seen devices which loose the config over suspend/resume,
which suggests it is in RAM. I recently added a fix for these which
saves the config at boot, which suggests that at least on the device
model with the suspend/resume issue the config is loaded into the chip
by the BIOS.

But I'm not sure that this is the case everywhere. Most other models
likely have the config in NVRAM.

I guess this is the same as with the firmware, and it differs per
model. I know for sure that their are RAM only models which need
the firmware loaded at boot, these are mostly found on ARM devs,
but I have one X86 devices (which currently does not work) which
also has RAM only and thus needs Linux to load the firmware
(which is not supported atm). These RAM only models, presumably
also have only RAM for the config.

Regards,

Hans


  reply	other threads:[~2020-11-23  8:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-29 17:03 [PATCH 0/3] Firmware loading option Andrej Valek
2020-10-29 17:03 ` [PATCH 1/3] Input: goodix - add option to disable firmware loading Andrej Valek
2020-10-29 20:36   ` Dmitry Torokhov
2020-10-30  8:45     ` Valek, Andrej
2020-10-30  9:56     ` Hans de Goede
2020-10-30 11:02       ` Valek, Andrej
2020-11-23  6:53       ` Dmitry Torokhov
2020-11-23  8:46         ` Hans de Goede [this message]
2020-11-10  9:07   ` [PATCH v3 1/4] " Andrej Valek
2020-11-10 18:15   ` [PATCH v4 " Andrej Valek
2020-10-29 17:03 ` [PATCH 2/3] dt-bindings: goodix Andrej Valek
2020-11-04 21:44   ` Rob Herring
2020-11-10  9:07   ` [PATCH v3 2/4] dt-bindings: touchscreen: goodix: add info about disabling FW loading Andrej Valek
2020-11-10 13:43     ` Rob Herring
2020-11-10 18:15   ` [PATCH v4 " Andrej Valek
2020-11-16 17:08     ` Rob Herring
2020-10-29 17:03 ` [PATCH 3/3] Input: atmel_mxt_ts - add option to disable firmware loading Andrej Valek
2020-11-10  9:07   ` [PATCH v3 3/4] " Andrej Valek
2020-11-10 18:15   ` [PATCH v4 " Andrej Valek
2020-11-23  6:56     ` Dmitry Torokhov
2020-11-06 10:05 ` [PATCH v2 0/3] Firmware loading option Andrej Valek
2020-11-06 10:05 ` [PATCH v2 1/3] Input: st1232 - add support resolution reading Andrej Valek
2020-11-10  9:07   ` [PATCH v3 4/4] " Andrej Valek
2020-11-10 18:15   ` [PATCH v4 " Andrej Valek
2020-11-06 10:05 ` [PATCH v2 2/3] dt-bindings: goodix Andrej Valek
2020-11-09 21:37   ` Rob Herring
2020-11-06 10:05 ` [PATCH v2 3/3] Input: goodix - add option to disable firmware loading Andrej Valek
2020-11-10  9:07 ` [PATCH v3 0/4] Firmware loading option Andrej Valek
2020-11-10 18:15 ` [PATCH v4 " Andrej Valek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=07a7e9e2-0c82-fdc0-2db0-5011b94816e2@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andrej.valek@siemens.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hadess@hadess.net \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nick@shmanahar.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.