| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <C246CAC1457055469EF09E3A7AC4E11A4A56E3C5@XAP-PVEXMBX01.xlnx.xilinx.com>
Date: Fri, 18 Mar 2016 12:43:26 +0000
From: Appana Durga Kedareswara Rao <appana.durga.rao@...inx.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
CC: "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
"vinod.koul@...el.com" <vinod.koul@...el.com>,
Michal Simek <michals@...inx.com>,
Soren Brinkmann <sorenb@...inx.com>,
"moritz.fischer@...us.com" <moritz.fischer@...us.com>,
"luis@...ethencourt.com" <luis@...ethencourt.com>,
Anirudha Sarangi <anirudh@...inx.com>,
"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 2/7] dmaengine: xilinx_vdma: Add quirks support to
differentiate differnet IP cores
Hi Laurent Pinchart,
Thanks for reviewing the patch.
> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@...asonboard.com]
> Sent: Thursday, March 17, 2016 12:49 PM
> To: Appana Durga Kedareswara Rao
> Cc: dan.j.williams@...el.com; vinod.koul@...el.com; Michal Simek; Soren
> Brinkmann; Appana Durga Kedareswara Rao; moritz.fischer@...us.com;
> luis@...ethencourt.com; Anirudha Sarangi; dmaengine@...r.kernel.org; linux-
> arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH 2/7] dmaengine: xilinx_vdma: Add quirks support to
> differentiate differnet IP cores
>
> Hello,
>
> On Tuesday 15 March 2016 22:53:07 Kedareswara rao Appana wrote:
> > This patch adds quirks support in the driver to differentiate
> > differnet IP
>
> s/differnet/different/
Ok will modify In the V2.
>
> (and in the subject line too)
>
> With this series applied the driver will not be vdma-specific anymore. The
> xilinx_vdma_ prefix will thus be confusing. I'd bite the bullet and apply a global
> rename to functions that are not shared between the three DMA IP core types
> before patch 2/7.
Ok will modify the General API's that will be shared across the DMA's to the dma instead of vdma in the v2.
>
> > cores.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@...inx.com>
> > ---
> > drivers/dma/xilinx/xilinx_vdma.c | 36
> > ++++++++++++++++++++++++++++++------
> > 1 file changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/dma/xilinx/xilinx_vdma.c
> > b/drivers/dma/xilinx/xilinx_vdma.c index 7ab6793..f682bef 100644
> > --- a/drivers/dma/xilinx/xilinx_vdma.c
> > +++ b/drivers/dma/xilinx/xilinx_vdma.c
> > @@ -139,6 +139,8 @@
> > /* Delay loop counter to prevent hardware failure */
> > #define XILINX_VDMA_LOOP_COUNT 1000000
> >
> > +#define AXIVDMA_SUPPORT BIT(0)
> > +
> > /**
> > * struct xilinx_vdma_desc_hw - Hardware Descriptor
> > * @next_desc: Next Descriptor Pointer @0x00 @@ -240,6 +242,7 @@
> > struct xilinx_vdma_chan {
> > * @chan: Driver specific VDMA channel
> > * @has_sg: Specifies whether Scatter-Gather is present or not
> > * @flush_on_fsync: Flush on frame sync
> > + * @quirks: Needed for different IP cores
> > */
> > struct xilinx_vdma_device {
> > void __iomem *regs;
> > @@ -248,6 +251,15 @@ struct xilinx_vdma_device {
> > struct xilinx_vdma_chan
> *chan[XILINX_VDMA_MAX_CHANS_PER_DEVICE];
> > bool has_sg;
> > u32 flush_on_fsync;
> > + u32 quirks;
> > +};
> > +
> > +/**
> > + * struct xdma_platform_data - DMA platform structure
> > + * @quirks: quirks for platform specific data.
> > + */
> > +struct xdma_platform_data {
>
> This isn't platform data but device information, I'd rename the structure to
> xdma_device_info (and update the description accordingly).
Ok will modify in v2.
>
> > + u32 quirks;
>
> I don't think you should call this field quirks as it stores the IP core type.
> Quirks usually refer to non-standard behaviour that requires specific handling in
> the driver, possibly in the form of work-arounds. I would thus call the field type
> and document it as "DMA IP core type". Similarly I'd rename AXIVDMA_SUPPORT
> to XDMA_TYPE_VDMA.
Sure will modify in the v2.
>
> > };
> >
> > /* Macros */
> > @@ -1239,6 +1251,16 @@ static struct dma_chan
> > *of_dma_xilinx_xlate(struct of_phandle_args *dma_spec, return
> > dma_get_slave_channel(&xdev->chan[chan_id]->common);
> > }
> >
> > +static const struct xdma_platform_data xvdma_def = {
> > + .quirks = AXIVDMA_SUPPORT,
> > +};
> > +
> > +static const struct of_device_id xilinx_vdma_of_ids[] = {
> > + { .compatible = "xlnx,axi-vdma-1.00.a", .data = &xvdma_def},
>
> Missing space before }, did you run checkpatch ?
Yes I ran check patch it didn't troughed any warning ok will fix in v2.
>
> As the xdma_platform_data structure contains a single integer, you could store
> it in the .data field directly.
Ok will fix in v2.
>
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, xilinx_vdma_of_ids);
> > +
> > /**
> > * xilinx_vdma_probe - Driver probe function
> > * @pdev: Pointer to the platform_device structure @@ -1251,6 +1273,7
> > @@ static int xilinx_vdma_probe(struct platform_device
> > *pdev) struct xilinx_vdma_device *xdev;
> > struct device_node *child;
> > struct resource *io;
> > + const struct of_device_id *match;
> > u32 num_frames;
> > int i, err;
> >
> > @@ -1259,6 +1282,13 @@ static int xilinx_vdma_probe(struct
> > platform_device
> > *pdev) if (!xdev)
> > return -ENOMEM;
> >
> > + match = of_match_node(xilinx_vdma_of_ids, pdev->dev.of_node);
>
> You can use of_device_get_match_data() to get the data directly.
Ok will fix in v2
>
> > + if (match && match->data) {
>
> I don't think this condition can ever be false.
OK will fix in v2.
Regards,
Kedar.
>
> > + const struct xdma_platform_data *data = match->data;
> > +
> > + xdev->quirks = data->quirks;
> > + }
> > +
> > xdev->dev = &pdev->dev;
> >
> > /* Request and map I/O memory */
> > @@ -1356,12 +1386,6 @@ static int xilinx_vdma_remove(struct
> > platform_device
> > *pdev) return 0;
> > }
> >
> > -static const struct of_device_id xilinx_vdma_of_ids[] = {
> > - { .compatible = "xlnx,axi-vdma-1.00.a",},
> > - {}
> > -};
> > -MODULE_DEVICE_TABLE(of, xilinx_vdma_of_ids);
> > -
> > static struct platform_driver xilinx_vdma_driver = {
> > .driver = {
> > .name = "xilinx-vdma",
>
> --
> Regards,
>
> Laurent Pinchart
Powered by blists - more mailing lists