On Wed, Dec 02, 2015 at 08:25:08AM +0100, Martin Sperl wrote:
> > On 01.12.2015, at 22:29, Mark Brown <broonie@xxxxxxxxxx> wrote:
> >> On Mon, Nov 30, 2015 at 01:04:53PM +0000, kernel@xxxxxxxxxxxxxxxx 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.
Attachment:
signature.asc
Description: PGP signature
