On Fri, Apr 10, 2020 at 12:27:02PM +0200, Stephan Gerhold wrote: > Hi Dmitry, > > On Thu, Apr 09, 2020 at 03:15:03PM -0700, Dmitry Torokhov wrote: > > Hi Stephan, > > > > On Sun, Apr 05, 2020 at 07:09:03PM +0200, Stephan Gerhold wrote: > > > MMS345L is another first generation touch screen from Melfas, > > > which uses the same registers as MMS152. > > > > > > However, using I2C_M_NOSTART for it causes errors when reading: > > > > > > i2c i2c-0: sendbytes: NAK bailout. > > > mms114 0-0048: __mms114_read_reg: i2c transfer failed (-5) > > > > > > The driver works fine as soon as I2C_M_NOSTART is removed. > > > > > > Add a separate melfas,mms345l binding, and make use of I2C_M_NOSTART > > > only for MMS114 and MMS152. > > > > > > Reviewed-by: Andi Shyti <andi@xxxxxxxxxxx> > > > Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > > > > Sorry for sitting on this for so long. I looked around, and I think that > > instead of adding separate handling for 345L we shoudl simply drop the > > "no start" bit for everyone. I am not sure what the original version > > tried to do here, as far as I can see in various public Android trees the > > driver for these devices does not use I2C_M_NOSTART. > > > > Actually, I wonder if the difference is not in the touch controller that > > is being used, but rather in the I2C controller the device in connected > > to. > > > > I would like to apply the version of the patch below. > > Thanks a lot for your reply! > > Your suggested patch below works fine on my MMS345L device > (with the melfas,mms152 compatible). Applying it would allow > the touchscreen to work on my device in mainline :) > > I wonder if we should still add a separate melfas,mms345l compatible: > > The only remaining difference to MMS152 at the moment is the > MMS152_COMPAT_GROUP register. It seems to be used for something > different on MMS345L, so when it is printed in mms114_get_version() > it just displays some garbage: > > TSP FW Rev: bootloader 0x6 / core 0x26 / config 0x26, Compat group: \x06 > > (It used to actually print it as some ASCII control character but > apparently that was fixed...) > > It's not really critical, but in case we find more differences I think > it's better to directly add a separate compatible string to the device > tree. > > If you agree with this I can send a separate patch with the new > compatible string once you have applied the patch below. Yes, if there is legitimate differences between the chips that would be a good reason to add a new compatible string. Please send a separate patch with updated justification and I will apply it. Thank you. -- Dmitry
