On Mon, Nov 26, 2012 at 11:20:32AM +0100, Per Förlin wrote:
> On 11/22/2012 06:37 PM, Russell King - ARM Linux wrote:
> > On Thu, Nov 22, 2012 at 06:28:30PM +0100, Per Forlin wrote:
> >> On Wed, Nov 21, 2012 at 5:50 PM, Russell King - ARM Linux
> >> <linux@xxxxxxxxxxxxxxxx> wrote:
> >>> On Wed, Nov 21, 2012 at 05:13:55PM +0100, Per Forlin wrote:
> >>>> On Wed, Nov 21, 2012 at 4:38 PM, Russell King - ARM Linux
> >>>> <linux@xxxxxxxxxxxxxxxx> wrote:
> >>>>> On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
> >>>>>> /*
> >>>>>> + * Validate mmc prerequisites
> >>>>>> + */
> >>>>>> +static int mmci_validate_data(struct mmci_host *host,
> >>>>>> + struct mmc_data *data)
> >>>>>> +{
> >>>>>> + if (!data)
> >>>>>> + return 0;
> >>>>>> +
> >>>>>> + if (!host->variant->non_power_of_2_blksize &&
> >>>>>> + !is_power_of_2(data->blksz)) {
> >>>>>> + dev_err(mmc_dev(host->mmc),
> >>>>>> + "unsupported block size (%d bytes)\n", data->blksz);
> >>>>>> + return -EINVAL;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (data->sg->offset & 3) {
> >>>>>> + dev_err(mmc_dev(host->mmc),
> >>>>>> + "unsupported alginment (0x%x)\n", data->sg->offset);
> >>>>>> + return -EINVAL;
> >>>>>> + }
> >>>>>
> >>>>> Why? What's the reasoning behind this suddenly introduced restriction?
> >>>>> readsl()/writesl() copes just fine with non-aligned pointers. It may be
> >>>>> that your DMA engine can not, but that's no business interfering with
> >>>>> non-DMA transfers, and no reason to fail such transfers.
> >>>>>
> >>>>> If your DMA engine can't do that then its your DMA engine code which
> >>>>> should refuse to prepare the transfer.
> >>>>>
> >>>>> Yes, that means problems with the way things are ordered - or it needs a
> >>>>> proper API where DMA engine can export these kinds of properties.
> >>>> The alignment constraint is related to PIO, sg_miter and that FIFO
> >>>> access must be done with 4 bytes.
> >>>
> >>> Total claptrap. No it isn't.
> >>>
> >>> - sg_miter just deals with bytes, and number of bytes transferred; there
> >>> is no word assumptions in that code. Indeed many ATA disks transfer
> >>> by half-word accesses so such a restriction would be insane.
> >>>
> >>> - the FIFO access itself needs to be 32-bit words, so readsl or writesl
> >>> (or their io* equivalents must be used).
> >>>
> >>> - but - and this is the killer item to your argument as I said above -
> >>> readsl and writesl _can_ take misaligned pointers and cope with them
> >>> fine.
> >>>
> >>> The actual alignment of the buffer address is totally irrelevant here.
> >>>
> >>> What isn't irrelevant is the _number_ of bytes to be transferred, but
> >>> that's something totally different and completely unrelated from
> >>> data->sg->offset.
> >> Let's try again :)
> >>
> >> Keep in mind that the mmc -block layer is aligned so from that aspect
> >> everything is fine.
> >> SDIO may have any length or alignment but sg-len is always 1.
> >>
> >> There is just one sg-element and one continues buffer.
> >>
> >> sg-miter splits the continues buffer in chunks that may not be aligned
> >> with 4 byte size. It depends on the start address alignment of the
> >> buffer.
> >>
> >> Is it more clear now?
> >
> > Is this more clear: you may be passed a single buffer which is misaligned.
> > We cope with that just fine. There is *no* reason to reject that transfer
> > because readsl/writesl cope just fine with it.
> >
> The MMCI driver doesn't support alignment smaller than 4 bytes (it may
> result in data corruption).
Explain yourself. That's what's lacking here. I'm explaining why I
think you're wrong, but you're just asserting all the time that I'm
wrong without giving any real details.
> There are two options
> 1. Either one should fix the driver to support it. Currently the driver
> only supports miss-alignment of the last sg-miter buffer.
> 2. Or be kind to inform the user that the alignment is not supported.
Look, it's very very simple.
If you have a multi-sg transfer, and the pointer starts off being
misaligned, the first transfer to the end of the page _MAY_ be a
non-word aligned number of bytes. _THAT_ is what you should be checking.
_THAT_ is what the limitation is in the driver. _NOT_ that the pointer
is misaligned.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
