regmap-spi will split data and address between two transfers
in the same message, so max_[read|write] must include space
for the address and padding
Signed-off-by: Lucas Tanure <[email protected]>
---
Changes v3:
New series
drivers/base/regmap/regmap-spi.c | 4 ++++
drivers/base/regmap/regmap.c | 15 +++++++++++++++
include/linux/regmap.h | 3 +++
3 files changed, 22 insertions(+)
diff --git a/drivers/base/regmap/regmap-spi.c b/drivers/base/regmap/regmap-spi.c
index 0e6552e57ecf..1434c502e340 100644
--- a/drivers/base/regmap/regmap-spi.c
+++ b/drivers/base/regmap/regmap-spi.c
@@ -123,6 +123,10 @@ static const struct regmap_bus *regmap_get_spi_bus(struct spi_device *spi,
bus->free_on_exit = true;
bus->max_raw_read = max_size;
bus->max_raw_write = max_size;
+
+ if (spi_max_message_size(spi) != SIZE_MAX)
+ bus->max_combined_rw = spi_max_message_size(spi);
+
return bus;
}
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 21a0c2562ec0..a99152f010f8 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -735,6 +735,7 @@ struct regmap *__regmap_init(struct device *dev,
struct regmap *map;
int ret = -EINVAL;
enum regmap_endian reg_endian, val_endian;
+ size_t reg_pad_size;
int i, j;
if (!config)
@@ -840,6 +841,20 @@ struct regmap *__regmap_init(struct device *dev,
if (bus) {
map->max_raw_read = bus->max_raw_read;
map->max_raw_write = bus->max_raw_write;
+ if (bus->max_combined_rw) {
+ reg_pad_size = map->format.reg_bytes + map->format.pad_bytes;
+
+ if (map->max_raw_read + reg_pad_size > bus->max_combined_rw)
+ map->max_raw_read -= reg_pad_size;
+ if (map->max_raw_write + reg_pad_size > bus->max_combined_rw)
+ map->max_raw_write -= reg_pad_size;
+
+ if (map->max_raw_read < map->format.buf_size ||
+ map->max_raw_write < map->format.buf_size) {
+ ret = -EINVAL;
+ goto err_hwlock;
+ }
+ }
}
map->dev = dev;
map->bus = bus;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e3c9a25a853a..a720f578b8e6 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -506,6 +506,8 @@ typedef void (*regmap_hw_free_context)(void *context);
* @max_raw_read: Max raw read size that can be used on the bus.
* @max_raw_write: Max raw write size that can be used on the bus.
* @free_on_exit: kfree this on exit of regmap
+ * @max_combined_rw: Max size for raw_read + raw_write, when they are issued
+ * together as part of the same message
*/
struct regmap_bus {
bool fast_io;
@@ -523,6 +525,7 @@ struct regmap_bus {
enum regmap_endian val_format_endian_default;
size_t max_raw_read;
size_t max_raw_write;
+ size_t max_combined_rw;
bool free_on_exit;
};
--
2.33.0
On Wed, Sep 15, 2021 at 01:09:51PM +0100, Lucas Tanure wrote:
> regmap-spi will split data and address between two transfers
> in the same message, so max_[read|write] must include space
> for the address and padding
>
> Signed-off-by: Lucas Tanure <[email protected]>
> ---
>
> Changes v3:
> New series
The changelog in patch 1 claimed that v2 was the first version of a new
series. Which is it? If you're going to include an inter-version
changelog it should be meaningful, just writing something for the sake
of providing a changelog tends to only cause confusion.
> + if (spi_max_message_size(spi) != SIZE_MAX)
> + bus->max_combined_rw = spi_max_message_size(spi);
> +
Why is this conditional? The whole point in returning SIZE_MAX is that
users don't need to care if there's a limit or not, any client logic
checking sizes will always work.
> + if (map->max_raw_read < map->format.buf_size ||
> + map->max_raw_write < map->format.buf_size) {
> + ret = -EINVAL;
> + goto err_hwlock;
> + }
> + }
This is at best an assert that the logic in _init() is self-consistent
so just silently returning an error doesn't seem like the right thing to
do, it's going to be super obscure for users.