diff mbox

[2/3] spi: add initial set of spi_transfer transformation methods

Message ID 1448888695-2260-3-git-send-email-kernel@martin.sperl.org (mailing list archive)
State New, archived
Headers show

Commit Message

Martin Sperl Nov. 30, 2015, 1:04 p.m. UTC
From: Martin Sperl <kernel@martin.sperl.org>

added the following spi_transformation methods:
* spi_split_transfers_first_page_len_not_aligned -
  this splits spi_transfers that are not aligned on rx_buf/tx_buf
  this will create 2 or 3 transfers, where the first 1/2 transfers are
  just there to allow the last transfer to be fully aligned. These first
  transfers will have a length less than alignment.
* spi_split_transfers_maxsize
  this splits the spi_transfer into multiple independent spi_transfers
  all of which will be of size max_size or smaller.

To start these shall get used by the individual drivers in prepare_message,
but some may get moved into spi-core with the correct parametrization in
spi_master.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/spi/spi.c       |  392 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi.h |   27 ++++
 2 files changed, 419 insertions(+)

Comments

Mark Brown Dec. 1, 2015, 9:29 p.m. UTC | #1
On Mon, Nov 30, 2015 at 01:04:53PM +0000, kernel@martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
> 
> added the following spi_transformation methods:
> * spi_split_transfers_first_page_len_not_aligned -
>   this splits spi_transfers that are not aligned on rx_buf/tx_buf
>   this will create 2 or 3 transfers, where the first 1/2 transfers are
>   just there to allow the last transfer to be fully aligned. These first
>   transfers will have a length less than alignment.
> * spi_split_transfers_maxsize
>   this splits the spi_transfer into multiple independent spi_transfers
>   all of which will be of size max_size or smaller.

Please split this up for review, as I'm fairly sure has been discussed
before smaller, more focused patches with clear changelogs describing
what they are suppose to do are much easier to review.  Start by adding
the framework changes, then add one transformation at once.  This is a
very big patch full of fiddly, unexplained code.

> To start these shall get used by the individual drivers in prepare_message,
> but some may get moved into spi-core with the correct parametrization in
> spi_master.

As discussed I'm not a big fan of this approach.

> +/* the spi_resource structure used */
> +struct spi_res_replaced_transfers {
> +	spi_res_release_t release;
> +	struct list_head replaced_transfers;
> +	int inserted;
> +	struct spi_transfer xfers[];
> +};

This quite clearly isn't a struct spi_resource, nor does it contain
one...

> +static void __spi_split_transfers_fixup_transfer_addr_and_len(
> +	struct spi_transfer *xfer, int count, int shift)
> +{
> +	int i;
> +
> +	/* fix up transfer length */
> +	xfer[0].len = shift;
> +
> +	/* shift all the addresses arround */
> +	for (i = 1; i < count; i++) {
> +		xfer[i].len -= shift;
> +		xfer[i].tx_buf += xfer[i].tx_buf ? shift : 0;
> +		xfer[i].rx_buf += xfer[i].rx_buf ? shift : 0;
> +		xfer[i].tx_dma += xfer[i].tx_dma ? shift : 0;
> +		xfer[i].rx_dma += xfer[i].rx_dma ? shift : 0;
> +	}
> +}

It's really not clear what this is supposed to do.  Among other things
"shift" appears to be misnamed and anything using tx_dma and rx_dma is
worrying.

I did read a bit further but like I said at the top this is a lot of
complicated code that's not split up enough to be clear.
Martin Sperl Dec. 2, 2015, 7:25 a.m. UTC | #2
> On 01.12.2015, at 22:29, Mark Brown <broonie@kernel.org> wrote:
> 
>> On Mon, Nov 30, 2015 at 01:04:53PM +0000, kernel@martin.sperl.org wrote:
>> From: Martin Sperl <kernel@martin.sperl.org>
>> 
>> added the following spi_transformation methods:
>> * spi_split_transfers_first_page_len_not_aligned -
>>  this splits spi_transfers that are not aligned on rx_buf/tx_buf
>>  this will create 2 or 3 transfers, where the first 1/2 transfers are
>>  just there to allow the last transfer to be fully aligned. These first
>>  transfers will have a length less than alignment.
>> * spi_split_transfers_maxsize
>>  this splits the spi_transfer into multiple independent spi_transfers
>>  all of which will be of size max_size or smaller.
> 
> Please split this up for review, as I'm fairly sure has been discussed
> before smaller, more focused patches with clear changelogs describing
> what they are suppose to do are much easier to review.  Start by adding
> the framework changes, then add one transformation at once.  This is a
> very big patch full of fiddly, unexplained code.

I can split this in two or 3, as I already have a prototype that allows for
merging multiple transfers into one transfer by copying data arround.

> 
>> To start these shall get used by the individual drivers in prepare_message,
>> but some may get moved into spi-core with the correct parametrization in
>> spi_master.
> 
> As discussed I'm not a big fan of this approach.
It is just a first step.

As far as I can see different hw may require different policies.

If there is a common code to handle all the variations then we can
Implement that, but I have no idea what that could be.

That is why I took this prepare message approach for now as it
allows most control from the driver perspective (and is not 
breaking existing drivers)

As said: this can get moved to the core when we have found the
correct parametrization for driver.
Parameters that come to mind are:
* dma_alignment
* max_transfersize (for which there exists a patch already)

> 
>> +/* the spi_resource structure used */
>> +struct spi_res_replaced_transfers {
>> +    spi_res_release_t release;
>> +    struct list_head replaced_transfers;
>> +    int inserted;
>> +    struct spi_transfer xfers[];
>> +};
> 
> This quite clearly isn't a struct spi_resource, nor does it contain
> one...
It is done the same way as devm_kalloc uses struct devres:
the magic devres struct is hidden by the devres framework. 
Only the payload data pointer is returned, which is what this
structure describes.

You want it as a more explicit member instead?

> 
>> +static void __spi_split_transfers_fixup_transfer_addr_and_len(
>> +    struct spi_transfer *xfer, int count, int shift)
>> +{
>> +    int i;
>> +
>> +    /* fix up transfer length */
>> +    xfer[0].len = shift;
>> +
>> +    /* shift all the addresses arround */
>> +    for (i = 1; i < count; i++) {
>> +        xfer[i].len -= shift;
>> +        xfer[i].tx_buf += xfer[i].tx_buf ? shift : 0;
>> +        xfer[i].rx_buf += xfer[i].rx_buf ? shift : 0;
>> +        xfer[i].tx_dma += xfer[i].tx_dma ? shift : 0;
>> +        xfer[i].rx_dma += xfer[i].rx_dma ? shift : 0;
>> +    }
>> +}
> 
> It's really not clear what this is supposed to do.  Among other things
> "shift" appears to be misnamed and anything using tx_dma and rx_dma is
> worrying.
Essentially this will handle count spi-transfers that have all identical data
(Except for transfer_list) initially - especially Len and tx/rx buf.
The first of those transfers is set its length to shift 
(no need to modify rx/tx_buf)
The others are reduced by that length and the pointers are shifted up,
So that they are now starting at the address after the first transfer and
Transfer the correct number of bytes.

Rx/tx_dma is handled because it is there and it's address needs also to be
accurately Shifted by that offset. I know that it is a depreciated interface,
but as it is still there it needs to be supported...


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Dec. 3, 2015, 12:36 a.m. UTC | #3
On Wed, Dec 02, 2015 at 08:25:08AM +0100, Martin Sperl wrote:
> > On 01.12.2015, at 22:29, Mark Brown <broonie@kernel.org> wrote:
> >> On Mon, Nov 30, 2015 at 01:04:53PM +0000, kernel@martin.sperl.org wrote:

Please use normal formatting for e-mails with blank lines between
paragraphs, it makes things much easier to read.

> >> +/* the spi_resource structure used */
> >> +struct spi_res_replaced_transfers {
> >> +    spi_res_release_t release;
> >> +    struct list_head replaced_transfers;
> >> +    int inserted;
> >> +    struct spi_transfer xfers[];
> >> +};

> > This quite clearly isn't a struct spi_resource, nor does it contain
> > one...

> It is done the same way as devm_kalloc uses struct devres:
> the magic devres struct is hidden by the devres framework. 
> Only the payload data pointer is returned, which is what this
> structure describes.

> You want it as a more explicit member instead?

If it's not actually a resource and is really just a structure that
happens to be managed with a resource then describe it as such, I'm
complaining about the confusing naming here.

> >> +static void __spi_split_transfers_fixup_transfer_addr_and_len(
> >> +    struct spi_transfer *xfer, int count, int shift)
> >> +{
> >> +    int i;

> >> +    /* fix up transfer length */
> >> +    xfer[0].len = shift;

> > It's really not clear what this is supposed to do.  Among other things
> > "shift" appears to be misnamed and anything using tx_dma and rx_dma is
> > worrying.

> Essentially this will handle count spi-transfers that have all identical data
> (Except for transfer_list) initially - especially Len and tx/rx buf.
> The first of those transfers is set its length to shift 
> (no need to modify rx/tx_buf)
> The others are reduced by that length and the pointers are shifted up,
> So that they are now starting at the address after the first transfer and
> Transfer the correct number of bytes.

I'm sorry but the above is still very opaque - why would we have some
list of transfers that all have identical data?  I *think* the intention
here is to do some realignment and that shift is really an offset to
move things by?

> Rx/tx_dma is handled because it is there and it's address needs also to be
> accurately Shifted by that offset. I know that it is a depreciated interface,
> but as it is still there it needs to be supported...

It's really quite broken already for most users.
Martin Sperl Dec. 3, 2015, 6:22 a.m. UTC | #4
> On 03.12.2015, at 01:36, Mark Brown <broonie@kernel.org> wrote:
> 
> On Wed, Dec 02, 2015 at 08:25:08AM +0100, Martin Sperl wrote:
>>>> On 01.12.2015, at 22:29, Mark Brown <broonie@kernel.org> wrote:
>>>> On Mon, Nov 30, 2015 at 01:04:53PM +0000, kernel@martin.sperl.org wrote:
> 
> Please use normal formatting for e-mails with blank lines between
> paragraphs, it makes things much easier to read.
> 
>>>> +/* the spi_resource structure used */
>>>> +struct spi_res_replaced_transfers {
>>>> +    spi_res_release_t release;
>>>> +    struct list_head replaced_transfers;
>>>> +    int inserted;
>>>> +    struct spi_transfer xfers[];
>>>> +};
> 
>>> This quite clearly isn't a struct spi_resource, nor does it contain
>>> one...
> 
>> It is done the same way as devm_kalloc uses struct devres:
>> the magic devres struct is hidden by the devres framework. 
>> Only the payload data pointer is returned, which is what this
>> structure describes.
> 
>> You want it as a more explicit member instead?
> 
> If it's not actually a resource and is really just a structure that
> happens to be managed with a resource then describe it as such, I'm
> complaining about the confusing naming here.

Ok understood now - I feared you meant you disliked the whole approach. 

> 
>>>> +static void __spi_split_transfers_fixup_transfer_addr_and_len(
>>>> +    struct spi_transfer *xfer, int count, int shift)
>>>> +{
>>>> +    int i;
> 
>>>> +    /* fix up transfer length */
>>>> +    xfer[0].len = shift;
> 
>>> It's really not clear what this is supposed to do.  Among other things
>>> "shift" appears to be misnamed and anything using tx_dma and rx_dma is
>>> worrying.
> 
>> Essentially this will handle count spi-transfers that have all identical data
>> (Except for transfer_list) initially - especially Len and tx/rx buf.
>> The first of those transfers is set its length to shift 
>> (no need to modify rx/tx_buf)
>> The others are reduced by that length and the pointers are shifted up,
>> So that they are now starting at the address after the first transfer and
>> Transfer the correct number of bytes.
> 
> I'm sorry but the above is still very opaque - why would we have some
> list of transfers that all have identical data?  I *think* the intention
> here is to do some realignment and that shift is really an offset to
> move things by?

Yes, so probably the naming is wrong.
As for identical data: the spi_transfer_replace method also copies the
first transfer to to be replaced to all the newly created transfers
Before replacing/splicing them into spi_message.transfers.

Anyway: there is something probably not working for the alignment
code when rx/tx_buf are mis-aligned by different offsets (say 1 and 2)
even though I ran all those extensive transfer tests on bcm2835 and
they run fine there.

> 
>> Rx/tx_dma is handled because it is there and it's address needs also to be
>> accurately Shifted by that offset. I know that it is a depreciated interface,
>> but as it is still there it needs to be supported...
> 
> It's really quite broken already for most users.
If it is broken, then why not remove those members?
Or set those pointers to null in spi_verify (or at least croak about it
being depreciated)

With the current situation the code needs to handle those transfers
correctly - especially if we want to enable it by default in spi-core
for all spi-masters eventually.

One last thing:
Where do you prefer to run those transfer transformations?
In the master-loop (before/after prepare message)?
During spi_verify (or after it)?

The advantage of spi-verify would be that we potentially run those
transformations in a different thread than message-pump allowing
to make better use of multiple CPUs. (This assumes that spi-async
is used or multiple drivers submit to spi_master concurrently,
so that queueing occurs and the message-pump thread is woken).

Actually the same argument of potential concurrency would also 
support the move of scatter/gather list generation into the same
place (which is essentially just another transformation)

I will rewrite those patches separating them out a bit more
and resubmitting them still based on the dev-res approach,
But renaming those structures.--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Dec. 3, 2015, 2:34 p.m. UTC | #5
On Thu, Dec 03, 2015 at 07:22:10AM +0100, Martin Sperl wrote:
> > On 03.12.2015, at 01:36, Mark Brown <broonie@kernel.org> wrote:

> >> Rx/tx_dma is handled because it is there and it's address needs also to be
> >> accurately Shifted by that offset. I know that it is a depreciated interface,
> >> but as it is still there it needs to be supported...

> > It's really quite broken already for most users.

> If it is broken, then why not remove those members?

There are still a couple of users and someone needs to go through and
fix them.  I don't know which if any systems they run on, it's possible
that they manage to work as a result of the DMA mappings not getting in
the way the systems they're being used on (if they're being used at all)
or the controller drivers they're used with being equally aged.

> One last thing:
> Where do you prefer to run those transfer transformations?
> In the master-loop (before/after prepare message)?
> During spi_verify (or after it)?

Remember that we can't do any allocations in _verify() as it can run in
atomic context so while we want to check if we can take the message then
we are most likely going to be unable to implement transformations.
Martin Sperl Dec. 3, 2015, 3:33 p.m. UTC | #6
> On 03.12.2015, at 15:34, Mark Brown <broonie@kernel.org> wrote:
> 
> On Thu, Dec 03, 2015 at 07:22:10AM +0100, Martin Sperl wrote:
>>> On 03.12.2015, at 01:36, Mark Brown <broonie@kernel.org> wrote:
> 
>>>> Rx/tx_dma is handled because it is there and it's address needs also to be
>>>> accurately Shifted by that offset. I know that it is a depreciated interface,
>>>> but as it is still there it needs to be supported...
> 
>>> It's really quite broken already for most users.
> 
>> If it is broken, then why not remove those members?
> 
> There are still a couple of users and someone needs to go through and
> fix them.  I don't know which if any systems they run on, it's possible
> that they manage to work as a result of the DMA mappings not getting in
> the way the systems they're being used on (if they're being used at all)
> or the controller drivers they're used with being equally aged.

So if we make use of this “optimization” globally we will need to support it,
otherwise we break it.

That was one of the reasons why I am advocating (for now) that we
make it an option that has to get enabled in the individual drivers.

But we could minimize the coding by providing a “default” implementation
that could get assigned to spi_master.prepare_message.

So an spi_master would only need to assign that, which makes it clear that
this has been tested with this specific HW (which is my biggest concern).

We could also create a separate spi_master.transform_message method, which
the drivers assign to the default or custom implementation and which
(eventually) may get assigned a default if NULL and dma_alignment
as well as (the future) max_transfer_size is set.

The advantage of this would be that we could (at a later stage) move the
invocation translation method to a different location - say: queuing it
in a different worker thread for translation when there are more than one
spi_message in the queue (if it is submitted from idle we would execute in
place at somewhere in __spi_pump_messages).

Just exploring some options...

Would it make any sense to separate it out as described, so that we can do
something like this later?

>> One last thing:
>> Where do you prefer to run those transfer transformations?
>> In the master-loop (before/after prepare message)?
>> During spi_verify (or after it)?
> 
> Remember that we can't do any allocations in _verify() as it can run in
> atomic context so while we want to check if we can take the message then
> we are most likely going to be unable to implement transformations.
I came to realize that in the meantime as well - it was just an idea
how we could defer such things to a second CPU...--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Dec. 3, 2015, 6:04 p.m. UTC | #7
On Thu, Dec 03, 2015 at 04:33:52PM +0100, Martin Sperl wrote:
> > On 03.12.2015, at 15:34, Mark Brown <broonie@kernel.org> wrote:

> > There are still a couple of users and someone needs to go through and
> > fix them.  I don't know which if any systems they run on, it's possible
> > that they manage to work as a result of the DMA mappings not getting in
> > the way the systems they're being used on (if they're being used at all)
> > or the controller drivers they're used with being equally aged.

> So if we make use of this “optimization” globally we will need to support it,
> otherwise we break it.

I'm relatively OK with breaking these users TBH, I'm not convinced any
of them are actually being run and it's probably easier to fix by fixing
the users when they're engaged.  Also if we do this in the message queue
thread then we're OK anyway since anything that uses these will old
enough to be using a custom message queue anyway so the risk is less
than it might appear.

> > Remember that we can't do any allocations in _verify() as it can run in
> > atomic context so while we want to check if we can take the message then
> > we are most likely going to be unable to implement transformations.

> I came to realize that in the meantime as well - it was just an idea
> how we could defer such things to a second CPU...

We could potentially try doing this in both call sites - we already have
special casing for spi_sync().  It's more fiddly but it could be done as
a future step.
diff mbox

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index eecbbe1..7576131 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -145,6 +145,9 @@  SPI_STATISTICS_TRANSFER_BYTES_HISTO(14, "16384-32767");
 SPI_STATISTICS_TRANSFER_BYTES_HISTO(15, "32768-65535");
 SPI_STATISTICS_TRANSFER_BYTES_HISTO(16, "65536+");
 
+SPI_STATISTICS_SHOW(transfers_split_maxsize, "%lu");
+SPI_STATISTICS_SHOW(transfers_split_unaligned, "%lu");
+
 static struct attribute *spi_dev_attrs[] = {
 	&dev_attr_modalias.attr,
 	NULL,
@@ -182,6 +185,8 @@  static struct attribute *spi_device_statistics_attrs[] = {
 	&dev_attr_spi_device_transfer_bytes_histo14.attr,
 	&dev_attr_spi_device_transfer_bytes_histo15.attr,
 	&dev_attr_spi_device_transfer_bytes_histo16.attr,
+	&dev_attr_spi_device_transfers_split_maxsize.attr,
+	&dev_attr_spi_device_transfers_split_unaligned.attr,
 	NULL,
 };
 
@@ -224,6 +229,8 @@  static struct attribute *spi_master_statistics_attrs[] = {
 	&dev_attr_spi_master_transfer_bytes_histo14.attr,
 	&dev_attr_spi_master_transfer_bytes_histo15.attr,
 	&dev_attr_spi_master_transfer_bytes_histo16.attr,
+	&dev_attr_spi_master_transfers_split_maxsize.attr,
+	&dev_attr_spi_master_transfers_split_unaligned.attr,
 	NULL,
 };
 
@@ -2103,6 +2110,391 @@  void spi_res_release(struct spi_master *master,
 	}
 }
 EXPORT_SYMBOL_GPL(spi_res_release);
+/*-------------------------------------------------------------------------*/
+
+/* Core methods for spi_message alterations */
+
+/* the spi_resource structure used */
+struct spi_res_replaced_transfers {
+	spi_res_release_t release;
+	struct list_head replaced_transfers;
+	int inserted;
+	struct spi_transfer xfers[];
+};
+
+static void __spi_replace_transfers_release(struct spi_master *master,
+					    struct spi_message *msg,
+					    void *res)
+{
+	struct spi_res_replaced_transfers *srt = res;
+	int i;
+
+	/* call extra callback */
+	if (srt->release)
+		srt->release(master, msg, res);
+
+	/* insert transfers back into the message ahead of xfers[0] */
+	list_splice(&srt->replaced_transfers, &srt->xfers[0].transfer_list);
+
+	/* remove the formerly inserted entries */
+	for (i = 0; i < srt->inserted; i++)
+		list_del(&srt->xfers[i].transfer_list);
+}
+
+/**
+ * spi_replace_transfers - replace transfers with several transfers
+ *                         and register change with spi_message.resources
+ * @msg: the spi_message we work upon
+ * @xfer: the spi_transfer we want to replace
+ * @remove: number of transfers to remove
+ * @insert: the number of transfers we want to insert
+ * @release: extra release code necessary in some circumstances
+ *
+ * Returns: pointer to array of newly inserted transfers,
+ *          NULL in case of errors
+ */
+struct spi_transfer *spi_replace_transfers(struct spi_message *msg,
+					   struct spi_transfer *xfer,
+					   int remove, int insert,
+					   spi_res_release_t release)
+{
+	int i;
+	size_t size = insert * sizeof(struct spi_transfer)
+		      + sizeof(struct spi_res_replaced_transfers);
+	struct spi_res_replaced_transfers *srt;
+	struct spi_transfer *next;
+
+	if (unlikely(insert < 1))
+		return NULL;
+
+	/* allocate the structure */
+	srt = spi_res_alloc(msg->spi, __spi_replace_transfers_release,
+			    size, 0);
+	if (unlikely(!srt))
+		return NULL;
+
+	/* the release code to use before running the generic release */
+	srt->release = release;
+
+	/* create copy of the given xfer with identical settings */
+	srt->inserted = insert;
+	for (i = 0; i < insert ; i++) {
+		/* copy all settings */
+		memcpy(&srt->xfers[i], xfer, sizeof(*xfer));
+
+		/* for anything but the last transfer, clear some settings */
+		if (i < insert - 1) {
+			srt->xfers[i].cs_change = 0;
+			srt->xfers[i].delay_usecs = 0;
+		}
+
+		/* add before the xfer to remove itself */
+		list_add_tail(&srt->xfers[i].transfer_list,
+			      &xfer->transfer_list);
+	}
+
+	/* remove the requested number of transfers */
+	for (i = 0; i < remove; i++, xfer = next) {
+		/* check for error case - we want to remove list_head...*/
+		if (&xfer->transfer_list == &msg->transfers) {
+			dev_err(&msg->spi->dev,
+				"requested to remove more spi_transfers than are available\n");
+			spi_res_free(srt);
+			return NULL;
+		}
+		/* get the next xfer for later */
+		next = list_next_entry(xfer, transfer_list);
+
+		/* and remove the current transfer from the list of transfers */
+		list_del_init(&xfer->transfer_list);
+
+		/* and add it to the list in spi_res_replaced_transfers */
+		INIT_LIST_HEAD(&srt->replaced_transfers);
+		list_add(&xfer->transfer_list, &srt->replaced_transfers);
+	}
+
+	/* and register it */
+	spi_res_add(msg, srt);
+
+	/* return the head of the list */
+	return &srt->xfers[0];
+}
+EXPORT_SYMBOL_GPL(spi_replace_transfers);
+
+/* core spi_transfer transformation */
+
+static void __spi_split_transfers_fixup_transfer_addr_and_len(
+	struct spi_transfer *xfer, int count, int shift)
+{
+	int i;
+
+	/* fix up transfer length */
+	xfer[0].len = shift;
+
+	/* shift all the addresses arround */
+	for (i = 1; i < count; i++) {
+		xfer[i].len -= shift;
+		xfer[i].tx_buf += xfer[i].tx_buf ? shift : 0;
+		xfer[i].rx_buf += xfer[i].rx_buf ? shift : 0;
+		xfer[i].tx_dma += xfer[i].tx_dma ? shift : 0;
+		xfer[i].rx_dma += xfer[i].rx_dma ? shift : 0;
+	}
+}
+
+static int __spi_split_transfers_first_page_len_not_aligned(
+	struct spi_master *master,
+	struct spi_message *message,
+	struct spi_transfer *xfer,
+	size_t alignment_mask)
+{
+	int count;
+	struct spi_transfer *xfers;
+	const char *tx_start, *rx_start; /* the rx/tx_buf address */
+	const char *tx_end, *rx_end; /* the last byte of the transfer */
+	size_t tx_start_page, rx_start_page; /* the "page address" for start */
+	size_t tx_end_page, rx_end_page; /* the "page address" for end */
+	size_t tx_start_align, rx_start_align; /* alignment of buf address */
+
+	/* calculate the necessary values */
+	tx_start = xfer->tx_buf;
+	tx_start_page = (size_t)tx_start & (PAGE_SIZE - 1);
+	tx_start_align = ((size_t)tx_start & alignment_mask);
+	tx_end = tx_start ? &tx_start[xfer->len - 1] : NULL;
+	tx_end_page = (size_t)tx_end & (PAGE_SIZE - 1);
+
+	rx_start = xfer->rx_buf;
+	rx_start_page = (size_t)rx_start & (PAGE_SIZE - 1);
+	rx_start_align = ((size_t)rx_start & alignment_mask);
+	rx_end = rx_start ? &tx_start[xfer->len - 1] : NULL;
+	rx_end_page = (size_t)rx_end & (PAGE_SIZE - 1);
+
+	/* if we do not cross a page for either rx or tx,
+	 * then there is nothing to do...
+	 */
+	if ((tx_start_page == tx_end_page) &&
+	    (rx_start_page == rx_end_page))
+		return 0;
+
+	/* calculate how many transfers we need to replace the current */
+	count = 1;
+	if (rx_start_align)
+		count++;
+	if ((tx_start_align) &&
+	    (tx_start_align != rx_start_align) &&
+	    (tx_start != rx_start))
+		count++;
+
+	/* send a one-time warning */
+	dev_warn_once(&message->spi->dev,
+		      "unaligned spi_transfers produced by spi_device driver - please fix driver\n");
+
+	/* create replacement */
+	xfers = spi_replace_transfers(message, xfer, 1, count, NULL);
+	if (!xfers)
+		return -ENOMEM;
+
+	/* now we fix up the transfer pointer and transfer len */
+	if (count == 2) {
+		__spi_split_transfers_fixup_transfer_addr_and_len(
+			xfers, 2, max(tx_start_align, rx_start_align));
+	} else {
+		if (tx_start_align < rx_start_align) {
+			__spi_split_transfers_fixup_transfer_addr_and_len(
+				&xfers[0], 3,
+				tx_start_align);
+			__spi_split_transfers_fixup_transfer_addr_and_len(
+				&xfers[1], 2,
+				rx_start_align - tx_start_align);
+		} else {
+			__spi_split_transfers_fixup_transfer_addr_and_len(
+				&xfers[0], 3,
+				rx_start_align);
+			__spi_split_transfers_fixup_transfer_addr_and_len(
+				&xfers[1], 2,
+				tx_start_align - rx_start_align);
+		}
+	}
+
+	/* increment statistics counters */
+	SPI_STATISTICS_INCREMENT_FIELD(&master->statistics,
+				       transfers_split_unaligned);
+	SPI_STATISTICS_INCREMENT_FIELD(&message->spi->statistics,
+				       transfers_split_unaligned);
+
+	return 0;
+}
+
+/**
+ * spi_split_tranfers_first_page_len_not_aligned - split spi transfers into
+ *                                                 two transfers on the
+ *                                                 first page boundary, when
+ *                                                 the start address is not
+ *                                                 aligned
+ * @master:       the @spi_master for this transfer
+ * @message:      the @spi_message to transform
+ * @when_can_dma: true if we only should execute when we can use dma
+ * @alignment:    the requested alignment
+ * @min_size:     the minimum transfer when to apply this
+ *
+ * Return: status of transformation
+ *
+ * This implements one of the ways that (dma-enabled) HW may be constrained
+ * in this case the HW is able to do unaligned transfers, but a DMA-transfer
+ * is always of size align from a register perspective (even if it only
+ * writes < align bytes back to ram).
+ * As there is no guarantee that the next logical page is actually adjectant
+ * for the dma perspective, we need to split the first page of a transfer
+ * into separate transfers (1 or 2 depending on alignment of rx_buf and
+ * tx_buf respectively).
+ * this does not make sure that the individual transfers that are
+ * added are address, aligned, but it makes sure that each of those
+ * new transfers.len is < alignment.
+ *
+ */
+int spi_split_transfers_first_page_len_not_aligned(
+	struct spi_master *master,
+	struct spi_message *message,
+	bool when_can_dma,
+	size_t alignment)
+{
+	struct spi_device *spi = message->spi;
+	struct spi_transfer *_xfer, *xfer = NULL;
+	size_t alignment_mask = alignment - 1;
+	int ret;
+
+	/* if when_can_dma is set, but can_dma is unset,
+	 * something major is wrong...
+	 */
+	if (when_can_dma && (!master->can_dma)) {
+		dev_err(&master->dev,
+			"configured without can_dma, but requests can_dma to be set calling first_page_len_not_aligned\n");
+		return -EINVAL;
+	}
+
+	/* iterate over the transfer_list in a safe manner
+	 * there is a posibility of replacement and we need to make sure
+	 * we run over all the entries
+	 */
+	xfer = list_prepare_entry(xfer, &message->transfers, transfer_list);
+	list_for_each_entry_safe_continue(xfer, _xfer,
+					  &message->transfers,
+					  transfer_list) {
+		if (when_can_dma && (!master->can_dma(master, spi, xfer)))
+			continue;
+		if (((size_t)xfer->rx_buf & alignment_mask) ||
+		    ((size_t)xfer->tx_buf & alignment_mask)) {
+			ret = __spi_split_transfers_first_page_len_not_aligned(
+				master, message, xfer, alignment_mask);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_split_transfers_first_page_len_not_aligned);
+
+int __spi_split_transfer_maxsize(struct spi_master *master,
+				 struct spi_message *msg,
+				 struct spi_transfer *xfer,
+				 size_t maxsize)
+{
+	int count = DIV_ROUND_UP(xfer->len, maxsize);
+	size_t offset = 0;
+	int i;
+	struct spi_transfer *xfers;
+
+	/* create replacement */
+	xfers = spi_replace_transfers(msg, xfer, 1, count, NULL);
+	if (!xfers)
+		return -ENOMEM;
+
+	/* now handle each of those newly inserted xfers */
+	for (i = 0; i < count; i++) {
+		/* the first transfer just needs the length modified,
+		 * so do not change the pointers
+		 */
+		if (i) {
+			/* update rx_buf if it is not tx_buf */
+			if (xfers[i].rx_buf &&
+			    (xfers[i].tx_buf != xfers[i].rx_buf)) {
+				/* modify the rx_buf */
+				xfers[i].rx_buf += offset;
+				/* this is actually not used with any
+				 * scatter_lists, but it is still there,
+				 * so we have to support it
+				 */
+				if (xfers[i].rx_dma)
+					xfers[i].rx_dma += offset;
+			}
+			/* update tx_buf */
+			if (xfers[i].tx_buf) {
+				/* modify the rx_buf */
+				xfers[i].tx_buf += offset;
+				/* this is actually not used with any
+				 * scatter_lists, but it is still there,
+				 * so we have to support it...
+				 */
+				if (xfers[i].tx_dma)
+					xfers[i].tx_dma += offset;
+			}
+		}
+		/* modify length */
+		if (i < count - 1) /* for all but the last transfer */
+			xfers[i].len = maxsize;
+		else /* the last one is most likley not max_size */
+			xfers[i].len -= offset;
+
+		/* increment offset */
+		offset += maxsize;
+	}
+
+	/* increment statistics counters */
+	SPI_STATISTICS_INCREMENT_FIELD(&master->statistics,
+				       transfers_split_maxsize);
+	SPI_STATISTICS_INCREMENT_FIELD(&msg->spi->statistics,
+				       transfers_split_maxsize);
+
+	return 0;
+}
+
+/**
+ * spi_split_tranfers_maxsize - split spi transfers into multiple transfers
+ *                              when an individual transfer exceeds a
+ *                              certain size
+ * @master:    the @spi_master for this transfer
+ * @message:   the @spi_message to transform
+ * @max_size:  the maximum when to apply this
+ *
+ * Return: status of transformation
+ */
+
+int spi_split_transfers_maxsize(struct spi_master *master,
+				struct spi_message *msg,
+				size_t maxsize)
+{
+	struct spi_transfer *_xfer, *xfer = NULL;
+	int ret;
+
+	/* iterate over the transfer_list in a safe manner
+	 * there is a posibility of replacement and we need to make sure
+	 * we run over all the entries
+	 */
+	xfer = list_prepare_entry(xfer, &msg->transfers, transfer_list);
+	list_for_each_entry_safe_continue(xfer, _xfer,
+					  &msg->transfers,
+					  transfer_list) {
+		if (xfer->len > maxsize) {
+			ret = __spi_split_transfer_maxsize(
+				master, msg, xfer, maxsize);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_split_transfers_maxsize);
 
 /*-------------------------------------------------------------------------*/
 
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 7e74e0e..8075c93 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -53,6 +53,11 @@  extern struct bus_type spi_bus_type;
  *
  * @transfer_bytes_histo:
  *                 transfer bytes histogramm
+ *
+ * @transfers_split_maxsize:
+ *                 number of transfers split because of len exceeds maxsize
+ * @transfers_split_unaligned:
+ *                 number of transfers split because of unaligned transfers
  */
 struct spi_statistics {
 	spinlock_t		lock; /* lock for the whole structure */
@@ -72,6 +77,10 @@  struct spi_statistics {
 
 #define SPI_STATISTICS_HISTO_SIZE 17
 	unsigned long transfer_bytes_histo[SPI_STATISTICS_HISTO_SIZE];
+
+	unsigned long           transfers_split_maxsize;
+	unsigned long           transfers_split_unaligned;
+
 };
 
 void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
@@ -607,6 +616,24 @@  extern void spi_res_free(void *res);
 extern void spi_res_release(struct spi_master *master,
 			    struct spi_message *message);
 
+/* SPI transfer modifications using spi_res */
+
+extern struct spi_transfer *spi_replace_transfers(
+	struct spi_message *msg,
+	struct spi_transfer *xfer,
+	int remove, int insert,
+	spi_res_release_t release);
+
+extern int spi_split_transfers_first_page_len_not_aligned(
+	struct spi_master *master,
+	struct spi_message *message,
+	bool when_can_dma,
+	size_t alignment);
+
+extern int spi_split_transfers_maxsize(struct spi_master *master,
+				       struct spi_message *msg,
+				       size_t maxsize);
+
 /*---------------------------------------------------------------------------*/
 
 /*