Messages in this thread |  | | Subject | Re: [PATCH v2] mfd: stm32-timers: Use dma_request_chan() instead dma_request_slave_channel() | From | Peter Ujfalusi <> | Date | Tue, 12 May 2020 17:04:48 +0300 |
| |
On 16/04/2020 11.46, Lee Jones wrote: > On Tue, 07 Jan 2020, Peter Ujfalusi wrote: > >> dma_request_slave_channel() is a wrapper on top of dma_request_chan() >> eating up the error code. >> >> By using dma_request_chan() directly the driver can support deferred >> probing against DMA. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> --- >> Hi, >> >> Changes since v1: >> - Fall back to PIO mode only in case of ENODEV and report all other errors >> >> Regards, >> Peter >> >> drivers/mfd/stm32-timers.c | 32 +++++++++++++++++++++++--------- >> 1 file changed, 23 insertions(+), 9 deletions(-) > > Apologies for not spotting this patch sooner, it had slipped through > the net. If this happens again, please just submit a [RESEND].
No issues, I have also forgot about it ;)
>> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c >> index efcd4b980c94..add603359124 100644 >> --- a/drivers/mfd/stm32-timers.c >> +++ b/drivers/mfd/stm32-timers.c >> @@ -167,10 +167,11 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata) >> regmap_write(ddata->regmap, TIM_ARR, 0x0); >> } >> >> -static void stm32_timers_dma_probe(struct device *dev, >> +static int stm32_timers_dma_probe(struct device *dev, >> struct stm32_timers *ddata) >> { >> int i; >> + int ret = 0; >> char name[4]; >> >> init_completion(&ddata->dma.completion); >> @@ -179,14 +180,23 @@ static void stm32_timers_dma_probe(struct device *dev, >> /* Optional DMA support: get valid DMA channel(s) or NULL */ >> for (i = STM32_TIMERS_DMA_CH1; i <= STM32_TIMERS_DMA_CH4; i++) { >> snprintf(name, ARRAY_SIZE(name), "ch%1d", i + 1); >> - ddata->dma.chans[i] = dma_request_slave_channel(dev, name); >> + ddata->dma.chans[i] = dma_request_chan(dev, name); >> } >> - ddata->dma.chans[STM32_TIMERS_DMA_UP] = >> - dma_request_slave_channel(dev, "up"); >> - ddata->dma.chans[STM32_TIMERS_DMA_TRIG] = >> - dma_request_slave_channel(dev, "trig"); >> - ddata->dma.chans[STM32_TIMERS_DMA_COM] = >> - dma_request_slave_channel(dev, "com"); >> + ddata->dma.chans[STM32_TIMERS_DMA_UP] = dma_request_chan(dev, "up"); >> + ddata->dma.chans[STM32_TIMERS_DMA_TRIG] = dma_request_chan(dev, "trig"); >> + ddata->dma.chans[STM32_TIMERS_DMA_COM] = dma_request_chan(dev, "com"); >> + >> + for (i = STM32_TIMERS_DMA_CH1; i < STM32_TIMERS_MAX_DMAS; i++) { >> + if (IS_ERR(ddata->dma.chans[i])) { >> + /* Save the first error code to return */ >> + if (PTR_ERR(ddata->dma.chans[i]) != -ENODEV && !ret) >> + ret = PTR_ERR(ddata->dma.chans[i]); >> + >> + ddata->dma.chans[i] = NULL; >> + } >> + } > > In my mind, it doesn't make sense to keep requesting channels if an > error has occurred. Please remove all of the added complexity caused > by the for() loop and simply check the return value after each call to > dma_request_chan(), returning immediately on error.
Yes. I have intentionally done the patch this way to _not_ change the behavior in any ways as it is not up to me to decide what is good or bad for a driver I can not test.
> >> + return ret; >> } >
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki [unhandled content-type:application/pgp-keys]
|  |