| Message ID | 20160905132952.27280-2-julian@jusst.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Warren |
| Headers | show |
On 09/05/2016 07:29 AM, Julian Scheel wrote: > This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. > The pmic shall be initialised in board specific code, as it is done for > jetson-tk1 and meerkat. It can't be assumed that all boards have the pmic on > the same i2c bus. You can't just revert this, since boards such as nyan-big and venice2 rely on this function being called. Admittedly jetson-tk1 does call this a second time right now, but none of the other boards appear to. > Change-Id: I02d279b63ca72e143fadd85d4df68a929a658a12 Remove that line for upstream patches.
On 06.09.16 18:50, Stephen Warren wrote: > On 09/05/2016 07:29 AM, Julian Scheel wrote: >> This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. >> The pmic shall be initialised in board specific code, as it is done for >> jetson-tk1 and meerkat. It can't be assumed that all boards have the >> pmic on >> the same i2c bus. > > You can't just revert this, since boards such as nyan-big and venice2 > rely on this function being called. Admittedly jetson-tk1 does call this > a second time right now, but none of the other boards appear to. Sorry, I missed to adapt the other boards. I don't see another way than having this in each of the boards, as parameters might change, do you? >> Change-Id: I02d279b63ca72e143fadd85d4df68a929a658a12 > > Remove that line for upstream patches. Sorry, slipped through. -Julian
On 09/06/2016 12:04 PM, Julian Scheel wrote: > On 06.09.16 18:50, Stephen Warren wrote: >> On 09/05/2016 07:29 AM, Julian Scheel wrote: >>> This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. >>> The pmic shall be initialised in board specific code, as it is done for >>> jetson-tk1 and meerkat. It can't be assumed that all boards have the >>> pmic on >>> the same i2c bus. >> >> You can't just revert this, since boards such as nyan-big and venice2 >> rely on this function being called. Admittedly jetson-tk1 does call this >> a second time right now, but none of the other boards appear to. > > Sorry, I missed to adapt the other boards. I don't see another way than > having this in each of the boards, as parameters might change, do you? You can also parametrize the common code; either make it use #defines for the bus/device/... number and set those defines in a board-specific location, or call functions to retrieve that data which boards need to implement in a board-specific location.
On 06.09.2016 20:49, Stephen Warren wrote: > On 09/06/2016 12:04 PM, Julian Scheel wrote: >> On 06.09.16 18:50, Stephen Warren wrote: >>> On 09/05/2016 07:29 AM, Julian Scheel wrote: >>>> This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. >>>> The pmic shall be initialised in board specific code, as it is done for >>>> jetson-tk1 and meerkat. It can't be assumed that all boards have the >>>> pmic on >>>> the same i2c bus. >>> >>> You can't just revert this, since boards such as nyan-big and venice2 >>> rely on this function being called. Admittedly jetson-tk1 does call this >>> a second time right now, but none of the other boards appear to. I checked this again and in fact the only other board besides jetson-tk1 having CONFIG_AS3722_POWER set is nyan big. All the other tegra boards do only call what is in board/nvidia/venice2/as3722_init.c:pmic_enable_cpu_vdd(void). >> Sorry, I missed to adapt the other boards. I don't see another way than >> having this in each of the boards, as parameters might change, do you? > > You can also parametrize the common code; either make it use #defines > for the bus/device/... number and set those defines in a board-specific > location, or call functions to retrieve that data which boards need to > implement in a board-specific location. Taking into account that currently 2 out of 26 tegra boards use this I would prefer not to do this in board2.c, but simply move it to the boards nvidia_board_init. Would you be ok with this? -Julian
On 09/12/2016 01:33 AM, Julian Scheel wrote: > On 06.09.2016 20:49, Stephen Warren wrote: >> On 09/06/2016 12:04 PM, Julian Scheel wrote: >>> On 06.09.16 18:50, Stephen Warren wrote: >>>> On 09/05/2016 07:29 AM, Julian Scheel wrote: >>>>> This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. >>>>> The pmic shall be initialised in board specific code, as it is done >>>>> for >>>>> jetson-tk1 and meerkat. It can't be assumed that all boards have the >>>>> pmic on >>>>> the same i2c bus. >>>> >>>> You can't just revert this, since boards such as nyan-big and venice2 >>>> rely on this function being called. Admittedly jetson-tk1 does call >>>> this >>>> a second time right now, but none of the other boards appear to. > > I checked this again and in fact the only other board besides jetson-tk1 > having CONFIG_AS3722_POWER set is nyan big. All the other tegra boards > do only call what is in > board/nvidia/venice2/as3722_init.c:pmic_enable_cpu_vdd(void). cei-tk1-som has been recently added too; it's very similar to Jetson TK1. >>> Sorry, I missed to adapt the other boards. I don't see another way than >>> having this in each of the boards, as parameters might change, do you? >> >> You can also parametrize the common code; either make it use #defines >> for the bus/device/... number and set those defines in a board-specific >> location, or call functions to retrieve that data which boards need to >> implement in a board-specific location. > > Taking into account that currently 2 out of 26 tegra boards use this I > would prefer not to do this in board2.c, but simply move it to the > boards nvidia_board_init. Would you be ok with this? It's fine to move the call to as3722_init() into board files rather than common files. However, note that this needs to be done atomically in a single commit; each commit in the series needs to compile and operate correctly on all boards. As such, simply reverting "tegra: Add support for setting up a as3722 PMIC" won't work, since that'll temporarily prevent at least venice2's tegra_lcd_pmic_init() from working, since it appears to relie on the call to as3722_init().
On 12.09.16 19:02, Stephen Warren wrote: > On 09/12/2016 01:33 AM, Julian Scheel wrote: >> On 06.09.2016 20:49, Stephen Warren wrote: >>> On 09/06/2016 12:04 PM, Julian Scheel wrote: >>>> On 06.09.16 18:50, Stephen Warren wrote: >>>>> On 09/05/2016 07:29 AM, Julian Scheel wrote: >>>>>> This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. >>>>>> The pmic shall be initialised in board specific code, as it is done >>>>>> for >>>>>> jetson-tk1 and meerkat. It can't be assumed that all boards have the >>>>>> pmic on >>>>>> the same i2c bus. >>>>> >>>>> You can't just revert this, since boards such as nyan-big and venice2 >>>>> rely on this function being called. Admittedly jetson-tk1 does call >>>>> this >>>>> a second time right now, but none of the other boards appear to. >> >> I checked this again and in fact the only other board besides jetson-tk1 >> having CONFIG_AS3722_POWER set is nyan big. All the other tegra boards >> do only call what is in >> board/nvidia/venice2/as3722_init.c:pmic_enable_cpu_vdd(void). > > cei-tk1-som has been recently added too; it's very similar to Jetson TK1. > >>>> Sorry, I missed to adapt the other boards. I don't see another way than >>>> having this in each of the boards, as parameters might change, do you? >>> >>> You can also parametrize the common code; either make it use #defines >>> for the bus/device/... number and set those defines in a board-specific >>> location, or call functions to retrieve that data which boards need to >>> implement in a board-specific location. >> >> Taking into account that currently 2 out of 26 tegra boards use this I >> would prefer not to do this in board2.c, but simply move it to the >> boards nvidia_board_init. Would you be ok with this? > > It's fine to move the call to as3722_init() into board files rather than > common files. However, note that this needs to be done atomically in a > single commit; each commit in the series needs to compile and operate > correctly on all boards. Sure I'll replace the revert patch with a dedicated move to board patch. > As such, simply reverting "tegra: Add support > for setting up a as3722 PMIC" won't work, since that'll temporarily > prevent at least venice2's tegra_lcd_pmic_init() from working, since it > appears to relie on the call to as3722_init(). Actually I don't see as3722_init being called on venice2 at all. CONFIG_AS3722_POWER is not set in include/configs/venice2.h or anywhere else, which would affect venice2. Am I missing something? -Julian
On 09/12/2016 11:10 AM, Julian Scheel wrote: > On 12.09.16 19:02, Stephen Warren wrote: >> On 09/12/2016 01:33 AM, Julian Scheel wrote: >>> On 06.09.2016 20:49, Stephen Warren wrote: >>>> On 09/06/2016 12:04 PM, Julian Scheel wrote: >>>>> On 06.09.16 18:50, Stephen Warren wrote: >>>>>> On 09/05/2016 07:29 AM, Julian Scheel wrote: >>>>>>> This reverts commit 346451b5888c4663bccef4c5203357319aa41f99. >>>>>>> The pmic shall be initialised in board specific code, as it is done >>>>>>> for >>>>>>> jetson-tk1 and meerkat. It can't be assumed that all boards have the >>>>>>> pmic on >>>>>>> the same i2c bus. >>>>>> >>>>>> You can't just revert this, since boards such as nyan-big and venice2 >>>>>> rely on this function being called. Admittedly jetson-tk1 does call >>>>>> this >>>>>> a second time right now, but none of the other boards appear to. >>> >>> I checked this again and in fact the only other board besides jetson-tk1 >>> having CONFIG_AS3722_POWER set is nyan big. All the other tegra boards >>> do only call what is in >>> board/nvidia/venice2/as3722_init.c:pmic_enable_cpu_vdd(void). >> >> cei-tk1-som has been recently added too; it's very similar to Jetson TK1. >> >>>>> Sorry, I missed to adapt the other boards. I don't see another way >>>>> than >>>>> having this in each of the boards, as parameters might change, do you? >>>> >>>> You can also parametrize the common code; either make it use #defines >>>> for the bus/device/... number and set those defines in a board-specific >>>> location, or call functions to retrieve that data which boards need to >>>> implement in a board-specific location. >>> >>> Taking into account that currently 2 out of 26 tegra boards use this I >>> would prefer not to do this in board2.c, but simply move it to the >>> boards nvidia_board_init. Would you be ok with this? >> >> It's fine to move the call to as3722_init() into board files rather than >> common files. However, note that this needs to be done atomically in a >> single commit; each commit in the series needs to compile and operate >> correctly on all boards. > > Sure I'll replace the revert patch with a dedicated move to board patch. > >> As such, simply reverting "tegra: Add support >> for setting up a as3722 PMIC" won't work, since that'll temporarily >> prevent at least venice2's tegra_lcd_pmic_init() from working, since it >> appears to relie on the call to as3722_init(). > > Actually I don't see as3722_init being called on venice2 at all. > CONFIG_AS3722_POWER is not set in include/configs/venice2.h or anywhere > else, which would affect venice2. Am I missing something? Sorry, I meant nyan-big not venice2.
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index f08af72..af5095e 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -7,7 +7,6 @@ #include <common.h> #include <dm.h> -#include <errno.h> #include <ns16550.h> #include <linux/compiler.h> #include <linux/sizes.h> @@ -37,7 +36,6 @@ #include <asm/arch-tegra/mmc.h> #endif #include <asm/arch-tegra/xusb-padctl.h> -#include <power/as3722.h> #include <i2c.h> #include <spi.h> #include "emc.h" @@ -147,11 +145,6 @@ int board_init(void) debug("Memory controller init failed: %d\n", err); # endif # endif /* CONFIG_TEGRA_PMU */ -#ifdef CONFIG_AS3722_POWER - err = as3722_init(NULL); - if (err && err != -ENODEV) - return err; -#endif #endif /* CONFIG_SYS_I2C_TEGRA */ #ifdef CONFIG_USB_EHCI_TEGRA