From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vikash Garodia Subject: Re: [PATCH v9 1/5] venus: firmware: add routine to reset ARM9 Date: Mon, 01 Oct 2018 21:14:46 +0530 Message-ID: References: <1537314192-26892-1-git-send-email-vgarodia@codeaurora.org> <1537314192-26892-2-git-send-email-vgarodia@codeaurora.org> <97b94b9b-f028-cb8b-a3db-67626dc517ab@linaro.org> <175fcecf3be715d2a20b71746c648f1e@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Stanimir Varbanov Cc: Alexandre Courbot , Hans Verkuil , Mauro Carvalho Chehab , robh@kernel.org, mark.rutland@arm.com, Andy Gross , Arnd Bergmann , bjorn.andersson@linaro.org, Linux Media Mailing List , LKML , linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, devicetree@vger.kernel.org, linux-media-owner@vger.kernel.org List-Id: devicetree@vger.kernel.org Hi Stanimir, On 2018-10-01 20:00, Stanimir Varbanov wrote: > Hi, > > On 09/20/2018 06:31 AM, Alexandre Courbot wrote: >> On Thu, Sep 20, 2018 at 2:55 AM Vikash Garodia >> wrote: >>> >>> On 2018-09-19 20:30, Stanimir Varbanov wrote: >>>> Hi Alex, >>>> >>>> On 09/19/2018 10:32 AM, Alexandre Courbot wrote: >>>>> On Wed, Sep 19, 2018 at 8:43 AM Vikash Garodia >>>>> wrote: >>>>>> >>>>>> Add routine to reset the ARM9 and brings it out of reset. Also >>>>>> abstract the Venus CPU state handling with a new function. This >>>>>> is in preparation to add PIL functionality in venus driver. >>>>>> >>>>>> Signed-off-by: Vikash Garodia >>>>>> --- >>>>>> drivers/media/platform/qcom/venus/core.h | 2 ++ >>>>>> drivers/media/platform/qcom/venus/firmware.c | 33 >>>>>> ++++++++++++++++++++++++ >>>>>> drivers/media/platform/qcom/venus/firmware.h | 11 ++++++++ >>>>>> drivers/media/platform/qcom/venus/hfi_venus.c | 13 +++------- >>>>>> drivers/media/platform/qcom/venus/hfi_venus_io.h | 7 +++++ >>>>>> 5 files changed, 57 insertions(+), 9 deletions(-) >>>>>> >>>>>> diff --git a/drivers/media/platform/qcom/venus/core.h >>>>>> b/drivers/media/platform/qcom/venus/core.h >>>>>> index 2f02365..dfd5c10 100644 >>>>>> --- a/drivers/media/platform/qcom/venus/core.h >>>>>> +++ b/drivers/media/platform/qcom/venus/core.h >>>>>> @@ -98,6 +98,7 @@ struct venus_caps { >>>>>> * @dev: convenience struct device pointer >>>>>> * @dev_dec: convenience struct device pointer for decoder >>>>>> device >>>>>> * @dev_enc: convenience struct device pointer for encoder >>>>>> device >>>>>> + * @no_tz: a flag that suggests presence of trustzone >>>>> >>>>> Looks like it suggests the absence of trustzone? >>>>> >>>>> Actually I would rename it as use_tz and set it if TrustZone is >>>>> used. >>>>> This would avoid double-negative statements like what we see below. >>>> >>>> I find this suggestion reasonable. >>> >>> Initially i planned to keep it as a positive flag. The reason behind >>> keeping it >>> as no_tz was to keep the default value of this flag to 0 indicating >>> tz >>> is present >>> by default. >>> I can switch it to use_tz though and set it in firmware_init after >>> the >>> presence of >>> firmware node is checked. >> >> Making sure the flag is explicitly initialized instead of relying on >> default initialization is another good reason to have that change >> IMHO. :) > > Vikash, care to send a new version, or will fix that with follow up > patches? I will provide a new series with the suggested change. Thanks, Vikash