Bjorn, thanks for the comments! On 02/08/2017 01:32 AM, Bjorn Andersson wrote: > On Tue 07 Feb 05:10 PST 2017, Stanimir Varbanov wrote: > >> * firmware loader >> > > I like the way this turns out, just some style comments below. > > [..] >> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c >> new file mode 100644 >> index 000000000000..4057696abaf5 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/venus/firmware.c >> @@ -0,0 +1,151 @@ >> +/* >> + * Copyright (C) 2017 Linaro Ltd. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 and >> + * only version 2 as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include <linux/dma-mapping.h> >> +#include <linux/firmware.h> >> +#include <linux/kernel.h> >> +#include <linux/of.h> >> +#include <linux/of_reserved_mem.h> >> +#include <linux/slab.h> >> +#include <linux/qcom_scm.h> >> +#include <linux/soc/qcom/mdt_loader.h> >> + >> +#define VENUS_FIRMWARE_NAME "venus.mdt" >> +#define VENUS_PAS_ID 9 >> +#define VENUS_FW_MEM_SIZE SZ_8M >> + >> +struct firmware_mem { >> + struct device dev; >> + void *mem_va; >> + phys_addr_t mem_phys; >> + size_t mem_size; >> +}; >> + >> +static struct firmware_mem fw; > > Rather than operating on a global variable I think you should either > return your firmware_mem pointer or the device pointer to the caller of > venus_boot() and have the core pass that back into venus_shutdown(). I will take your comments and will pass struct device *fw_dev as an argument of venus_boot. Also I will move memory allocation in venus_boot and by that way I don't need to keep memory attributes from above structure. -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html