On 11/23/23 09:24, Matti Vaittinen wrote:
On 11/23/23 02:26, Marek Vasut wrote:
On 11/22/23 13:17, Matti Vaittinen wrote:
On 11/21/23 05:10, Marek Vasut wrote:
..snip
I like this table-based look-up for write (and read) of scales.
Looking at this I see an analogy to some of the regulator stuff, like
for example the ramp-up values. What I do very much like in the
regulator subsystem is the drivers/regulator/helpers.c
I wonder if similar approach would be usable in IIO as well? I mean,
providing readily written iio_regmap_read/write_raw_<functionality>()
and iio_available_*() helpers for the simple devices where we just
have value-register mapping? I mean, driver would just populate
something like:
struct iio_scale_desc {
int *scale_val_table;
int *scale_val2_table;
int num_scales;
You'd also need type here (fractional, int+micro, ...), right ?
Well, my thinking was to go with baby-steps. Eg, start by supporting
just int+micro - but yes. As I wrote below, this can be expanded by
allowing specifying the type.
int scale_reg_addr;
int scale_reg_mask;
};
and call helper like
int iio_regmap_read_raw_scale(struct iio_dev *idev,
struct iio_scale_desc *sd, int *val,
int *val2)"
provided by IIO framework.
Similar helper for writing new scales and getting available scales.
Later this could be expanded by allowing specifying the type of
provided values (in the example case, IIO_VAL_INT_PLUS_x - but maybe
this would be extensible (and worth) to support also the other options?)
... snip
The only thing I would wonder about is, should such a thing go into
regmap so it can be reused cross-subsystem instead of making this iio
specific ?
I definitely think a relation "register value" <=> "item from a table"
is very much used also outside the IIO. So yes, a generic regmap helper
for doing write as a "look value from table and write corresponding
value to a register" and "read value from register and return me a
corresponding item from a table" would be very usable.
There is a tradeoff when doing a generic one instead of making it
targeted for IIO use. Supporting different types of data is likely to
make the code a bit hairy. Also, the IIO way of having these IIO_VAL_*
flags does probably require IIO - specific wrappers in any case.
I had some spare time so drafted following:
+struct reg_val_table {
+ int *reg_vals;
+ int *vals;
+ int num_vals;
+};
...
+/**
+ * regtable_find_val - find a value matching register setting
+ *
+ * Search given table for value mathcing a register setting.
+ *
+ * @table: Table from which the register setting - value pairs are
+ * searched.
+ * @reg: Register value for which the matching physical value is
+ * searched.
+ * @val: Pointer to location where the found value will be stored.
+ *
+ * returns: 0 on success, negative errno if table is invalid or match is
+ * not found.
+ */
+int regtable_find_val(const struct reg_val_table *table, int reg, int *val)
+/**
+ * regtable_find_reg - find a register setting matching given value.
+ *
+ * Search given table for a register setting matching a value.
+ *
+ * @table: Table from which the register setting - value pairs are
+ * searched.
+ * @val: Value for which the matching register setting is searched.
+ * @reg: Pointer to location where the found register value will be
+ * stored.
+ *
+ * returns: 0 on success, negative errno if table is invalid or match is
+ * not found.
+ */
+int regtable_find_reg(const struct reg_val_table *table, int val, int *reg)
+/**
+ * regtable_find_greater_than_val - find the closest greater val and reg
+ *
+ * Search given table for the smallest value which is still greater than
+ * the given value. Both the found value and corresponding register
+ * setting are returned unless given pointers are NULL.
+ *
+ * @table: Table from which the register setting - value pairs are
+ * searched.
+ * @val_cmp: Value to which the values stored in table are compared to.
+ * @reg: NULL or pointer to location where the matching register
+ * setting value will be stored.
+ * @val: NULL or pointer to location where the found value will be
+ * stored.
+ *
+ * returns: 0 on success, negative errno if table is invalid or match is
+ * not found.
+ */
+int regtable_find_greater_than_val(const struct reg_val_table *table,
int val_cmp,
+ int *reg, int *val)
+/**
+ * regtable_find_smaller_than_val - find the closest smaller val and reg
+ *
+ * Search given table for the greatest value which is still smaller than
+ * the given value. Both the found value and corresponding register
+ * setting are returned unless given pointers are NULL.
+ *
+ * @table: Table from which the register setting - value pairs are
+ * searched.
+ * @val_cmp: Value to which the values stored in table are compared to.
+ * @reg: NULL or pointer to location where the matching register
+ * setting value will be stored.
+ * @val: NULL or pointer to location where the found value will be
+ * stored.
+ *
+ * returns: 0 on success, negative errno if table is invalid or match is
+ * not found.
+ */
+int regtable_find_smaller_than_val(const struct reg_val_table *table,
+ int val_cmp, int *reg, int *val)
and
+struct regmap_regval_table {
+ const struct reg_val_table table;
+ int reg;
+ int mask;
+};
+/**
+ * regmap_table_value_set - update register to match
human-understandable value
+ * @map: Register map
+ * @table: Table describing register-value, human-readable value
relation
+ * value: Human understandable value to configure in hardware.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int regmap_table_value_set(struct regmap *map,
+ const struct regmap_regval_table *table, int
value)
+/**
+ * regmap_table_value_get - return human-understandable configuration
+ *
+ * Reads hardware or regmap cache for current hardware configuration and
+ * converts the read register value to human understandable entity.
+ * @map: Register map
+ * @table: Table describing register-value, human-readable value
relation
+ * value: Human understandable value to configure in hardware.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int regmap_table_value_get(struct regmap *map,
+ const struct regmap_regval_table *table, int
*value)
(for anyone interested, whole thing + tests can be found from:
https://github.com/M-Vaittinen/linux/commits/regtable/
Just last 3 commits.)
I am however having difficulties in seeing how this could be utilized by
IIO, which tends to rely on values being represented by two integers
(val and val2).
Any suggestions regarding this idea? I'm wondering if I should just
scrap this and try seeing if I can make an IIO-specific helper(s) - or
if someone sees this would bring additional value worth an proper RFC? I
don't want to sen an RFC for people to properly review if this idea is
just plain stupid :)
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
