On 12/17/23 15:06, Jonathan Cameron wrote:
On Fri, 15 Dec 2023 14:06:32 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
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
Maybe use rounding terminology rather than greater than?
regtable_find_val_roundup()?
Would be much better indeed. Thanks!
+ * 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_val_rounddown()?
Yes.
+/**
+ * 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).
Two integers and a type to make it harder still... IIO_VAL_INT_PLUS_MICRO etc
though I guess that might not need representing as generally the caller
would know what that was. Fixed point (ish) is a pain, but not come up with a better
presentation yet :(
I think the IIO-representation is fine. Sure it sucks that the real
world is set up in such a imperfect way that we do need fractions, but
as we do, IIO-way is just fine.
The thing IIO 'stuff' requires (and is not available in this draft) is
64 storage bits for the values. I guess I could increase the size in
tables to use u64 - and have an IIO-specific layer which could
pack/unpack the val and val2 in that 64 bits appropriately - but I'm not
sure it's worth the hassle. Besides, when users only need 32bits, 64bit
tables would be waste.
Adding support for both 32 and 64 bit tables would probably work - but
maybe this is just an overkill for a simple task. Creating just
IIO-specific helpers would be much leaner.
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 :)
It seems useful in general but I guess it's a question of whether you can find
enough users to justify it.
You're right of course. I think my question should've been if someone
can instantly think a type of devices that could benefit from these
helpers :)
Anyways, bit Thank You for the input! Your help is much appreciated as
always! :) Let's see if my "hands NOT full" time continues so I can
rethink this.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
