Re: [U-Boot] [EXT] Re: [PATCH 1/3] arm64: mvebu: Trigger PCI devices scan at early init stage
Hi, Simon, On 04/01/2017 07:23 AM, Simon Glass wrote: External Email -- Hi Konstanitin, On 30 March 2017 at 07:58, Konstantin Porotchkin <kos...@marvell.com> wrote: On 03/30/2017 04:31 PM, Stefan Roese wrote: (adding Simon to Cc for PCI related question) On 28.03.2017 17:36, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Add PCIe initialization at early init stage. This operation has a side effect of detecting all PCIe plug-in cards, so the operator is not obligated to issue "pci enum" command though CLI for this purpose. I'm not sure, if this should be handled this way. Simon, how is such a default PCI scan with DM supposed to get done? Is there a way do do this automatically without the need that the user has to issue "pci enum" manually? I was not sure either, but did not see any other way of doing so. I asked to add this change by our Robot/Jenkins automation test team. It seems reasonable. We actually have some platforms that require PCI buses to be probed before we know what devices are in the system, and some of these are important. For example, if your network controller is on PCI then U-Boot will not know about it (unless you have it in the device tree) until PCI is probed. I am wondering whether we should add a uclass flag that indicates that uclass members should be automatically probed on start-up? It would not be set for SATA, but would be for PCI. Thank you for your explanation. So, as the bottom line - Can I leave the PCIe init call in place and remove the SATA devices walk through from this patch? In my case the PCIe devices should be initialized for detection of a network card. Thanks Kosta Also convert the SATA first device scan to a walk through all availabel SATA devices. This should be done in a separate patch. But seeing this, won't this SATA / AHCI code be gone completely from this file, once this is converted into a "real" DM AHCI / SCSI driver (please look at my preliminary patch for this). Will check your patch, thank you. Maybe this change has to be completely removed if Simon guide me to the right solution for automatic PCIe enumeration. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/5] arm64: mvebu: a8k: Add support for NAND clock get
Hi, Stefan, On 03/30/2017 04:15 PM, Stefan Roese wrote: On 28.03.2017 17:16, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Implement mvebu_get_nand_clock call for A8K family. This function is used by PXA3XX NAND driver. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Igal Liberman <ig...@marvell.com> Cc: Nadav Haklai <nad...@marvell.com> --- arch/arm/mach-mvebu/armada8k/cpu.c | 16 1 file changed, 16 insertions(+) diff --git a/arch/arm/mach-mvebu/armada8k/cpu.c b/arch/arm/mach-mvebu/armada8k/cpu.c index 2325e9a..e18ca6b 100644 --- a/arch/arm/mach-mvebu/armada8k/cpu.c +++ b/arch/arm/mach-mvebu/armada8k/cpu.c @@ -110,3 +110,19 @@ void reset_cpu(ulong ignored) reg &= ~(1 << RFU_SW_RESET_OFFSET); writel(reg, RFU_GLOBAL_SW_RST); } + +#ifdef CONFIG_NAND_PXA3XX Do we really need to have this code conditionally compiled here? I can remove it, just wanted not to increase the code size if not needed. Do you think it is excessive? +/* Return NAND clock in Hz */ +u32 mvebu_get_nand_clock(void) +{ +unsigned long NAND_FLASH_CLK_CTRL = 0xF2440700UL; I know that some of this code is historically done this way. But with DT available now, isn't it possible to at least get the base address of such registers from the DT instead of hardcoding it? I see what you saying and I agree it is not as elegant as it could be. The only problem is that the NAND clock register is not part of the SoC NAND unit, so the IO base taken from DTS entry for NAND device is not really useful here. NAND unit and its clock configuration register are only sharing the CP0 base - 0xF200. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] arm64: mvebu: Trigger PCI devices scan at early init stage
On 03/30/2017 04:31 PM, Stefan Roese wrote: (adding Simon to Cc for PCI related question) On 28.03.2017 17:36, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Add PCIe initialization at early init stage. This operation has a side effect of detecting all PCIe plug-in cards, so the operator is not obligated to issue "pci enum" command though CLI for this purpose. I'm not sure, if this should be handled this way. Simon, how is such a default PCI scan with DM supposed to get done? Is there a way do do this automatically without the need that the user has to issue "pci enum" manually? I was not sure either, but did not see any other way of doing so. I asked to add this change by our Robot/Jenkins automation test team. Also convert the SATA first device scan to a walk through all availabel SATA devices. This should be done in a separate patch. But seeing this, won't this SATA / AHCI code be gone completely from this file, once this is converted into a "real" DM AHCI / SCSI driver (please look at my preliminary patch for this). Will check your patch, thank you. Maybe this change has to be completely removed if Simon guide me to the right solution for automatic PCIe enumeration. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Igal Liberman <ig...@marvell.com> Cc: Nadav Haklai <nad...@marvell.com> --- arch/arm/mach-mvebu/arm64-common.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c index 8f02655..ff28750 100644 --- a/arch/arm/mach-mvebu/arm64-common.c +++ b/arch/arm/mach-mvebu/arm64-common.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -142,8 +143,16 @@ int arch_early_init_r(void) break; } -/* Cause the SATA device to do its early init */ -uclass_first_device(UCLASS_AHCI, ); +/* Cause the SATA devices to do their early init */ +for (uclass_first_device(UCLASS_AHCI, ); + dev; + uclass_next_device()) +; + +#ifdef CONFIG_DM_PCI +/* Trigger PCIe devices detection */ +pci_init(); +#endif return 0; } Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 07/10] mvebu: a37xx: Add init for ESPRESSBin Topaz switch
On 02/15/2017 11:07 AM, Konstantin Porotchkin wrote: Hi, Joe, On 02/14/2017 07:17 PM, Joe Hershberger wrote: On Tue, Feb 14, 2017 at 6:32 AM, Stefan Roese <s...@denx.de> wrote: > (added Joe to Cc as network custodian) > > > On 14.02.2017 13:13, Konstantin Porotchkin wrote: >> >> Hi, Stefan, >> >> On 2/14/2017 13:49, Stefan Roese wrote: >>> >>> Hi Kosta, >>> >>> On 13.02.2017 14:38, kos...@marvell.com wrote: >>>> >>>> From: Konstantin Porotchkin <kos...@marvell.com> >>>> >>>> Implement the board-specific network init function for >>>> ESPRESSOBin community board, setting the on-board Topaz >>>> switch port to forward mode and allow network connection >>>> through any of the available Etherenet ports. >>>> >>>> Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> >>>> Cc: Stefan Roese <s...@denx.de> >>>> Cc: Igal Liberman <ig...@marvell.com> >>>> --- >>>> board/Marvell/mvebu_db-88f3720/board.c | 49 >>>> ++ >>>> 1 file changed, 49 insertions(+) >>>> >>>> diff --git a/board/Marvell/mvebu_db-88f3720/board.c >>>> b/board/Marvell/mvebu_db-88f3720/board.c >>>> index 3337f3f..45098ce 100644 >>>> --- a/board/Marvell/mvebu_db-88f3720/board.c >>>> +++ b/board/Marvell/mvebu_db-88f3720/board.c >>>> @@ -6,6 +6,7 @@ >>>> >>>> #include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> @@ -156,3 +157,51 @@ int board_xhci_enable(void) >>>> >>>> return 0; >>>> } >>>> + >>>> +static int mii_multi_chip_mode_write(struct mii_dev *bus, int >>>> dev_smi_addr, >>>> + int smi_addr, int reg, u16 value) >>>> +{ >>>> +u16 data = 0; >>>> + >>>> +if (bus->write(bus, dev_smi_addr, 0, 1, value) != 0) { >>>> +printf("Error writing to the PHY addr=%02x reg=%02x\n", >>>> + smi_addr, reg); >>>> +return -EFAULT; >>>> +} >>>> + >>>> +data = (1 << 15) | (1 << 12) | (1 << 10) | (smi_addr << 5) | reg; >>>> +if (bus->write(bus, dev_smi_addr, 0, 0, data) != 0) { >>>> +printf("Error writing to the PHY addr=%02x reg=%02x\n", >>>> + smi_addr, reg); >>>> +return -EFAULT; >>>> +} >>>> + >>>> +return 0; >>>> +} >>>> + >>>> + >>>> +int board_network_enable(struct mii_dev *bus) >>>> +{ >>>> +if (!of_machine_is_compatible("marvell,armada-3720-espressobin")) >>>> +return 0; >>>> + >>>> +/* >>>> + * FIXME: remove this code once Topaz driver gets available >>>> + * A3720 Community Board Only >>>> + * Configure Topaz switch (88E6341) >>>> + * Set port 0,1,2,3 to forwarding Mode >>>> + */ >>> >>> >>> Just checking: Is this "Topaz switch driver" something thats being >>> worked on or in the queue to do? >> >> >> I currently do not have it in my queue. >> I think the driver exists in the kernel (or will exist in 4.10/4.11 >> release), so we may want to port it if required. >> Which switch operations are needed at u-bot stage? > > > I'm not 100% sure if there is anything really "needed" other than > to get some ports into operation for the ethernet driver connected > to this switch. So it might be that such a few register writes > are acceptable - I'm pretty sure other boards do it this way as > well. > > On the other hand you could take a look at the > "drivers/net/phy/mv88e61xx.c" switch driver. Might be that this is > something similar to what you want / need. I think the switch driver to model after is drivers/net/vsc9953.c - there is a command: cmd/ethsw.c / include ethsw.h that implements the framework (doc/README.t1040-l2switch). I will check this code, thank you for the reference! There is also the drivers/net/cpsw.c that just hard-codes the config. Eth switches have varying levels of support. What level of support are you able to implement? I am not really sure about level of support required by the u-boot. The Linux driver conf
Re: [U-Boot] [PATCH 04/10] arm64: a37xx: Handle pin controls in early board init
On 02/14/2017 02:25 PM, Konstantin Porotchkin wrote: On 2/14/2017 14:21, Stefan Roese wrote: On 14.02.2017 13:07, Konstantin Porotchkin wrote: Hi, Stefan, On 2/14/2017 13:43, Stefan Roese wrote: Hi Kosta, On 13.02.2017 14:38, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Fix the default pin control values in a board-specific function on early board init stage. This fix allows the NETA driver to work in RGMII mode until the full-featured pin control driver gets introduced. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Igal Liberman <ig...@marvell.com> --- board/Marvell/mvebu_db-88f3720/board.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/board/Marvell/mvebu_db-88f3720/board.c b/board/Marvell/mvebu_db-88f3720/board.c index edf88c7..3337f3f 100644 --- a/board/Marvell/mvebu_db-88f3720/board.c +++ b/board/Marvell/mvebu_db-88f3720/board.c @@ -19,9 +19,33 @@ DECLARE_GLOBAL_DATA_PTR; #define I2C_IO_REG_0_SATA_OFF2 #define I2C_IO_REG_0_USB_H_OFF1 +#define PINCTRL_NB_REG_VALUE0x000173fa +#define PINCTRL_SB_REG_VALUE0x7a23 + I am aware that this is a temporary solution, but are these values correct for the A3720-DB or only the ESPRESSBin board? They are good for the DB board as well. Actually without this change the NETA driver will crash if we try to ping the server. Okay. And do you have any ideas on when this pinctrl driver might be available? I will query our team that is responsible for A37xx features. I think they are currently working on SATA/SCSI issues discovered when moved to the new code base. Hope the pin control will be the next task, but I have to ensure it. Just got an update - the pin control driver task is scheduled to March. BTW: You are now using the "Marvell/mvebu_db-88f3720" board directory for multiple board and not only the A3720-DB. I would prefer to see a rename of the board directory before this, like we've done to the A7k/8k directory. What do you think? Agree, I can do it. Should we change it in this patch series or introduce an additional patch later? We have no chance to get this patchset into this release, so we have a bit of time for the next one. I would prefer a clean switch and add this rename as one of the first patches in the next version of this patchset. Ok, got you, I will work on this change. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/10] mvebu: a37xx: Add init for ESPRESSBin Topaz switch
Hi, Joe, On 02/14/2017 07:17 PM, Joe Hershberger wrote: On Tue, Feb 14, 2017 at 6:32 AM, Stefan Roese <s...@denx.de> wrote: > (added Joe to Cc as network custodian) > > > On 14.02.2017 13:13, Konstantin Porotchkin wrote: >> >> Hi, Stefan, >> >> On 2/14/2017 13:49, Stefan Roese wrote: >>> >>> Hi Kosta, >>> >>> On 13.02.2017 14:38, kos...@marvell.com wrote: >>>> >>>> From: Konstantin Porotchkin <kos...@marvell.com> >>>> >>>> Implement the board-specific network init function for >>>> ESPRESSOBin community board, setting the on-board Topaz >>>> switch port to forward mode and allow network connection >>>> through any of the available Etherenet ports. >>>> >>>> Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> >>>> Cc: Stefan Roese <s...@denx.de> >>>> Cc: Igal Liberman <ig...@marvell.com> >>>> --- >>>> board/Marvell/mvebu_db-88f3720/board.c | 49 >>>> ++ >>>> 1 file changed, 49 insertions(+) >>>> >>>> diff --git a/board/Marvell/mvebu_db-88f3720/board.c >>>> b/board/Marvell/mvebu_db-88f3720/board.c >>>> index 3337f3f..45098ce 100644 >>>> --- a/board/Marvell/mvebu_db-88f3720/board.c >>>> +++ b/board/Marvell/mvebu_db-88f3720/board.c >>>> @@ -6,6 +6,7 @@ >>>> >>>> #include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> @@ -156,3 +157,51 @@ int board_xhci_enable(void) >>>> >>>> return 0; >>>> } >>>> + >>>> +static int mii_multi_chip_mode_write(struct mii_dev *bus, int >>>> dev_smi_addr, >>>> + int smi_addr, int reg, u16 value) >>>> +{ >>>> +u16 data = 0; >>>> + >>>> +if (bus->write(bus, dev_smi_addr, 0, 1, value) != 0) { >>>> +printf("Error writing to the PHY addr=%02x reg=%02x\n", >>>> + smi_addr, reg); >>>> +return -EFAULT; >>>> +} >>>> + >>>> +data = (1 << 15) | (1 << 12) | (1 << 10) | (smi_addr << 5) | reg; >>>> +if (bus->write(bus, dev_smi_addr, 0, 0, data) != 0) { >>>> +printf("Error writing to the PHY addr=%02x reg=%02x\n", >>>> + smi_addr, reg); >>>> +return -EFAULT; >>>> +} >>>> + >>>> +return 0; >>>> +} >>>> + >>>> + >>>> +int board_network_enable(struct mii_dev *bus) >>>> +{ >>>> +if (!of_machine_is_compatible("marvell,armada-3720-espressobin")) >>>> +return 0; >>>> + >>>> +/* >>>> + * FIXME: remove this code once Topaz driver gets available >>>> + * A3720 Community Board Only >>>> + * Configure Topaz switch (88E6341) >>>> + * Set port 0,1,2,3 to forwarding Mode >>>> + */ >>> >>> >>> Just checking: Is this "Topaz switch driver" something thats being >>> worked on or in the queue to do? >> >> >> I currently do not have it in my queue. >> I think the driver exists in the kernel (or will exist in 4.10/4.11 >> release), so we may want to port it if required. >> Which switch operations are needed at u-bot stage? > > > I'm not 100% sure if there is anything really "needed" other than > to get some ports into operation for the ethernet driver connected > to this switch. So it might be that such a few register writes > are acceptable - I'm pretty sure other boards do it this way as > well. > > On the other hand you could take a look at the > "drivers/net/phy/mv88e61xx.c" switch driver. Might be that this is > something similar to what you want / need. I think the switch driver to model after is drivers/net/vsc9953.c - there is a command: cmd/ethsw.c / include ethsw.h that implements the framework (doc/README.t1040-l2switch). I will check this code, thank you for the reference! There is also the drivers/net/cpsw.c that just hard-codes the config. Eth switches have varying levels of support. What level of support are you able to implement? I am not really sure about level of support required by the u-boot. The Linux driver configures the 3 output ports of this switch as lan0, lan1 and wan interfaces, so they are presented to the kernel as separate NICs. So if I set the NFS server on lan0 and the cable connected to lan1, the connection attempt will fail. The u-boot code however just sets the switch ports to follow all the traffic to the CPU. So when I tfttpload image using default neta0 interface, any switch port will work for that. Anyway, I will check what is supported by the reference code you just pointed and check what I can provide. I personally not an expert in this Topaz switch internals and may need to request help from other Marvell teams for doing something smarter than the code already provided in this patch. Thanks, -Joe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] arm64: a37xx: Handle pin controls in early board init
On 2/14/2017 14:21, Stefan Roese wrote: On 14.02.2017 13:07, Konstantin Porotchkin wrote: Hi, Stefan, On 2/14/2017 13:43, Stefan Roese wrote: Hi Kosta, On 13.02.2017 14:38, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Fix the default pin control values in a board-specific function on early board init stage. This fix allows the NETA driver to work in RGMII mode until the full-featured pin control driver gets introduced. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Igal Liberman <ig...@marvell.com> --- board/Marvell/mvebu_db-88f3720/board.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/board/Marvell/mvebu_db-88f3720/board.c b/board/Marvell/mvebu_db-88f3720/board.c index edf88c7..3337f3f 100644 --- a/board/Marvell/mvebu_db-88f3720/board.c +++ b/board/Marvell/mvebu_db-88f3720/board.c @@ -19,9 +19,33 @@ DECLARE_GLOBAL_DATA_PTR; #define I2C_IO_REG_0_SATA_OFF2 #define I2C_IO_REG_0_USB_H_OFF1 +#define PINCTRL_NB_REG_VALUE0x000173fa +#define PINCTRL_SB_REG_VALUE0x7a23 + I am aware that this is a temporary solution, but are these values correct for the A3720-DB or only the ESPRESSBin board? They are good for the DB board as well. Actually without this change the NETA driver will crash if we try to ping the server. Okay. And do you have any ideas on when this pinctrl driver might be available? I will query our team that is responsible for A37xx features. I think they are currently working on SATA/SCSI issues discovered when moved to the new code base. Hope the pin control will be the next task, but I have to ensure it. BTW: You are now using the "Marvell/mvebu_db-88f3720" board directory for multiple board and not only the A3720-DB. I would prefer to see a rename of the board directory before this, like we've done to the A7k/8k directory. What do you think? Agree, I can do it. Should we change it in this patch series or introduce an additional patch later? We have no chance to get this patchset into this release, so we have a bit of time for the next one. I would prefer a clean switch and add this rename as one of the first patches in the next version of this patchset. Ok, got you, I will work on this change. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/10] mvebu: a37xx: Add init for ESPRESSBin Topaz switch
Hi, Stefan, On 2/14/2017 13:49, Stefan Roese wrote: Hi Kosta, On 13.02.2017 14:38, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Implement the board-specific network init function for ESPRESSOBin community board, setting the on-board Topaz switch port to forward mode and allow network connection through any of the available Etherenet ports. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Igal Liberman <ig...@marvell.com> --- board/Marvell/mvebu_db-88f3720/board.c | 49 ++ 1 file changed, 49 insertions(+) diff --git a/board/Marvell/mvebu_db-88f3720/board.c b/board/Marvell/mvebu_db-88f3720/board.c index 3337f3f..45098ce 100644 --- a/board/Marvell/mvebu_db-88f3720/board.c +++ b/board/Marvell/mvebu_db-88f3720/board.c @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -156,3 +157,51 @@ int board_xhci_enable(void) return 0; } + +static int mii_multi_chip_mode_write(struct mii_dev *bus, int dev_smi_addr, + int smi_addr, int reg, u16 value) +{ +u16 data = 0; + +if (bus->write(bus, dev_smi_addr, 0, 1, value) != 0) { +printf("Error writing to the PHY addr=%02x reg=%02x\n", + smi_addr, reg); +return -EFAULT; +} + +data = (1 << 15) | (1 << 12) | (1 << 10) | (smi_addr << 5) | reg; +if (bus->write(bus, dev_smi_addr, 0, 0, data) != 0) { +printf("Error writing to the PHY addr=%02x reg=%02x\n", + smi_addr, reg); +return -EFAULT; +} + +return 0; +} + + +int board_network_enable(struct mii_dev *bus) +{ +if (!of_machine_is_compatible("marvell,armada-3720-espressobin")) +return 0; + +/* + * FIXME: remove this code once Topaz driver gets available + * A3720 Community Board Only + * Configure Topaz switch (88E6341) + * Set port 0,1,2,3 to forwarding Mode + */ Just checking: Is this "Topaz switch driver" something thats being worked on or in the queue to do? I currently do not have it in my queue. I think the driver exists in the kernel (or will exist in 4.10/4.11 release), so we may want to port it if required. Which switch operations are needed at u-bot stage? +mii_multi_chip_mode_write(bus, 1, 16, 4, 0x7f); +mii_multi_chip_mode_write(bus, 1, 17, 4, 0x7f); +mii_multi_chip_mode_write(bus, 1, 18, 4, 0x7f); +mii_multi_chip_mode_write(bus, 1, 19, 4, 0x7f); +/* RGMII Delay on Port 0*/ +mii_multi_chip_mode_write(bus, 1, 16, 1, 0xe002); +/* Power up PHY 1, 2, 3 */ +mii_multi_chip_mode_write(bus, 1, 28, 25, 0x1140); +mii_multi_chip_mode_write(bus, 1, 28, 24, 0x9620); +mii_multi_chip_mode_write(bus, 1, 28, 24, 0x9640); +mii_multi_chip_mode_write(bus, 1, 28, 24, 0x9660); + +return 0; +} Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] arm64: a37xx: Handle pin controls in early board init
Hi, Stefan, On 2/14/2017 13:43, Stefan Roese wrote: Hi Kosta, On 13.02.2017 14:38, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> Fix the default pin control values in a board-specific function on early board init stage. This fix allows the NETA driver to work in RGMII mode until the full-featured pin control driver gets introduced. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Igal Liberman <ig...@marvell.com> --- board/Marvell/mvebu_db-88f3720/board.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/board/Marvell/mvebu_db-88f3720/board.c b/board/Marvell/mvebu_db-88f3720/board.c index edf88c7..3337f3f 100644 --- a/board/Marvell/mvebu_db-88f3720/board.c +++ b/board/Marvell/mvebu_db-88f3720/board.c @@ -19,9 +19,33 @@ DECLARE_GLOBAL_DATA_PTR; #define I2C_IO_REG_0_SATA_OFF2 #define I2C_IO_REG_0_USB_H_OFF1 +#define PINCTRL_NB_REG_VALUE0x000173fa +#define PINCTRL_SB_REG_VALUE0x7a23 + I am aware that this is a temporary solution, but are these values correct for the A3720-DB or only the ESPRESSBin board? They are good for the DB board as well. Actually without this change the NETA driver will crash if we try to ping the server. BTW: You are now using the "Marvell/mvebu_db-88f3720" board directory for multiple board and not only the A3720-DB. I would prefer to see a rename of the board directory before this, like we've done to the A7k/8k directory. What do you think? Agree, I can do it. Should we change it in this patch series or introduce an additional patch later? int board_early_init_f(void) { -/* Nothing to do (yet), perhaps later some pin-muxing etc */ +const void *blob = gd->fdt_blob; +const char *bank_name; +const char *compat = "marvell,armada-3700-pinctl"; +int off, len; +void __iomem *addr; + +/* FIXME + * Temporary WA for setting correct pin control values + * until the real pin control driver is awailable. + */ +off = fdt_node_offset_by_compatible(blob, -1, compat); +while (off != -FDT_ERR_NOTFOUND) { +bank_name = fdt_getprop(blob, off, "bank-name", ); +addr = (void __iomem *)fdtdec_get_addr_size_auto_noparent( +blob, off, "reg", 0, NULL, true); +if (!strncmp(bank_name, "armada-3700-nb", len)) +writel(PINCTRL_NB_REG_VALUE, addr); +else if (!strncmp(bank_name, "armada-3700-sb", len)) +writel(PINCTRL_SB_REG_VALUE, addr); + +off = fdt_node_offset_by_compatible(blob, off, compat); +} return 0; } Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 07:24 PM, Marek Vasut wrote: On 02/09/2017 05:43 PM, Konstantin Porotchkin wrote: On 02/09/2017 06:15 PM, Marek Vasut wrote: On 02/09/2017 04:54 PM, Konstantin Porotchkin wrote: On 02/09/2017 05:36 PM, Marek Vasut wrote: On 02/09/2017 04:30 PM, Konstantin Porotchkin wrote: On 02/09/2017 03:37 PM, Marek Vasut wrote: On 02/09/2017 12:32 PM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v5: - Extended clocks description in documentation - Removed print for regulator not found case doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 29 +++ drivers/usb/host/Kconfig | 1 + drivers/usb/host/xhci-mvebu.c | 13 +- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..6cc370c --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,29 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a platform clocks that should be enabled/configured + upon interface initialization. May not exist on all platforms. This is probably block clock then ? Otherwise, Acked-by: Marek Vasut <ma...@denx.de> Otherwise the the internal SoC clock does not require gating/muxing or any other configuration for making this USB host adapter running. Not sure if I understood your question well. Well, do these clock drive the USB block or do they drive the register interface or what ? This entry is generic and applicable to all XHCI controllers, so it is hard to answer your question. It supposes to be a clock that drives the data transfer. It can be directly connected to the internal clock generator and divider or pass though an additional gate/mux. In the last case it can be inhibited or redirected. So it's a PHY clock then ? Or XHCI block clock ? marvell.xhci-usb.txt probably doesn't contain generic stuff, but marvell XHCI implementation specific stuff, right ? No, in this particular case this entry is generic. That is why I proposed to remove it in the past. For the purpose of mvebu XHCI driver this entry is not required. In general and in Marvell case particularly, every unit is driven by some kind of internal clock. And those internal clock can never ever be gated ? That's some odd design, I would not expect that on an advanced ARM chip ... I guess marvell is not power conscious ? :) The example contradicts what you just said though, it points into some clock module ... Yes, it can be gated. It is a gated clock coming from system controller. This XHCI driver supports two different SoC families, so the real clock names may vary. Please help me to understand what is missing in this description? Should I add "this clock is a gated unit clock driven by system controller" to th description? Should I drop this description file and submit it in a separate patch? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 06:15 PM, Marek Vasut wrote: On 02/09/2017 04:54 PM, Konstantin Porotchkin wrote: > > On 02/09/2017 05:36 PM, Marek Vasut wrote: >> On 02/09/2017 04:30 PM, Konstantin Porotchkin wrote: >>> >>> >>> On 02/09/2017 03:37 PM, Marek Vasut wrote: >>>> On 02/09/2017 12:32 PM, kos...@marvell.com wrote: >>>>> From: Konstantin Porotchkin <kos...@marvell.com> >>>>> >>>>> The USB device should linked to VBUS regulator through "vbus-supply" >>>>> DTS property. >>>>> This patch adds handling for "vbus-supply" property inside the USB >>>>> device entry for turning on the VBUS regulator upon the host adapter >>>>> probe. >>>>> >>>>> Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> >>>>> Cc: Stefan Roese <s...@denx.de> >>>>> Cc: Marek Vasut <ma...@denx.de> >>>>> Cc: Nadav Haklai <nad...@marvell.com> >>>>> Cc: Neta Zur Hershkovits <n...@marvell.com> >>>>> Cc: Igal Liberman <ig...@marvell.com> >>>>> Cc: Haim Boot <ha...@marvell.com> >>>>> --- >>>>> Changes for v5: >>>>> - Extended clocks description in documentation >>>>> - Removed print for regulator not found case >>>>> >>>>> doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 29 >>>>> +++ >>>>> drivers/usb/host/Kconfig | 1 + >>>>> drivers/usb/host/xhci-mvebu.c | 13 +- >>>>> 3 files changed, 42 insertions(+), 1 deletion(-) >>>>> create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>>> >>>>> diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>>> b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>>> new file mode 100644 >>>>> index 000..6cc370c >>>>> --- /dev/null >>>>> +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>>> @@ -0,0 +1,29 @@ >>>>> +Marvell SOC USB controllers >>>>> + >>>>> +This controller is integrated in Armada 3700/8K. >>>>> +It uses the same properties as a generic XHCI host controller >>>>> + >>>>> +Required properties : >>>>> + - compatible: should be one or more of: >>>>> + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs >>>>> + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs >>>>> + - reg: should contain address and length of the standard XHCI >>>>> + register set for the device. >>>>> + - interrupts: one XHCI interrupt should be described here. >>>>> + >>>>> +Optional properties: >>>>> + - clocks: reference to a platform clocks that should be >>>>> enabled/configured >>>>> + upon interface initialization. May not exist on all platforms. >>>> >>>> This is probably block clock then ? >>>> >>>> Otherwise, >>>> Acked-by: Marek Vasut <ma...@denx.de> >>> Otherwise the the internal SoC clock does not require gating/muxing or >>> any other configuration for making this USB host adapter running. >>> Not sure if I understood your question well. >> >> Well, do these clock drive the USB block or do they drive the register >> interface or what ? > This entry is generic and applicable to all XHCI controllers, so it is > hard to answer your question. It supposes to be a clock that drives the > data transfer. It can be directly connected to the internal clock > generator and divider or pass though an additional gate/mux. In the last > case it can be inhibited or redirected. So it's a PHY clock then ? Or XHCI block clock ? marvell.xhci-usb.txt probably doesn't contain generic stuff, but marvell XHCI implementation specific stuff, right ? No, in this particular case this entry is generic. That is why I proposed to remove it in the past. For the purpose of mvebu XHCI driver this entry is not required. In general and in Marvell case particularly, every unit is driven by some kind of internal clock. >>>>> + - vbus-supply : If present, specifies the fixed regulator to be >>>>> turned on >>>>> + for providing power to the USB VBUS rail. >>>>> + >>>>> +Example: >>
Re: [U-Boot] [PATCH v5 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 05:36 PM, Marek Vasut wrote: On 02/09/2017 04:30 PM, Konstantin Porotchkin wrote: > > > On 02/09/2017 03:37 PM, Marek Vasut wrote: >> On 02/09/2017 12:32 PM, kos...@marvell.com wrote: >>> From: Konstantin Porotchkin <kos...@marvell.com> >>> >>> The USB device should linked to VBUS regulator through "vbus-supply" >>> DTS property. >>> This patch adds handling for "vbus-supply" property inside the USB >>> device entry for turning on the VBUS regulator upon the host adapter >>> probe. >>> >>> Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> >>> Cc: Stefan Roese <s...@denx.de> >>> Cc: Marek Vasut <ma...@denx.de> >>> Cc: Nadav Haklai <nad...@marvell.com> >>> Cc: Neta Zur Hershkovits <n...@marvell.com> >>> Cc: Igal Liberman <ig...@marvell.com> >>> Cc: Haim Boot <ha...@marvell.com> >>> --- >>> Changes for v5: >>> - Extended clocks description in documentation >>> - Removed print for regulator not found case >>> >>> doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 29 >>> +++ >>> drivers/usb/host/Kconfig | 1 + >>> drivers/usb/host/xhci-mvebu.c | 13 +- >>> 3 files changed, 42 insertions(+), 1 deletion(-) >>> create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> >>> diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> new file mode 100644 >>> index 000..6cc370c >>> --- /dev/null >>> +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> @@ -0,0 +1,29 @@ >>> +Marvell SOC USB controllers >>> + >>> +This controller is integrated in Armada 3700/8K. >>> +It uses the same properties as a generic XHCI host controller >>> + >>> +Required properties : >>> + - compatible: should be one or more of: >>> + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs >>> + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs >>> + - reg: should contain address and length of the standard XHCI >>> + register set for the device. >>> + - interrupts: one XHCI interrupt should be described here. >>> + >>> +Optional properties: >>> + - clocks: reference to a platform clocks that should be >>> enabled/configured >>> + upon interface initialization. May not exist on all platforms. >> >> This is probably block clock then ? >> >> Otherwise, >> Acked-by: Marek Vasut <ma...@denx.de> > Otherwise the the internal SoC clock does not require gating/muxing or > any other configuration for making this USB host adapter running. > Not sure if I understood your question well. Well, do these clock drive the USB block or do they drive the register interface or what ? This entry is generic and applicable to all XHCI controllers, so it is hard to answer your question. It supposes to be a clock that drives the data transfer. It can be directly connected to the internal clock generator and divider or pass though an additional gate/mux. In the last case it can be inhibited or redirected. >>> + - vbus-supply : If present, specifies the fixed regulator to be >>> turned on >>> + for providing power to the USB VBUS rail. >>> + >>> +Example: >>> +cpm_usb3_0: usb3@50 { >>> +compatible = "marvell,armada-8k-xhci", >>> + "generic-xhci"; >>> +reg = <0x50 0x4000>; >>> +interrupts = ; >>> +clocks = <_syscon0 1 22>; >>> +vbus-supply = <_usb3h0_vbus>; >>> +status = "disabled"; >>> +}; >>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >>> index 5129a57..0bf8274 100644 >>> --- a/drivers/usb/host/Kconfig >>> +++ b/drivers/usb/host/Kconfig >>> @@ -25,6 +25,7 @@ config USB_XHCI_MVEBU >>> bool "MVEBU USB 3.0 support" >>> default y >>> depends on ARCH_MVEBU >>> +select DM_REGULATOR >>> help >>>Choose this option to add support for USB 3.0 driver on mvebu >>>SoCs, which includes Armada8K, Armada3700 and other Armada >>> diff --git a/drivers/usb/host/xhci-mvebu.c >>> b/drivers/usb/host/
Re: [U-Boot] [PATCH v5 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 03:37 PM, Marek Vasut wrote: On 02/09/2017 12:32 PM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v5: - Extended clocks description in documentation - Removed print for regulator not found case doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 29 +++ drivers/usb/host/Kconfig | 1 + drivers/usb/host/xhci-mvebu.c | 13 +- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..6cc370c --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,29 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a platform clocks that should be enabled/configured + upon interface initialization. May not exist on all platforms. This is probably block clock then ? Otherwise, Acked-by: Marek Vasut <ma...@denx.de> Otherwise the the internal SoC clock does not require gating/muxing or any other configuration for making this USB host adapter running. Not sure if I understood your question well. + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: + cpm_usb3_0: usb3@50 { + compatible = "marvell,armada-8k-xhci", +"generic-xhci"; + reg = <0x50 0x4000>; + interrupts = ; + clocks = <_syscon0 1 22>; + vbus-supply = <_usb3h0_vbus>; + status = "disabled"; + }; diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 5129a57..0bf8274 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -25,6 +25,7 @@ config USB_XHCI_MVEBU bool "MVEBU USB 3.0 support" default y depends on ARCH_MVEBU + select DM_REGULATOR help Choose this option to add support for USB 3.0 driver on mvebu SoCs, which includes Armada8K, Armada3700 and other Armada diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..d880af1 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "xhci.h" @@ -44,12 +45,22 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci_platdata *plat = dev_get_platdata(dev); struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; - int len; + int len, ret; + struct udevice *regulator; ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(>hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len); + ret = device_get_supply_regulator(dev, "vbus-supply", ); + if (!ret) { + ret = regulator_set_enable(regulator, true); + if (ret) { + printf("Failed to turn ON the VBUS regulator\n"); + return ret; + } + } + /* Enable USB xHCI (VBUS, reset etc) in board specific code */ board_xhci_enable(); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 12:39 PM, Marek Vasut wrote: On 02/09/2017 11:39 AM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v4: - Move the VBUS supply handling to use regulator framework - Select DM_REGULATOR by Kconfig for mvebu XHCI driver doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 29 +++ drivers/usb/host/Kconfig | 1 + drivers/usb/host/xhci-mvebu.c | 14 ++- 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..cd21115 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,29 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a clock I asked in the previous version , multiple times , what clock are these? Yep, we discussed these clocks already. Do you want me to add a notice here? Like "the platform clocks that should be enabled upon initialization. may not exist on all platforms" + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: + cpm_usb3_0: usb3@50 { + compatible = "marvell,armada-8k-xhci", +"generic-xhci"; + reg = <0x50 0x4000>; + interrupts = ; + clocks = <_syscon0 1 22>; + vbus-supply = <_usb3h0_vbus>; + status = "disabled"; + }; + diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 5129a57..0bf8274 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -25,6 +25,7 @@ config USB_XHCI_MVEBU bool "MVEBU USB 3.0 support" default y depends on ARCH_MVEBU + select DM_REGULATOR help Choose this option to add support for USB 3.0 driver on mvebu SoCs, which includes Armada8K, Armada3700 and other Armada diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..35d89ab 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "xhci.h" @@ -44,12 +45,23 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci_platdata *plat = dev_get_platdata(dev); struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; - int len; + int len, ret; + struct udevice *regulator; ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(>hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len); + ret = device_get_supply_regulator(dev, "vbus-supply", ); + if (!ret) { + ret = regulator_set_enable(regulator, true); + if (ret) { + printf("Failed to turn ON the VBUS regulator\n"); + return ret; + } + } else + printf("No VBUS regulator found\n"); This should be debug() , looks good otherwise. OK, will change it to "debug" /* Enable USB xHCI (VBUS, reset etc) in board specific code */ board_xhci_enable(); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 11:09 AM, Marek Vasut wrote: On 02/09/2017 10:08 AM, Konstantin Porotchkin wrote: On 02/09/2017 10:32 AM, Marek Vasut wrote: On 02/09/2017 09:00 AM, Konstantin Porotchkin wrote: On 02/08/2017 06:42 PM, Marek Vasut wrote: On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 06:04 PM, Marek Vasut wrote: On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 05:35 PM, Marek Vasut wrote: On 02/08/2017 04:34 PM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v3: - Moved VBUS control from private GPIO to a fixed regulator - Rebase on top of master branch doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 drivers/usb/host/xhci-mvebu.c | 31 +++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a clock What clock ? Why are clock optional ? This probably needs clock-names too. This is the way it defined in Linux Kernel. Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it. From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it. DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT. I see what you saying. I will move the "clocks" entry to the "mandatory" section. Why ? What clock are those ? Are they mandatory ? They are not mandatory. This entry can be used for enabling gated clocks on a specific platform. However Kernel code does not handle missing clock entry as an error. It just assumes that the clock is not gated and therefore should not be enabled upon host controller probe. So maybe I got you wrong. My feeling was that you requested to make this entry mandatory. I have no idea what close those are (based on the description), so I cannot decide either way. If this is something which is present only on selected platforms, then they are optional indeed. Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry. Then why are these clock here at all ? Because this is an optional parameter and can be used for enabling gated clock if one is defined. So these are different clock from the SERDES clock, yes ? As far as I know the SERDES entry defines the clock speed, which affects the initialization flow including the clock dividers. The clock in USb entry looks like for gating only. Should I simply remove this property from the text file? See above. I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: +cpm_usb3_0: usb3@50 { +compatible = "marvell,armada-8k-xhci", + "generic-xhci"; +reg = <0x50 0x4000>; +interrupts = ; +clocks = <_syscon0 1 22>; +
Re: [U-Boot] [PATCH v3 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/09/2017 10:32 AM, Marek Vasut wrote: On 02/09/2017 09:00 AM, Konstantin Porotchkin wrote: On 02/08/2017 06:42 PM, Marek Vasut wrote: On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 06:04 PM, Marek Vasut wrote: On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 05:35 PM, Marek Vasut wrote: On 02/08/2017 04:34 PM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v3: - Moved VBUS control from private GPIO to a fixed regulator - Rebase on top of master branch doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 drivers/usb/host/xhci-mvebu.c | 31 +++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a clock What clock ? Why are clock optional ? This probably needs clock-names too. This is the way it defined in Linux Kernel. Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it. From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it. DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT. I see what you saying. I will move the "clocks" entry to the "mandatory" section. Why ? What clock are those ? Are they mandatory ? They are not mandatory. This entry can be used for enabling gated clocks on a specific platform. However Kernel code does not handle missing clock entry as an error. It just assumes that the clock is not gated and therefore should not be enabled upon host controller probe. So maybe I got you wrong. My feeling was that you requested to make this entry mandatory. Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry. Then why are these clock here at all ? Because this is an optional parameter and can be used for enabling gated clock if one is defined. Should I simply remove this property from the text file? See above. I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: +cpm_usb3_0: usb3@50 { +compatible = "marvell,armada-8k-xhci", + "generic-xhci"; +reg = <0x50 0x4000>; +interrupts = ; +clocks = <_syscon0 1 22>; +vbus-supply = <_usb3h0_vbus>; +status = "disabled"; +}; diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED Just make the driver depend on REGULATOR_FIXED I think the drive
Re: [U-Boot] [PATCH v3 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
On 02/08/2017 06:42 PM, Marek Vasut wrote: On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 06:04 PM, Marek Vasut wrote: On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 05:35 PM, Marek Vasut wrote: On 02/08/2017 04:34 PM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v3: - Moved VBUS control from private GPIO to a fixed regulator - Rebase on top of master branch doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 drivers/usb/host/xhci-mvebu.c | 31 +++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a clock What clock ? Why are clock optional ? This probably needs clock-names too. This is the way it defined in Linux Kernel. Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it. From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it. DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT. I see what you saying. I will move the "clocks" entry to the "mandatory" section. Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry. Should I simply remove this property from the text file? See above. I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: +cpm_usb3_0: usb3@50 { +compatible = "marvell,armada-8k-xhci", + "generic-xhci"; +reg = <0x50 0x4000>; +interrupts = ; +clocks = <_syscon0 1 22>; +vbus-supply = <_usb3h0_vbus>; +status = "disabled"; +}; diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED Just make the driver depend on REGULATOR_FIXED I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-) Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board. Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, whi
Re: [U-Boot] [PATCH v3 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
Hi, Marek, On 02/08/2017 06:04 PM, Marek Vasut wrote: On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote: Hi, Marek, On 02/08/2017 05:35 PM, Marek Vasut wrote: On 02/08/2017 04:34 PM, kos...@marvell.com wrote: From: Konstantin Porotchkin <kos...@marvell.com> The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe. Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> Cc: Stefan Roese <s...@denx.de> Cc: Marek Vasut <ma...@denx.de> Cc: Nadav Haklai <nad...@marvell.com> Cc: Neta Zur Hershkovits <n...@marvell.com> Cc: Igal Liberman <ig...@marvell.com> Cc: Haim Boot <ha...@marvell.com> --- Changes for v3: - Moved VBUS control from private GPIO to a fixed regulator - Rebase on top of master branch doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 drivers/usb/host/xhci-mvebu.c | 31 +++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a clock What clock ? Why are clock optional ? This probably needs clock-names too. This is the way it defined in Linux Kernel. Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it. From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it. Should I simply remove this property from the text file? I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: +cpm_usb3_0: usb3@50 { +compatible = "marvell,armada-8k-xhci", + "generic-xhci"; +reg = <0x50 0x4000>; +interrupts = ; +clocks = <_syscon0 1 22>; +vbus-supply = <_usb3h0_vbus>; +status = "disabled"; +}; diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED Just make the driver depend on REGULATOR_FIXED I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-) Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board. +const void *fdt = gd->fdt_blob; +int node = dev->of_offset; +const fdt32_t *regulator; +int size; +/* + * The VBUS supply regulator is not probed automatically + * Trigger the regulator probe upon USB port bring up + */ +regulator = fdt_getprop(fdt, node, "vbus-supply", ); I think this should use regulator_*() calls from include/power/regulator.h I can call regulator_set_enable() at the end, but I have to locate it first and get the udev for it. However it will be enabled already at the time I will call this function. +if (regulator) { +uint32_t phandle; +struct udevice *config; +int reg_node, ret; + +phand
Re: [U-Boot] [PATCH v3 4/6] mvebu: usb: xhci: Add VBUS regulator supply to the host driver
Hi, Marek, On 02/08/2017 05:35 PM, Marek Vasut wrote: On 02/08/2017 04:34 PM, kos...@marvell.com wrote: > From: Konstantin Porotchkin <kos...@marvell.com> > > The USB device should linked to VBUS regulator through "vbus-supply" > DTS property. > This patch adds handling for "vbus-supply" property inside the USB > device entry for turning on the VBUS regulator upon the host adapter probe. > > Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de > Signed-off-by: Konstantin Porotchkin <kos...@marvell.com> > Cc: Stefan Roese <s...@denx.de> > Cc: Marek Vasut <ma...@denx.de> > Cc: Nadav Haklai <nad...@marvell.com> > Cc: Neta Zur Hershkovits <n...@marvell.com> > Cc: Igal Liberman <ig...@marvell.com> > Cc: Haim Boot <ha...@marvell.com> > --- > Changes for v3: > - Moved VBUS control from private GPIO to a fixed regulator > - Rebase on top of master branch > > > doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 > drivers/usb/host/xhci-mvebu.c | 31 +++ > 2 files changed, 59 insertions(+) > create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt > > diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt > new file mode 100644 > index 000..672a829 > --- /dev/null > +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt > @@ -0,0 +1,28 @@ > +Marvell SOC USB controllers > + > +This controller is integrated in Armada 3700/8K. > +It uses the same properties as a generic XHCI host controller > + > +Required properties : > + - compatible: should be one or more of: > + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs > + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs > + - reg: should contain address and length of the standard XHCI > + register set for the device. > + - interrupts: one XHCI interrupt should be described here. > + > +Optional properties: > + - clocks: reference to a clock What clock ? Why are clock optional ? This probably needs clock-names too. This is the way it defined in Linux Kernel. I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on > + - vbus-supply : If present, specifies the fixed regulator to be turned on > + for providing power to the USB VBUS rail. > + > +Example: > +cpm_usb3_0: usb3@50 { > +compatible = "marvell,armada-8k-xhci", > + "generic-xhci"; > +reg = <0x50 0x4000>; > +interrupts = ; > +clocks = <_syscon0 1 22>; > +vbus-supply = <_usb3h0_vbus>; > +status = "disabled"; > +}; > diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c > index 46eb937..149f6a4 100644 > --- a/drivers/usb/host/xhci-mvebu.c > +++ b/drivers/usb/host/xhci-mvebu.c > @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) > struct mvebu_xhci *ctx = dev_get_priv(dev); > struct xhci_hcor *hcor; > int len; > +#ifdef CONFIG_DM_REGULATOR_FIXED Just make the driver depend on REGULATOR_FIXED I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled > +const void *fdt = gd->fdt_blob; > +int node = dev->of_offset; > +const fdt32_t *regulator; > +int size; > > +/* > + * The VBUS supply regulator is not probed automatically > + * Trigger the regulator probe upon USB port bring up > + */ > +regulator = fdt_getprop(fdt, node, "vbus-supply", ); > +if (regulator) { > +uint32_t phandle; > +struct udevice *config; > +int reg_node, ret; > + > +phandle = fdt32_to_cpu(*regulator); > +reg_node = fdt_node_offset_by_phandle(fdt, phandle); > +if (reg_node < 0) { > +dev_err(dev, "vbus-supply has invalid phandle\n"); > +return -EINVAL; > +} > +ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR, > + reg_node, ); > +if (ret) { > +dev_err(dev, "failed to get VBUS regulator device\n"); > +return ret; Where is the regulator enabled ? The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered. > +} > +} > +#else > +debug("VBUS regulator support is missing\n"); > +#endif > ctx->hcd = (struct xhci_hccr *)plat->hcd_base; > len = HC_LENGTH(xhci_readl(>hcd->cr_capbase)); > hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len); > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] New UCLASS_PINCTRL driver - probe is not called for all nodes
Hi, Simon, Thank you for your reply. In order to activate pin control function using "pinctrl-0" property, the device driver itself has to be aware of the pin control existence, right? So if I put such property under SPI controller, the SPI controller driver has to handle call to the pin control driver methods, right? However my current target is to trigger setup for all existent pin controllers regardless of the connected device entries. Unfortunately not all drivers are aware of the pin controller properties. For instance current SPI and I2C drivers does not trigger the pin controller "probe" method regardless of the "pinctrl-0" property presence in FDT. Regards Konstantin On Fri, Nov 18, 2016 at 3:14 AM, Simon Glass <s...@chromium.org> wrote: > Hi Konstantin, > > On 15 November 2016 at 06:56, Konstantin Porotchkin <kos...@gmail.com> > wrote: > > Hi, All, > > > > I am currently porting the Marvell (mvebu) pin control driver for > Armada-8K > > family to the current u-boot sources. > > The Armada 8K SoC is a hybrid chip that contains several interconnected > > dies in a single package. > > Each such device (AP, CP0, CP1) has an independent pin controller with > > different memory mapping. > > The DTS for such configuration looks like the following: > > / { > > ap806 { > > config-space { > > pinctl: pinctl@6F4000 { > > ... > > }; > > }; > > }; > > cp110-master { > > config-space { > > cpm_pinctl: pinctl@44000 { > > ... > > }; > > }; > > }; > > cp110-slave { > > config-space { > > cps_pinctl: pinctl@44000 { > > ... > > }; > > }; > > }; > > }; > > > > I expect that my driver "probe" method will be called 3 times - one for > > every controller. > > However, according to my test, only the first controller is probed > > (pinctl@6F4000). > > Two others are listed in the DM tree, but are not active (not probed). > > > > I can do a trick and sequentially call uclass_get_device() function for > > the UCLASS_PINCTRL type, causing all 3 controller to be probed and > > activated. > > However I think this is not the way it should work. > > Is my assumption wrong and such hybrid devices should use the above trick > > for bringing up all controllers in the package? > > They should be activated automatically by devices that use them. This > is the pinctrl-0 property in the device. Can you take a look at why > that is not working? > > Specifically, see pinctrl_select_state() in device_probe(). > > Regards, > Simon > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] New UCLASS_PINCTRL driver - probe is not called for all nodes
Hi, All, I am currently porting the Marvell (mvebu) pin control driver for Armada-8K family to the current u-boot sources. The Armada 8K SoC is a hybrid chip that contains several interconnected dies in a single package. Each such device (AP, CP0, CP1) has an independent pin controller with different memory mapping. The DTS for such configuration looks like the following: / { ap806 { config-space { pinctl: pinctl@6F4000 { ... }; }; }; cp110-master { config-space { cpm_pinctl: pinctl@44000 { ... }; }; }; cp110-slave { config-space { cps_pinctl: pinctl@44000 { ... }; }; }; }; I expect that my driver "probe" method will be called 3 times - one for every controller. However, according to my test, only the first controller is probed (pinctl@6F4000). Two others are listed in the DM tree, but are not active (not probed). I can do a trick and sequentially call uclass_get_device() function for the UCLASS_PINCTRL type, causing all 3 controller to be probed and activated. However I think this is not the way it should work. Is my assumption wrong and such hybrid devices should use the above trick for bringing up all controllers in the package? Thank you beforehand for your help Konstantin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot