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, Minkyu Kang. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
