Hi
Thanks for the review...
>
> Hi Kedar,
>
>
> On 04-01-2017 06:54, Kedareswara rao Appana wrote:
> > When VDMA is configured for more than one frame in the h/w for example
> > h/w is configured for n number of frames and user Submits n number of
> > frames and triggered the DMA using issue_pending API.
> > In the current driver flow we are submitting one frame at a time but
> > we should submit all the n number of frames at one time as the h/w Is
> > configured for n number of frames.
> >
> > This patch fixes this issue.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xxxxxxxxxx>
>
> Looks fine. I have a couple of minor comments, if you address them you can add
> "Reviewed-by: Jose Abreu <joabreu@xxxxxxxxxxxx>"
> in next version.
Thanks...
Sure will fix the comments and will add your' s Reviewed-by....
[snip]
> > + /*
> > + * Note: When VDMA is built with default h/w configuration
> > + * On the S2MM(recv) side user should submit frames upto
> > + * H/W configured. If users submits less than h/w configured
> > + * VDMA engine tries to write to a invalid location
> > + * Results undefined behaviour/memory corruption.
> > + *
> > + * If user would like to submit frames less than h/w capable
> > + * On S2MM side please enable debug info 13 at the h/w level
> > + * It will allows the frame buffers numbers to be modified at runtime.
> > + */
> > + if (!chan->has_fstoreconfig && chan->direction == DMA_DEV_TO_MEM
> &&
> > + chan->desc_pendingcount < chan->num_frms) {
> > + dev_dbg(chan->dev, "Frame Store Configuration is not enabled
> at the");
> > + dev_dbg(chan->dev, " H/w level enable Debug info 13 at the
> h/w level");
> > + dev_dbg(chan->dev, " OR Submit the frames upto h/w
> Capable\n\r");
> > +
> > + return;
> > + }
>
> Hmm, may dev_warn would be more suitable because with dev_dbg and no
> dynamic debug enabled user will not know what happened. Also, I am aware
> that in direction DMA_MEM_TO_DEV there will be no corruption in PC side but it
> will be corruption in VDMA side because it will read from invalid memory
> locations. Maybe drop the check for channel direction.
Sure will fix it in the next version....
>
> I am also not fancy about dropping prints that are not grep'able (you do not
> break line in each print so a user searching for the whole string will not find it).
> Try to do a line break in each print or change the string to be smaller.
>
Sure will add a line break in each print in the next version...
Regards,
Kedar.
> Best regards,
> Jose Miguel Abreu
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
