Re: [PATCH v6 3/4] media: coda: use genalloc API

[Paul Gortmaker <paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>]

On 12-11-16 11:51 AM, Philipp Zabel wrote:
> Am Freitag, den 16.11.2012, 11:00 -0500 schrieb Paul Gortmaker:
>> On 12-11-16 10:21 AM, Philipp Zabel wrote:
>>> Am Freitag, den 16.11.2012, 10:08 -0500 schrieb Paul Gortmaker:
>>>> On 12-11-16 05:30 AM, Philipp Zabel wrote:
>>>>> This patch depends on "genalloc: add a global pool list,
>>>>> allow to find pools by phys address", which provides the
>>>>> of_get_named_gen_pool function.
>>>>>
>>>>> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>>>> ---
>>>>>  drivers/media/platform/Kconfig |    3 +--
>>>>>  drivers/media/platform/coda.c  |   47 ++++++++++++++++++++++++++++------------
>>>>>  2 files changed, 34 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>>>>> index 181c768..09d45c6 100644
>>>>> --- a/drivers/media/platform/Kconfig
>>>>> +++ b/drivers/media/platform/Kconfig
>>>>> @@ -130,10 +130,9 @@ if V4L_MEM2MEM_DRIVERS
>>>>>  
>>>>>  config VIDEO_CODA
>>>>>  	tristate "Chips&Media Coda multi-standard codec IP"
>>>>> -	depends on VIDEO_DEV && VIDEO_V4L2 && ARCH_MXC
>>>>> +	depends on VIDEO_DEV && VIDEO_V4L2
>>>>
>>>> What was the logic for reducing the dependency scope here?
>>>> Your commit log doesn't mention that at all, and when I see
>>>> things like that, I predict allyesconfig build failures,
>>>> unless there is a similar dependency elsewhere that isn't
>>>> visible in just the context of this patch alone.
>>>>
>>>> P.
>>>
>>> iram_alloc and iram_free are i.MX specific wrappers around
>>> gen_pool_alloc and gen_pool_free, located in <mach/iram.h>.
>>> Those were responsible for the dependency in the first place.
>>
>> So when I do an allyesconfig for sparc, or parisc or alpha,
>> and VIDEO_CODA gets selected, it will build just fine then?
> 
> I don't know, as I don't have compilers for those available right now.
> I'd like to know if it doesn't, though. It builds fine on x86 and mips,
> for example.

Probably worthwhile to watch the linux-next builds once you
know your commit(s) will be present there, since it has a
wide arch coverage.

> 
>> My point was that when you remove the ARCH_MXC dep, this
>> probably gets opened up as a viable option to a _lot_ more
>> platforms than you might want it exposed to.
> 
> I don't see the problem. Isn't this a good thing?

Well, it can be, if it was intentional, and if the hardware
is genuinely architecture agnostic.  On the other hand, I
don't think it makes sense to be building arm specific drivers
on sparc (or similar) just because we can.  It just adds to
the overall build coverage overhead for minimal gain.

Paul.
--

> 
> regards
> Philipp
>