On Friday 21 May 2010 11:22:19 am Matthew Dharm wrote:
> On Fri, May 21, 2010 at 08:25:19AM -0600, Steve McKown wrote:
> > Nice! I think the data written to /sys/blah/blah/configuration should
> > have a magic string at the beginning and a CRC field at the end, so the
> > driver can validate the input. This would prevent accidental writing of
> > bad data to the cp210x, and perhaps mitigate the need for the lock/unlock
> > files.
>
> I disagree here. Adding the requirement for a header w/ magic string and
> CRC means that, practically, you need a userspace library to use this
> interface.
>
> Maybe you need a "magic string" to unlock (instead of just "echo 1", you
> could use "echo UnLockMe".
>
> But the individual value fields should be
> easily readable/writeable without hoops to jump through.
Since the USB code already provides a mechanism to read the descriptor fields,
we only need to provide the ability to write those descriptors that are
writable, and both read and write the port configuration.
By 'easily readable/writable', do you mean relative to a human manipulating
the data? The port configuration structure as sent to the cp210x is shown
below. Bit positions in the __u16 fields correspond to different
communicaitons pins on the internal port, like GPIO_0, RxD, SUSPEND, etc.
/* Port config definitions */
struct cp210x_port_state {
__u16 mode; /* Push-pull = 1, Open-drain = 0 */
__u16 lowPower;
__u16 latch; /* Logic high = 1, Logic low = 0 */
} __attribute__((packed));
struct cp210x_port_config {
struct cp210x_port_state reset;
struct cp210x_port_state suspend;
__u8 enhancedFxn;
} __attribute__((packed));
The driver could of course accept an ascii representation of this data, then
parse it internally to create the necessary binary structure to push to the
device. Does this logic best reside in the driver? One alternative is a
userspace utility that a creates the necessary binary data as an output
stream that could be written via sysfs.
>
> As for preventing "accidental writing of bad data" -- there are two parts
> to this: "accidental writing" and "bad data". "Accidental writing" is
> easily solved with lock/unlock.
> "Bad Data" can't be prevented, unless you
> can validate the data somehow (which, in the case of VID/PID or strings,
> you really can't do).
I really think the configuration data should be (1) somewhat hard to change,
and (2) relatively easy to verify as valid at time of write to a cp210x.
(1) Why somewhat hard to change? The port configuration is intimately tied to
the hardware design. Change it and break the device. Applications
interacting with the device likely rely on the integrity of the descriptor
fields. All configuration data, except serial number, are constant across
all devices of the same model/version/revision. I view this kind of
configuration data as part of the device firmware, loaded once at production
time. Changing it after the fact is largely unnecessary and only a source
for problems for users and support people alike.
(2) Why verify at time of write to device? Better to catch data corruption as
soon as possible. Consider a production environment where 1,000 devices are
programmed a week. Failing to catch a corrupted configuration for even a day
means non-trivial unit rework and increased production costs. Because the
configuration may be unchanged for several years (at least for us small
shops!), the possibility of its corruption from various causes human and
other is that much greater, as it sits there on the production server.
These are the reasons for my suggestion of a userspace program to create the
configuration data, and having it contain a CRC allowing the driver to check
validity before programming the cp210x. The former provides a barrier to
unnecessary reconfigurations, but not too burdensome of one, I think, for the
necessary configurations. The latter protects against a number of sources of
data corruption that would exist in a real production environment. And if a
user space program creates the configuration output anyway, its incremental
cost in development and operational terms is negligible.
Thanks,
Steve
>
> Thus, I think the lock/unlock paradigm works better here.
>
> Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
