Hi, Le vendredi 26 avril 2019 à 21:18 +0530, Jagan Teki a écrit : > On Fri, Apr 26, 2019 at 9:08 PM Paul Kocialkowski > <paul.kocialkowski@xxxxxxxxxxx> wrote: > > Hi, > > > > Le vendredi 26 avril 2019 à 20:48 +0530, Jagan Teki a écrit : > > > On Fri, Apr 26, 2019 at 8:42 PM Paul Kocialkowski > > > <paul.kocialkowski@xxxxxxxxxxx> wrote: > > > > Hi, > > > > > > > > On Fri, 2019-04-26 at 20:31 +0530, Jagan Teki wrote: > > > > > On Fri, Apr 26, 2019 at 8:24 PM Paul Kocialkowski > > > > > <paul.kocialkowski@xxxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > > > > > > > On Fri, 2019-04-26 at 20:15 +0530, Jagan Teki wrote: > > > > > > > On Fri, Apr 26, 2019 at 8:04 PM Paul Kocialkowski > > > > > > > <paul.kocialkowski@xxxxxxxxxxx> wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On Fri, 2019-04-26 at 19:57 +0530, Jagan Teki wrote: > > > > > > > > > On Fri, Apr 26, 2019 at 7:47 PM Paul Kocialkowski > > > > > > > > > <paul.kocialkowski@xxxxxxxxxxx> wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On Thu, 2019-04-25 at 23:04 +0530, Jagan Teki wrote: > > > > > > > > > > > Add initial support for Orangepi RK3399 board. > > > > > > > > > > > > > > > > > > > > > > Specification > > > > > > > > > > > - Rockchip RK3399 > > > > > > > > > > > - 2GB/4GB DDR3 > > > > > > > > > > > - 16GB eMMC > > > > > > > > > > > - SD card slot > > > > > > > > > > > > > > > > > > > > Looks like you're missing u-boot,dm-pre-reloc to have it working, which > > > > > > > > > > will need to be introduced when moving to rk3399-u-boot.dtsi. > > > > > > > > > > > > > > > > > > Look like you are confused or doesn't check the patch. This patch have > > > > > > > > > rk3399-orangepi-u-boot.dtsi which included rk3399-u-boot.dtsi that has > > > > > > > > > u-boot,dm-pre-reloc for sdmmc. > > > > > > > > > > > > > > > > Well no, in your v3.1 patch, you no longer have u-boot,dm-pre-reloc in > > > > > > > > rk3399-u-boot.dtsi, so you need to add it in the device dts and remove > > > > > > > > it in your second series when you add it back to rk3399-u-boot.dtsi. > > > > > > > > > > > > > > Which u-boot,dm-pre-reloc are you taking about? > > > > > > > > > > > > > > v3.1 has u-boot,dm-pre-reloc for sdmmc, please check it again [1] > > > > > > > which were used by subsequent boards on the same series. > > > > > > > > > > > > > > The diff between v3 vs v3.1 is like v3 removed u-boot,dm-pre-reloc on > > > > > > > existing board dts files thought that it will include > > > > > > > rk3399-u-boot.dtsi will automatically, but not ie fixed in v3.1 > > > > > > > > > > > > > > > It's not okay to submit the board with broken MMC support and fix it in > > > > > > > > a subsequent series. > > > > > > > > > > > > > > Again, nothing broken. existing boards or dts(i) files are untouched. > > > > > > > Added only initial rk3399-u-boot.dtsi with sdmmc u-boot,dm-pre-reloc > > > > > > > node to make use of new boards. and the same reused by next series > > > > > > > so-that I can add binman global to all rk3399 boards. > > > > > > > > > > > > Okay I think I'm getting there. So the Orangepi uses the new scheme > > > > > > (including rk3399-u-boot.dtsi which has u-boot,dm-pre-reloc) while all > > > > > > other boards are still using the previous scheme after the series. > > > > > > > > > > > > This is very confusing and I really think you should keep u-boot,dm- > > > > > > pre-reloc in the orange pi dts file and then make the transition with > > > > > > all the other boards. You're mixing multiple logical steps which makes > > > > > > it really hard to understand what's going on. > > > > > > > > > > > > More to that, introducing the rk3399-u-boot.dtsi is a logical step that > > > > > > should be grouped with your second series, not the first one. In your > > > > > > first series, boards have u-boot,dm-pre-reloc in their per-device dtsi. > > > > > > > > > > > > Could you respin the two series to group changes by logic changes > > > > > > instead of the current state of interleaved changes? > > > > > > > > > > Okay, to make it clear I can send v4 with v3.1 included and updated > > > > > commit log. but I the new series about binman remains same since the > > > > > changes on first patch on the series are relevant. > > > > > > > > I think it makes no sense to introduce rk3399-u-boot.dts as part of the > > > > series currently in v3. You need to remove the patch (currently v3.1) > > > > from this series and group it with the new series (binman-related). > > > > > > > > Otherwise, you're mixing two unrelated logical changes across two > > > > series. > > > > > > rk3399-u-boot.dtsi is require for sdboot on that boards, whole idea is > > > to untouch Linux dts files and get introduced initial > > > rk3399-u-boot.dtsi which doesn't effect any board dts files but use > > > new boards. > > > > Yes I think I properly understand that now. The problem is that > > currently, rk3399 boards do not use rk3399-u-boot.dtsi. In the first > > series, you are introducing the file and including it in the newly- > > introduced devices, but not in the previous ones. This is inconsistent. > > > > Then, in the second series, you are moving existing boards to using > > rk3399-u-boot.dtsi, except for the boards you introduced in the first > > series. This is inconsistent. > > > > Do you see the problem? I feel like you are missing it and only > > focusing on the fact that the series works in practice, which is not > > the issue. > > Not again. > > First series, I get introduced rk3399-u-boot.dtsi, and only the new > boards are using this file and next series rest of the boards are > using which indeed a valid step. what is inconsistent here, I don't > understand. Yes, what you are describing is exactly the issue. It does not make sense to introduce a new common u-boot dtsi for rk3399 and add new devices that use this file *before* switching existing devices to the new common u-boot dtsi for rk3399 in the same series. Globally with your series, you are doing 3 things: - Introducing a common RK3399 u-boot dtsi with the required pre-reloc entries; - Adding new RK3399 devices and syncing the dtsi files from Linux; - Introducing u-boot-rockchip-with-spl.bin. So it should be 3 distinct series, not two. As you can see, you can either do step 1 and then step 2 or step 2 and then step 1, but you cannot mix parts of step 1 into step 2 and vice-versa. Is it clearer explained this way? Cheers, Paul > > > rk3399-u-boot.dtsi is required for binman for adding binman changes, > > > and indeed binman series has some issues which can take time to merge > > > and other one can do it before. > > > > Yes, it can be done before in the first series (so things will not > > break), but it should not be done that way for the reason I'm talking > > about. Again, the way changes are split and organized is as import as > > the technical changes. It's not sufficient that the series works, it > > also has to make sense by grouping logical changes. > > Don't agree your point, let me send v4 will see what Philipp and other > can think off. > > thanks for your comments, > Jagan. _______________________________________________ Linux-rockchip mailing list Linux-rockchip@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/linux-rockchip
