On Thursday, July 19, 2018 9:54:04 PM CEST Nick Dyer wrote: > On Wed, Jul 18, 2018 at 06:21:30PM +0200, Paweł Chmiel wrote: > > On Tuesday, July 17, 2018 10:00:05 PM CEST Nick Dyer wrote: > > > On Tue, Jul 17, 2018 at 08:16:25PM +0200, Paweł Chmiel wrote: > > > > This patch adds optional regulators, which can be used to power > > > > up touchscreen. After enabling regulators, we need to wait 150msec. > > > > This value is taken from official driver. > > > > > > > > It was tested on Samsung Galaxy i9000 (based on Samsung S5PV210 SOC). > > > > > > > > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@xxxxxxxxx> > > > > --- > > > > Changes from v1: > > > > - Enable regulators only if reset_gpio is present. > > > > - Switch from devm_regulator_get_optional to devm_regulator_get > > > > --- > > > > drivers/input/touchscreen/atmel_mxt_ts.c | 46 ++++++++++++++++++++++++++++++-- > > > > 1 file changed, 44 insertions(+), 2 deletions(-) > > > > > > Hi Pawel- > > > > > > I see you've borrowed some of the logic from the patch I wrote a while > > > back (see https://github.com/ndyer/linux/commit/8e9687e41ed062 ) > > Actually, i was looking at https://github.com/atmel-maxtouch/linux/blob/maxtouch-v3.14/drivers/input/touchscreen/atmel_mxt_ts.c (and didn't saw Your patch till now). > > Are You going to submit it? (it has more functionalities - for example > > suspend mode read from device tree). > > Getting that work upstream has stalled for a couple of years because I > changed jobs. I have actually started recently to dust it off again, it > was later on in my queue but if you have the time to work on it that is > great. > > > > The correct behaviour according to Atmel should be: > > > > > > * Make RESET zero > > > * Bring up regulators > > > * Wait for a period to settle (150 msec) > > > * Release RESET > > > * Wait for 100 msec whilst device gets through bootloader > > > * Wait for CHG line assert before reading info block > > > > > > I can't see the first and last steps in your patch at present. > > About first step - reset_gpio is readed by using > > devm_gpiod_get_optional with GPIOD_OUT_LOW flag, so i think (but might > > be wrong) that we don't need to set this gpio value again to 0 before > > enabling regulators, > > I see what you mean - that is fair enough. > > > since currently only place where reset_gpio is used is in driver probe > > (in Your patch it is used in other cases/places - for example in > > mxt_start/stop, when we enable regulators). > > About missing wait after releasing reset, shouldn't this be separate > > patch (since currently driver is not doing it)? I can prepare it and > > send with other in next version. > > According to the maxtouch documentation, it isn't ready for comms until > the firmware asserts the CHG line. I've seen a bunch of devices that get > by without an explicit wait because the board file does the power on, > and by the time the driver gets to probe it's a few hundred ms later > anyway, so it doesn't matter. But if we put it all in the driver, it > will attempt to read the info block straight after the 100 msec delay > without waiting for CHG, and I suspect we'll end up with occasional > probe failures. It'll depend on the maxtouch device, though: they have a > range of different power on timings. > > Which platform are you doing this for? Is it a Chromebook? No, it's Samsung Galaxy S (i9000) phone with S5PV210 Samsung Soc. I'm preparing v3 version with separate patch adding this wait/delay. > > > Thanks for feedback > > > > > > The only downside with this approach is that there are a lot of > > > delays during driver probe, but I believe the asynchronous probe stuff > > > that's landed since I wrote the original patch should address that. > > > > > > cheers > > > > > > Nick > > > > > > > } > > > > @@ -3116,6 +3154,10 @@ static int mxt_remove(struct i2c_client *client) > > > > struct mxt_data *data = i2c_get_clientdata(client); > > > > > > > > disable_irq(data->irq); > > > > + if (data->reset_gpio) { > > > > + regulator_disable(data->avdd_reg); > > > > + regulator_disable(data->vdd_reg); > > > > + } > > > > sysfs_remove_group(&client->dev.kobj, &mxt_attr_group); > > > > mxt_free_input_device(data); > > > > mxt_free_object_table(data); > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
