On Thu, Mar 10, 2022 at 08:13:51PM +0100, Miquel Raynal wrote:
> andriy.shevchenko@xxxxxxxxxxxxxxx wrote on Thu, 10 Mar 2022 20:06:25
> +0200:
> > On Thu, Mar 10, 2022 at 05:16:48PM +0100, Miquel Raynal wrote:
> > > From: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
...
> > > + /* If we have a valid fifosize and DMA support, try hooking up DMA */
> > > + if (p->fifosize && data->dma_capable) {
> >
> > > + if (reg & DW_UART_CPR_DMA_EXTRA)
> > > + data->dma_capable = 1;
> >
> > How many designs will be broken by this change?
>
> My understanding was that CPR registers where always synthesized until
> now even though it was not mandatory and that the RZN1 SoC was the
> first one to not embed it. My hope was that people using this driver
> would have brought "external" CPR support earlier if they needed it,
> but I understand this assumption might be wrong.
>
> Anyway, I also hesitated to do something more custom for the RZN1 I'll
> try something else.
My point is that you have put this requirement to the above conditionals.
Have you checked all supported platforms that announce CPR that that bit
is set when DMA is indeed in use?
AFAIK on Intel SoCs the native UART DMA signalling is not used, the
integration between DMA and UART is done differently because it requires
more signals to connect. It might be that I misread the documentation
and this is not the case and we indeed set that bit as well.
Also, what to do with the platforms that have no CPR but support DMA currently?
...
> > > + unsigned int dma_capable:1;
> >
> > Note, we use up->dma == NULL for no-DMA, no additional flag is needed.
> > Just make sure that for your platform you enable DMA by filling that.
>
> dma_capable is just a capability the SoC has. It was discovered at
> probe time and should be saved to know, later, if DMA can be hooked up
> or not. At the time we look at the CPR register we don't yet have DMA
> fields populated so its too early to set up->dma to NULL.
'up->dma == NULL' check will tell you that, no?
--
With Best Regards,
Andy Shevchenko
