Re: [PATCH 10/53] Input: atmel_mxt_ts - Add memory access interface via sysfs

Dmitry Torokhov wrote:

quoted

We have made a deliberate choice to implement this via sysfs rather than
debugfs since it needs to work on devices that don't have debugfs enabled.

Then they will have to enable it. Re-implementing something because
someone might not enable needed subsystem is not a good idea. Let's
say somebody disabled I2C - will you write your own implementation because
of that? Of course not, you just say that for given functionality it
is a prerequisite.

debugfs is a debug filesystem. This interface is useful for purposes which
are not debug. I have to be pragmatic: I don't see debugfs enabled on most
shipping Android devices, and however much I tell them to enable debugfs
doesn't seem to hold much weight.

It's partly path dependence - it was implemented like this because regmap
wasn't in mainline at the point when I wrote it. Having a dependency on
regmap would now be a API break complicating support of customers using
older kernels than mainline. I would also have to update a bunch of
software and documentation and people to know about the two different APIs.
The existing implementation already appears in shipping devices, so it is
well tested.

quoted

In addition, there are some quirks about the way in which we have to
read/write registers which means regmap isn't a good fit.

Could you please elaborate more on this?

- the mxt chip caches the I2C read pointer, so you can get a performance
optimisation by not sending the address on every read/write (I haven't
implemented this yet but plan to)
- the address pointer can wrap around when you read the T5 message
processor, which would confuse regmap
- we require I2C retries in some cases due to way the chip handles sleep states
- I can't see how to map the object protocol (used on mxt chips) into the
way regmap treats register ranges

I can look into porting on top of regmap. But it seems a pity to pepper
regmap with atmel_mxt_ts quirks just to save on three small functions in
the driver.

Thanks for your time to look at this.

N