diff for duplicates of <1372806786.8183.127@snotra> diff --git a/a/1.txt b/N1/1.txt index 83798eb..3d847bd 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,86 +1,86 @@ On 06/30/2013 10:46:18 PM, hongbo.zhang@freescale.com wrote: > From: Hongbo Zhang <hongbo.zhang@freescale.com> ->=20 -> This patch adds support to 8-channel DMA engine, thus the driver =20 +> +> This patch adds support to 8-channel DMA engine, thus the driver > works for both > the new 8-channel and the legacy 4-channel DMA engines. ->=20 +> > Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com> > --- -> drivers/dma/fsldma.c | 48 =20 +> drivers/dma/fsldma.c | 48 > ++++++++++++++++++++++++++++++++++-------------- > drivers/dma/fsldma.h | 4 ++-- > 2 files changed, 36 insertions(+), 16 deletions(-) ->=20 +> > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c > index 4fc2980..0f453ea 100644 > --- a/drivers/dma/fsldma.c > +++ b/drivers/dma/fsldma.c -> @@ -1119,27 +1119,33 @@ static irqreturn_t fsldma_ctrl_irq(int irq, =20 +> @@ -1119,27 +1119,33 @@ static irqreturn_t fsldma_ctrl_irq(int irq, > void *data) -> struct fsldma_device *fdev =3D data; +> struct fsldma_device *fdev = data; > struct fsldma_chan *chan; -> unsigned int handled =3D 0; +> unsigned int handled = 0; > - u32 gsr, mask; > + u8 chan_sr[round_up(FSL_DMA_MAX_CHANS_PER_DEVICE, 4)]; > + u32 gsr; > int i; ->=20 -> - gsr =3D (fdev->feature & FSL_DMA_BIG_ENDIAN) ? in_be32(fdev->regs) -> - : =20 +> +> - gsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ? in_be32(fdev->regs) +> - : > in_le32(fdev->regs); -> - mask =3D 0xff000000; +> - mask = 0xff000000; > - dev_dbg(fdev->dev, "IRQ: gsr 0x%.8x\n", gsr); > + memset(&chan_sr, 0, sizeof(chan_sr)); -> + gsr =3D (fdev->feature & FSL_DMA_BIG_ENDIAN) ? =20 +> + gsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ? > in_be32(fdev->regs0) -> + : =20 +> + : > in_le32(fdev->regs0); > + memcpy(&chan_sr[0], &gsr, 4); > + dev_dbg(fdev->dev, "IRQ: gsr0 0x%.8x\n", gsr); > + -> + if (of_device_is_compatible(fdev->dev->of_node, =20 +> + if (of_device_is_compatible(fdev->dev->of_node, > "fsl,eloplus-dma2")) { -NACK; Figure out what sort of device you've got when you first probe =20 -the device, and store the information for later. Do not call device =20 +NACK; Figure out what sort of device you've got when you first probe +the device, and store the information for later. Do not call device tree stuff in an interrupt handler. -> + gsr =3D (fdev->feature & FSL_DMA_BIG_ENDIAN) ? +> + gsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ? > + in_be32(fdev->regs1) : in_le32(fdev->regs1); > + memcpy(&chan_sr[4], &gsr, 4); > + dev_dbg(fdev->dev, "IRQ: gsr1 0x%.8x\n", gsr); > + } -Do these memcpy()s get inlined? If not (and maybe even if they do), =20 +Do these memcpy()s get inlined? If not (and maybe even if they do), it'd be better to use a union instead. -Wait a second -- how are we even getting into this code on these new =20 -DMA controllers? All 85xx-family DMA controllers use fsldma_chan_irq =20 +Wait a second -- how are we even getting into this code on these new +DMA controllers? All 85xx-family DMA controllers use fsldma_chan_irq directly. -> @@ -1341,13 +1349,22 @@ static int fsldma_of_probe(struct =20 +> @@ -1341,13 +1349,22 @@ static int fsldma_of_probe(struct > platform_device *op) > INIT_LIST_HEAD(&fdev->common.channels); ->=20 +> > /* ioremap the registers for use */ -> - fdev->regs =3D of_iomap(op->dev.of_node, 0); +> - fdev->regs = of_iomap(op->dev.of_node, 0); > - if (!fdev->regs) { > - dev_err(&op->dev, "unable to ioremap registers\n"); -> + fdev->regs0 =3D of_iomap(op->dev.of_node, 0); +> + fdev->regs0 = of_iomap(op->dev.of_node, 0); > + if (!fdev->regs0) { > + dev_err(&op->dev, "unable to ioremap register0\n"); -> err =3D -ENOMEM; +> err = -ENOMEM; > goto out_free_fdev; > } ->=20 -> + if (of_device_is_compatible(op->dev.of_node, =20 +> +> + if (of_device_is_compatible(op->dev.of_node, > "fsl,eloplus-dma2")) { Not "fsl,eloplusplus-dma"? :-) -More seriously, if we're sticking with this "elo" naming, maybe =20 -"fsl,elo3-dma" would be better. It would be odd to have "2" in the =20 +More seriously, if we're sticking with this "elo" naming, maybe +"fsl,elo3-dma" would be better. It would be odd to have "2" in the name of the third generation of this hardware. > diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h @@ -89,18 +89,18 @@ name of the third generation of this hardware. > +++ b/drivers/dma/fsldma.h > @@ -112,10 +112,10 @@ struct fsldma_chan_regs { > }; ->=20 +> > struct fsldma_chan; > -#define FSL_DMA_MAX_CHANS_PER_DEVICE 4 > +#define FSL_DMA_MAX_CHANS_PER_DEVICE 8 ->=20 +> > struct fsldma_device { > - void __iomem *regs; /* DGSR register base */ > + void __iomem *regs0, *regs1; /* DGSR registers */ Either give these meaningful names, or use an array. Or both (dgsr[2]). -Or just get rid of this, since I don't see why we need DGSR1 at all, as =20 +Or just get rid of this, since I don't see why we need DGSR1 at all, as previously noted. --Scott= +-Scott diff --git a/a/content_digest b/N1/content_digest index 47f98a5..853a32c 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,97 +3,98 @@ "Subject\0Re: [PATCH 2/2] DMA: Freescale: update driver to support 8-channel DMA engine\0" "Date\0Tue, 2 Jul 2013 18:13:06 -0500\0" "To\0<hongbo.zhang@freescale.com>\0" - "Cc\0vinod.koul@intel.com" - linux-kernel@vger.kernel.org - vakul@freescale.com + "Cc\0<vinod.koul@intel.com>" + <djbw@fb.com> + <vakul@freescale.com> + <leoLi@freescale.com> + <linux-kernel@vger.kernel.org> Hongbo Zhang <hongbo.zhang@freescale.com> - djbw@fb.com - " linuxppc-dev@lists.ozlabs.org\0" + " <linuxppc-dev@lists.ozlabs.org>\0" "\00:1\0" "b\0" "On 06/30/2013 10:46:18 PM, hongbo.zhang@freescale.com wrote:\n" "> From: Hongbo Zhang <hongbo.zhang@freescale.com>\n" - ">=20\n" - "> This patch adds support to 8-channel DMA engine, thus the driver =20\n" + "> \n" + "> This patch adds support to 8-channel DMA engine, thus the driver \n" "> works for both\n" "> the new 8-channel and the legacy 4-channel DMA engines.\n" - ">=20\n" + "> \n" "> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>\n" "> ---\n" - "> drivers/dma/fsldma.c | 48 =20\n" + "> drivers/dma/fsldma.c | 48 \n" "> ++++++++++++++++++++++++++++++++++--------------\n" "> drivers/dma/fsldma.h | 4 ++--\n" "> 2 files changed, 36 insertions(+), 16 deletions(-)\n" - ">=20\n" + "> \n" "> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c\n" "> index 4fc2980..0f453ea 100644\n" "> --- a/drivers/dma/fsldma.c\n" "> +++ b/drivers/dma/fsldma.c\n" - "> @@ -1119,27 +1119,33 @@ static irqreturn_t fsldma_ctrl_irq(int irq, =20\n" + "> @@ -1119,27 +1119,33 @@ static irqreturn_t fsldma_ctrl_irq(int irq, \n" "> void *data)\n" - "> \tstruct fsldma_device *fdev =3D data;\n" + "> \tstruct fsldma_device *fdev = data;\n" "> \tstruct fsldma_chan *chan;\n" - "> \tunsigned int handled =3D 0;\n" + "> \tunsigned int handled = 0;\n" "> -\tu32 gsr, mask;\n" "> +\tu8 chan_sr[round_up(FSL_DMA_MAX_CHANS_PER_DEVICE, 4)];\n" "> +\tu32 gsr;\n" "> \tint i;\n" - ">=20\n" - "> -\tgsr =3D (fdev->feature & FSL_DMA_BIG_ENDIAN) ? in_be32(fdev->regs)\n" - "> -\t\t\t\t\t\t : =20\n" + "> \n" + "> -\tgsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ? in_be32(fdev->regs)\n" + "> -\t\t\t\t\t\t : \n" "> in_le32(fdev->regs);\n" - "> -\tmask =3D 0xff000000;\n" + "> -\tmask = 0xff000000;\n" "> -\tdev_dbg(fdev->dev, \"IRQ: gsr 0x%.8x\\n\", gsr);\n" "> +\tmemset(&chan_sr, 0, sizeof(chan_sr));\n" - "> +\tgsr =3D (fdev->feature & FSL_DMA_BIG_ENDIAN) ? =20\n" + "> +\tgsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ? \n" "> in_be32(fdev->regs0)\n" - "> +\t\t\t\t\t\t : =20\n" + "> +\t\t\t\t\t\t : \n" "> in_le32(fdev->regs0);\n" "> +\tmemcpy(&chan_sr[0], &gsr, 4);\n" "> +\tdev_dbg(fdev->dev, \"IRQ: gsr0 0x%.8x\\n\", gsr);\n" "> +\n" - "> +\tif (of_device_is_compatible(fdev->dev->of_node, =20\n" + "> +\tif (of_device_is_compatible(fdev->dev->of_node, \n" "> \"fsl,eloplus-dma2\")) {\n" "\n" - "NACK; Figure out what sort of device you've got when you first probe =20\n" - "the device, and store the information for later. Do not call device =20\n" + "NACK; Figure out what sort of device you've got when you first probe \n" + "the device, and store the information for later. Do not call device \n" "tree stuff in an interrupt handler.\n" "\n" - "> +\t\tgsr =3D (fdev->feature & FSL_DMA_BIG_ENDIAN) ?\n" + "> +\t\tgsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ?\n" "> +\t\t\tin_be32(fdev->regs1) : in_le32(fdev->regs1);\n" "> +\t\tmemcpy(&chan_sr[4], &gsr, 4);\n" "> +\t\tdev_dbg(fdev->dev, \"IRQ: gsr1 0x%.8x\\n\", gsr);\n" "> +\t}\n" "\n" - "Do these memcpy()s get inlined? If not (and maybe even if they do), =20\n" + "Do these memcpy()s get inlined? If not (and maybe even if they do), \n" "it'd be better to use a union instead.\n" "\n" - "Wait a second -- how are we even getting into this code on these new =20\n" - "DMA controllers? All 85xx-family DMA controllers use fsldma_chan_irq =20\n" + "Wait a second -- how are we even getting into this code on these new \n" + "DMA controllers? All 85xx-family DMA controllers use fsldma_chan_irq \n" "directly.\n" "\n" - "> @@ -1341,13 +1349,22 @@ static int fsldma_of_probe(struct =20\n" + "> @@ -1341,13 +1349,22 @@ static int fsldma_of_probe(struct \n" "> platform_device *op)\n" "> \tINIT_LIST_HEAD(&fdev->common.channels);\n" - ">=20\n" + "> \n" "> \t/* ioremap the registers for use */\n" - "> -\tfdev->regs =3D of_iomap(op->dev.of_node, 0);\n" + "> -\tfdev->regs = of_iomap(op->dev.of_node, 0);\n" "> -\tif (!fdev->regs) {\n" "> -\t\tdev_err(&op->dev, \"unable to ioremap registers\\n\");\n" - "> +\tfdev->regs0 =3D of_iomap(op->dev.of_node, 0);\n" + "> +\tfdev->regs0 = of_iomap(op->dev.of_node, 0);\n" "> +\tif (!fdev->regs0) {\n" "> +\t\tdev_err(&op->dev, \"unable to ioremap register0\\n\");\n" - "> \t\terr =3D -ENOMEM;\n" + "> \t\terr = -ENOMEM;\n" "> \t\tgoto out_free_fdev;\n" "> \t}\n" - ">=20\n" - "> +\tif (of_device_is_compatible(op->dev.of_node, =20\n" + "> \n" + "> +\tif (of_device_is_compatible(op->dev.of_node, \n" "> \"fsl,eloplus-dma2\")) {\n" "\n" "Not \"fsl,eloplusplus-dma\"? :-)\n" "\n" - "More seriously, if we're sticking with this \"elo\" naming, maybe =20\n" - "\"fsl,elo3-dma\" would be better. It would be odd to have \"2\" in the =20\n" + "More seriously, if we're sticking with this \"elo\" naming, maybe \n" + "\"fsl,elo3-dma\" would be better. It would be odd to have \"2\" in the \n" "name of the third generation of this hardware.\n" "\n" "> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h\n" @@ -102,20 +103,20 @@ "> +++ b/drivers/dma/fsldma.h\n" "> @@ -112,10 +112,10 @@ struct fsldma_chan_regs {\n" "> };\n" - ">=20\n" + "> \n" "> struct fsldma_chan;\n" "> -#define FSL_DMA_MAX_CHANS_PER_DEVICE 4\n" "> +#define FSL_DMA_MAX_CHANS_PER_DEVICE 8\n" - ">=20\n" + "> \n" "> struct fsldma_device {\n" "> -\tvoid __iomem *regs;\t/* DGSR register base */\n" "> +\tvoid __iomem *regs0, *regs1;\t/* DGSR registers */\n" "\n" "Either give these meaningful names, or use an array. Or both (dgsr[2]).\n" "\n" - "Or just get rid of this, since I don't see why we need DGSR1 at all, as =20\n" + "Or just get rid of this, since I don't see why we need DGSR1 at all, as \n" "previously noted.\n" "\n" - -Scott= + -Scott -010faa37bc87b4b736870d01edb6c82a3aef3dfb6622585713036cecd1b50586 +6ab08ed7b7d8a5ee13d8c132f510049265a9a4db2d7f8d698f3f8ef4279b67cb
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.