| Message ID | 7898ac737e1cce9542de662f1266d94bd451659e.1457695169.git.moinejf@free.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Fri, Mar 11, 2016 at 12:01:29PM +0100, Jean-Francois Moine wrote: > Some DMA transfers, as for H3 audio, ask for 4 as a burst value. > > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > --- > drivers/dma/sun6i-dma.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c > index 3579ee7..7c98c0d 100644 > --- a/drivers/dma/sun6i-dma.c > +++ b/drivers/dma/sun6i-dma.c > @@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst) > switch (maxburst) { > case 1: > return 0; > + case 4: > + return 1; This is true only for the H3. For the other SoCs that we support, the only valid values are 0 and 2, so we need to reject those values. We should do that based on the compatible. The easiest solution would be to expose the available burst sizes in the probe, and just our new one if we match that compatible, and any invalid burst size would be rejected by the framework. Vinod, any objection to that? Thanks! Maxime
On Fri, 11 Mar 2016 13:06:01 +0100 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Fri, Mar 11, 2016 at 12:01:29PM +0100, Jean-Francois Moine wrote: > > Some DMA transfers, as for H3 audio, ask for 4 as a burst value. [snip] > > @@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst) > > switch (maxburst) { > > case 1: > > return 0; > > + case 4: > > + return 1; > > This is true only for the H3. > > For the other SoCs that we support, the only valid values are 0 and 2, > so we need to reject those values. > > We should do that based on the compatible. > > The easiest solution would be to expose the available burst sizes in > the probe, and just our new one if we match that compatible, and any > invalid burst size would be rejected by the framework. Vinod, any > objection to that? Do you think that we should also check if the requested ports are valid, i.e. have a list/bitmap of the possible input/output ports per SoC, instead of just only the ID of the max port?
On Fri, Mar 11, 2016 at 05:28:58PM +0100, Jean-Francois Moine wrote: > On Fri, 11 Mar 2016 13:06:01 +0100 > Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > > > On Fri, Mar 11, 2016 at 12:01:29PM +0100, Jean-Francois Moine wrote: > > > Some DMA transfers, as for H3 audio, ask for 4 as a burst value. > [snip] > > > @@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst) > > > switch (maxburst) { > > > case 1: > > > return 0; > > > + case 4: > > > + return 1; > > > > This is true only for the H3. > > > > For the other SoCs that we support, the only valid values are 0 and 2, > > so we need to reject those values. > > > > We should do that based on the compatible. > > > > The easiest solution would be to expose the available burst sizes in > > the probe, and just our new one if we match that compatible, and any > > invalid burst size would be rejected by the framework. Vinod, any > > objection to that? > > Do you think that we should also check if the requested ports are > valid, i.e. have a list/bitmap of the possible input/output ports per > SoC, instead of just only the ID of the max port? That would require having a map of the possible endpoints for a given device, which would be quite difficult to accomplish. I don't think customer drivers have access to that anyway, only the provider driver is parsing the DT to retrieve the ID. And we already trust the DT for so many things it would be a bit pointless to check that. Maxime
On Fri, 18 Mar 2016 16:09:46 +0100 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Fri, Mar 11, 2016 at 05:28:58PM +0100, Jean-Francois Moine wrote: [snip] > > Do you think that we should also check if the requested ports are > > valid, i.e. have a list/bitmap of the possible input/output ports per > > SoC, instead of just only the ID of the max port? > > That would require having a map of the possible endpoints for a given > device, which would be quite difficult to accomplish. > > I don't think customer drivers have access to that anyway, only the > provider driver is parsing the DT to retrieve the ID. And we already > trust the DT for so many things it would be a bit pointless to check > that. The ports are already checked against 'nr_max_requests' which is the index of the higher port. I think that this check exists because the ports are given by the DMA clients. If this check is needed, as 'nr_max_requests' is lower than 32, 2 maps (in and out) on 32 bits would do the job. If this check is not needed (because we trust the DTs), the actual check (port <= nr_max_requests) could be removed.
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c index 3579ee7..7c98c0d 100644 --- a/drivers/dma/sun6i-dma.c +++ b/drivers/dma/sun6i-dma.c @@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst) switch (maxburst) { case 1: return 0; + case 4: + return 1; case 8: return 2; default:
Some DMA transfers, as for H3 audio, ask for 4 as a burst value. Signed-off-by: Jean-Francois Moine <moinejf@free.fr> --- drivers/dma/sun6i-dma.c | 2 ++ 1 file changed, 2 insertions(+)