Orolia RW disciplining parameters

[CharlesParent]

• Add condition to read Orolia's ART card serial at the right place
• Implement disciplining parameters R/W in EEPROM:
  • IOCTL for mRO misc devices can be used to R/W disciplining parameters from/to at24 EEPROM.
  • R/W operations are done through the I2C interface of the EEPROM (Write operation implemented from at24 regmap device write behaviour)

[vvfedorenko]

@CharlesParent so right now we have 2 different situations of failure - the one when we don't have dev, another - no adap. In the latter one we have to make put_device but we shouldn't do it in the first one. I would go with goto to end point and put_device if we have valid dev pointer. that will simplify code a bit

[vvfedorenko]

and one more point - all the kernel code is following the "reverse Christmas tree" style of declaring variables in function. It would be great to follow this way in your PR too

[CharlesParent]

@vvfedorenko calling put_device(dev) with dev = NULL will return automatically

[vvfedorenko]

calling put_device(dev) with dev = NULL will return automatically

Ok, fair, but we still have to carefully control error paths

[CharlesParent]

By the way:

scripts/checkpatch.pl -f ptp_ocp.c --types=reverse_xmas_tree                           cparent@orolia2s.com@LAPTOPCP
total: 0 errors, 0 warnings, 4339 lines checked

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

ptp_ocp.c has no obvious style problems and is ready for submission.

NOTE: Used message types: REVERSE_XMAS_TREE

[CharlesParent]

@vvfedorenko I added gotos like what was done on read_eeprom. Is this sufficient to cleanly control error paths ?

[CharlesParent]

@jlemon Should userspace rather send address and offset of EEPROM's space to read/write ?

We decided to store the calibration parameters in EEPROM because these parameters are unique to the mRO50 soldered on a given ART Card. Originally everything was stored in oscillatord's config file, but it looks smarter to us to separate what is inherent to the card inside the EEPROM from what corresponds to how the card is seen by the server.

[CharlesParent]

@jlemon Another reason to store these parameters in EEPROM is to protect them from unwanted modifications. Also the IOCTL interface in temporary as Vadim wants to use netlink interface in the future

[jlemon]

It looks like the diff is also exposing the eeprom to user space. If we're doing that, then the best approach would be just reading/writing the parameters to the eeprom directly instead of from the driver. No need for ioctls/netlink in this case.

[CharlesParent]

@jlemon @leoleovich if I remember the well the point was to get red rid of the eeprom file from userspace to make sure data would not be altered by mistake.

Indeed it is way simpler to use a file from userspace to r/w data on the eeprom, but the point was to control operations on the eeprom using either ioctls in the driver and netlink protocol.

@leoleovich could you please tell us if the eeprom file would fit the security requirements or not ?

[jlemon]

This isn't a security issue, since the eeprom can be read/written through userspace i2c tools anyway.

The kernel really shouldn't know about the definition of disciplining_parameters, as this isn't used, it's just a binary blob. I'd prefer to not use an ioctl for this since there are other ways to accomplish the same thing.

[CharlesParent]

@jlemon I understood from @vvfedorenko that the end goal was to get rid of ioctls for this and use netlink protocol.

Could we stick with a ioctl for the moment to R/W a binary blob on the eeprom (used as disciplining parameters in userspace only) ?

[jlemon]

Sure - let me update the driver to cleanly bring in the nvmem support for reading/writing the eeprom. The ioctl can then just use that to read/write a binary blob to userspace.