Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT

[Sylwester Nawrocki <sylvester.nawrocki@gmail.com>]

Hi Kgene,

Cc: Rob Herring

On 08/01/2012 11:00 AM, Kukjin Kim wrote:
>>>>> +* Samsung Exynos5 Gscaler device
>>>>> +
>>>>> +Gscaler is used for scaling and color space conversion on EXYNOS5
>> SoCs.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: should be "samsung,exynos5250-gsc"
>>>>
>>>> IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can
>> use
>>>> same gscaler driver.
>>
>> In addition to the below explanation, perhaps it's obvious, but the driver
>> can claim compatibility with multiple devices, i.e. match with multiple
>> 'compatible' properties.
>>
> 
> The name of exact model is 'gscaler' for EXYNOS5 SoCs not only for
> EXYNOS5250. So the driver which has been submitted is also 'exynos-gsc' not
> 'exynos5250-gsc'. Note that there is no gscaler on EXYNOS4 SoCs now, it can
> be either 'exynos5-gsc' or 'exynos-gsc'. In addition, since some
> peripherals/drivers can be used for multiple SoCs and actually it does, I'm
> still thinking the compatible should be represent its usage. I don't know
> why restricted name is much clearer. And if multiple 'compatible' is
> required, we can add it later.

Sorry, but I cannot agree with that. :) There is no exynos5 SoC, it's just
a common name for the whole series that includes: exynos5250, exynos5450 ones 
and more. We shouldn't use such common name for just a subset of exynos5 chips, 
in case different GScaler versions get deployed in future SoCs.

It wouldn't be clear what specific SoCs the "samsung,exynos5-gsc" compatible 
string applies to, would it ? I believe there are already minor differences
in GScaler parameters on currently available exynos5 SoC. The variant data
structures are used to handle this and the compatible string determines which
variant data structure is selected during driver's initialization.
If you use a wildcard 'compatible' string this won't be possible any more.

Also it would look odd IMO to have two compatible strings like:
compatible = "samsung,exynos5-gsc", "samsung,exynos5400-gsc";

Please also see:
http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

--

Regards,
Sylwester