2016-03-02 17:53:27

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RESPIN 1/6] dt-bindings: EXYNOS: Add exynos-srom device tree binding

On Thu, Feb 25, 2016 at 02:03:37PM +0530, Pankaj Dubey wrote:
> This patch adds exynos-srom binding information for SROM Controller
> driver on Exynos SoCs.
>
> CC: Rob Herring <[email protected]>
> CC: Mark Rutland <[email protected]>
> CC: Ian Campbell <[email protected]>
> CC: [email protected]
> Signed-off-by: Pankaj Dubey <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Kukjin Kim <[email protected]>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../devicetree/bindings/memory-controllers/exynos-srom.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
> new file mode 100644
> index 0000000..33886d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
> @@ -0,0 +1,12 @@
> +SAMSUNG Exynos SoCs SROM Controller driver.
> +
> +Required properties:
> +- compatible : Should contain "samsung,exynos-srom".

There should be SoC specific compatible strings in addition to this.

> +
> +- reg: offset and length of the register set
> +
> +Example:
> + sromc@12570000 {

This should be memory-controller@...

> + compatible = "samsung,exynos-srom";
> + reg = <0x12570000 0x10>;

The next patch changes the size to 0x14?

BTW, you could just combine both patches. I guess you didn't to maintain
authorship?

Rob


2016-03-05 04:40:50

by Pankaj Dubey

[permalink] [raw]
Subject: Re: [RESPIN 1/6] dt-bindings: EXYNOS: Add exynos-srom device tree binding

Hi,

On Wednesday 02 March 2016 11:23 PM, Rob Herring wrote:
> On Thu, Feb 25, 2016 at 02:03:37PM +0530, Pankaj Dubey wrote:
>> This patch adds exynos-srom binding information for SROM Controller
>> driver on Exynos SoCs.
>>
>> CC: Rob Herring <[email protected]>
>> CC: Mark Rutland <[email protected]>
>> CC: Ian Campbell <[email protected]>
>> CC: [email protected]
>> Signed-off-by: Pankaj Dubey <[email protected]>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> Signed-off-by: Kukjin Kim <[email protected]>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> .../devicetree/bindings/memory-controllers/exynos-srom.txt | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>> new file mode 100644
>> index 0000000..33886d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>> @@ -0,0 +1,12 @@
>> +SAMSUNG Exynos SoCs SROM Controller driver.
>> +
>> +Required properties:
>> +- compatible : Should contain "samsung,exynos-srom".
>
> There should be SoC specific compatible strings in addition to this.
>

I do not understood need to additional need to SoC specific compatible,
as of now all SoCs having SROM can work with exynos specific common
compatible. Will you see any issues in this?

>> +
>> +- reg: offset and length of the register set
>> +
>> +Example:
>> + sromc@12570000 {
>
> This should be memory-controller@...
>
>> + compatible = "samsung,exynos-srom";
>> + reg = <0x12570000 0x10>;
>
> The next patch changes the size to 0x14?
>
> BTW, you could just combine both patches. I guess you didn't to maintain
> authorship?

OK. Will squash both of these patches, unless Pavel do not any objections.

Thanks,
Pankaj Dubey
>
> Rob
>
>

2016-03-05 06:17:21

by Pankaj Dubey

[permalink] [raw]
Subject: Re: [RESPIN 1/6] dt-bindings: EXYNOS: Add exynos-srom device tree binding

Hi

On Wednesday 02 March 2016 11:23 PM, Rob Herring wrote:
> On Thu, Feb 25, 2016 at 02:03:37PM +0530, Pankaj Dubey wrote:
>> This patch adds exynos-srom binding information for SROM Controller
>> driver on Exynos SoCs.
>>
>> CC: Rob Herring <[email protected]>
>> CC: Mark Rutland <[email protected]>
>> CC: Ian Campbell <[email protected]>
>> CC: [email protected]
>> Signed-off-by: Pankaj Dubey <[email protected]>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> Signed-off-by: Kukjin Kim <[email protected]>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> .../devicetree/bindings/memory-controllers/exynos-srom.txt | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>> new file mode 100644
>> index 0000000..33886d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>> @@ -0,0 +1,12 @@
>> +SAMSUNG Exynos SoCs SROM Controller driver.
>> +
>> +Required properties:
>> +- compatible : Should contain "samsung,exynos-srom".
>
> There should be SoC specific compatible strings in addition to this.
>

I do not understood need of additional SoC specific compatible here. As
of now all SoCs having SROM can work with exynos specific common
compatible. Do you see any issues if we do not have SoC specific
compatible here.

>> +
>> +- reg: offset and length of the register set
>> +
>> +Example:
>> + sromc@12570000 {
>
> This should be memory-controller@...
>
>> + compatible = "samsung,exynos-srom";
>> + reg = <0x12570000 0x10>;
>
> The next patch changes the size to 0x14?
>
> BTW, you could just combine both patches. I guess you didn't to maintain
> authorship?
>

OK. Will squash both of these patches, unless Pavel do not have any
objections to this.

Thanks,
Pankaj Dubey

> Rob
>
>

2016-03-08 07:06:04

by Pavel Fedin

[permalink] [raw]
Subject: RE: [RESPIN 1/6] dt-bindings: EXYNOS: Add exynos-srom device tree binding

Hello!

>> BTW, you could just combine both patches. I guess you didn't to
>> maintain authorship?

>OK. Will squash both of these patches, unless Pavel do not any objections.

I don't.

Kind regards.

2016-03-11 02:38:21

by Pankaj Dubey

[permalink] [raw]
Subject: Re: [RESPIN 1/6] dt-bindings: EXYNOS: Add exynos-srom device tree binding



On Saturday 05 March 2016 11:48 AM, pankaj.dubey wrote:
> Hi
>
> On Wednesday 02 March 2016 11:23 PM, Rob Herring wrote:
>> On Thu, Feb 25, 2016 at 02:03:37PM +0530, Pankaj Dubey wrote:
>>> This patch adds exynos-srom binding information for SROM Controller
>>> driver on Exynos SoCs.
>>>
>>> CC: Rob Herring <[email protected]>
>>> CC: Mark Rutland <[email protected]>
>>> CC: Ian Campbell <[email protected]>
>>> CC: [email protected]
>>> Signed-off-by: Pankaj Dubey <[email protected]>
>>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>>> Signed-off-by: Kukjin Kim <[email protected]>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> .../devicetree/bindings/memory-controllers/exynos-srom.txt | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>> new file mode 100644
>>> index 0000000..33886d5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>> @@ -0,0 +1,12 @@
>>> +SAMSUNG Exynos SoCs SROM Controller driver.
>>> +
>>> +Required properties:
>>> +- compatible : Should contain "samsung,exynos-srom".
>>
>> There should be SoC specific compatible strings in addition to this.
>>
>
> I do not understood need of additional SoC specific compatible here. As
> of now all SoCs having SROM can work with exynos specific common
> compatible. Do you see any issues if we do not have SoC specific
> compatible here.
>

Gentle ping.

Thanks,
Pankaj Dubey

2016-04-04 03:35:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RESPIN 1/6] dt-bindings: EXYNOS: Add exynos-srom device tree binding

On 05.03.2016 13:42, pankaj.dubey wrote:
> Hi,
>
> On Wednesday 02 March 2016 11:23 PM, Rob Herring wrote:
>> On Thu, Feb 25, 2016 at 02:03:37PM +0530, Pankaj Dubey wrote:
>>> This patch adds exynos-srom binding information for SROM Controller
>>> driver on Exynos SoCs.
>>>
>>> CC: Rob Herring <[email protected]>
>>> CC: Mark Rutland <[email protected]>
>>> CC: Ian Campbell <[email protected]>
>>> CC: [email protected]
>>> Signed-off-by: Pankaj Dubey <[email protected]>
>>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>>> Signed-off-by: Kukjin Kim <[email protected]>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> .../devicetree/bindings/memory-controllers/exynos-srom.txt | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>> new file mode 100644
>>> index 0000000..33886d5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.txt
>>> @@ -0,0 +1,12 @@
>>> +SAMSUNG Exynos SoCs SROM Controller driver.
>>> +
>>> +Required properties:
>>> +- compatible : Should contain "samsung,exynos-srom".
>>
>> There should be SoC specific compatible strings in addition to this.
>>
>
> I do not understood need to additional need to SoC specific compatible,
> as of now all SoCs having SROM can work with exynos specific common
> compatible. Will you see any issues in this?

Dear Rob, Pankaj,

I would like to proceed with this patchset. I guess your comment here
was about to change the compatible from generic exynos-like to a model
specific. How about this:
- compatible : Should contain "samsung,exynos4210-srom".

Is this okay?

(the oldest chip mentioning SROM controller as separate block is Exynos4210)


Best regards,
Krzysztof