| Message ID | 91ae80f172192fa4dbc8bf8ee5a9068661592de6.1445935215.git.p.fedin@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>; >
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
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
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
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
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
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
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>;
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(+)