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

[Pavel Fedin]

 Hello!

> > +   sromc: sromc@1225 {
> > +   compatible = "samsung,exynos-srom";
> > +   reg = <0x1225 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Krzysztof Kozlowski]

On 28.10.2015 16:06, Pavel Fedin wrote:
>  Hello!
> 
>>> +   sromc: sromc@1225 {
>>> +   compatible = "samsung,exynos-srom";
>>> +   reg = <0x1225 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Pavel Fedin]

 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Pankaj Dubey]

Hi

On Wednesday 28 October 2015 12:54 PM, Krzysztof Kozlowski wrote:

On 28.10.2015 16:06, Pavel Fedin wrote:

  Hello!


+   sromc: sromc@1225 {
+   compatible = "samsung,exynos-srom";
+   reg = <0x1225 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Krzysztof Kozlowski]

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@1225 {
> +compatible = "samsung,exynos-srom";
> +reg = <0x1225 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Pavel Fedin]

 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Pavel Fedin]

This machine uses own SoC device tree file, add missing part.

Signed-off-by: Pavel Fedin 
---
 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 = <0x1000 0x100>;
};
 
+   sromc: sromc@1225 {
+   compatible = "samsung,exynos-srom";
+   reg = <0x1225 0x10>;
+   };
+
pmu_system_controller: system-controller@1004 {
compatible = "samsung,exynos5410-pmu", "syscon";
reg = <0x1004 0x5000>;
-- 
2.4.4

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

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

[Krzysztof Kozlowski]

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 
> ---
>  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 = <0x1000 0x100>;
>   };
>  
> + sromc: sromc@1225 {
> + compatible = "samsung,exynos-srom";
> + reg = <0x1225 0x10>;

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

Best regards,
Krzysztof

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

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