Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Sun, 29 Oct 2023 03:34:59 +0000
From:   Dmitry Torokhov <dmitry.torokhov@gmail.com>
To:     James Hilliard <james.hilliard1@gmail.com>
Cc:     linux-input@vger.kernel.org,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Input: cyttsp5 - improve error handling and remove
 regmap
Message-ID: <ZT3S43_eMdwHWu2u@google.com>
References: <20231025013939.353553-1-james.hilliard1@gmail.com>
 <ZTwWmHC7Wcd5iwqS@google.com>
 <CADvTj4oH=3Q3EShC-FM9ob7EnvFe4t2LHyDEwr-e7=G8M=UzYg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CADvTj4oH=3Q3EShC-FM9ob7EnvFe4t2LHyDEwr-e7=G8M=UzYg@mail.gmail.com>
Precedence: bulk

On Sat, Oct 28, 2023 at 03:31:00AM -0600, James Hilliard wrote:
> On Fri, Oct 27, 2023 at 1:59 PM Dmitry Torokhov <dmitry.torokhov@gmail.com>
> wrote:
> 
> > Hi James,
> >
> > On Tue, Oct 24, 2023 at 07:39:38PM -0600, James Hilliard wrote:
> > > The vendor cyttsp5 driver does not use regmap for i2c support, it
> > > would appear this is due to regmap not providing sufficient levels
> > > of control to handle various error conditions that may be present
> > > under some configuration/firmware variants.
> > >
> > > To improve reliability lets refactor the cyttsp5 i2c interface to
> > > function more like the vendor driver and implement some of the error
> > > handling retry/recovery techniques present there.
> >
> > Sorry but you need to elaborate more on what is missing in regmap and
> > how vendor code is better. In my experience vendors rarely follow kernel
> > development and either are not aware of the latest kernel APIs, or they
> > simply have the driver written to what we had in 3.x kernels and have
> > not really updated it since then.
> >
> 
> I'm unaware of a way to do essentially raw reads when using regmap, for
> example I don't know of a way to implement the cyttsp5_deassert_read
> function using regmap, maybe there's a way I'm not aware of however?

What is wrong with current way of reading from the input register? It
should clear the interrupt line.

> 
> In general the issue with regmap seems to be that regmap always does
> operations against specific registers and prevents doing raw i2c operations
> needed to handle some hardware/firmware issues for some variants.

What are those issues and why do they need raw access.

> 
> Note that I'm not exactly doing things the same way the vendor driver does,
> I have simplified the error recovery/retry code paths in the startup
> function.
> 
> 
> >
> > >
> > > As part of this rather than assuming the device is in bootloader mode
> > > we should first check that the device is in bootloader and only
> > > attempt to launch the app if it actually is in the bootloader.
> >
> > I would prefer if this was split into a separate patch.
> >
> 
> I think this change is somewhat intertwined with the probe retry/recovery
> logic
> changes and is a bit tricky to split out without breaking the startup
> sequence
> from my testing at least.

I understand that it might be tricky but each logical change should
stand on its own.

Thanks.

-- 
Dmitry