linux.kernel - 26 new messages in 18 topics - digest

linux.kernel
http://groups.google.com/group/linux.kernel?hl=en

linux.kernel@googlegroups.com

Today's topics:

* batman-adv: Use seq_overflow - 2 messages, 2 authors
http://groups.google.com/group/linux.kernel/t/4c6be54c4cfbdd71?hl=en
* ARM: dts: provide DMA config to pxamci - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/e347c8b46599ce3c?hl=en
* usb: phy: msm: Add support for secondary PHY control - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/531d1582b5d30b15?hl=en
* Staging: TIDSPBRIDGE: Use vm_iomap_memory for mmap-ing instead of remap_pfn_
range - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/c85a4526f6e76b9b?hl=en
* sched: Add tracepoints related to NUMA task migration - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/5d38f47da52ad28e?hl=en
* phy: Add provision for tuning phy. - 3 messages, 2 authors
http://groups.google.com/group/linux.kernel/t/d26d42c3b09f58d4?hl=en
* X86/KVM: enable Intel MPX for KVM - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/335543543d4dfb99?hl=en
* mtd: st_spi_fsm: Allocate resources and register with MTD framework - 1
messages, 1 author
http://groups.google.com/group/linux.kernel/t/8eb175d8e03582a7?hl=en
* dma: tegra: Use runtime_pm for clk enable/disable - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/6766cc6ffea941f6?hl=en
* mm/migrate: remove result argument on page allocation function for migration
- 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/fab8622d4d72e03b?hl=en
* usb: dwc3: use quirks to know if a particualr platform doesn't have PHY - 2
messages, 2 authors
http://groups.google.com/group/linux.kernel/t/185df9b46897a043?hl=en
* mm/zswap: change zswap to writethrough cache - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/83a057c0f19449e9?hl=en
* sched/numa: fix set cpupid on page migration twice against normal page - 4
messages, 1 author
http://groups.google.com/group/linux.kernel/t/2879b635b367c8c6?hl=en
* mfd: menelaus: Start to use irqdomain - 2 messages, 1 author
http://groups.google.com/group/linux.kernel/t/0018ab22f96f187d?hl=en
* net: of_mdio: Scan PHYs which have device_type set to ethernet-phy - 1
messages, 1 author
http://groups.google.com/group/linux.kernel/t/098803c3358dfc40?hl=en
* dma: mv_xor: remove mv_desc_get_dest_addr() - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/1dba4c15122332c3?hl=en
* Documentation: pci: Minor cleanup - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/cda14d708591570c?hl=en
* Documentation: pci: Add pcie-howto.txt - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/d50a5a1acc7d6131?hl=en

==============================================================================
TOPIC: batman-adv: Use seq_overflow
http://groups.google.com/group/linux.kernel/t/4c6be54c4cfbdd71?hl=en
==============================================================================

== 1 of 2 ==
Date: Wed, Dec 11 2013 12:30 am
From: Joe Perches


On Wed, 2013-12-11 at 08:05 +0000, Al Viro wrote:
> On Wed, Dec 11, 2013 at 07:55:26AM +0000, Al Viro wrote:
>
> > This sucker should return 0. Insufficiently large buffer will be handled
> > by caller, TYVM, if you give that caller a chance to do so. Returning 1
> > from ->show() is a bug in almost all cases, and definitely so in this one.
> >
> > Just in case somebody decides that above is worth copying: It Is Not.
> > Original code is buggy, plain and simple. This one trades the older
> > bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
> > "silently skip an entry entirely whenever the buffer is too small".
> >
> > Don't Do That.
>
> Pardon - Joe has made seq_overflow return -1 instead of true. Correction
> to the above, then - s/This trades.*\./This is just as buggy./

Yeah, I started to use true/false, 0/1, but thought
I needed to match what seq_printf/seq_vprintf does.

> Conclusion is still the same - Don't Do That. Returning -1 on insufficiently
> large buffer is a bug, plain and simple.

int seq_vprintf(struct seq_file *m, const char *f, va_list args)
{
int len;

if (m->count < m->size) {
len = vsnprintf(m->buf + m->count, m->size - m->count, f, args);
if (m->count + len < m->size) {
m->count += len;
return 0;
}
}
seq_set_overflow(m);
return -1;
}
EXPORT_SYMBOL(seq_vprintf);

int seq_printf(struct seq_file *m, const char *f, ...)
{
int ret;
va_list args;

va_start(args, f);
ret = seq_vprintf(m, f, args);
va_end(args);

return ret;
}
EXPORT_SYMBOL(seq_printf);

> And this patch series is completely misguided - it doesn't fix any bugs
> *and* it provides a misleading example for everyone. See the reaction
> right in this thread, proposing to spread the same bug to currently
> working iterators.

Anyway, changing seq_overflow is easy enough

You prefer this?

bool seq_overflow(struct seq_file *seq)
{
return m->count == m->size;
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 2 of 2 ==
Date: Wed, Dec 11 2013 12:40 am
From: Al Viro


On Wed, Dec 11, 2013 at 12:26:17AM -0800, Joe Perches wrote:
> On Wed, 2013-12-11 at 08:05 +0000, Al Viro wrote:
> > On Wed, Dec 11, 2013 at 07:55:26AM +0000, Al Viro wrote:
> >
> > > This sucker should return 0. Insufficiently large buffer will be handled
> > > by caller, TYVM, if you give that caller a chance to do so. Returning 1
> > > from ->show() is a bug in almost all cases, and definitely so in this one.
> > >
> > > Just in case somebody decides that above is worth copying: It Is Not.
> > > Original code is buggy, plain and simple. This one trades the older
> > > bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
> > > "silently skip an entry entirely whenever the buffer is too small".
> > >
> > > Don't Do That.
> >
> > Pardon - Joe has made seq_overflow return -1 instead of true. Correction
> > to the above, then - s/This trades.*\./This is just as buggy./
>
> Yeah, I started to use true/false, 0/1, but thought
> I needed to match what seq_printf/seq_vprintf does.
>
> > Conclusion is still the same - Don't Do That. Returning -1 on insufficiently
> > large buffer is a bug, plain and simple.
>
> int seq_vprintf(struct seq_file *m, const char *f, va_list args)
> {
> int len;
>
> if (m->count < m->size) {
> len = vsnprintf(m->buf + m->count, m->size - m->count, f, args);
> if (m->count + len < m->size) {
> m->count += len;
> return 0;
> }
> }
> seq_set_overflow(m);
> return -1;
> }
> EXPORT_SYMBOL(seq_vprintf);
>
> int seq_printf(struct seq_file *m, const char *f, ...)
> {
> int ret;
> va_list args;
>
> va_start(args, f);
> ret = seq_vprintf(m, f, args);
> va_end(args);
>
> return ret;
> }
> EXPORT_SYMBOL(seq_printf);
>
> > And this patch series is completely misguided - it doesn't fix any bugs
> > *and* it provides a misleading example for everyone. See the reaction
> > right in this thread, proposing to spread the same bug to currently
> > working iterators.
>
> Anyway, changing seq_overflow is easy enough
>
> You prefer this?
>
> bool seq_overflow(struct seq_file *seq)
> {
> return m->count == m->size;
> }

I prefer a series that starts with fixing the obvious bugs (i.e. places
where we return seq_printf/seq_puts/seq_putc return value from ->show()).
All such places should return 0. Then we need to look at the remaining
places that check return value of seq_printf() et.al. And decide whether
the callers really care about it.

Theoretically, there is a legitimate case when we want to look at that
return value. Namely,
seq_print(...)
if (!overflowed)
do tons of expensive calculations
generate more output
return 0
That is the reason why those guys hadn't been returning void to start with.
And yes, it was inviting bugs with ->show() returning -1 on overflows.
Bad API design, plain and simple.

I'm not sure we actually have any instances of that legitimate case, TBH.
_IF_ we do, we ought to expose seq_overflow() (with saner name - this one
invites the same "it's an error, need to report it" kind of bugs) and use
it in such places. But that needs to be decided on per-caller basis. And
I'd expect that there would be few enough such places after we kill the
obvious bugs.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





==============================================================================
TOPIC: ARM: dts: provide DMA config to pxamci
http://groups.google.com/group/linux.kernel/t/e347c8b46599ce3c?hl=en
==============================================================================

== 1 of 1 ==
Date: Wed, Dec 11 2013 12:30 am
From: Robert Jarzmik


Daniel Mack <zonque@gmail.com> writes:

> In particular, the pxa camera driver is something Robert (cc) wanted to
> have a look at. Robert, any updates on this?
Oh yes, sorry, it's been monthes, I've been very busy.

This is my last status :
- my current patch is in (1)

- this patch doesn't work yet

- if my memory serves me well, I have reached the conclusion that dmaengine
pxa-mpp driver doesn't allow pxa_camera to work properly in the current
state. This is a long time since I checked, so take the following with
caution until I have checked again.

This assertion is based on this required behaviour :
- video buffers are queued, let's say 5
=> 5 dma are "prepared"

- video buffers are submitted
=> 5 dma xfers are submitted

- the userspace polls for each finished frame (ie. dma xfer termination), and
on the third one, resubmits the 2 finished frames
=> scatter-gathers should mot be recomputed, just xfer should be
resubmitted, with cache sync operations and first/last descriptors chaining

- the pxa_camera driver needs a way to know if a dma channel is still running,
for missed chaining transfers, because if a channel stopped, the dma xfers
should not be submitted until IRQ "begin of frame" is encoutered. I didn't
find a way for that.

Now, I will retry this weekend, but if I remember correctly the issues I had
were :
- a transfer cannot be resubmitted, it has to be freed and recreated
- if it is resubmitted, the completion is not signaled by the tasklet

Cheers.

--
Robert

(1)
commit 4741ba6 (pxa_camera_dmaengine, backup/pxa_camera_dmaengine)
Author: Robert Jarzmik <robert.jarzmik@free.fr>
Date: Sat Aug 24 21:13:38 2013 +0200

WIP: pxa_camera dmaengine conversion

Only slightly tested, without much success.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

diff --git a/drivers/media/platform/soc_camera/pxa_camera.c b/drivers/media/platform/soc_camera/pxa_camera.c
index d4df305..903ea03 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -9,7 +9,7 @@
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*/
-
+#define DEBUG 1
#include <linux/init.h>
#include <linux/module.h>
#include <linux/io.h>
@@ -28,6 +28,9 @@
#include <linux/clk.h>
#include <linux/sched.h>
#include <linux/slab.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dma/mmp-pdma.h>

#include <media/v4l2-common.h>
#include <media/v4l2-dev.h>
@@ -37,7 +40,6 @@

#include <linux/videodev2.h>

-#include <mach/dma.h>
#include <linux/platform_data/camera-pxa.h>

#define PXA_CAM_VERSION "0.0.6"
@@ -174,21 +176,13 @@ enum pxa_camera_active_dma {
DMA_V = 0x4,
};

-/* descriptor needed for the PXA DMA engine */
-struct pxa_cam_dma {
- dma_addr_t sg_dma;
- struct pxa_dma_desc *sg_cpu;
- size_t sg_size;
- int sglen;
-};
-
/* buffer for one video frame */
struct pxa_buffer {
/* common v4l buffer stuff -- must be first */
struct videobuf_buffer vb;
enum v4l2_mbus_pixelcode code;
/* our descriptor lists for Y, U and V channels */
- struct pxa_cam_dma dmas[3];
+ struct dma_async_tx_descriptor *descs[3];
int inwork;
enum pxa_camera_active_dma active_dma;
};
@@ -206,7 +200,7 @@ struct pxa_camera_dev {
void __iomem *base;

int channels;
- unsigned int dma_chans[3];
+ struct dma_chan *dma_chans[3];

struct pxacamera_platform_data *pdata;
struct resource *res;
@@ -221,7 +215,6 @@ struct pxa_camera_dev {
spinlock_t lock;

struct pxa_buffer *active;
- struct pxa_dma_desc *sg_tail[3];

u32 save_cicr[5];
};
@@ -273,40 +266,70 @@ static void free_buffer(struct videobuf_queue *vq, struct pxa_buffer *buf)
videobuf_waiton(vq, &buf->vb, 0, 0);
videobuf_dma_unmap(vq->dev, dma);
videobuf_dma_free(dma);
-
- for (i = 0; i < ARRAY_SIZE(buf->dmas); i++) {
- if (buf->dmas[i].sg_cpu)
- dma_free_coherent(ici->v4l2_dev.dev,
- buf->dmas[i].sg_size,
- buf->dmas[i].sg_cpu,
- buf->dmas[i].sg_dma);
- buf->dmas[i].sg_cpu = NULL;
- }
+ /* FIXME: free buf->descs[0..2] */

buf->vb.state = VIDEOBUF_NEEDS_INIT;
+
+ dev_dbg(icd->parent, "%s end (vb=0x%p) 0x%08lx %d\n", __func__,
+ &buf->vb, buf->vb.baddr, buf->vb.bsize);
}

-static int calculate_dma_sglen(struct scatterlist *sglist, int sglen,
- int sg_first_ofs, int size)
+static struct scatterlist *videobuf_sg_splice(struct scatterlist *sglist,
+ int sglen, int offset, int size,
+ int *new_sg_len)
{
- int i, offset, dma_len, xfer_len;
- struct scatterlist *sg;
+ struct scatterlist *sg0, *sg;
+ int nfirst, i, dma_len, xfer_len;

- offset = sg_first_ofs;
- for_each_sg(sglist, sg, sglen, i) {
+ sg0 = kmalloc(sizeof(struct scatterlist) * sglen, GFP_KERNEL);
+ if (!sg0)
+ return NULL;
+ for_each_sg(sglist, sg, sglen, nfirst) {
dma_len = sg_dma_len(sg);
-
+ if (offset < dma_len)
+ break;
/* PXA27x Developer's Manual 27.4.4.1: round up to 8 bytes */
- xfer_len = roundup(min(dma_len - offset, size), 8);
-
- size = max(0, size - xfer_len);
- offset = 0;
- if (size == 0)
+ xfer_len = roundup(min(dma_len - offset, offset), 8);
+ offset = max(0, offset - xfer_len);
+ }
+ BUG_ON(sg_is_last(&sglist[nfirst]));
+ for_each_sg(&sglist[nfirst], sg, sglen - nfirst, i) {
+ sg0[i] = sglist[nfirst];
+ sg0[i].offset = offset;
+ sg_dma_len(&sg0[i]) -= offset;
+ if (size <= sg_dma_len(sg))
break;
+ offset = 0;
+ size -= roundup(sg_dma_len(sg), 8);
+ }
+ if (size) {
+ sg_dma_len(&sg0[i]) = size;
+ *new_sg_len = i + 1;
+ } else {
+ *new_sg_len = i;
}

- BUG_ON(size != 0);
- return i + 1;
+ return sg0;
+}
+static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
+ enum pxa_camera_active_dma act_dma);
+
+static void pxa_camera_dma_irq_y(void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+ pxa_camera_dma_irq(pcdev, DMA_Y);
+}
+
+static void pxa_camera_dma_irq_u(void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+ pxa_camera_dma_irq(pcdev, DMA_U);
+}
+
+static void pxa_camera_dma_irq_v(void *data)
+{
+ struct pxa_camera_dev *pcdev = data;
+ pxa_camera_dma_irq(pcdev, DMA_V);
}

/**
@@ -317,91 +340,68 @@ static int calculate_dma_sglen(struct scatterlist *sglist, int sglen,
* @channel: dma channel (0 => 'Y', 1 => 'U', 2 => 'V')
* @cibr: camera Receive Buffer Register
* @size: bytes to transfer
- * @sg_first: first element of sg_list
- * @sg_first_ofs: offset in first element of sg_list
+ * @offset: offset in videobuffer of the first byte to transfer
*
* Prepares the pxa dma descriptors to transfer one camera channel.
- * Beware sg_first and sg_first_ofs are both input and output parameters.
*
- * Returns 0 or -ENOMEM if no coherent memory is available
+ * Returns 0 if success or -ENOMEM if no memory is available
*/
static int pxa_init_dma_channel(struct pxa_camera_dev *pcdev,
struct pxa_buffer *buf,
struct videobuf_dmabuf *dma, int channel,
- int cibr, int size,
- struct scatterlist **sg_first, int *sg_first_ofs)
+ int cibr, int size, int offset)
{
- struct pxa_cam_dma *pxa_dma = &buf->dmas[channel];
- struct device *dev = pcdev->soc_host.v4l2_dev.dev;
- struct scatterlist *sg;
- int i, offset, sglen;
- int dma_len = 0, xfer_len = 0;
+ struct dma_chan *dma_chan = pcdev->dma_chans[channel];
+ struct scatterlist *sg = NULL;
+ int ret, sglen;
+ struct dma_slave_config config;
+ struct dma_async_tx_descriptor *tx;

- if (pxa_dma->sg_cpu)
- dma_free_coherent(dev, pxa_dma->sg_size,
- pxa_dma->sg_cpu, pxa_dma->sg_dma);
+ dmaengine_terminate_all(dma_chan);

- sglen = calculate_dma_sglen(*sg_first, dma->sglen,
- *sg_first_ofs, size);
-
- pxa_dma->sg_size = (sglen + 1) * sizeof(struct pxa_dma_desc);
- pxa_dma->sg_cpu = dma_alloc_coherent(dev, pxa_dma->sg_size,
- &pxa_dma->sg_dma, GFP_KERNEL);
- if (!pxa_dma->sg_cpu)
+ sg = videobuf_sg_splice(dma->sglist, dma->sglen, offset, size, &sglen);
+ if (!sg)
return -ENOMEM;

- pxa_dma->sglen = sglen;
- offset = *sg_first_ofs;
-
- dev_dbg(dev, "DMA: sg_first=%p, sglen=%d, ofs=%d, dma.desc=%x\n",
- *sg_first, sglen, *sg_first_ofs, pxa_dma->sg_dma);
-
+ memset(&config, 0, sizeof(config));
+ config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; /* FIXME? */
+ config.src_maxburst = 8;
+ config.src_addr = pcdev->res->start + cibr;
+ config.direction = DMA_DEV_TO_MEM;

- for_each_sg(*sg_first, sg, sglen, i) {
- dma_len = sg_dma_len(sg);
-
- /* PXA27x Developer's Manual 27.4.4.1: round up to 8 bytes */
- xfer_len = roundup(min(dma_len - offset, size), 8);
-
- size = max(0, size - xfer_len);
-
- pxa_dma->sg_cpu[i].dsadr = pcdev->res->start + cibr;
- pxa_dma->sg_cpu[i].dtadr = sg_dma_address(sg) + offset;
- pxa_dma->sg_cpu[i].dcmd =
- DCMD_FLOWSRC | DCMD_BURST8 | DCMD_INCTRGADDR | xfer_len;
-#ifdef DEBUG
- if (!i)
- pxa_dma->sg_cpu[i].dcmd |= DCMD_STARTIRQEN;
-