diff mbox

[v2,1/4] ARM: dts: Add SROMc to Exynos 5410

Message ID 91ae80f172192fa4dbc8bf8ee5a9068661592de6.1445935215.git.p.fedin@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Fedin Oct. 27, 2015, 8:43 a.m. UTC
This machine uses own SoC device tree file, add missing part.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 arch/arm/boot/dts/exynos5410.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Krzysztof Kozlowski Oct. 28, 2015, 2:12 a.m. UTC | #1
On 27.10.2015 17:43, Pavel Fedin wrote:
> This machine uses own SoC device tree file, add missing part.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5410.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
> index 4603356..af85aad 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -101,6 +101,11 @@
>  			reg = <0x10000000 0x100>;
>  		};
>  
> +		sromc: sromc@12250000 {
> +			compatible = "samsung,exynos-srom";
> +			reg = <0x12250000 0x10>;

Isn't 0x10 too small (SROM_BC3 won't be mapped)?

Best regards,
Krzysztof

> +		};
> +
>  		pmu_system_controller: system-controller@10040000 {
>  			compatible = "samsung,exynos5410-pmu", "syscon";
>  			reg = <0x10040000 0x5000>;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Fedin Oct. 28, 2015, 7:06 a.m. UTC | #2
Hello!

> > +		sromc: sromc@12250000 {
> > +			compatible = "samsung,exynos-srom";
> > +			reg = <0x12250000 0x10>;
> 
> Isn't 0x10 too small (SROM_BC3 won't be mapped)?

 Muhaha, indeed, thanks for noticing this.
 By the way, i've just checked exynos4.dtsi and exynos5.dtsi, they specify the same size. Did reviewers overlook this small thing?
Shouldn't it be fixed then?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Oct. 28, 2015, 7:24 a.m. UTC | #3
On 28.10.2015 16:06, Pavel Fedin wrote:
>  Hello!
> 
>>> +		sromc: sromc@12250000 {
>>> +			compatible = "samsung,exynos-srom";
>>> +			reg = <0x12250000 0x10>;
>>
>> Isn't 0x10 too small (SROM_BC3 won't be mapped)?
> 
>  Muhaha, indeed, thanks for noticing this.
>  By the way, i've just checked exynos4.dtsi and exynos5.dtsi, they specify the same size. Did reviewers overlook this small thing?

Yep, I pointed that 0x100 (from first version of patchset) is too big...
but did not exactly check the length of new value.

> Shouldn't it be fixed then?

Yes. It hasn't been pulled yet by arm-soc... Let's wait Kukjin's opinion
how to deal with exynos[45].dtsi.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Fedin Oct. 28, 2015, 7:25 a.m. UTC | #4
Hello!

> Yes. It hasn't been pulled yet by arm-soc... Let's wait Kukjin's opinion
> how to deal with exynos[45].dtsi.

 I can simply include it into my patchset, two more lines aren't a problem for me.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Oct. 28, 2015, 9:30 a.m. UTC | #5
Hi

On Wednesday 28 October 2015 12:54 PM, Krzysztof Kozlowski wrote:
> On 28.10.2015 16:06, Pavel Fedin wrote:
>>   Hello!
>>
>>>> +		sromc: sromc@12250000 {
>>>> +			compatible = "samsung,exynos-srom";
>>>> +			reg = <0x12250000 0x10>;
>>>
>>> Isn't 0x10 too small (SROM_BC3 won't be mapped)?
>>
>>   Muhaha, indeed, thanks for noticing this.
>>   By the way, i've just checked exynos4.dtsi and exynos5.dtsi, they specify the same size. Did reviewers overlook this small thing?
>
> Yep, I pointed that 0x100 (from first version of patchset) is too big...
> but did not exactly check the length of new value.
>

Yes, once you pointed out I checked UM for Exynos4415, Exynos5250, 
Exynos5420 and Exynos5410 and all these manuals talks about SROM_BC{0-3} 
only. There is no offset such as SROM_BC{4,5} at least in these SoC 
manuals. Accordingly I modified size from 0x100 to 0x10. But looks like 
I missed to remove SROM_BC{{4,5} from exynos-srom.h. I checked only 
these registers are used in the driver, so as such it should not cause 
any issue in driver as of now, only we have some redundant entry in 
exynos-srom.h which can be removed if it's not applicable for any of 
Exynos SoC, after confirmation from Kukjin.


>> Shouldn't it be fixed then?
>
> Yes. It hasn't been pulled yet by arm-soc... Let's wait Kukjin's opinion
> how to deal with exynos[45].dtsi.
>

> Best regards,
> Krzysztof
>

Thanks,
Pankaj Dubey
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Oct. 28, 2015, 9:36 a.m. UTC | #6
On 28.10.2015 18:30, Pankaj Dubey wrote:
> Hi
> 
> On Wednesday 28 October 2015 12:54 PM, Krzysztof Kozlowski wrote:
>> On 28.10.2015 16:06, Pavel Fedin wrote:
>>>   Hello!
>>>
>>>>> +        sromc: sromc@12250000 {
>>>>> +            compatible = "samsung,exynos-srom";
>>>>> +            reg = <0x12250000 0x10>;
>>>>
>>>> Isn't 0x10 too small (SROM_BC3 won't be mapped)?
>>>
>>>   Muhaha, indeed, thanks for noticing this.
>>>   By the way, i've just checked exynos4.dtsi and exynos5.dtsi, they
>>> specify the same size. Did reviewers overlook this small thing?
>>
>> Yep, I pointed that 0x100 (from first version of patchset) is too big...
>> but did not exactly check the length of new value.
>>
> 
> Yes, once you pointed out I checked UM for Exynos4415, Exynos5250,
> Exynos5420 and Exynos5410 and all these manuals talks about SROM_BC{0-3}
> only. There is no offset such as SROM_BC{4,5} at least in these SoC
> manuals. Accordingly I modified size from 0x100 to 0x10. But looks like
> I missed to remove SROM_BC{{4,5} from exynos-srom.h. I checked only
> these registers are used in the driver, so as such it should not cause
> any issue in driver as of now, only we have some redundant entry in
> exynos-srom.h which can be removed if it's not applicable for any of
> Exynos SoC, after confirmation from Kukjin.

I was not referring to SROM_BC[45] but to SROM_BC3 which has the offset
of 0x10.

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Fedin Oct. 28, 2015, 9:39 a.m. UTC | #7
Hello!

> There is no offset such as SROM_BC{4,5} at least in these SoC
> manuals. Accordingly I modified size from 0x100 to 0x10.

 0x10 is indeed an offset for SROM_BC3. But, it occupies 4 bytes by itself, and you have to include it, because you are describing
*SIZE* of the region. :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
index 4603356..af85aad 100644
--- a/arch/arm/boot/dts/exynos5410.dtsi
+++ b/arch/arm/boot/dts/exynos5410.dtsi
@@ -101,6 +101,11 @@ 
 			reg = <0x10000000 0x100>;
 		};
 
+		sromc: sromc@12250000 {
+			compatible = "samsung,exynos-srom";
+			reg = <0x12250000 0x10>;
+		};
+
 		pmu_system_controller: system-controller@10040000 {
 			compatible = "samsung,exynos5410-pmu", "syscon";
 			reg = <0x10040000 0x5000>;