Re: [PATCH v2 4/5] [media] exynos-mscl: Add DT bindings for M-Scaler driver

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

On 08/19/2013 02:57 PM, Inki Dae wrote:
>> -----Original Message-----
>> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
>> owner@vger.kernel.org] On Behalf Of Shaik Ameer Basha
>> Sent: Monday, August 19, 2013 7:59 PM
>> To: linux-media@vger.kernel.org; linux-samsung-soc@vger.kernel.org
>> Cc: s.nawrocki@samsung.com; posciak@google.com; arun.kk@samsung.com;
>> shaik.ameer@samsung.com
>> Subject: [PATCH v2 4/5] [media] exynos-mscl: Add DT bindings for M-Scaler
>> driver
>>
>> This patch adds the DT binding documentation for the exynos5

You may want to say to which specific SoC it applies.

>> based M-Scaler device driver.
>>
>> Signed-off-by: Shaik Ameer Basha<shaik.ameer@samsung.com>
>> ---
>>   .../devicetree/bindings/media/exynos5-mscl.txt     |   34
>> ++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/exynos5-
>> mscl.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/exynos5-mscl.txt
>> b/Documentation/devicetree/bindings/media/exynos5-mscl.txt
>> new file mode 100644
>> index 0000000..5c9d1b1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/exynos5-mscl.txt
>> @@ -0,0 +1,34 @@
>> +* Samsung Exynos5 M-Scaler device
>> +
>> +M-Scaler is used for scaling, blending, color fill and color space
>> +conversion on EXYNOS5 SoCs.
>> +
>> +Required properties:
>> +- compatible: should be "samsung,exynos5-mscl"

What is an exact name of this IP in the datasheet ?

> If Exynos5410/5420 have same IP,
> "samsung,exynos5410-mscl" for M Scaler IP in Exynos5410/5420"
>
> Else,
> Compatible: should be one of the following:
> (a) "samsung,exynos5410-mscl" for M Scaler IP in Exynos5410"
> (b) "samsung,exynos5420-mscl" for M Scaler IP in Exynos5420"

Yes, except I suspect "mscl" is incorrect. It sounds like an unclear
abbreviation of real name of the IP. It likely should be "mscaler".

>> +- reg: should contain M-Scaler physical address location and length.
>> +- interrupts: should contain M-Scaler interrupt number
>> +- clocks: should contain the clock number according to CCF

Hmm, this sounds like a Linux specific term in the binding. Perhaps:

  - clocks: should contain the M-Scaler clock specifier, from the common
	   clock bindings

?
>> +- clock-names: should be "mscl"
>> +
>> +Example:
>> +
>> +	mscl_0: mscl@0x12800000 {

s/0x//

>> +		compatible = "samsung,exynos5-mscl";
>
> "samsung,exynos5410-mscl";
>
>> +		reg =<0x12800000 0x1000>;
>> +		interrupts =<0 220 0>;
>> +		clocks =<&clock 381>;
>> +		clock-names = "mscl";
>> +	};
>> +
>> +Aliases:
>> +Each M-Scaler node should have a numbered alias in the aliases node,
>> +in the form of msclN, N = 0...2. M-Scaler driver uses these aliases
>> +to retrieve the device IDs using "of_alias_get_id()" call.

So except in debug logs and for selecting variant data (which is same for
all IP instances) are the aliases used for anything else ?
I suspect you could do without these aliases. Device name includes start
address of the IP register region, so that could be used to identify the
M-Scaler instance in the logs.

--
Regards,
Sylwester