diff mbox

[2/3,media] exynos4-is: Annotate unused functions

Message ID 1375425134-17080-2-git-send-email-sachin.kamat@linaro.org
State Superseded
Headers show

Commit Message

Sachin Kamat Aug. 2, 2013, 6:32 a.m. UTC
__is_set_init_isp_aa and fimc_is_hw_set_tune currently do not have
any callers. However these functions may be used in the future. Hence
instead of deleting them, staticize and annotate them with __maybe_unused
flag to avoid compiler warnings.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/media/platform/exynos4-is/fimc-is-param.c |    2 +-
 drivers/media/platform/exynos4-is/fimc-is-regs.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Sachin Kamat Aug. 5, 2013, 5:12 a.m. UTC | #1
Hi Sylwester,

On 2 August 2013 12:02, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> __is_set_init_isp_aa and fimc_is_hw_set_tune currently do not have
> any callers. However these functions may be used in the future. Hence
> instead of deleting them, staticize and annotate them with __maybe_unused
> flag to avoid compiler warnings.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Thanks for applying the other 2 patches in this series. What is your
opinion about this one?
Does this look good or do you prefer to delete the code altogether?

> ---
>  drivers/media/platform/exynos4-is/fimc-is-param.c |    2 +-
>  drivers/media/platform/exynos4-is/fimc-is-regs.c  |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/exynos4-is/fimc-is-param.c b/drivers/media/platform/exynos4-is/fimc-is-param.c
> index a353be0..9bf3ddd 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is-param.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is-param.c
> @@ -287,7 +287,7 @@ void __is_set_sensor(struct fimc_is *is, int fps)
>         fimc_is_set_param_bit(is, PARAM_ISP_OTF_INPUT);
>  }
>
> -void __is_set_init_isp_aa(struct fimc_is *is)
> +static void __maybe_unused __is_set_init_isp_aa(struct fimc_is *is)
>  {
>         struct isp_param *isp;
>
> diff --git a/drivers/media/platform/exynos4-is/fimc-is-regs.c b/drivers/media/platform/exynos4-is/fimc-is-regs.c
> index 63c68ec..cf2e13a 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is-regs.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is-regs.c
> @@ -96,7 +96,7 @@ int fimc_is_hw_set_param(struct fimc_is *is)
>         return 0;
>  }
>
> -int fimc_is_hw_set_tune(struct fimc_is *is)
> +static int __maybe_unused fimc_is_hw_set_tune(struct fimc_is *is)
>  {
>         fimc_is_hw_wait_intmsr0_intmsd0(is);
>
> --
> 1.7.9.5
>
Hi Sachin,

On 08/05/2013 07:12 AM, Sachin Kamat wrote:
> On 2 August 2013 12:02, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> > __is_set_init_isp_aa and fimc_is_hw_set_tune currently do not have
>> > any callers. However these functions may be used in the future. Hence
>> > instead of deleting them, staticize and annotate them with __maybe_unused
>> > flag to avoid compiler warnings.
>> >
>> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Thanks for applying the other 2 patches in this series. What is your
> opinion about this one?
> Does this look good or do you prefer to delete the code altogether?

Thanks for your work on this. I think it would be better to call those
functions somewhere instead, e.g. in the fimc-is initialization routine,
until there is a user interface available for this 3A control.
fimc_is_hw_set_tune() just needs a private control a think. Let me see
if I can come up with at least some intermediate patch to achieve this,
so the warnings can be eliminated. I wouldn't like to take such steps
backwards, marking those functions static an unused.

Regards,
Sylwester
Sachin Kamat Aug. 5, 2013, 11:35 a.m. UTC | #3
Hi Sylwester,

On 5 August 2013 16:35, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> Hi Sachin,
>
> On 08/05/2013 07:12 AM, Sachin Kamat wrote:
>> On 2 August 2013 12:02, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>> > __is_set_init_isp_aa and fimc_is_hw_set_tune currently do not have
>>> > any callers. However these functions may be used in the future. Hence
>>> > instead of deleting them, staticize and annotate them with __maybe_unused
>>> > flag to avoid compiler warnings.
>>> >
>>> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> Thanks for applying the other 2 patches in this series. What is your
>> opinion about this one?
>> Does this look good or do you prefer to delete the code altogether?
>
> Thanks for your work on this. I think it would be better to call those
> functions somewhere instead, e.g. in the fimc-is initialization routine,
> until there is a user interface available for this 3A control.
> fimc_is_hw_set_tune() just needs a private control a think. Let me see
> if I can come up with at least some intermediate patch to achieve this,
> so the warnings can be eliminated. I wouldn't like to take such steps
> backwards, marking those functions static an unused.

Marking it unused is just a stop gap solution to silence unnecessary warnings.
However if you can come up with some users for these functions, then
that would be great
and right thing to do.
diff mbox

Patch

diff --git a/drivers/media/platform/exynos4-is/fimc-is-param.c b/drivers/media/platform/exynos4-is/fimc-is-param.c
index a353be0..9bf3ddd 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-param.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-param.c
@@ -287,7 +287,7 @@  void __is_set_sensor(struct fimc_is *is, int fps)
 	fimc_is_set_param_bit(is, PARAM_ISP_OTF_INPUT);
 }
 
-void __is_set_init_isp_aa(struct fimc_is *is)
+static void __maybe_unused __is_set_init_isp_aa(struct fimc_is *is)
 {
 	struct isp_param *isp;
 
diff --git a/drivers/media/platform/exynos4-is/fimc-is-regs.c b/drivers/media/platform/exynos4-is/fimc-is-regs.c
index 63c68ec..cf2e13a 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-regs.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-regs.c
@@ -96,7 +96,7 @@  int fimc_is_hw_set_param(struct fimc_is *is)
 	return 0;
 }
 
-int fimc_is_hw_set_tune(struct fimc_is *is)
+static int __maybe_unused fimc_is_hw_set_tune(struct fimc_is *is)
 {
 	fimc_is_hw_wait_intmsr0_intmsd0(is);