On Fri, Jun 07, 2013 at 05:11:28PM +0100, Nick Dyer wrote: > Mark Brown wrote: > > I thought there was this protocol you're concerned aboout, not raw > > registers? Presenting the actual data in binary form seems sane, how > > one gets to the data is the issue. > OK, so we seem to have gone round in a circle here. Initially I understood > you to say that providing a binary read/write attribute for access to the > configuration data was not acceptable. What was being proposed was just mapping the register map straight into userspace and implementing the entire protocol in userspace. Having the unparsed attributes visible themselves is relatively normal. > >> The chip itself will enforce which registers are read-only (by ignoring > >> writes) or write-only (by returning zeros if read). Encoding all this > >> information in the driver would just make it more brittle in the face of > >> touch controller firmware updates. > > So not only do you interact with the firmware via this protocol but the > > actual hardware register map is unstable > The register map is fixed at firmware compile time. The driver contains > code which parses the object table and figures out the correct register > offsets which are used on the particular chip that it is talking to. > The user space tools that we have written contain an equivalent parser. Is > it the duplication of this code that is your concern? I don't think you're using the usual definition of "register map" here. You seem to be switching between talking about this object model the device has and device registers - perhaps the objects are also registers sometimes? > Are you saying that your concern is that user space shouldn't be able to > directly access these registers, for example to trigger a reset? In which > case, how should user space reset the chip if required? If the application layer can reset the device underneath the driver that doesn't seem awesome; similarly if the application layer is having to worry about the device getting powered off underneath it this doesn't seem great either. > >> It would be possible for the driver to intermediate for some of the > >> registers that it cares about. For example, if we change the screen width > >> then the driver could reinitialise the input device. But I can't see that > >> it makes sense if you are changing several settings in a row for the input > >> device to be reinitialised several times. It is far less buggy to provide a > >> single way of tearing down everything and reinitialising (which could be > >> simply triggered from user space) than to encode all of the dependencies > >> (which is bound to introduce bugs). > > I am having an extremely hard time connecting anything you're talking > > about here with the discussion at hand I'm afraid... > I'm trying to provide the background to this design as you've requested, I > apologise if you find it confusing. You appear to be assuming a great deal of familiarity with the specifics of this device here. Where does all thi stuff about reinitialsing the device come from? What are these dependencies and what does all this have to do with setting parameters? > > Nobody is talking about changing the protocol for interacting with the > > device. The discussion here is about the ABI the driver offers to the > > application layer. > OK. I think that the ABI offered to the application layer should also be > object protocol, implemented over a binary attribute, which is what the > patch under discussion does. Is the problem that I need to provide better > documentation of object protocol? As Greg says you do need to document any new sysfs ABIs you're adding anyway. However if this is some stateful protocol you're implementing then does it really map onto sysfs well, sysfs attributes are normally more just data values? Won't the driver end up getting into a fight with the magic userspace stuff if the driver has no idea what the magic userspace is doing? How would suspend and resume work?
Attachment:
signature.asc
Description: Digital signature
