linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size
@ 2021-09-15 12:09 Lucas Tanure
  2021-09-15 12:09 ` [PATCH v3 2/2] regmap: spi: Check raw_[read|write] against max message size Lucas Tanure
  2021-09-15 12:34 ` [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Lucas Tanure @ 2021-09-15 12:09 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Rafael J . Wysocki
  Cc: linux-kernel, patches, Lucas Tanure

Set regmap raw read/write from spi max_transfer_size
so regmap_raw_read/write can split the access into chunks

Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
---

Changes v2:
New series

Changes v3:
None

 drivers/base/regmap/regmap-spi.c | 36 ++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/base/regmap/regmap-spi.c b/drivers/base/regmap/regmap-spi.c
index c1894e93c378..0e6552e57ecf 100644
--- a/drivers/base/regmap/regmap-spi.c
+++ b/drivers/base/regmap/regmap-spi.c
@@ -109,13 +109,37 @@ static const struct regmap_bus regmap_spi = {
 	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
+static const struct regmap_bus *regmap_get_spi_bus(struct spi_device *spi,
+						   const struct regmap_config *config)
+{
+	struct spi_master *master = spi->master;
+	struct regmap_bus *bus = NULL;
+	size_t max_size = spi_max_transfer_size(spi);
+
+	if (max_size != SIZE_MAX) {
+		bus = kmemdup(&regmap_spi, sizeof(*bus), GFP_KERNEL);
+		if (!bus)
+			return ERR_PTR(-ENOMEM);
+		bus->free_on_exit = true;
+		bus->max_raw_read = max_size;
+		bus->max_raw_write = max_size;
+		return bus;
+	}
+
+	return &regmap_spi;
+}
+
 struct regmap *__regmap_init_spi(struct spi_device *spi,
 				 const struct regmap_config *config,
 				 struct lock_class_key *lock_key,
 				 const char *lock_name)
 {
-	return __regmap_init(&spi->dev, &regmap_spi, &spi->dev, config,
-			     lock_key, lock_name);
+	const struct regmap_bus *bus = regmap_get_spi_bus(spi, config);
+
+	if (IS_ERR(bus))
+		return ERR_CAST(bus);
+
+	return __regmap_init(&spi->dev, bus, &spi->dev, config, lock_key, lock_name);
 }
 EXPORT_SYMBOL_GPL(__regmap_init_spi);
 
@@ -124,8 +148,12 @@ struct regmap *__devm_regmap_init_spi(struct spi_device *spi,
 				      struct lock_class_key *lock_key,
 				      const char *lock_name)
 {
-	return __devm_regmap_init(&spi->dev, &regmap_spi, &spi->dev, config,
-				  lock_key, lock_name);
+	const struct regmap_bus *bus = regmap_get_spi_bus(spi, config);
+
+	if (IS_ERR(bus))
+		return ERR_CAST(bus);
+
+	return __devm_regmap_init(&spi->dev, bus, &spi->dev, config, lock_key, lock_name);
 }
 EXPORT_SYMBOL_GPL(__devm_regmap_init_spi);
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] regmap: spi: Check raw_[read|write] against max message size
  2021-09-15 12:09 [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Lucas Tanure
@ 2021-09-15 12:09 ` Lucas Tanure
  2021-09-15 12:51   ` Mark Brown
  2021-09-15 12:34 ` [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Lucas Tanure @ 2021-09-15 12:09 UTC (permalink / raw)
  To: Mark Brown, Greg Kroah-Hartman, Rafael J . Wysocki
  Cc: linux-kernel, patches, Lucas Tanure

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 <tanureal@opensource.cirrus.com>
---

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size
  2021-09-15 12:09 [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Lucas Tanure
  2021-09-15 12:09 ` [PATCH v3 2/2] regmap: spi: Check raw_[read|write] against max message size Lucas Tanure
@ 2021-09-15 12:34 ` Mark Brown
  2021-09-15 12:37   ` Lucas tanure
  1 sibling, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-09-15 12:34 UTC (permalink / raw)
  To: Lucas Tanure
  Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel, patches

[-- Attachment #1: Type: text/plain, Size: 603 bytes --]

On Wed, Sep 15, 2021 at 01:09:50PM +0100, Lucas Tanure wrote:
> Set regmap raw read/write from spi max_transfer_size
> so regmap_raw_read/write can split the access into chunks
> 
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> ---
> 
> Changes v2:
> New series
> 
> Changes v3:
> None

How does this relate to the previous free standing copy of this patch
you sent only two days ago on Monday?  You've dropped Charles'
Reviewed-by so I guess there must be some change but this changelog
claims there's nothing (and appears to claim that v2 was the first
version...).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size
  2021-09-15 12:34 ` [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Mark Brown
@ 2021-09-15 12:37   ` Lucas tanure
  2021-09-15 12:57     ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Lucas tanure @ 2021-09-15 12:37 UTC (permalink / raw)
  To: Mark Brown; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel, patches

On 9/15/21 13:34, Mark Brown wrote:
> On Wed, Sep 15, 2021 at 01:09:50PM +0100, Lucas Tanure wrote:
>> Set regmap raw read/write from spi max_transfer_size
>> so regmap_raw_read/write can split the access into chunks
>>
>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
>> ---
>>
>> Changes v2:
>> New series
>>
>> Changes v3:
>> None
> 
> How does this relate to the previous free standing copy of this patch
> you sent only two days ago on Monday?  You've dropped Charles'
> Reviewed-by so I guess there must be some change but this changelog
> claims there's nothing (and appears to claim that v2 was the first
> version...).
> 
Hi,

Yes, its exactly the same, I forgot to add the second patch the previous 
one.
I will resend with Charles review-by.

Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] regmap: spi: Check raw_[read|write] against max message size
  2021-09-15 12:09 ` [PATCH v3 2/2] regmap: spi: Check raw_[read|write] against max message size Lucas Tanure
@ 2021-09-15 12:51   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-09-15 12:51 UTC (permalink / raw)
  To: Lucas Tanure
  Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel, patches

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]

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 <tanureal@opensource.cirrus.com>
> ---
> 
> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size
  2021-09-15 12:37   ` Lucas tanure
@ 2021-09-15 12:57     ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-09-15 12:57 UTC (permalink / raw)
  To: Lucas tanure
  Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel, patches

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On Wed, Sep 15, 2021 at 01:37:03PM +0100, Lucas tanure wrote:
> On 9/15/21 13:34, Mark Brown wrote:

> Yes, its exactly the same, I forgot to add the second patch the previous
> one.
> I will resend with Charles review-by.

Please allow a bit more time in case people are reviewing your whole
series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-15 12:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 12:09 [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Lucas Tanure
2021-09-15 12:09 ` [PATCH v3 2/2] regmap: spi: Check raw_[read|write] against max message size Lucas Tanure
2021-09-15 12:51   ` Mark Brown
2021-09-15 12:34 ` [PATCH v3 1/2] regmap: spi: Set regmap max raw r/w from max_transfer_size Mark Brown
2021-09-15 12:37   ` Lucas tanure
2021-09-15 12:57     ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).