Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Sat, 30 Sep 2023 16:27:17 +0200
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Jeff LaBundy <jeff@labundy.com>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Henrik Rydberg <rydberg@bitmath.org>,
        linux-input@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Jonathan Albrieux <jonathan.albrieux@gmail.com>
Subject: Re: [PATCH 2/2] Input: add Himax HX852x(ES) touchscreen driver
Message-ID: <ZRgwRfr7aW_Ww0oL@gerhold.net>
References: <20230913-hx852x-v1-0-9c1ebff536eb@gerhold.net>
 <20230913-hx852x-v1-2-9c1ebff536eb@gerhold.net>
 <ZQYUe46/rj8jqNvg@nixie71>
 <ZQcx7oQyL6RM06Jt@gerhold.net>
 <ZROaqRiWa6ReVH/D@nixie71>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZROaqRiWa6ReVH/D@nixie71>
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Jeff,

On Tue, Sep 26, 2023 at 09:59:53PM -0500, Jeff LaBundy wrote:
> On Sun, Sep 17, 2023 at 07:05:50PM +0200, Stephan Gerhold wrote:
> > On Sat, Sep 16, 2023 at 03:47:55PM -0500, Jeff LaBundy wrote:
> > > On Wed, Sep 13, 2023 at 03:25:30PM +0200, Stephan Gerhold wrote:
> > > > From: Jonathan Albrieux <jonathan.albrieux@gmail.com>
> [...]
> > > > +static int hx852x_probe(struct i2c_client *client)
> > > > +{
> > > > +	struct device *dev = &client->dev;
> > > > +	struct hx852x *hx;
> > > > +	int error, i;
> > > > +
> > > > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
> > > > +				     I2C_FUNC_SMBUS_WRITE_BYTE |
> > > > +				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
> > > > +				     I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
> > > > +		dev_err(dev, "not all i2c functionality supported\n");
> > > > +		return -ENXIO;
> > > > +	}
> > > > +
> > > > +	hx = devm_kzalloc(dev, sizeof(*hx), GFP_KERNEL);
> > > > +	if (!hx)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	hx->client = client;
> > > > +	hx->input_dev = devm_input_allocate_device(dev);
> > > > +	if (!hx->input_dev)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	hx->input_dev->name = "Himax HX852x";
> > > > +	hx->input_dev->id.bustype = BUS_I2C;
> > > > +	hx->input_dev->open = hx852x_input_open;
> > > > +	hx->input_dev->close = hx852x_input_close;
> > > > +
> > > > +	i2c_set_clientdata(client, hx);
> > > > +	input_set_drvdata(hx->input_dev, hx);
> > > > +
> > > > +	hx->supplies[0].supply = "vcca";
> > > > +	hx->supplies[1].supply = "vccd";
> > > > +	error = devm_regulator_bulk_get(dev, ARRAY_SIZE(hx->supplies), hx->supplies);
> > > > +	if (error < 0)
> > > > +		return dev_err_probe(dev, error, "failed to get regulators");
> > > > +
> > > > +	hx->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> > > > +	if (IS_ERR(hx->reset_gpiod))
> > > > +		return dev_err_probe(dev, error, "failed to get reset gpio");
> > > 
> > > Can the reset GPIO be optional?
> > > 
> > 
> > I'm afraid I have no idea if the controller needs this or not. Would it
> > be better to keep it required until someone confirms otherwise or have
> > it optional for the other way around?
> 
> If you have a datasheet handy, or your hardware provides a means for you to
> test and confirm whether reset can be left out, I would make the reset GPIO
> optional. Often times, these controllers are part of a module and reset may
> be tied high locally as opposed to adding another signal to a flex cable.
> 
> If you have no way to confirm, I would keep it as required for now; it is not
> too cumbersome to be changed later if the need arises on different hardware.
> 

I don't have a datasheet unfortunately. :(

However, I tried to simulate this case on my board by keeping the reset
GPIO permanently de-asserted (i.e. high because of active-low). The
results are not entirely conclusive: The controller seems to respond to
commands and the initial configuration is read correctly. However, it
does not report any touch events. As soon as I add the temporary
assertion of the reset signal back it works fine again.

I suspect toggling the reset signal might be required to make the
controller come properly out of reset. I'll keep it required to be sure.

Thanks,
Stephan