RE: [PATCH v7 3/9] Input: goodix - write configuration data to device

[Tirdea, Irina]

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx]
> Sent: 14 October, 2015 9:59
> To: Tirdea, Irina
> Cc: Bastien Nocera; Aleksei Mamlin; linux-input@xxxxxxxxxxxxxxx; Mark Rutland; Purdila, Octavian; linux-kernel@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v7 3/9] Input: goodix - write configuration data to device
> 
> On Thu, Oct 08, 2015 at 01:19:29PM +0300, Irina Tirdea wrote:
> > Goodix devices can be configured by writing custom data to the device at
> > init. The configuration data is read with request_firmware from
> > "goodix_<id>_cfg.bin", where <id> is the product id read from the device
> > (e.g.: goodix_911_cfg.bin for Goodix GT911, goodix_9271_cfg.bin for
> > GT9271).
> >
> > The configuration information has a specific format described in the Goodix
> > datasheet. It includes X/Y resolution, maximum supported touch points,
> > interrupt flags, various sensitivity factors and settings for advanced
> > features (like gesture recognition).
> >
> > Before writing the firmware, it is necessary to reset the device. If
> > the device ACPI/DT information does not declare gpio pins (needed for
> > reset), writing the firmware will not be available for these devices.
> >
> > This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
> > driver gt9xx.c for Android (publicly available in Android kernel
> > trees for various devices).
> >
> > Signed-off-by: Octavian Purdila <octavian.purdila@xxxxxxxxx>
> > Signed-off-by: Irina Tirdea <irina.tirdea@xxxxxxxxx>
> > ---
> >  drivers/input/touchscreen/goodix.c | 229 +++++++++++++++++++++++++++++++------
> >  1 file changed, 196 insertions(+), 33 deletions(-)
> >
<snip>
> > +/**
> > + * goodix_config_cb - Callback to finish device init
> > + *
> > + * @ts: our goodix_ts_data pointer
> > + *
> > + * request_firmware_wait callback that finishes
> > + * initialization of the device.
> > + */
> > +static void goodix_config_cb(const struct firmware *cfg, void *ctx)
> > +{
> > +	struct goodix_ts_data *ts = (struct goodix_ts_data *)ctx;
> > +	int error;
> > +
> > +	if (cfg) {
> > +		/* send device configuration to the firmware */
> > +		error = goodix_send_cfg(ts, cfg);
> > +		if (error)
> > +			goto err_release_cfg;
> > +	}
> > +	goodix_configure_dev(ts);
> > +
> > +err_release_cfg:
> > +	kfree(ts->cfg_name);
> > +	release_firmware(cfg);
> 
> You need to use completion to signal remove() (and also probably
> suspend/resume in the subsequent patches) that you are done handling
> config, otherwise if you do bind/unbind via sysfs in a tight loop you
> will observe a nice crash.
> 
> Thanks.
> 

Right, missed that. Will fix in next version.

Thanks,
Irina

<snip>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/