Hi, On Sun, Sep 17, 2017 at 05:19:53AM +0200, Stefan Brüns wrote: > + ret = of_property_read_u32(np, "dma-channels", &sdc->num_pchans); > + if (ret && !sdc->num_pchans) { > + dev_err(&pdev->dev, "Can't get dma-channels.\n"); > + return ret; > + } > + > + if (sdc->num_pchans > DMA_MAX_CHANNELS) { > + dev_err(&pdev->dev, "Number of dma-channels out of range.\n"); > + return -EINVAL; > + } > + > + ret = of_property_read_u32(np, "dma-requests", &sdc->max_request); > + if (ret && !sdc->max_request) { > + dev_info(&pdev->dev, "Missing dma-requests, using %u.\n", > + DMA_CHAN_MAX_DRQ); > + sdc->max_request = DMA_CHAN_MAX_DRQ; > + } > + > + if (sdc->max_request > DMA_CHAN_MAX_DRQ) { > + dev_err(&pdev->dev, "Value of dma-requests out of range.\n"); > + return -EINVAL; > + } I'm not really convinced about these two checks. They don't catch all errors (the range between the actual number of channels / DRQ and the maximum allowed per the registers), they might increase in the future too, and if we want to make that check actually working, we would have to duplicate the number of requests and channels into the driver. Which is kind of the opposite of what we're trying to do here :) Once removed, Acked-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Attachment:
signature.asc
Description: PGP signature