Hi Minkyu, On 28 May 2014 07:02, Minkyu Kang <mk7.k...@samsung.com> wrote: > Dear Simon Glass, > > On 27/05/14 09:01, Simon Glass wrote: >> Hi Minkyu, >> >> On 25 May 2014 21:15, Minkyu Kang <mk7.k...@samsung.com> wrote: >>> Dear Simon, >>> >>> On 23/05/14 02:27, Simon Glass wrote: >>>> Hi Minkyu, >>>> >>>> On 21 May 2014 15:20, Minkyu Kang <mk7.k...@samsung.com> wrote: >>>>> On 22/05/14 03:58, Simon Glass wrote: >>>>>> Hi Minkyu, >>>>>> >>>>>> On 21 May 2014 00:05, Minkyu Kang <mk7.k...@samsung.com> wrote: >>>>>>> On 20/05/14 20:47, Simon Glass wrote: >>>>>>>> Hi Minkyu, >>>>>>>> >>>>>>>> On 15 May 2014 00:51, Minkyu Kang <mk7.k...@samsung.com> wrote: >>>>>>>>> On 03/04/14 08:24, Simon Glass wrote: >>>>>>>>>> From: Aaron Durbin <adur...@chromium.org> >>>>>>>>>> >>>>>>>>>> The TSP65090 is a PMIC on some exynos5 boards. The init function is >>>>>>>>>> called for the TPS65090 pmic. If that device is not a part of the >>>>>>>>>> device >>>>>>>>>> tree (returns -ENODEV) then continue. Otherwise return a failure. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Aaron Durbin <adur...@chromium.org> >>>>>>>>>> Signed-off-by: Simon Glass <s...@chromium.org> >>>>>>>>>> Reviewed-by: Simon Glass <s...@chromium.org> >>>>>>>>>> --- >>>>>>>>>> >>>>>>>>>> Changes in v2: >>>>>>>>>> - Move code to exynos5-dt.c >>>>>>>>>> - Fix comment style >>>>>>>>>> - Add #ifdef around tps65090 code >>>>>>>>>> >>>>>>>>>> board/samsung/smdk5250/exynos5-dt.c | 13 +++++++++++++ >>>>>>>>>> 1 file changed, 13 insertions(+) >>>>>>>>>> >>>>>>>>>> diff --git a/board/samsung/smdk5250/exynos5-dt.c >>>>>>>>>> b/board/samsung/smdk5250/exynos5-dt.c >>>>>>>>>> index 1a64b9b..2c1cf8a 100644 >>>>>>>>>> --- a/board/samsung/smdk5250/exynos5-dt.c >>>>>>>>>> +++ b/board/samsung/smdk5250/exynos5-dt.c >>>>>>>>>> @@ -20,6 +20,7 @@ >>>>>>>>>> #include <asm/arch/sromc.h> >>>>>>>>>> #include <power/pmic.h> >>>>>>>>>> #include <power/max77686_pmic.h> >>>>>>>>>> +#include <power/tps65090_pmic.h> >>>>>>>>>> #include <tmu.h> >>>>>>>>>> >>>>>>>>>> DECLARE_GLOBAL_DATA_PTR; >>>>>>>>>> @@ -164,7 +165,19 @@ int exynos_power_init(void) >>>>>>>>>> >>>>>>>>>> #ifdef CONFIG_POWER_MAX77686 >>>>>>>>>> ret = max77686_init(); >>>>>>>>>> + if (ret) >>>>>>>>>> + return ret; >>>>>>>>>> #endif >>>>>>>>>> +#ifdef CONFIG_POWER_TPS65090 >>>>>>>>>> + /* >>>>>>>>>> + * The TPS65090 may not be in the device tree. If so, it is not >>>>>>>>>> + * an error. >>>>>>>>> >>>>>>>>> Then, how we can initialise the tps65090? >>>>>>>> >>>>>>>> It is initialised if a suitable node is found in the device tree. If >>>>>>>> the device tree does not have it, then the hardware is assumed to not >>>>>>>> have this chip. >>>>>>> >>>>>>> then I think, it's an error. >>>>>>> Why you said, it is not an error? >>>>>> >>>>>> I may be misunderstanding your question, but I'll try to answer what I >>>>>> think you are asking. >>>>>> >>>>>> The device tree contains nodes for hardware in the machine that you >>>>>> want to initialise, and information about each one. Devices can be >>>>>> enabled or disabled by including or removing this information from the >>>>>> device tree (or marking a device disabled with a status = "disabled" >>>>>> property in the node). >>>>>> >>>>>> The tps65090 chip is not used in all exynos5-dt boards, but is used in >>>>>> some. For example smdk5250 does not have it, but snow does. So we use >>>>>> the device tree to specify the difference, including (on snow) things >>>>>> like the tps65090, the display bridge chip, etc. >>>>>> >>>>> >>>>> Hm, it doesn't make sense. >>>>> Then it(#define CONFIG_POWER_TPS65090) should go into each config files. >>>>> Not in exynos5-dt.h. >>>>> Please modify it and patch 6/12 also. >>>> >>>> The way it works at present is that there is a single exynos5-dt.h >>>> file which is used by all exynos5 boards. To the extent possible we >>>> have avoided putting particular features in things like snow.h and >>>> smdk5250.h - they just include exynos5-dt.h without changes. >>>> >>>> The idea is that we can have one U-Boot binary that runs on any >>>> exynos5 device, rather than lots of different options. This makes it >>>> much easier to test changes sine we only need to build it once. If >>>> every exynos5 board has different #defines then there are more >>>> combinations to build and test. This is similar to what the kernel >>>> does with arch/arm/mach-exynos/mach-exynos5-dt.c. >>>> >>>> Using this approach the only differences between smdk5250, daisy, >>>> snow, spring, etc. are in the device tree. I'd really like to avoid >>>> changing this now. It is easy enough for people to add their own >>>> private variants of these locally if they want to, but for mainline I >>>> believe it is better to be more generic. >>> >>> I totally understood what you assert. >>> But I can't agree with you. >>> Do you think if we collect all features at exynos5-dt.h then >>> it can be generic? even if some boards doesn't have the device? >>> I think no. It just wrong definition. >>> We should separate exynos5 generic features and board specific features. >> >> Yes my intent is that we have an exynos5 build which (given the right >> device tree) can support any board. That has been the intent of the >> device tree effort for a while, and similar to the kernel approach. >> >> What is the objection of having the driver compiled in for a board >> that doesn't have the device? Is it the extra 1KB of code space for >> smdk5250 etc.? > > Yes, it's a tiny part of binary. > But if we allow such things.. then it can be grew more and more. > >> >> Anyway, it doesn't necessarily have to be exynos5-dt.h. If you really >> don't like that, we could create a new generic exynos5 board and I can >> target that instead. But I really want to avoid having to build >> multiple U-Boots for each board if possible. > > I want to define CONFIG_POWER_TPS65090 at snow.h and other each board's > config file that have a device. > But, I know what you want. > So I will merge this patchset. > But at future, I'll not allow any exception. > It was not right way I think. > I just want to keep clean our codes. > That's all.
Thanks for being flexible. It is good to get this applied. I'll take a look at how to improve this to separate out the 'generic' board from the others. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
