lkml.org 
[lkml]   [2019]   [Apr]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/5] media: platform: Aspeed: Make reserved memory optional
From
Date

On 4/3/19 1:01 AM, Andrew Jeffery wrote:
>
> On Wed, 3 Apr 2019, at 04:55, Eddie James wrote:
>> Reserved memory doesn't need to be required; system memory would work
>> fine.
> I had to do a bit of legwork to understand what you were doing here. My
> understanding is that we allocate out of the default CMA region if the
> memory-region property isn't specified. Is that what you're expecting?
> Could be helpful to be a little less terse in the commit message.


Correct.


>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> drivers/media/platform/aspeed-video.c | 6 +-----
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c
>> b/drivers/media/platform/aspeed-video.c
>> index 55c55a6..8144fe3 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -1608,11 +1608,7 @@ static int aspeed_video_init(struct aspeed_video
>> *video)
>> return PTR_ERR(video->vclk);
>> }
>>
>> - rc = of_reserved_mem_device_init(dev);
>> - if (rc) {
>> - dev_err(dev, "Unable to reserve memory\n");
>> - return rc;
>> - }
>> + of_reserved_mem_device_init(dev);
> You're ignoring *all* errors here with the expectation that the cause is the
> missing memory-region property. However, other errors can propagate
> out of of_reserved_mem_device_init() - e.g. ENOMEM. Rather than remove
> error checking, I think you should explicitly test for ENODEV, which is what is
> returned if the memory-region property is absent.


But it doesn't matter if it fails for any reason, any DMA allocation
should fall back to default CMA memory. In the case of ENOMEM or other
errors, then the later calls to allocate DMA may fail and we can deal
with it then.


Thanks,

Eddie


>
> Cheers,
>
> Andrew
>
>>
>> rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>> if (rc) {
>> --
>> 1.8.3.1
>>
>>

\
 
 \ /
  Last update: 2019-04-03 16:37    [W:0.069 / U:3.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site