Re: [PATCH 00/17] video: dw_hdmi: Support Vendor PHY
On 2023-12-15 7:13 am, Kever Yang wrote: Hi Jagan, On 2023/12/15 14:36, Jagan Teki wrote: Hi Heiko/Kerver/Anatoloj, On Mon, Dec 11, 2023 at 2:30 PM Jagan Teki wrote: Unlike RK3399, Sunxi/Meson DW HDMI the new Rockchip SoC Rk3328 would support external vendor PHY with DW HDMI chip. Support this vendor PHY by adding new platform PHY ops via DW HDMI driver and call the respective generic phy from platform driver code. This series tested in RK3328 with 1080p (1920x1080) resolution. Patch 0001/0005: Support Vendor PHY Patch 0006/0008: VOP extension for win, dsp offsets Patch 0009/0010: RK3328 VOP, HDMI clocks Patch 0011: Rockchip Inno HDMI PHY Patch 0012: RK3328 HDMI driver Patch 0013: RK3328 VOP driver Patch 0014/0017: Enable HDMI Out for RK3328 Importent: One pontential issues is that Linux HDMI out on RK3328 has effected by this patchset as I wouldn't find any relation or clue. [ 0.752016] Loading compiled-in X.509 certificates [ 0.787796] inno_hdmi_phy_rk3328_clk_recalc_rate: parent 2400 [ 0.788391] inno-hdmi-phy ff43.phy: inno_hdmi_phy_rk3328_clk_recalc_rate rate 14850 vco 14850 [ 0.798353] rockchip-drm display-subsystem: bound ff37.vop (ops vop_component_ops) [ 0.799403] dwhdmi-rockchip ff3c.hdmi: supply avdd-0v9 not found, using dummy regulator [ 0.800288] rk_iommu ff373f00.iommu: Enable stall request timed out, status: 0x4b [ 0.801131] dwhdmi-rockchip ff3c.hdmi: supply avdd-1v8 not found, using dummy regulator [ 0.802056] rk_iommu ff373f00.iommu: Disable paging request timed out, status: 0x4b [ 0.803233] dwhdmi-rockchip ff3c.hdmi: Detected HDMI TX controller v2.11a with HDCP (inno_dw_hdmi_phy2) [ 0.805355] dwhdmi-rockchip ff3c.hdmi: registered DesignWare HDMI I2C bus driver [ 0.808769] rockchip-drm display-subsystem: bound ff3c.hdmi (ops dw_hdmi_rockchip_ops) [ 0.810869] [drm] Initialized rockchip 1.0.0 20140818 for display-subsystem on minor 0 The only way I can use Linux HDMI by disabling IOMMU or support disable-iommu link for RK3328 via DT [1]. [1] https://www.spinics.net/lists/devicetree/msg605124.html Is anyone aware of this issue? I did post the patches for Linux IOMMU but seems not a proper solution. Any suggestions? I'm not expert in HDMI/VOP, so I can't provide a suitable solution in the kernel, but here is the reason why we need patch to workaround the issue in the kernel: - The VOP driver working in U-Boot is non-IOMMU mode, and the VOP access DDR by physical address; - The VOP driver working in kernel with IOMMU enabled(by default), the VOP access DDR with virtual address(by IOMMU); - The VOP is keep working in kernel before kernel VOP driver is enabled, and the IOMMU driver will be enabled by the Linux PM framework, since the IOMMU is not correctly configured at this point, the VOP will access unknown space(the original physical address in U-Boot) convert by IOMMU; So we need to disable the IOMMU temporary in kernel startup before VOP driver is enabled. If U-Boot isn't handing off an active framebuffer, then it should be U-Boot's responsibility to stop the VOP before it exits; if on the other hand it is, then it can now use the "iommu-addresses" DT property (see the reserved-memory schema) on the framebuffer region, and we should just need a bit of work in the IOMMU driver to ensure that is respected during the period between the IOMMU initialising and the Linux VOP driver subsequently taking over (i.e. so it won't get stuck on an unexpected page fault as seems to be happening above). The IOMMU aspect of that ought to be fairly straightforward; the trickier part might be the runtime PM aspect to ensure the IOMMU doesn't let itself go idle and actually turn anything off during that period. I also still think that doing the full rk_iommu_disable() upon runtime suspend is wrong, but that's more of a thing which confounds the underlying issue here, rather than being the problem in itself. Thanks, Robin.
Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC
On 2023-09-06 19:10, Laurentiu Tudor wrote: On 9/6/2023 8:21 PM, Robin Murphy wrote: On 2023-09-06 17:01, Laurentiu Tudor wrote: MC being a plain DMA master as any other device in the SoC and being live at OS boot time, as soon as the SMMU is probed it will immediately start triggering faults because there is no mapping in the SMMU for the MC. Pre-create such a mapping in the SMMU, being the OS's responsibility to preserve it. Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do is explicitly turn it *off*, therefore programming other registers appears to be a complete waste of time. No, it doesn't enable SMMU but it does mark a SMR as valid for MC FW. And the ARM SMMU driver subtly preserves it, see [1] (it's late and I might be wrong, but I'll double check tomorrow). :-) No, that sets the SMR valid bit *if* the corresponding entry is allocated and marked as valid in the software state in smmu->smrs, which at probe time it isn't, because that's only just been allocated and is still zero-initialised. Unless, that is, arm_smmu_rmr_install_bypass_smr() found a reserved region and preallocated an entry to honour it. But even those entries are still constructed from scratch; we can't do anything with the existing SMR/S2CR register contents in general since they may be uninitialised random reset values, so we don't even look. Pay no attention to the qcom_smmu_cfg_probe() hack either - that only exists on the promise that the relevant platforms couldn't have their firmware updated to use proper RMRs. You're already doing the right thing in patch #2, so there's no need to waste code on doing a pointless wrong thing as well. Thanks, Robin. All that should matter to the OS, and that it is responsible for upholding, is the reserved memory regions from patch #2. For instance, if the OS is Linux, literally the first thing arm_smmu_device_reset() does is rewrite all the S2CRs and SMRs without so much as looking. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm/arm-smmu/arm-smmu.c#n894 --- Best Regards, Laurentiu Signed-off-by: Laurentiu Tudor --- arch/arm/cpu/armv8/fsl-layerscape/soc.c | 26 --- .../asm/arch-fsl-layerscape/immap_lsch3.h | 9 +++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 3bfdc3f77431..870b99838ab5 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -376,6 +376,18 @@ void bypass_smmu(void) val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & ~(SCR0_USFCFG_MASK); out_le32(SMMU_NSCR0, val); } + +void setup_smmu_mc_bypass(int icid, int mask) +{ + u32 val; + + val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) | + (mask << SMMU_SMR_MASK_SHIFT); + out_le32(SMMU_REG_SMR(0), val); + val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK; + out_le32(SMMU_REG_S2CR(0), val); +} + void fsl_lsch3_early_init_f(void) { erratum_rcw_src(); @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void) bypass_smmu(); #endif -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \ - defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \ - defined(CONFIG_ARCH_LX2162A) +#ifdef CONFIG_ARCH_LS1028A + set_icids(); +#endif + +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A) + set_icids(); + setup_smmu_mc_bypass(0x300, 0); +#endif + +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A) set_icids(); + setup_smmu_mc_bypass(0x4000, 0); #endif } diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h index ca5e33379ba9..bec5355adaed 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h @@ -190,6 +190,15 @@ #define SCR0_CLIENTPD_MASK 0x0001 #define SCR0_USFCFG_MASK 0x0400 +#define SMMU_REG_SMR(n) (SMMU_BASE + 0x800 + ((n) << 2)) +#define SMMU_REG_S2CR(n) (SMMU_BASE + 0xc00 + ((n) << 2)) +#define SMMU_SMR_VALID_MASK 0x8000 +#define SMMU_SMR_MASK_MASK 0x +#define SMMU_SMR_MASK_SHIFT 16 +#define SMMU_SMR_ID_MASK 0x +#define SMMU_SMR_ID_SHIFT 0 +#define SMMU_S2CR_EXIDVALID_VALID_MASK 0x0400 +#define SMMU_S2CR_TYPE_BYPASS_MASK 0x0001 /* PCIe */ #define CFG_SYS_PCIE1_ADDR (CONFIG_SYS_IMMR + 0x240)
Re: [PATCH 3/5] armv8: fsl-layerscape: create bypass smmu mapping for MC
On 2023-09-06 17:01, Laurentiu Tudor wrote: MC being a plain DMA master as any other device in the SoC and being live at OS boot time, as soon as the SMMU is probed it will immediately start triggering faults because there is no mapping in the SMMU for the MC. Pre-create such a mapping in the SMMU, being the OS's responsibility to preserve it. Does U-Boot enable the SMMU? AFAICS the only thing it knows how to do is explicitly turn it *off*, therefore programming other registers appears to be a complete waste of time. All that should matter to the OS, and that it is responsible for upholding, is the reserved memory regions from patch #2. For instance, if the OS is Linux, literally the first thing arm_smmu_device_reset() does is rewrite all the S2CRs and SMRs without so much as looking. Thanks, Robin. Signed-off-by: Laurentiu Tudor --- arch/arm/cpu/armv8/fsl-layerscape/soc.c | 26 --- .../asm/arch-fsl-layerscape/immap_lsch3.h | 9 +++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c b/arch/arm/cpu/armv8/fsl-layerscape/soc.c index 3bfdc3f77431..870b99838ab5 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c @@ -376,6 +376,18 @@ void bypass_smmu(void) val = (in_le32(SMMU_NSCR0) | SCR0_CLIENTPD_MASK) & ~(SCR0_USFCFG_MASK); out_le32(SMMU_NSCR0, val); } + +void setup_smmu_mc_bypass(int icid, int mask) +{ + u32 val; + + val = SMMU_SMR_VALID_MASK | (icid << SMMU_SMR_ID_SHIFT) | + (mask << SMMU_SMR_MASK_SHIFT); + out_le32(SMMU_REG_SMR(0), val); + val = SMMU_S2CR_EXIDVALID_VALID_MASK | SMMU_S2CR_TYPE_BYPASS_MASK; + out_le32(SMMU_REG_S2CR(0), val); +} + void fsl_lsch3_early_init_f(void) { erratum_rcw_src(); @@ -402,10 +414,18 @@ void fsl_lsch3_early_init_f(void) bypass_smmu(); #endif -#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS1028A) || \ - defined(CONFIG_ARCH_LS2080A) || defined(CONFIG_ARCH_LX2160A) || \ - defined(CONFIG_ARCH_LX2162A) +#ifdef CONFIG_ARCH_LS1028A + set_icids(); +#endif + +#if defined(CONFIG_ARCH_LS1088A) || defined(CONFIG_ARCH_LS2080A) + set_icids(); + setup_smmu_mc_bypass(0x300, 0); +#endif + +#if defined(CONFIG_ARCH_LX2160A) || defined(CONFIG_ARCH_LX2162A) set_icids(); + setup_smmu_mc_bypass(0x4000, 0); #endif } diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h index ca5e33379ba9..bec5355adaed 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h @@ -190,6 +190,15 @@ #define SCR0_CLIENTPD_MASK0x0001 #define SCR0_USFCFG_MASK 0x0400 +#define SMMU_REG_SMR(n) (SMMU_BASE + 0x800 + ((n) << 2)) +#define SMMU_REG_S2CR(n) (SMMU_BASE + 0xc00 + ((n) << 2)) +#define SMMU_SMR_VALID_MASK0x8000 +#define SMMU_SMR_MASK_MASK 0x +#define SMMU_SMR_MASK_SHIFT16 +#define SMMU_SMR_ID_MASK 0x +#define SMMU_SMR_ID_SHIFT 0 +#define SMMU_S2CR_EXIDVALID_VALID_MASK 0x0400 +#define SMMU_S2CR_TYPE_BYPASS_MASK 0x0001 /* PCIe */ #define CFG_SYS_PCIE1_ADDR(CONFIG_SYS_IMMR + 0x240)
Re: [PATCH 2/2] doc: add Arm Juno board documentation
p-fw.bin --soc-fw soc-fw.bin \ + --hw-config hw-config.bin ... --nt-fw /path/to/your/u-boot.bin fip.bin +$ cp fip.bin /mnt/juno/SOFTWARE/fip.bin + +Unmount the USB mass storage device and reboot the board, the new ``fip.bin`` +will be automatically written to the NOR flash and then used. + +Rebuilding Trusted Firmware +^^^ +You can also generate a new FIP image by compiling Arm Trusted Firmware, +and providing ``u-boot.bin`` as the BL33 file. For that you can either build +the required `SCP firmware`_ yourself, or just extract the existing +version from your ``fip.bin`` (as above): + +.. code-block:: bash + +$ mkdir /tmp/juno; cd /tmp/juno +$ fiptool unpack /mnt/juno/SOFTWARE/fip.bin + +Then build TF-A: + +.. code-block:: bash + +$ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git +$ cd trusted-firmware-a +$ make CROSS_COMPILE=aarch64-linux-gnu- PLAT=juno DEBUG=1 \ + SCP_BL2=/tmp/juno/scp-fw.bin BL33=/path/to/your/u-boot.bin fiptool all fip +$ cp build/juno/debug/bl1.bin build/juno/debug/fip.bin /mnt/juno/SOFTWARE + +Then umount the USB device, and reboot, as above. + +Device trees + +The device tree files for the boards are maintained in the Linux kernel +repository. They end up in the root of the SD card, as ``juno.dtb``, +``juno-r1.dtb``, and ``juno-r2.dtb``, respectively. The MCP firmware will copy +the one matching the board revision into the NOR flash, into the ``board.dtb`` While this is technically not untrue for the typical case, I wonder if it might be a slightly misleading oversimplification. AFAIK the MCC doesn't have any awareness beyond looking for configuration files in one of the \SITE1\HBI0262[BCD] directories based on the board ID. It is the images.txt files in there wherein in the stock firmware configuration plays the trick of associating different DTB filenames with the same NOR address. This might matter if anyone's tinkered with that configuration already, or possibly still has a really ancient firmware setup (I have a vague memory of something wacky with both r0 and r1 DTBs present and EDK2 deciding which one to pass to Linux). Also I believe the stock setup assumes DTBs in \SOFTWARE, rather than the root directory. At least that's how my r2 here seems to be (not that I've ever actually used it without loading a custom one from GRUB...) Thanks, Robin. +partition. U-Boot picks its control DTB from there, you can pass this on to +a kernel using ``$fdtcontroladdr``. +You can update the DTBs anytime, by building them using the ``dtbs`` make +target from a Linux kernel tree, then just copying the generated binaries +to the SD card. + +.. _`Juno development board`: https://developer.arm.com/tools-and-software/development-boards/juno-development-board +.. _`SCP firmware`: https://github.com/ARM-software/SCP-firmware.git
Re: [PATCH] rockchip: rk3288: Add OF board setup
On 2020-07-03 11:10, Jagan Teki wrote: On Thu, Jul 2, 2020 at 7:26 PM Robin Murphy wrote: On 2020-07-02 09:48, Jagan Teki wrote: The new rk3288 revision rk3288w has some changes with respect to legacy rk3288 like hclk_vio and usb host0 ohci. In order to work these on the same in Linux kernel update the compatible the root compatible with rockchip,rk3288w before booting. So, this support during of board setup code of rk3288. Signed-off-by: Jagan Teki --- arch/arm/mach-rockchip/Kconfig | 1 + arch/arm/mach-rockchip/rk3288/rk3288.c | 26 ++ 2 files changed, 27 insertions(+) diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index b1008a5058..822d8d4e9c 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -98,6 +98,7 @@ config ROCKCHIP_RK322X config ROCKCHIP_RK3288 bool "Support Rockchip RK3288" select CPU_V7A + select OF_BOARD_SETUP select SUPPORT_SPL select SPL select SUPPORT_TPL diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c b/arch/arm/mach-rockchip/rk3288/rk3288.c index 804abe8a1b..8a682675e6 100644 --- a/arch/arm/mach-rockchip/rk3288/rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c @@ -115,6 +115,32 @@ int rk_board_late_init(void) return rk3288_board_late_init(); } +#ifdef CONFIG_OF_BOARD_SETUP + +#define RK3288_HDMI_PHYS 0xff98 +#define RK3288W_HDMI_REV 0x1A +#define HDMI_CONFIG0_ID 0x04 + +int ft_board_setup(void *blob, bd_t *bd) +{ + u8 config0; + int ret; + + config0 = readb(RK3288_HDMI_PHYS + HDMI_CONFIG0_ID); + if (config0 == RK3288W_HDMI_REV) { + ret = fdt_setprop_string(blob, 0, + "compatible", "rockchip,rk3288w"); Does this end up replacing the entire top-level compatible property? i.e. from: compatible = "vendor,board\0rockchip,rk3288"; to just: compatible = "rockchip,rk3288w"; If so, that's a bit of a problem for various drivers that care about the actual board compatible rather than the SoC. Yes, It looks replacing the entire compatible. I think the root compatible is mostly untouchable because of this reason. But, if we skip the root compatible and trying to replace individual nodes for W revision then it requires extra registration code on in the Linux drivers like Linux clock driver is registering the clock with rockchip,rk3288-cru but updating this compatible with rockchip,rk3288w-cru will require another registration code. Having common rockchip,rk3288w can be possible to check any code in the tree. Right, it's definitely reasonable to update the top-level SoC compatible, we just need to be prepared to deal with a whole string list here. It looks like libfdt doesn't offer any nice helpers for inserting/removing/updating in string lists, but given that the SoC should be the last entry here we might be able to cheat a bit - just get the whole raw property, check that the final characters are exactly "rockchip,rk3288", and if so append a "w" to the end and write it back. Robin.
Re: [PATCH] rockchip: rk3288: Add OF board setup
On 2020-07-02 09:48, Jagan Teki wrote: The new rk3288 revision rk3288w has some changes with respect to legacy rk3288 like hclk_vio and usb host0 ohci. In order to work these on the same in Linux kernel update the compatible the root compatible with rockchip,rk3288w before booting. So, this support during of board setup code of rk3288. Signed-off-by: Jagan Teki --- arch/arm/mach-rockchip/Kconfig | 1 + arch/arm/mach-rockchip/rk3288/rk3288.c | 26 ++ 2 files changed, 27 insertions(+) diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index b1008a5058..822d8d4e9c 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -98,6 +98,7 @@ config ROCKCHIP_RK322X config ROCKCHIP_RK3288 bool "Support Rockchip RK3288" select CPU_V7A + select OF_BOARD_SETUP select SUPPORT_SPL select SPL select SUPPORT_TPL diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c b/arch/arm/mach-rockchip/rk3288/rk3288.c index 804abe8a1b..8a682675e6 100644 --- a/arch/arm/mach-rockchip/rk3288/rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c @@ -115,6 +115,32 @@ int rk_board_late_init(void) return rk3288_board_late_init(); } +#ifdef CONFIG_OF_BOARD_SETUP + +#define RK3288_HDMI_PHYS 0xff98 +#define RK3288W_HDMI_REV 0x1A +#define HDMI_CONFIG0_ID0x04 + +int ft_board_setup(void *blob, bd_t *bd) +{ + u8 config0; + int ret; + + config0 = readb(RK3288_HDMI_PHYS + HDMI_CONFIG0_ID); + if (config0 == RK3288W_HDMI_REV) { + ret = fdt_setprop_string(blob, 0, +"compatible", "rockchip,rk3288w"); Does this end up replacing the entire top-level compatible property? i.e. from: compatible = "vendor,board\0rockchip,rk3288"; to just: compatible = "rockchip,rk3288w"; If so, that's a bit of a problem for various drivers that care about the actual board compatible rather than the SoC. Robin. + if (ret < 0) { + printf("failed to set rk3288w compatible (ret=%d)\n", + ret); + return ret; + } + } + + return 0; +} +#endif + static int do_clock(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {
Re: [PATCH v2] mmc: sdhci: Fix HISPD bit handling
On 2020-06-09 15:01, Jagan Teki wrote: SDHCI HISPD bits need to be configured based on desired mmc timings mode and some HISPD quirks. So, handle the HISPD bit based on the mmc computed selected mode(timing parameter) rather than fixed mmc card clock frequency. Linux handle the HISPD similar like this in below commit, commit <501639bf2173> ("mmc: sdhci: fix SDHCI_QUIRK_NO_HISPD_BIT handling") This eventually fixed the mmc write issue observed in rk3399 sdhci controller. Bug log for refernece, => gpt write mmc 0 $partitions Writing GPT: mmc write failed ** Can't write to device 0 ** ** Can't write to device 0 ** error! Cc: Kever Yang Cc: Peng Fan Reviewed-by: Jaehoon Chung Signed-off-by: Jagan Teki --- Changes for v2: - collect Jaehoon R-b drivers/mmc/sdhci.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 92cc8434af..280b8c88eb 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -594,14 +594,21 @@ static int sdhci_set_ios(struct mmc *mmc) ctrl &= ~SDHCI_CTRL_4BITBUS; } - if (mmc->clock > 2600) - ctrl |= SDHCI_CTRL_HISPD; - else - ctrl &= ~SDHCI_CTRL_HISPD; - - if ((host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) || - (host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) - ctrl &= ~SDHCI_CTRL_HISPD; + if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT) || Should that be "&&" rather than "||"? Otherwise this will always evaluate to true unless *both* quirks are set, which isn't equivalent to the check being removed above. Robin. + !(host->quirks & SDHCI_QUIRK_BROKEN_HISPD_MODE)) { + if (mmc->selected_mode == MMC_HS || + mmc->selected_mode == SD_HS || + mmc->selected_mode == MMC_DDR_52 || + mmc->selected_mode == MMC_HS_200 || + mmc->selected_mode == MMC_HS_400 || + mmc->selected_mode == UHS_SDR25 || + mmc->selected_mode == UHS_SDR50 || + mmc->selected_mode == UHS_SDR104 || + mmc->selected_mode == UHS_DDR50) + ctrl |= SDHCI_CTRL_HISPD; + else + ctrl &= ~SDHCI_CTRL_HISPD; + } sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
Re: [PATCH 5/8] pci: Add Rockchip PCIe controller driver
On 2020-04-25 8:36 pm, Jagan Teki wrote: On Sun, Apr 26, 2020 at 12:23 AM Mark Kettenis wrote: From: Jagan Teki Date: Sat, 25 Apr 2020 16:33:51 +0530 Add Rockchip PCIe controller driver for rk3399 platform. Driver support Gen1 by operating as a Root complex. Thanks to Patrick for initial work. Tried to get this to work on my firefly-rk3399 which made me notice some shortcomings: 1. The vpcie1v8 and vpcie0v9 supplies are optional, just like the vpcie3v3 supply. FWIW those are "non-optional" in Linux in the sense that supplies to the PCIE_AVDD_0V9 and PCIE_AVDD_1V8 pins of the SoC must physically exist, even if they aren't described. If U-Boot doesn't have the same "create a dummy regulator if none is specified" behaviour then you might need some slightly different logic there. The 3.3V and 12V supplies on the other hand may legitimately not be part of the board at all, depending on whether it implements a full-size slot, a mini-PCI/M.2 socket, a hard-wired endpoint chip, or just the data and clock signal pairs exposed on some non-standard connector. Robin.
Re: [PATCH 0/2] Reading size-cells and address-cells from a node should walk up the
Hi Matthias. Thanks for this. I can confirm that this fixes the reported problem. You have my Tested-By. FWIW, the changes look fine to me too. Cheers, Robin
Re: Bug: qemu_arm64: Cannot access the second flash bank
On Thu, 2020-01-09 at 15:57 +0100, Matthias Brugger wrote: [...] > The property expects size-cells to be two, but U-Boot will use one > cell if no > size-cells are defined in the device node (which is not the case) and > therefor > will see > > Bank1: Flashbase 0x0 0x0 Flashsize 0x400 > Bank2: Flashbase 0x400 0x0 Flashsize 0x400 My knowledge of DT is superficial. However, looking at the following lines from the spec: - A |spec|-compliant boot program shall supply #address-cells and #size-cells on all nodes that have children. - If missing, a client program should assume a default value of 2 for #address-cells, and a value of 1 for #size-cells. .. and contrasting with the root node and device node in question from the DTS for this platform: / { interrupt-parent = <0x8001>; #size-cells = <0x2>; #address-cells = <0x2>; compatible = "linux,dummy-virt"; . . flash@0 { bank-width = <0x4>; reg = <0x0 0x0 0x0 0x400 0x0 0x400 0x0 0x400>; compatible = "cfi-flash"; }; .. it seems to me that while the flash node is missing #size-cells, given that #size-cells _is_ defined in the parent node ("the node that has children") then that value (0x2) is the one u-boot should have used but didn't. Maybe the u-boot DT interpreting logic needs to check if the parent node also does not specify #size-cells before making the assumption that the value 1 is to be used ? Thanks, Robin
Re: Bug: qemu_arm64: Cannot access the second flash bank
Hi Matthias. On Thu, 2020-01-09 at 12:12 +0100, Matthias Brugger wrote: [...] > Can you pinpoint me to where I can find the DTS used by U-boot. As per my understanding the DTB for this virtual platform is generated by qemu and handed to u-boot. I dumped the DTB to the host filesystem using GDB and 'dump mem' then dtc'd it to get the DTS. It's at: https://pastebin.ubuntu.com/p/2KzPKdxddx/ > I tried to run that myself but wasn't able to see any output. Which > U-Boot > config do you use? rpi_3_defconfig? I hit this problem for the qemu_arm64 u-boot platform target for which I used qemu_arm64_defconfig [1]. Let me know if you need more info. Cheers, Robin [1]: https://github.com/u-boot/u-boot/blob/master/configs/qemu_arm64_defconfig
Bug: qemu_arm64: Cannot access the second flash bank
Hi folks. [CC'ing some hopefully relevant folks]. As of: commit 0ba41ce1b7816c229cc19e0621148b98f990cb68 libfdt: return correct value if #size-cells property is not present .. accesses to the second flash bank on the qemu_arm64 virtual board appear broken. To demonstrate, consider that the physical memory map for the 2 flash banks is: Bank 1: 0x_ - 0x03FC_ Bank 2: 0x0400_ - 0x7FC0_ Now, consider the abbreviated output of the flinfo command pre and post the above commit: Pre: === => flinfo Bank # 1: CFI conformant flash (32 x 16) Size: 64 MB in 256 Sectors Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018 Erase timeout: 16384 ms, write timeout: 3 ms Buffer write timeout: 3 ms, buffer size: 2048 bytes Sector Start Addresses: RO 0004 RO 0008 RO 000C0010 00140018001C00200024 . . 03E803EC03F003F403F8 03FC Bank # 2: CFI conformant flash (32 x 16) Size: 64 MB in 256 Sectors Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018 Erase timeout: 16384 ms, write timeout: 3 ms Buffer write timeout: 3 ms, buffer size: 2048 bytes Sector Start Addresses: 0400 RO 04040408040C0410 04140418041C04200424 . . 07E807EC07F007F407F8 07FC Post: => flinfo Bank # 1: CFI conformant flash (32 x 16) Size: 64 MB in 256 Sectors Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018 Erase timeout: 16384 ms, write timeout: 3 ms Buffer write timeout: 3 ms, buffer size: 2048 bytes Sector Start Addresses: RO 0004 RO 0008 RO 000C0010 00140018001C00200024 . . 03E803EC03F003F403F8 03FC Bank # 2: CFI conformant flash (32 x 16) Size: 64 MB in 256 Sectors Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018 Erase timeout: 16384 ms, write timeout: 3 ms Buffer write timeout: 3 ms, buffer size: 2048 bytes Sector Start Addresses: 400404408 40C410 41441841C 420424 . . 40003E840003EC40003F040 003F440003F8 40003FC As a result, the second bank is unusable for environment stores (CONFIG_ENV_ADDR is 0x400): => saveenv Saving Environment to Flash... Error: start and/or end address not on sector boundary Error: start and/or end address not on sector boundary Failed (1) Rewinding the u-boot repo to before this commit fixes the problem. Manually (uncleanly) reverting the commit and it's dependent commits fixes the problem. Here are the HEAD commits from the relevant repos that I used for the data above: qemu: commit dd5b0f95490883cd8bc7d070db8de70d5c979cbc u-boot: commit 6cb87cbb1475f668689f95911d1521ee6ba7f55c Here is the qemu invocation I used: $ dd if=/dev/zero of=./flash0-with-uboot.img bs=1M count=64 && dd if=/path/to/u-boot.bin of=./flash0-with-uboot.img conv=notrunc $ qemu-system-aarch64 -M virt -cpu cortex-a53 -m 1024M -nographic -drive if=pflash,format=raw,index=0,file=flash0-with-uboot.img -drive if=pflash,format=raw,index=1,file=flash1.img I'm happy to help test any fixes if and as needed. Cheers, Robin
Re: [U-Boot] [PATCH v3 5/5] doc: boards: Add rockchip documentation
On 17/10/2019 20:07, Jagan Teki wrote: Rockchip has documentation file, doc/README.rockchip but which is not so readable to add or understand the existing contents. Even the format that support is legacy readme in U-Boot. Add rockchip specific documentation file using new rst format, which describes the information about Rockchip supported boards and it's usage steps. Added minimal information about rk3288, rk3328, rk3368 and rk3399 boards and usage. This would indeed updated further based on the requirements and updates. Cc: Kever Yang Cc: Matwey V. Kornilov Signed-off-by: Jagan Teki --- doc/board/rockchip/index.rst| 10 +++ doc/board/rockchip/rockchip.rst | 125 2 files changed, 135 insertions(+) create mode 100644 doc/board/rockchip/index.rst create mode 100644 doc/board/rockchip/rockchip.rst diff --git a/doc/board/rockchip/index.rst b/doc/board/rockchip/index.rst new file mode 100644 index 00..0c377e9bbb --- /dev/null +++ b/doc/board/rockchip/index.rst @@ -0,0 +1,10 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. Copyright (C) 2019 Jagan Teki + +Rockchip + + +.. toctree:: + :maxdepth: 2 + + rockchip diff --git a/doc/board/rockchip/rockchip.rst b/doc/board/rockchip/rockchip.rst new file mode 100644 index 00..782a0f1c7a --- /dev/null +++ b/doc/board/rockchip/rockchip.rst @@ -0,0 +1,125 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. Copyright (C) 2019 Jagan Teki + +ROCKCHIP + + +About this +-- + +This document describes the information about Rockchip supported boards +and it's usage steps. + +Rockchip boards +--- + +Rockchip is SoC solutions provider for tablets & PCs, streaming media +TV boxes, AI audio & vision, IoT hardware. + +A wide range of Rockchip SoCs with associated boardsare supported in +mainline U-Boot. + +List of mainline supported rockchip boards: + +* rk3288 + - Evb-RK3288 + - Firefly-RK3288 + - mqmaker MiQi + - Phytec RK3288 PCM-947 + - PopMetal-RK3288 + - Radxa Rock 2 Square + - Tinker-RK3288 + - Google Jerry + - Google Mickey + - Google Minnie + - Google Speedy + - Amarula Vyasa-RK3288 +* rk3328 + - Rockchip RK3328 EVB + - Pine64 Rock64 +* rk3368 + - GeekBox + - PX5 EVB + - Rockchip sheep board + - Theobroma Systems RK3368-uQ7 SoM +* rk3399 + - 96boards RK3399 Ficus + - 96boards Rock960 + - Firefly-RK3399 Board + - Firefly ROC-RK3399-PC Board + - FriendlyElec NanoPC-T4 + - FriendlyElec NanoPi M4 + - FriendlyARM NanoPi NEO4 + - Google Bob + - Khadas Edge + - Khadas Edge-Captain + - Khadas Edge-V + - Orange Pi RK3399 Board + - Pine64 RockPro64 + - Radxa ROCK Pi 4 + - Rockchip RK3399 Evaluation Board + - Theobroma Systems RK3399-Q7 SoM + +Building + + +TF-A + + +TF-A would require to build for ARM64 Rockchip SoCs platforms. + +To build TF-A:: + +git clone https://github.com/ARM-software/arm-trusted-firmware.git Nit: it's probably worth updating that to https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git Robin. +cd arm-trusted-firmware +make realclean +make CROSS_COMPILE=aarch64-linux-gnu- PLAT=rk3399 + +Specify the PLAT= with desired rockchip platform to build TF-A for. + +U-Boot +^^ + +To build rk3328 boards:: + +export BL31=/path/to/arm-trusted-firmware/to/bl31.elf +make evb-rk3328_defconfig +make + +To build rk3288 boards:: + +make evb-rk3288_defconfig +make + +To build rk3368 boards:: + +export BL31=/path/to/arm-trusted-firmware/to/bl31.elf +make evb-px5_defconfig +make + +To build rk3399 boards:: + +export BL31=/path/to/arm-trusted-firmware/to/bl31.elf +make evb-rk3399_defconfig +make + +SD Card Flashing + + +To write an image that boots from an SD card (assumed to be /dev/sda): + +TPL + SPL:: + +sudo dd if=u-boot-rockchip-with-tpl-spl.bin of=/dev/sda seek=64 +sync + +TODO + + +- Add SPL-alone SD Card flashing steps +- Add rockchip idbloader image building +- Describe steps for eMMC flashing +- Add missing SoC's with it boards list + +.. Jagan Teki +.. Thu Oct 17 22:36:14 IST 2019 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RESEND PATCH v7 05/11] rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1
On 08/05/2019 06:41, Jagan Teki wrote: sdmmc cd pin is configured as RK_FUNC_GPIO which is wrong and indeed failed to detect the sdcard on the board with below error Card did not respond to voltage select! So, fix it by replacing RK_FUNC_GPIO with RK_FUNC_1 which is already defined in rk3399.dts so make use of same like other boards. AFAICS this should also be true of RockPro64 and (at least with the Linux DT) Firefly - those aren't grabbing &sdmmc_cd by default either. I imagine that U-Boot might also see similar problems on Gru, where the card detect signal is on a completely different GPIO. I'd note that in Linux, only rk3399-evb is actually *using* &sdmmc_cd - despite the fact that they claim it, nearly all the other boards also have "cd-gpios" and thus end up overriding the dedicated function with an implicit GPIO configuration anyway. Sapphire is the odd one out in using "broken-cd" as the less-efficient way of mitigating the runtime PM issue. Add these changes in -u-boot.dtsi to make Linux sync easy for future changes. Signed-off-by: Jagan Teki Reviewed-by: Kever Yang --- arch/arm/dts/rk3399-nanopi4-u-boot.dtsi | 9 + 1 file changed, 9 insertions(+) create mode 100644 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi diff --git a/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi new file mode 100644 index 00..20db99c0b8 --- /dev/null +++ b/arch/arm/dts/rk3399-nanopi4-u-boot.dtsi @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 Jagan Teki + */ + +&sdmmc { + pinctrl-names = "default"; That's already set in the base DTSI, so doesn't really need to be shadowed here. + pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>; +}; I suppose you could also delete the "cd-gpios" property to make it really clear what this override is for (and save a few bytes if it's going to be ignored anyway). Robin. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 08/14] rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1
On 16/04/2019 11:56, Jagan Teki wrote: sdmmc cd pin is configured as RK_FUNC_GPIO which is wrong and indeed failed to detect the sdcard on the board with below error Card did not respond to voltage select! So, fix it by replacing RK_FUNC_GPIO with RK_FUNC_1 which is already defined in rk3399.dts so make use of same like other boards. I guess the U-Boot dwmmc driver doesn't support using a GPIO? The reason we do this for Linux is that the dedicated function is not compatible with runtime power management - once we see that no card is present and suspend the idle controller, the CD logic is also powered off and thus no longer capable of generating the interrupt necessary to wake everything up again. The GPIO function of the same pin, however, is in an always-on power domain so is able to do the right thing. So it's not "wrong" as such, but this change should be fine for U-Boot as long as it never turns off PD_SD itself. Robin. Signed-off-by: Jagan Teki --- arch/arm/dts/rk3399-nanopi4.dtsi | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/arm/dts/rk3399-nanopi4.dtsi b/arch/arm/dts/rk3399-nanopi4.dtsi index d325e11728..5dc8a8de16 100644 --- a/arch/arm/dts/rk3399-nanopi4.dtsi +++ b/arch/arm/dts/rk3399-nanopi4.dtsi @@ -521,10 +521,6 @@ }; sdmmc { - sdmmc0_det_l: sdmmc0-det-l { - rockchip,pins = <0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>; - }; - sdmmc0_pwr_h: sdmmc0-pwr-h { rockchip,pins = <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>; }; @@ -582,7 +578,7 @@ cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>; disable-wp; pinctrl-names = "default"; - pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc0_det_l>; + pinctrl-0 = <&sdmmc_bus4 &sdmmc_clk &sdmmc_cmd &sdmmc_cd>; sd-uhs-sdr104; vmmc-supply = <&vcc3v0_sd>; vqmmc-supply = <&vcc_sdio>; ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Unable to saveenv to MMC
Thank you for the clarification. I've correct my issue by partitioning the MMC storage and formatting the partition FAT. Then burning the U-Boot Image to 8K in on the raw media. On Tue, Dec 18, 2018 at 12:48 AM Heinrich Schuchardt wrote: > On 12/6/18 7:12 PM, Robin Polak wrote: > > Hello. > > > > I'm having trouble persisting my environment variables to the SD Card > > onto which I have FAT formatted and then written U-Boot to using the > > following command: > > > > sudo dd if=u-boot-sunxi-with-spl.bin of=/dev/disk2 bs=1024 seek=8 > > > > I get the following error when booting a Linksprite_pcDuino3_Nano with > the > > SD Card I've built: > > > > U-Boot SPL 2018.11 (Dec 06 2018 - 17:57:48 +) > > DRAM: 1024 MiB > > CPU: 91200Hz, AXI/AHB/APB: 3/2/2 > > Trying to boot from MMC1 > > > > > > U-Boot 2018.11 (Dec 06 2018 - 17:57:48 +) Allwinner Technology > > > > CPU: Allwinner A20 (SUN7I) > > Model: LinkSprite pcDuino3 Nano > > I2C: ready > > DRAM: 1 GiB > > MMC: SUNXI SD/MMC: 0 > > Loading Environment from FAT... Unable to use mmc 0:0... In:serial > > Out: serial > > Err: serial > > SCSI: SATA link 0 timeout. > > AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode > > flags: ncq stag pm led clo only pmp pio slum part ccc apst > > > > Net: eth0: ethernet@1c5 > > starting USB... > > USB0: USB EHCI 1.00 > > USB1: USB OHCI 1.0 > > USB2: USB EHCI 1.00 > > USB3: USB OHCI 1.0 > > scanning bus 0 for devices... 1 USB Device(s) found > > scanning bus 2 for devices... 1 USB Device(s) found > >scanning usb for storage devices... 0 Storage Device(s) found > > Hit any key to stop autoboot: 0 > > => saveenv > > Saving Environment to FAT... Unable to use mmc 0:0... Failed (1) > > Partitions are numbered from 1. So partition 1 would be mmc 0:1. > > mmc 0:0 would require that there is no partition table and the FAT file > system starts in block 0. > > Please, check the value of CONFIG_ENV_FAT_DEVICE_AND_PART in your > configuration. The default is CONFIG_ENV_FAT_DEVICE_AND_PART="0:auto" > > With "auto" function blk_get_device_part_str() looks for the first > partition that has the bootable flag set. > > So to analyze your problem further please look at the output of > > sudo fdisk -l /dev/ > > assuming that you use a DOS partition table. > > Best regards > > Heinrich > > > > > Thank you for any light that may be shed upon this error. > > > > -- -- Robin Polak ro...@robinpolak.com 917-494-2080 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Unable to saveenv to MMC
The output I'm getting is: => ls mmc 0:0 ** Unrecognized filesystem type ** On Sun, Dec 9, 2018 at 1:43 AM wrote: > just try the ls-command i wrote in uboot-console ;) > > > Gesendet: Sonntag, 09. Dezember 2018 um 04:20 Uhr > Von: "Robin Polak" > An: fran...@public-files.de > Cc: u-boot@lists.denx.de > Betreff: Re: [U-Boot] Unable to saveenv to MMC > > How would I go about validating whether I can access the partition from > u-boot? > > On Fri, Dec 7, 2018 at 1:47 AM Frank Wunderlich [mailto:fran...@public-files.de]> wrote:can you try to access the > partiton from uboot? > > ls mmc 0:0 > > regards Frank > > > Von: "Robin Polak" mailto:ad...@robinpolak.com]> > > I'm having trouble persisting my environment variables to the SD Card > > onto which I have FAT formatted and then written U-Boot to using the > > following command: > > ... > > => saveenv > > Saving Environment to FAT... Unable to use mmc 0:0... Failed (1) > > -- > > --Robin polakro...@robinpolak.com[mailto:ro...@robinpolak.com] > 917-494-2080 > -- -- Robin Polak ro...@robinpolak.com 917-494-2080 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Unable to saveenv to MMC
How would I go about validating whether I can access the partition from u-boot? On Fri, Dec 7, 2018 at 1:47 AM Frank Wunderlich wrote: > can you try to access the partiton from uboot? > > ls mmc 0:0 > > regards Frank > > > Von: "Robin Polak" > > I'm having trouble persisting my environment variables to the SD Card > > onto which I have FAT formatted and then written U-Boot to using the > > following command: > > ... > > => saveenv > > Saving Environment to FAT... Unable to use mmc 0:0... Failed (1) > -- -- Robin Polak ro...@robinpolak.com 917-494-2080 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] Unable to saveenv to MMC
Hello. I'm having trouble persisting my environment variables to the SD Card onto which I have FAT formatted and then written U-Boot to using the following command: sudo dd if=u-boot-sunxi-with-spl.bin of=/dev/disk2 bs=1024 seek=8 I get the following error when booting a Linksprite_pcDuino3_Nano with the SD Card I've built: U-Boot SPL 2018.11 (Dec 06 2018 - 17:57:48 +) DRAM: 1024 MiB CPU: 91200Hz, AXI/AHB/APB: 3/2/2 Trying to boot from MMC1 U-Boot 2018.11 (Dec 06 2018 - 17:57:48 +) Allwinner Technology CPU: Allwinner A20 (SUN7I) Model: LinkSprite pcDuino3 Nano I2C: ready DRAM: 1 GiB MMC: SUNXI SD/MMC: 0 Loading Environment from FAT... Unable to use mmc 0:0... In:serial Out: serial Err: serial SCSI: SATA link 0 timeout. AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode flags: ncq stag pm led clo only pmp pio slum part ccc apst Net: eth0: ethernet@1c5 starting USB... USB0: USB EHCI 1.00 USB1: USB OHCI 1.0 USB2: USB EHCI 1.00 USB3: USB OHCI 1.0 scanning bus 0 for devices... 1 USB Device(s) found scanning bus 2 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found Hit any key to stop autoboot: 0 => saveenv Saving Environment to FAT... Unable to use mmc 0:0... Failed (1) Thank you for any light that may be shed upon this error. -- -- Robin Polak ro...@robinpolak.com 917-494-2080 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] efi_loader: Fix crash on 32-bit systems
Hi Alex. On Wed 14 Sep 08:34:47 2016, Alexander Graf wrote: [...] > Very nice catch! Thanks! [...] > Can you please double-check that this is the only place the type > mismatch happened? So I skimmed through the boot and run-time service API implementations and couldn't spot another instance of a mismatch. Ideally it would be neat to have an automated way to check for these things - maybe Coccinelle or something. > For this change however, the fix is already correct: > > Reviewed-by: Alexander Graf Thanks, Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] efi_loader: Fix crash on 32-bit systems
A type mismatch in the efi_allocate_pool boot service flow causes hazardous memory scribbling on 32-bit systems. This is efi_allocate_pool's prototype: static efi_status_t EFIAPI efi_allocate_pool(int pool_type, unsigned long size, void **buffer); Internally, it invokes efi_allocate_pages as follows: efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, (void*)buffer); This is efi_allocate_pages' prototype: efi_status_t efi_allocate_pages(int type, int memory_type, unsigned long pages, uint64_t *memory); The problem: efi_allocate_pages does this internally: *memory = addr; This fix in efi_allocate_pool uses a transitional uintptr_t cast to ensure the correct outcome, irrespective of the system's native word size. This was observed when bootefi'ing the EFI instance of FreeBSD's first stage bootstrap (boot1.efi) on a 32-bit ARM platform (Qemu VExpress + Cortex-a9). Signed-off-by: Robin Randhawa --- lib/efi_loader/efi_boottime.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index be6f5e8..784891b 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -134,9 +134,11 @@ static efi_status_t EFIAPI efi_allocate_pool(int pool_type, unsigned long size, void **buffer) { efi_status_t r; + efi_physical_addr_t t; EFI_ENTRY("%d, %ld, %p", pool_type, size, buffer); - r = efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, (void*)buffer); + r = efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, &t); + *buffer = (void *)(uintptr_t)t; return EFI_EXIT(r); } -- 2.9.0 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/12] imx:mx6 Support LDO bypass
On Thu, Feb 12, 2015 at 04:08:51PM -0800, Tim Harvey wrote: > On Wed, Feb 11, 2015 at 7:47 AM, Tim Harvey wrote: > > On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong wrote: > > > >>> > >>> This is very board dependent. Here you are referring to a board that > >>> has a reset input to the PMIC's from the IMX6's watchdog output. In > >>> this case, this reset routing/pinmux would be needed regardless of > >>> using ldo-bypass mode or not and that should just be a pinmux of the > >>> pin your using for WDOG_B. > >>> > >> There are two types of reboot in our chip by watchdog : SRS/warm reset > >> (Software Reset Signal) and WDOG_B(reset signal output to external pmic > >> to trigger next power cycle). In fact, warm reset can work most cases > >> except > >> ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass > >> here: > >> kernel may trigger warm reset at the lowest cpu freq setpoint, for example > >> VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable > >> mode > >> @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need > >> use > >> WDOG_B pin to reset external pmic if using ldo-byapss mode. > > > > Hi Robin, > > > > I understand that configuring WDOG_B external reset must be used when > > LDO bypass is used. This should be done in the kernel but you can also > > set this pinmux up in uboot in the board-specific init. > > > > If your kernel is providing the PMIC driver and cpufreq driver that > > alter the cpu core frequencies it must also be configuring pinmux so > > that WDOG_B gets routed to the pin that connects to the PMIC so any > > reset based on that wdog (SRS or timeout) issues a hard reset to the > > PMIC. Failure to do so is a kernel bug. I believe this is done > > properly in the Freescale kernel for the reference boards. > > > > If you are trying to take care of an issue caused by a watchdog reset > > (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB > > errata where signal doesn't go to the PMIC) then you should probably > > simply set the PMIC rails where they need to be for the 800MHz > > operation in the bootloader and not muck with the CPU frequency. > > Honestly, if your PMIC rails are too low for 800MHz on powerup your > > bootloader may not be stable anyway right? Note that the operating > > setpoints have the same SOC voltage for both 792MHz and 396MHz, its > > only the ARM voltage that is lower for 396 vs 792 and chances are your > > not going to have an issue just in the bootloader at that point. > > > > Robin, > > The issue your describing was interesting to me and I ran some tests > and found that on a board with no reset to the PMIC, an IMX6Q CPU, > ldo-bypass enabled, and the CPU freq set to 400MHz (such that VDD_ARM > rail set to 0.960V for 400MHz operation) the chip does not even come > out of reset (ie when SRS is set in the watchdog controller). So I > don't really see any ability to work around this in bootloader > software since you won't get there in this case. Yes, ldo-bypass mode depends on reset PMIC while reboot happen. But even on this feature supported boards, we'd better provide warm reset in ldo-enable mode,for example, warm reset can keep printed log in DDR while system crash in last time and trigger watchdog reset. Someone may use DCDC instead of PMIC, or some board didn't connect WDOG_B reset with PMIC, have to use ldo-enable mode. > > Possible solutions I can think of for boards without a PMIC reset is > to either blow the eFUSE so the chip comes up in 400MHz or in the > kernel never allow the VDD_ARM or VDD_SOC rail to go below where they > need to be for CPU startup (the only one that does is VDD_ARM) or > before soft-reboot make sure the cpufreq is at 800MHz or above (which > must be done at higher levels before single-cpu mode in > machine_restart). This also does not deal with the case of a watchdog > reset and/or a crash handler. > Yes, absolutely. But we suggest connecting PMIC reset pin with WDOG_B or just use ldo-enable mode if they are not very care of power number. > Are you findings different? > > Tim ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 07/12] imx:mx6 Support LDO bypass
On Wed, Feb 11, 2015 at 07:47:57AM -0800, Tim Harvey wrote: > On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong wrote: > > >> > >> This is very board dependent. Here you are referring to a board that > >> has a reset input to the PMIC's from the IMX6's watchdog output. In > >> this case, this reset routing/pinmux would be needed regardless of > >> using ldo-bypass mode or not and that should just be a pinmux of the > >> pin your using for WDOG_B. > >> > > There are two types of reboot in our chip by watchdog : SRS/warm reset > > (Software Reset Signal) and WDOG_B(reset signal output to external pmic > > to trigger next power cycle). In fact, warm reset can work most cases except > > ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: > > kernel may trigger warm reset at the lowest cpu freq setpoint, for example > > VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable > > mode > > @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use > > WDOG_B pin to reset external pmic if using ldo-byapss mode. > > Hi Robin, > > I understand that configuring WDOG_B external reset must be used when > LDO bypass is used. This should be done in the kernel but you can also > set this pinmux up in uboot in the board-specific init. > > If your kernel is providing the PMIC driver and cpufreq driver that > alter the cpu core frequencies it must also be configuring pinmux so > that WDOG_B gets routed to the pin that connects to the PMIC so any > reset based on that wdog (SRS or timeout) issues a hard reset to the > PMIC. Failure to do so is a kernel bug. I believe this is done > properly in the Freescale kernel for the reference boards. > pinmux is not enough. WDT bit of WDOGx_WCR need to be taken care of if you want to enable WDOG_B pin reset function. But unfortunately the current wdog driver of kernel not touch this write-once bit. To limit the impact from kernel, set this bit in u-boot. > If you are trying to take care of an issue caused by a watchdog reset > (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB > errata where signal doesn't go to the PMIC) then you should probably > simply set the PMIC rails where they need to be for the 800MHz > operation in the bootloader and not muck with the CPU frequency. > Honestly, if your PMIC rails are too low for 800MHz on powerup your > bootloader may not be stable anyway right? Note that the operating > setpoints have the same SOC voltage for both 792MHz and 396MHz, its > only the ARM voltage that is lower for 396 vs 792 and chances are your > not going to have an issue just in the bootloader at that point. > As you saw in another mail, bootloader is too late. It'll hang in ROM code. > > >> > >> What you are doing here is relying on a device-tree binding from the > >> Freescale 'vendor' kernel, which will NEVER make it upstream and this > >> is one of the issues I was running into getting ldo-bypass capability > >> upstream in the kernel. > >> > >> The issue here is that LDO bypass is dependent on the following things: > >> 1. your voltage rail requirements - which are dependent on the CPU > >> frequency (there is a nice table in the IMX6 datasheets of voltage on > >> each rail at each frequency operating point validated by Freescale). > >> The exception of always using the LDO for 1.2GHz is specified here as > >> well. > >> 2. you have a PMIC in your design on VDD_ARM_IN and VDD_SOC_IN rails > >> - this should be specified in the device-tree as well > >> 3. you have valid PMIC drivers configured > >> > >> In the kernel, its not desired to have a single device-tree node > >> called 'fsl,ldo-bypass' for an enable. Instead you need to make sure > >> that you PMIC regulators that are 'not' the internal IMX6 anatop > >> regulators. This property is not a mainline linux device-tree binding. > >> > > Yes, understood. But we have no better solution, because we need touch both > > internal anatop regulator and external pmic regulator in ldo-bypass mode, > > that > > bring kernel cpufreq driver code too messy > > I just don't see the point in having the CPU frequency changed in the > bootloader - leave this up to the OS kernel. I don't think any of this > patch should go into mainline uboot. > > I think my kernel patch I referenced provides you with a simple kernel > solution that de-couples ldo-bypass completely from the bootloader. > Can you please share me the patch link? Thanks. > > >&
Re: [U-Boot] [PATCH 07/12] imx:mx6 Support LDO bypass
On Tue, Feb 10, 2015 at 06:33:38AM -0800, Tim Harvey wrote: > On Fri, Jan 9, 2015 at 12:59 AM, Peng Fan wrote: > > The basic graph for voltage input is: > >VDDARM_IN ---> LDO_DIG(ARM) ---> VDD_ARM_CAP > >VDDSOC_IN ---> LDO_DIG(SOC) ---> VDD_SOC_CAP > > > > Hi Peng, > > Glad to see someone else interested in IMX6 LDO bypass mode. I've made > a couple of stabs at getting it supported in mainline but I haven't > had the time to follow-through yet there. > > > We can bypass the LDO to save power, if the board already has pmic. > > > > set_anatop_bypass is the function to do the bypass VDDARM and VDDSOC > > work. > > > > Current only set VDDARM_IN@1.175V/VDDSOC_IN@1.175V before ldo > > bypass switch. So until ldo bypass switch happened, these voltage > > setting is set in ldo-enable mode. But in datasheet, we need > > 1.15V + 125mV = 1.275V for VDDARM_IN. We need to downgrade cpufreq > > to 400Mhz and restore after ldo bypass mode switch. So add > > prep_anatop_bypass/finish_anatop_bypass/set_arm_freq_400M to do > > this work. > > > > LDO bypass is dependent on the flatten device tree file. If speed > > grading fuse is for 1.2GHz, enable LDO bypass and setup PMIC voltages. > > So add check for 1.2GHz core speed. So add check_1_2G function. > > This isn't quite how it works. If you are 'operating at 1.2GHz' > (supposing you had a processor cabable of it) you must use the LDO (to > avoid ripple sensitivity issues). > Hi Peng, the limitation is "must use ldo-enable mode in 1.2Ghz", NOT ldo-bypass > > > > In ldo-bypass mode, we need trigger WDOG_B pin to reset pmic in > > ldo-bypass mode. So add set_wdog_reset to do this work. > > This is very board dependent. Here you are referring to a board that > has a reset input to the PMIC's from the IMX6's watchdog output. In > this case, this reset routing/pinmux would be needed regardless of > using ldo-bypass mode or not and that should just be a pinmux of the > pin your using for WDOG_B. > There are two types of reboot in our chip by watchdog : SRS/warm reset (Software Reset Signal) and WDOG_B(reset signal output to external pmic to trigger next power cycle). In fact, warm reset can work most cases except ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: kernel may trigger warm reset at the lowest cpu freq setpoint, for example VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use WDOG_B pin to reset external pmic if using ldo-byapss mode. > > > > > > > diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c > > index 5f5f497..5d02755 100644 > > --- a/arch/arm/cpu/armv7/mx6/soc.c > > +++ b/arch/arm/cpu/armv7/mx6/soc.c > > @@ -18,6 +18,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -429,6 +430,146 @@ void s_init(void) > > writel(mask528, &anatop->pfd_528_clr); > > } > > > > +#ifdef CONFIG_LDO_BYPASS_CHECK > > +DECLARE_GLOBAL_DATA_PTR; > > +static int ldo_bypass; > > + > > +int check_ldo_bypass(void) > > +{ > > + const int *ldo_mode; > > + int node; > > + > > + /* get the right fdt_blob from the global working_fdt */ > > + gd->fdt_blob = working_fdt; > > + /* Get the node from FDT for anatop ldo-bypass */ > > + node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, > > + "fsl,imx6q-gpc"); > > + if (node < 0) { > > + printf("No gpc device node %d, force to ldo-enable.\n", > > node); > > + return 0; > > + } > > + ldo_mode = fdt_getprop(gd->fdt_blob, node, "fsl,ldo-bypass", NULL); > > + /* > > +* return 1 if "fsl,ldo-bypass = <1>", else return 0 if > > +* "fsl,ldo-bypass = <0>" or no "fsl,ldo-bypass" property > > +*/ > > + ldo_bypass = fdt32_to_cpu(*ldo_mode) == 1 ? 1 : 0; > > + > > + return ldo_bypass; > > +} > > What you are doing here is relying on a device-tree binding from the > Freescale 'vendor' kernel, which will NEVER make it upstream and this > is one of the issues I was running into getting ldo-bypass capability > upstream in the kernel. > > The issue here is that LDO bypass is dependent on the following things: > 1. your voltage rail requirements - which are dependent on the CPU > frequency (there is a nice table in the IMX6 datasheets of voltage on > each rail at each frequency operating point validated by Freescale). > The exception of always using the LDO for 1.2GHz is specified here as > well. > 2. you have a PMIC in your design on VDD_ARM_IN and VDD_SOC_IN rails > - this should be specified in the device-tree as well > 3. you have valid PMIC drivers configured > > In the kernel, its not desired to have a single device-tree node > called 'fsl,ldo-bypass' for an enable. Instead you need to make sure > that you PMIC regulators that are 'not' the internal IMX6 anatop > regulators.
[U-Boot] help on adding bootelf to u-boot
hi sir/madam I am trying to run bootelf command from u-boot. But the command is not inbuilt in it. So could you please let me know what is the procedure to compile.. -- Thanks and Regards *Robin Fernandes* Global Edge Software Ltd. Bangalore, India Tel : _+91 88 6164 8965_ Web : www.globaledgesoft.com Email: /robin.fernan...@globaledgesoft.com/ Have a Good day. - ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] net/dns.c: Fix endian conversion for big-endian in dns command
On Sun 16 Oct 2011 05:59, Bernhard Kaindl pondered: > From: Bernhard Kaindl > > net/dns.c used endian conversion macros wrongly (shorts in reply > were put swapped into CPU, and then ntohs() was used to swap it > back, which broke on big-endian). > > Fix this by using the correct linux conversion macro for reading > a unaligned short in network byte order: get_unaligned_be16() > Thanks to Mike Frysinger pointing at the best macro to use. > > Tested on big and little endian qemu boards (mips and versatile) This also tweaks some error messages and comments, which isn't captured in the change log? Otherwise, Ack from me. > Signed-off-by: Bernhard Kaindl > Cc: Pieter Voorthuijsen > Cc: Robin Getz > --- > net/dns.c | 20 > 1 files changed, 8 insertions(+), 12 deletions(-) > > diff --git a/net/dns.c b/net/dns.c > index b51d1bd..7a3f1f9 100644 > --- a/net/dns.c > +++ b/net/dns.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > > #include "dns.h" > > @@ -109,7 +110,6 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, > unsigned src, unsigned len) > int found, stop, dlen; > char IPStr[22]; > IPaddr_t IPAddress; > - short tmp; > > > debug("%s\n", __func__); > @@ -120,14 +120,14 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, > unsigned src, unsigned len) > debug("0x%p - 0x%.2x 0x%.2x 0x%.2x 0x%.2x\n", > pkt+i, pkt[i], pkt[i+1], pkt[i+2], pkt[i+3]); > > - /* We sent 1 query. We want to see more that 1 answer. */ > + /* We sent one query. We want to have a single answer: */ > header = (struct header *) pkt; > if (ntohs(header->nqueries) != 1) > return; > > /* Received 0 answers */ > if (header->nanswers == 0) { > - puts("DNS server returned no answers\n"); > + puts("DNS: host not found\n"); > NetState = NETLOOP_SUCCESS; > return; > } > @@ -139,9 +139,8 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, > unsigned src, unsigned len) > continue; > > /* We sent query class 1, query type 1 */ > - tmp = p[1] | (p[2] << 8); > - if (&p[5] > e || ntohs(tmp) != DNS_A_RECORD) { > - puts("DNS response was not A record\n"); > + if (&p[5] > e || get_unaligned_be16(p+1) != DNS_A_RECORD) { > + puts("DNS: response was not an A record\n"); > NetState = NETLOOP_SUCCESS; > return; > } > @@ -160,14 +159,12 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, > unsigned src, unsigned len) > } > debug("Name (Offset in header): %d\n", p[1]); > > - tmp = p[2] | (p[3] << 8); > - type = ntohs(tmp); > + type = get_unaligned_be16(p+2); > debug("type = %d\n", type); > if (type == DNS_CNAME_RECORD) { > /* CNAME answer. shift to the next section */ > debug("Found canonical name\n"); > - tmp = p[10] | (p[11] << 8); > - dlen = ntohs(tmp); > + dlen = get_unaligned_be16(p+10); > debug("dlen = %d\n", dlen); > p += 12 + dlen; > } else if (type == DNS_A_RECORD) { > @@ -181,8 +178,7 @@ DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, > unsigned src, unsigned len) > > if (found && &p[12] < e) { > > - tmp = p[10] | (p[11] << 8); > - dlen = ntohs(tmp); > + dlen = get_unaligned_be16(p+10); > p += 12; > memcpy(&IPAddress, p, 4); > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] standalone application export eth_receive: undefined index
Okay, I have put in my commands file #include So it could give me access to the eth_receive function. The strange thing is that when I use the eth_send function it doens't give any error. /Software/u-boot-1.3.3/board/ssysdatalogger/cmd_setup.c:25: undefined reference to `eth_receive' make: *** [u-boot] Fout 1 I know it is just some small switch somewhere but I don't see where I can find it. Thanks a lot Robin Theunis 2011/8/26 Wolfgang Denk : > Dear Robin Theunis, > > please keep the ML on Cc: > > In message > you > wrote: >> >> It will we a tool to update the environment variables over http/ a >> json request. It isn't a problem to opensource it. >> >> But how do I link my standalone application to u-boot? > > Don't make it a standalone application, but link it with the rest of > the U-Boot code. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > Misquotation is, in fact, the pride and privilege of the learned. A > widely-read man never quotes accurately, for the rather obvious > reason that he has read too widely. > - Hesketh Pearson _Common Misquotations_ introduction > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] standalone application export eth_receive: undefined index
Hi all I have a little problem. I need to access the eth_receive function in my standalone application. The U-boot version is 1.3.3 (I know it is old but it is a requirement). Also I have seen that I need to enable #define CONFIG_API but building with this option fails. Current platfom is a AT91RM9200, 16MB sdram, 16MB flash. If I comment out the switches in the net/eth.c net/net.c and include/net.h. It could work. I exported also eth_send. This function doesn't give me any problems when compiling the application. Is this a dummy problem? Or something else? Thanks Robin Theunis ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] integratorap: remove hardcoded 32MB memory cmdline
Hi Linus, Wolfgang, On Fri, Jul 15, 2011 at 11:34 AM, Linus Walleij wrote: >> On Thu, Jul 14, 2011 at 8:03 PM, Linus Walleij wrote: >> > On Thu, Jul 14, 2011 at 1:44 PM, Wolfgang Denk wrote: >> > >> >>> --- a/include/configs/integratorap.h >> >>> +++ b/include/configs/integratorap.h >> >> >> >> Please make sure to Cc: the board maintainer! >> > >> > Aw yes, that's one thing. Peter, are you still looking after the Integrator >> > AP board support in U-boot? >> >> I got this as autoreply from another conversation, so obviously Peter isn't >> working with this anymore, actually if I recall correctly, he has retired from >> ARM. This is correct, Peter retired a couple of months ago. >> I will attempt to find his private email address but I will propose a >> patch taking over as Integrator maintainer since I have the board and I >> suspect noone else i likely to be working with it. This is probably best for this. Many thanks. Regards, Philippe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] spi utility command support
On Wed 28 Jul 2010 10:01, shashikiran.bevenka...@wipro.com pondered: [snip] > The information contained in this electronic message and any attachments > to this message are intended for the exclusive use of the addressee(s) > and may contain proprietary, confidential or privileged information. If > you are not the intended recipient, you should not disseminate, distribute > or copy this e-mail. Please notify the sender immediately and destroy all > copies of this message and any attachments. Such disclaimers are inappropriate for mail sent to public lists. If your company automatically adds something like this to outgoing mail, and you can't convince them to stop, you might consider using a free web-based e-mail account. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Uboot application problem
Hi, I have a couple of custom application for uboot. I am able to build and run the application individually If i power up the board and run the first application, It executes without powering off the board if i load second application to same location, the system hangs. Both the application has been compiled for same location 0x8000. i have tested both application seperately, both are fine. But when i execute one after another without power off, then system hangs. i compiled one application for 0x8000 and another for 0x8100 then both application executes without requiring reboot. i am using the below lds file for applications. Can some one suggest if it is a stack issue or some other issue. i had defined usrstack area in my application and verified it using map file. but after execution of application i check that area, its not at all modified, its 0x, i think its using uboot's stack area not the one defined by my application. what is causing this behavior and how to resolve it ? Regards, Robin = OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) ENTRY(main) MEMORY { ram : o = 0x8000, l = 256k usrstack : o = 0x8300, l = 64k } SECTIONS { /* Code and Constants */ .text : { . = 0x00; __text_start = .; *.o(.text) *.o(.strings) *.o(.rodata) *.o(.rodata.*) *.o(.comment) *.o(.debug*) *(.rodata.*) *(.rodata) *(.eh_frame) . = ALIGN(32); __text_end = .; } > ram /* Initialized data */ .init : { . = ALIGN(32); __data_start = .; . = ALIGN(32); *(.data) *(.glue_7) *(.glue_7t) . = ALIGN(32); __data_end = .; . = ALIGN(32); } > ram .bss : { __bss_start = .; *(.bss) *(COMMON) . = ALIGN(32); __bss_end = .; } > ram /* Application stack */ .stack : { __stack_start = .; *(.stack) __stack_end = .; . = ALIGN(32); } > usrstack } --- DISCLAIMER: This e-mail and any attachment (s) is for authorised use by the intended recipient (s) only. It may contain proprietary material, confidential information and/or be subject to the legal privilege of iWave Systems Technologies Private Limited. If you have received this message in error, please notify the originator immediately. If you are not the intended recipient, you are notified that you are strictly prohibited from retaining, using, copying, alerting or disclosing the content of this message. Thank you for your co-operation. -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Uboot application problem
Hi, I have a couple of custom application for uboot. I am able to build and run the application individually If i power up the board and run the first application, It executes without powering off the board if i load second application to same location, the system hangs. Both the application has been compiled for same location 0x8000. i have tested both application seperately, both are fine. But when i execute one after another without power off, then system hangs. i compiled one application for 0x8000 and another for 0x8100 then both application executes without requiring reboot. i am using the below lds file for applications. Can some one suggest if it is a stack issue or some other issue. i had defined usrstack area in my application and verified it using map file. but after execution of application i check that area, its not at all modified, its 0x, i think its using uboot's stack area not the one defined by my application. what is causing this behavior and how to resolve it ? Regards, Robin = OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") OUTPUT_ARCH(arm) ENTRY(main) MEMORY { ram : o = 0x8000, l = 256k usrstack : o = 0x8300, l = 64k } SECTIONS { /* Code and Constants */ .text : { . = 0x00; __text_start = .; *.o(.text) *.o(.strings) *.o(.rodata) *.o(.rodata.*) *.o(.comment) *.o(.debug*) *(.rodata.*) *(.rodata) *(.eh_frame) . = ALIGN(32); __text_end = .; } > ram /* Initialized data */ .init : { . = ALIGN(32); __data_start = .; . = ALIGN(32); *(.data) *(.glue_7) *(.glue_7t) . = ALIGN(32); __data_end = .; . = ALIGN(32); } > ram .bss : { __bss_start = .; *(.bss) *(COMMON) . = ALIGN(32); __bss_end = .; } > ram /* Application stack */ .stack : { __stack_start = .; *(.stack) __stack_end = .; . = ALIGN(32); } > usrstack } --- DISCLAIMER: This e-mail and any attachment (s) is for authorised use by the intended recipient (s) only. It may contain proprietary material, confidential information and/or be subject to the legal privilege of iWave Systems Technologies Private Limited. If you have received this message in error, please notify the originator immediately. If you are not the intended recipient, you are notified that you are strictly prohibited from retaining, using, copying, alerting or disclosing the content of this message. Thank you for your co-operation. -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [Patch] ./net/net.c - make Microsoft dns servers happy with random_port() numbers
For some reason, (which I can't find any documentation on), if U-Boot gives a port number higher than 17500 to a Microsoft DNS server, the server will reply to port 17500, and U-Boot will ignore things (since that isn't the port it asked the DNS server to reply to). This fixes that by ensuring the random port number is less than 17500. Signed-off-by: Robin Getz --- net/net.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/net.c b/net/net.c index 595abd9..98d58e5 100644 --- a/net/net.c +++ b/net/net.c @@ -1872,11 +1872,13 @@ void copy_filename (char *dst, char *src, int size) #if defined(CONFIG_CMD_NFS) || defined(CONFIG_CMD_SNTP) || defined(CONFIG_CMD_DNS) /* - * make port a little random, but use something trivial to compute + * make port a little random (1024-17407) + * This keeps the math somewhat trivial to compute, and seems to work with + * all supported protocols/clients/servers */ unsigned int random_port(void) { - return 1024 + (get_timer(0) % 0x8000);; + return 1024 + (get_timer(0) % 0x4000); } #endif ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] About GPL
On Wed 16 Dec 2009 21:55, Rob Westfall pondered: > Where exactly is the line for what you have to provide vs what you > don't have to provide? > > We are building boards that are based on a standard board, but we have > obviously had to modify include/config/ to setup for our > hardware and create customized files to support our board. We have > NOT modified files outside of customizing a > /include/config/ and creating a board/ directory > based on a existing board that is already released from the cpu > vendor. The Free Software Foundation has this to say: http://www.gnu.org/licenses/gpl-faq.html#DistributingSourceIsInconvenient ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Uboot hangs on "starting kernel..."
Greetings. On Thu10 Dec 2009, at 20:29, Wierd O wrote: > I am not sure why the kernel stops short of loading. The other thing that > confuses me is that i couldnt know whther the proble is from the image or > uboot. You are assuming that the kernel 'stops short of loading'. It is quite likely that the kernel has gone along a fair but and attempted to bring up your board but has panicked at a very early stage before the kernel log buffer was flushed to the console thereby preventing you from getting a feel for what really went wrong. There could be any number of reasons for this of course. My recommendation would be to start hacking the kernel from a very early stage of initialisation. If you can get to Linux's '__log_buf' and print out the ASCII contents straight to the UART, I bet you'll get a handle on the problem. It'll most likely be a kernel misconfiguration issue but you'll get the idea one hopes. Cheers, Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Add debug message for Blackfin Ethernet Rx function.
Add a simple print for the Blackfin's Ethernet Rx function, so we can debug incomming Ethernet functions easier. Signed-off-by: Robin Getz --- drivers/net/bfin_mac.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c index 12d98c2..ec45b63 100644 --- a/drivers/net/bfin_mac.c +++ b/drivers/net/bfin_mac.c @@ -186,6 +186,9 @@ static int bfin_EMAC_recv(struct eth_device *dev) printf("Ethernet: bad frame\n"); break; } + + debug("%s: len = %d\n", __func__, length - 4); + NetRxPackets[rxIdx] = (volatile uchar *)(rxbuf[rxIdx]->FrmData->Dest); NetReceive(NetRxPackets[rxIdx], length - 4); -- 1.6.0.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Add Transfer Size Option to tftp
Optionally add RFC 2349 "Transfer Size Option", so we can minimize the time spent sending data over the UART (now print a single line during a tftp transfer). - If turned on (CONFIG_TFTP_TSIZE), U-Boot asks for the size of the file. - if receives the file size, a single line (50 chars) are printed. one hash mark == 2% of the file downloaded. - if it doesn't receive the file size (the server doesn't support RFC 2349, prints standard hash marks (one mark for each UDP frame). Signed-off-by: Robin Getz --- net/tftp.c | 42 -- 1 files changed, 40 insertions(+), 2 deletions(-) diff --git a/net/tftp.c b/net/tftp.c index 0fa7033..865f41a 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -56,6 +56,10 @@ static ulong TftpLastBlock; /* last packet sequence number received */ static ulong TftpBlockWrap; /* count of sequence number wraparounds */ static ulong TftpBlockWrapOffset;/* memory offset due to wrapping */ static int TftpState; +#ifdef CONFIG_TFTP_TSIZE +static int TftpTsize; /* The file size reported by the server */ +static short TftpNumchars; /* The number of hashes we printed */ +#endif #define STATE_RRQ 1 #define STATE_DATA 2 @@ -202,6 +206,10 @@ TftpSend (void) sprintf((char *)pkt, "%lu", TIMEOUT / 1000); debug("send option \"timeout %s\"\n", (char *)pkt); pkt += strlen((char *)pkt) + 1; +#ifdef CONFIG_TFTP_TSIZE + memcpy((char *)pkt, "tsize\\0", 8); + pkt += 8; +#endif /* try for more effic. blk size */ pkt += sprintf((char *)pkt,"blksize%c%d%c", 0,TftpBlkSizeOption,0); @@ -311,8 +319,14 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) simple_strtoul((char*)pkt+i+8,NULL,10); debug("Blocksize ack: %s, %d\n", (char*)pkt+i+8,TftpBlkSize); - break; } +#ifdef CONFIG_TFTP_TSIZE + if (strcmp ((char*)pkt+i,"tsize") == 0) { + TftpTsize = simple_strtoul((char*)pkt+i+6,NULL,10); + debug("size = %s, %d\n", +(char*)pkt+i+6, TftpTsize); + } +#endif } #ifdef CONFIG_MCAST_TFTP parse_multicast_oack((char *)pkt,len-1); @@ -338,7 +352,16 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) TftpBlockWrap++; TftpBlockWrapOffset += TftpBlkSize * TFTP_SEQUENCE_SIZE; printf ("\n\t %lu MB received\n\t ", TftpBlockWrapOffset>>20); - } else { + } +#ifdef CONFIG_TFTP_TSIZE + else if (TftpTsize) { + while (TftpNumchars < NetBootFileXferSize * 50 / TftpTsize) { + putc('#'); + TftpNumchars++; + } + } +#endif + else { if (((TftpBlock - 1) % 10) == 0) { putc ('#'); } else if ((TftpBlock % (10 * HASHES_PER_LINE)) == 0) { @@ -432,6 +455,13 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) * We received the whole thing. Try to * run it. */ +#ifdef CONFIG_TFTP_TSIZE + /* Print out the hash marks for the last packet received */ + while (TftpTsize && TftpNumchars < 49) { + putc('#'); + TftpNumchars++; + } +#endif puts ("\ndone\n"); NetState = NETLOOP_SUCCESS; } @@ -561,6 +595,10 @@ TftpStart (void) #ifdef CONFIG_MCAST_TFTP mcast_cleanup(); #endif +#ifdef CONFIG_TFTP_TSIZE + TftpTsize = 0; + TftpNumchars = 0; +#endif TftpSend (); } -- 1.6.0.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Mon 17 Aug 2009 16:20, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200908171555.31016.rg...@blackfin.uclinux.org> you wrote: > > > > > Why static int? This gives a random init value for the second and each > > > following TFTP transfers. > > > > Nope - it is set to zero on the start of every transfer. > > Right, I saw this later, at the end of your patch, but was too lazy to > change my whole message ;-) > > > > > +#ifdef CONFIG_TFTP_TSIZE > > > > + pkt += sprintf((char *)pkt,"tsize%c%d", 0,0); > > > > + pkt += strlen((char *)pkt) + 1; > > > > > > Looks to me as if you were adding the length twice? > > > > One zero is for the null char (delimiter), one zero is for ascii zero > > (length). > > Wel, the sprintf() already returns the number of output characters, > i. e. 7. > > pkt then points at the terminating '\0' character. strlen() should > always return 0, then. This makes not much sense to me. > > Also, why do you need sprintf() at all when the string is a constant > anyway? > > Why not simply: > > memcpy (pkt, "tsize\00", 7); > pkt += 7; That's is better - It was just a dumb copy/paste from the blksize option... > > > > + printf("%2d\b\b", NetBootFileXferSize * 100 / > > > > TftpTsize); > > > > + } > > > > +#endif > > > > > > Hm... maybe we should rather print hashes (say one '#' for each 2%, > > > resulting in a maximum of 50 characters output. > > > > > > Otherwise we actually increase the amount of characters sent over the > > > serial line (and the intention was to reduce it, right?) > > > > Yeah, it was to reduce the output - this was easier :) > > > > Plus spinning numbers are always nice. When you are doing a scp - do you > > look at the bar moving, or the numbers going up to 100%? I always look > > at the numbers... > > I know what you mean. OTOH, I tend to dislike such characters > sequences (with lots of backspace characters) because they always > mess up log files ;-) But they look pretty ... :) > > I'll re-work it for a single line of 50 hashes. (one '#' == 2% of the file). > > Thanks. Np. > > So - you are OK with they way it is done (other comments still apply of > > course). > > Right. Thanks a lot. New version tomorrow... Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Mon 17 Aug 2009 15:05, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200908171315.40365.rg...@blackfin.uclinux.org> you wrote: > > > > Comments welcome... > > I guess the code is largely untested? I tested it on a single machine. > > diff --git a/net/tftp.c b/net/tftp.c > > index 9544691..56db247 100644 > > --- a/net/tftp.c > > +++ b/net/tftp.c > > @@ -66,6 +66,9 @@ static ulong TftpLastBlock; /* last packet > > sequence number received */ > > static ulong TftpBlockWrap; /* count of sequence number > > wraparounds */ > > static ulong TftpBlockWrapOffset;/* memory offset due to > > wrapping*/ > > static int TftpState; > > +#ifdef CONFIG_TFTP_TSIZE > > +static int TftpTsize; /* Tsize */ > > +#endif > > Why static int? This gives a random init value for the second and each > following TFTP transfers. Nope - it is set to zero on the start of every transfer. > > @@ -212,6 +215,10 @@ TftpSend (void) > > sprintf((char *)pkt, "%lu", TIMEOUT / 1000); > > debug("send option \"timeout %s\"\n", (char *)pkt); > > pkt += strlen((char *)pkt) + 1; > > +#ifdef CONFIG_TFTP_TSIZE > > + pkt += sprintf((char *)pkt,"tsize%c%d", 0,0); > > + pkt += strlen((char *)pkt) + 1; > > Looks to me as if you were adding the length twice? One zero is for the null char (delimiter), one zero is for ascii zero (length). > > +#endif > > /* try for more effic. blk size */ > > pkt += sprintf((char *)pkt,"blksize%c%d%c", > > 0,TftpBlkSizeOption,0); > > @@ -321,8 +328,14 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, > > unsigned len) > > simple_strtoul((char*)pkt+i+8,NULL,10); > > debug("Blocksize ack: %s, %d\n", > > (char*)pkt+i+8,TftpBlkSize); > > - break; > > } > > +#ifdef CONFIG_TFTP_TSIZE > > + if (strcmp ((char*)pkt+i,"tsize") == 0) { > > + TftpTsize = > > simple_strtoul((char*)pkt+i+6,NULL,10); > > + debug("size = %s, %d\n", > > +(char*)pkt+i+6, TftpTsize); > > + } > > It seems you rely on TftpTsize to be zero otherwise. The current code > (with a static int) does not guarantee that, though. It is set to zero below on start. > > - } else { > > + } > > +#ifdef CONFIG_TFTP_TSIZE > > + else if (TftpTsize) { > > + > > + printf("%2d\b\b", NetBootFileXferSize * 100 / > > TftpTsize); > > + } > > +#endif > > Hm... maybe we should rather print hashes (say one '#' for each 2%, > resulting in a maximum of 50 characters output. > > Otherwise we actually increase the amount of characters sent over the > serial line (and the intention was to reduce it, right?) Yeah, it was to reduce the output - this was easier :) Plus spinning numbers are always nice. When you are doing a scp - do you look at the bar moving, or the numbers going up to 100%? I always look at the numbers... I'll re-work it for a single line of 50 hashes. (one '#' == 2% of the file). > > +#ifdef CONFIG_TFTP_TSIZE > > + if (TftpTsize) > > + puts("100%"); > > +#endif > > + > > puts_quiet("\ndone\n"); > > Here we see the bad consequences of this puts_quiet() stuff (which I > really dislike. The longer I see it, the more I tend to reject it. > > Here, the (necessary!) newline terminating the "100%" output, is not > always sent, only when puts_quiet() is "active". Yeah, sorry about that - I would switch the puts to the puts_quiet if that is picked up - I just never heard an ack or nack on it... > > @@ -550,6 +579,9 @@ TftpStart (void) > > #ifdef CONFIG_TFTP_TIME > > start_time = get_timer(0); > > #endif > > +#ifdef CONFIG_TFTP_TSIZE > > + TftpTsize = 0; > > +#endif > > Ah. I see :-) So - you are OK with they way it is done (other comments still apply of course). ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Sat 8 Aug 2009 05:50, Ben Warren pondered: > Allesandro, > > Alessandro Rubini wrote: > > I finally fixed the defrag code, testing with NFS as well. > > Didn't take performance figures, tough, for lack of time. > > > > I wanted to do "config + environment" for the NFS case, like tftp, but > > didnt' do the second step out of laziness (also, the source file has > > long lines while I have 80 columns). > > > > For the record, I added the check on ip_src and ip_dst, but everything > > stopped working. So I reverted that instead of entering a long > > debugging session. > > > > The CONFIG_NET_MAXDEFRAG argument is the actual payload, so I add NFS > > overhead to that figure, which is expected to a beautiful 4096 or > > 8192. I feel this is better than other options, as the person writing > > the config is not expected to know how much protocol overhead is > > there. > > > > Alessandro Rubini (4): > > net: defragment IP packets > > tftp: get the tftp block size from config file and from the > > environment I noticed that while playing with this, that if you set the "tftpblocksize" environment, do a transfer, and then clear it, it does not go back to the default setting. I was not sure if this was the intended or not, but this fixes it (and provides a small code size reduction when this option is not activated). Also wondering -- if the user sets the "tftpblocksize" to a number larger than IP_MAXUDP, the transfer will never finish. Should this be restricted here? diff --git a/net/tftp.c b/net/tftp.c index 9544691..56db247 100644 --- a/net/tftp.c +++ b/net/tftp.c +#ifdef CONFIG_TFTP_BLOCKSIZE /* Allow the user to choose tftpblocksize */ if ((ep = getenv("tftpblocksize")) != NULL) TftpBlkSizeOption = simple_strtol(ep, NULL, 10); + else + TftpBlkSizeOption = TFTP_MTU_BLOCKSIZE; debug("tftp block size is %i\n", TftpBlkSizeOption); +#endif ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Thu 13 Aug 2009 18:01, Wolfgang Denk pondered: > Dear Robin Getz, > In message <200908131747.20194.rg...@blackfin.uclinux.org> you wrote: > > The better thing to do (IMHO) - would be to print out the proper number of > > hashes, depending on the size of the file (and implement RFC 2349 at the > > same > > time) - not the number of packets (which is what happens today)... > > Agreed. Patches welcome! Something like this? - If turned on (CONFIG_TFTP_TSIZE), asks for the size of the file. - if receives the file size, prints out the percentage of the file downloaded. - when done, prints 100% - if it doesn't receive the file size (the server doesn't support RFC 2349, prints standard hash marks. Comments welcome... diff --git a/net/tftp.c b/net/tftp.c index 9544691..56db247 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -66,6 +66,9 @@ static ulong TftpLastBlock; /* last packet sequence number received */ static ulong TftpBlockWrap; /* count of sequence number wraparounds */ static ulong TftpBlockWrapOffset;/* memory offset due to wrapping */ static int TftpState; +#ifdef CONFIG_TFTP_TSIZE +static int TftpTsize; /* Tsize */ +#endif #define STATE_RRQ 1 #define STATE_DATA 2 @@ -212,6 +215,10 @@ TftpSend (void) sprintf((char *)pkt, "%lu", TIMEOUT / 1000); debug("send option \"timeout %s\"\n", (char *)pkt); pkt += strlen((char *)pkt) + 1; +#ifdef CONFIG_TFTP_TSIZE + pkt += sprintf((char *)pkt,"tsize%c%d", 0,0); + pkt += strlen((char *)pkt) + 1; +#endif /* try for more effic. blk size */ pkt += sprintf((char *)pkt,"blksize%c%d%c", 0,TftpBlkSizeOption,0); @@ -321,8 +328,14 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) simple_strtoul((char*)pkt+i+8,NULL,10); debug("Blocksize ack: %s, %d\n", (char*)pkt+i+8,TftpBlkSize); - break; } +#ifdef CONFIG_TFTP_TSIZE + if (strcmp ((char*)pkt+i,"tsize") == 0) { + TftpTsize = simple_strtoul((char*)pkt+i+6,NULL,10); + debug("size = %s, %d\n", +(char*)pkt+i+6, TftpTsize); + } +#endif } #ifdef CONFIG_MCAST_TFTP parse_multicast_oack((char *)pkt,len-1); @@ -348,7 +361,14 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) TftpBlockWrap++; TftpBlockWrapOffset += TftpBlkSize * TFTP_SEQUENCE_SIZE; printf ("\n\t %lu MB received\n\t ", TftpBlockWrapOffset>>20); - } else { + } +#ifdef CONFIG_TFTP_TSIZE + else if (TftpTsize) { + + printf("%2d\b\b", NetBootFileXferSize * 100 / TftpTsize); + } +#endif + else { if (((TftpBlock - 1) % 10) == 0) { puts_quiet("#"); } else if ((TftpBlock % (10 * HASHES_PER_LINE)) == 0) { @@ -442,6 +462,11 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) * We received the whole thing. Try to * run it. */ +#ifdef CONFIG_TFTP_TSIZE + if (TftpTsize) + puts("100%"); +#endif + puts_quiet("\ndone\n"); #ifdef CONFIG_TFTP_TIME end_time = get_timer(0); @@ -550,6 +579,9 @@ TftpStart (void) #ifdef CONFIG_TFTP_TIME start_time = get_timer(0); #endif +#ifdef CONFIG_TFTP_TSIZE + TftpTsize = 0; +#endif TftpTimeoutMSecs = TftpRRQTimeoutMSecs; TftpTimeoutCountMax = TftpRRQTimeoutCountMax; ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Wed 12 Aug 2009 17:30, Wolfgang Denk pondered: > Dear Ben Warren, > > In message <4a832bce.9060...@gmail.com> you wrote: > > > > Sure, if you don't mind re-compiling. I think it might be an > > opt-outable message via puts_quiet() > > It seems we start having a mess here, with features bound to other > features that have not even been agreeds about yet. > > I have to admit that I am no friend of this puts_quiet() thingy. > How much time do we really save on a normal system? A small fraction. On a high speed network, with a low speed UART, and block sizes of 512 (default with BSD tftp). With printing hashes... uartDownload 57600 6657 (ms)2,734,393 (bytes/sec) Without 57600 6418 (ms)2,836,219 (bytes/sec) As the network gets faster, and the UART gets slower - it will make a bigger difference. > Is this worth the inconsistent behaviour Most likely not. I'm not going to be offended if you NAK it. The better thing to do (IMHO) - would be to print out the proper number of hashes, depending on the size of the file (and implement RFC 2349 at the same time) - not the number of packets (which is what happens today)... > and the (IMHO much uglier) code? The code isn't uglier - one function is replaced with a different one. The name of the function is kind of crappy - but that is what I came up with late one evening... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Wed 12 Aug 2009 16:04, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200908121148.16431.rg...@blackfin.uclinux.org> you wrote: > > > > Some info for the docs, when I was troubleshooting a Ubuntu 9.04 > install. > > Good info, but bad format. Can you please submit this as a patch? There wasn't any existing docs to patch, and I thought that by the comment that Ben had made: On Mon 10 Aug 2009 15:57, Ben Warren pondered: > Robin Getz wrote: > > Feel free to add my Signed-off (once the docs have been updated > > explaining what this is all for). > > > > > I'll do that. Thanks for all your help. Was that he was going to add a patch for the docs... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Wed 12 Aug 2009 16:05, Ben Warren pondered: > Hi Robin, > > Robin Getz wrote: > > On Mon 10 Aug 2009 15:57, Ben Warren pondered: > > > >> Robin Getz wrote: > >> > >>> Thanks to Alessandro for putting it together. > >>> > >>> Feel free to add my Signed-off (once the docs have been updated > >>> explaining what this is all for). > >>> > >>> > >>> > >> I'll do that. Thanks for all your help. > >> > > > > Some info for the docs, when I was troubleshooting a Ubuntu 9.04 install. > > == > > > > It appears that some tftp servers (the older BSD version, > > Debian's "tftpd") doesn't support RFC 2348 (blksize), and always use > > a block size of 512 :( You need to make sure that you install the > > the Peter Anvin version, which does support RFC 2348, and blksize up > > to 65,464 bytes (Debian's "tftpd-hpa"): > > http://www.kernel.org/pub/software/network/tftp/ > > > Maybe it would make sense to have a message printed if the user > specifies a higher blocksize but the TFTP server doesn't respond to the > 'blksize' request. Thoughts? It doesn't happen today... We request 1468 (today), and if don't get an respose to the blksize request, it falls back to 512 (with the BSD tftpd) - no message to the user - it just takes longer, and people complain about a crappy network driver... To me - this is independent of the defragmentation patch. If we request 1468 (MTU) or 1469 (which will be fragmented) - the logic in tftp.c is the same... There is already a debug("Blocksize ack... in the code - so I'm assuming that if someone wanted to figure it out - they just need to turn on DEBUG in tftp.c - rather than printing it out all the time... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Make TFTP Quiet
On Wed 12 Aug 2009 15:48, Scott Wood pondered: > On Wed, Aug 12, 2009 at 12:02:33PM +0200, Detlev Zundel wrote: > > Hi Timur, > > > > >> +#ifdef CONFIG_TFTP_QUIET > > >> +#define puts_quiet(fmt) > > >> +#else > > >> +#define puts_quiet(fmt) puts(fmt); > > >> +#endif > > > > > > This looks backwards to me. I would do this: > > > > > > #ifdef CONFIG_TFTP_QUIET > > > #define puts(x) puts_quiet(x) > > > #endif > > > > > > That way, you don't need to change all of the puts calls to > > > puts_quiet. Plus, having the normal calls be "puts_quiet" that > > > changes to puts when QUIET is *not* enabled just feels wrong. > > > > Just as a general remark - I consider it a bad idea to "overload" well > > known functions with non-standard behaviour. This breaks the "principle > > of least surprise" which turns out to be very valuable. > > Technically, U-Boot's puts() is already non-standard (no automatic > newline)... > > But there's no redefinition in the original patch. It's just introducing > a new puts_quiet(). Yeah, I think Detlev was just commenting on Timur's suggestion to the patch, not on the original patch... I would be happy to change things to a debugX() - but this changes everyone's default behaviour (it becomes opt in, not opt-out), and most likely would cause some puzzlement when normally things don't print like they use to... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Mon 10 Aug 2009 15:57, Ben Warren pondered: > Robin Getz wrote: > > Thanks to Alessandro for putting it together. > > > > Feel free to add my Signed-off (once the docs have been updated > > explaining what this is all for). > > > > > I'll do that. Thanks for all your help. Some info for the docs, when I was troubleshooting a Ubuntu 9.04 install. == It appears that some tftp servers (the older BSD version, Debian's "tftpd") doesn't support RFC 2348 (blksize), and always use a block size of 512 :( You need to make sure that you install the the Peter Anvin version, which does support RFC 2348, and blksize up to 65,464 bytes (Debian's "tftpd-hpa"). http://www.kernel.org/pub/software/network/tftp/ How to tell which you have? Check your man page: "man tftpd" or ask for the version: Peter Anvin version: rg...@imhotep:~> /usr/sbin/in.tftpd -V tftp-hpa 0.48, with remap, with tcpwrappers rg...@imhotep:~> non-RFC 2348 one: rg...@imhotep:~> /usr/sbin/in.tftpd -V rg...@imhotep:~> Even when using the Peter Anvin version, block size can still be reduced via two methods: -B : limits the maximum permitted block size (-B 512) -r : rejects RFC 2347 TFTP options (-r blksize) Check your local /etc/xinetd.d/tftp or /etc/inetd.conf file to see how your tftpd server is being invoked. === -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add md5sum and sha1 commands...
On Mon 27 Jul 2009 00:07, Robin Getz pondered: > From: Robin Getz > > Now that we have sha1 and md5 in lib_generic, allow people to use them > on the command line, for checking downloaded files > > Signed-off-by: Robin Getz > > README |4 ++ > common/cmd_mem.c | 68 + > 2 files changed, 72 insertions(+) > Wolfgang - was there anything wrong with this? or did you want me to send again? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
On Wed 22 Jul 2009 10:04, jeffery palmer pondered: > We are looking for an http client now as well. Our major issue > revolves around the download times for tftp. > > Can Volkmar Uhlig kindly provide the patches? > > Our units automically update themselves inside of uboot giving us the > most control over our firmware. The issue is that it takes 20 minutes > via a DSL line in Africa to update our units. An http test showed that > the same firmware downloads in 30 seconds. We have also added things > like the blksize parameter to the uboot tftp client to get it down to > 20 minutes, our original download times were ~50 minutes. Jeff: Can you look at the recent changes to the net/next tree/branch? On a slow connection -- I saw things go from 15:28 download (tftpblocksize == 1468) to 1:47 (tftpblocksize == 16268) - more than a 8x difference. Even if you look at the your stated time of 50min - that should drop it to a more reasonable ~6ish ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add md5sum and sha1 commands...
On Tue 11 Aug 2009 15:21, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200908111357.25007.rg...@blackfin.uclinux.org> you wrote: > > > > > Now that we have sha1 and md5 in lib_generic, allow people to use them > > > on the command line, for checking downloaded files > > > > > > Signed-off-by: Robin Getz > > > > > > README |4 ++ > > > common/cmd_mem.c | 68 + > > > 2 files changed, 72 insertions(+) > > > > > > > Wolfgang - was there anything wrong with this? or did you want me to > > send again? > > No, there is nothing wrog with it. Looks good to me. But it is new > stuff, and the "next" branch is currently not so high on my priority > list. Is this urgent to you? No - I was just going to update, and if I dropped it by accident (still learning git) - I wanted to make sure it was in your inbox... :) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Make TFTP Quiet
On Mon 10 Aug 2009 16:55, Timur Tabi pondered: > On Mon, Aug 10, 2009 at 3:26 PM, Robin Getz wrote: > > --- a/net/tftp.c > > +++ b/net/tftp.c > > @@ -22,6 +22,16 @@ > > /* (for checking the image size) > > */ > > #define HASHES_PER_LINE 65 /* Number of "loading" > > hashes per line */ > > > > +#ifdef CONFIG_TFTP_QUIET > > +#define puts_quiet(fmt) > > +#else > > +#define puts_quiet(fmt) puts(fmt); > > +#endif > > This looks backwards to me. I would do this: > > #ifdef CONFIG_TFTP_QUIET > #define puts(x) puts_quiet(x) > #endif > > That way, you don't need to change all of the puts calls to > puts_quiet. Plus, having the normal calls be "puts_quiet" that > changes to puts when QUIET is *not* enabled just feels wrong. There are other puts that you don't want quiet... puts ("Starting again\n\n"); puts ("\nRetry count exceeded; starting again\n"); Otherwise - if you have a bad network - it will never output anything... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Make TFTP Quiet
>From bca49fb5e3045bc175e924999a4015804c39c5c6 Mon Sep 17 00:00:00 2001 From: Robin Getz I was using this when I was looking at some other recent tftp performance, and thought I would share. I really don't think it belongs, as it is (a) trivial and (b) mostly debug... (but I'm trying out some more things with git). This adds the ability to make tftp a little quieter, which saves some time printing hash marks on the console. It also has the ability to print some download stats for debugging network issues. Signed-off-by: Robin Getz --- net/tftp.c | 29 - 1 files changed, 24 insertions(+), 5 deletions(-) diff --git a/net/tftp.c b/net/tftp.c index 0fa7033..9544691 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -22,6 +22,16 @@ /* (for checking the image size) */ #define HASHES_PER_LINE65 /* Number of "loading" hashes per line */ +#ifdef CONFIG_TFTP_QUIET +#define puts_quiet(fmt) +#else +#define puts_quiet(fmt)puts(fmt); +#endif + +#ifdef CONFIG_TFTP_TIME +static ulong start_time, end_time; +#endif + /* * TFTP operations. */ @@ -340,9 +350,9 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) printf ("\n\t %lu MB received\n\t ", TftpBlockWrapOffset>>20); } else { if (((TftpBlock - 1) % 10) == 0) { - putc ('#'); + puts_quiet("#"); } else if ((TftpBlock % (10 * HASHES_PER_LINE)) == 0) { - puts ("\n\t "); + puts_quiet("\n\t "); } } @@ -432,7 +442,12 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) * We received the whole thing. Try to * run it. */ - puts ("\ndone\n"); + puts_quiet("\ndone\n"); +#ifdef CONFIG_TFTP_TIME + end_time = get_timer(0); + printf("Time to download == %ld ms\n", end_time - start_time); + printf("Download rate = %lld bytes/second\n", (long long)(NetBootFileXferSize) * 1000 / (end_time - start_time)); +#endif NetState = NETLOOP_SUCCESS; } break; @@ -460,7 +475,7 @@ TftpTimeout (void) #endif NetStartAgain (); } else { - puts ("T "); + puts_quiet("T "); NetSetTimeout (TftpTimeoutMSecs, TftpTimeout); TftpSend (); } @@ -530,7 +545,11 @@ TftpStart (void) printf ("Load address: 0x%lx\n", load_addr); - puts ("Loading: *\b"); + puts_quiet("Loading: *\b"); + +#ifdef CONFIG_TFTP_TIME + start_time = get_timer(0); +#endif TftpTimeoutMSecs = TftpRRQTimeoutMSecs; TftpTimeoutCountMax = TftpRRQTimeoutCountMax; -- 1.6.0.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] Network defrag
On Sat 8 Aug 2009 05:50, Ben Warren pondered: > Allesandro, > > Alessandro Rubini wrote: > > I finally fixed the defrag code, testing with NFS as well. > > Didn't take performance figures, tough, for lack of time. Checking out performance with tftp - server is Linux 2.6.27 located in Germany, client (U-Boot running net/next) is located in the USA. Packets fragmented, and arriving out of order. U-Boot image (214268 bytes) tftpblocksize download rate (bytes) (ms) (bytes/second) 512 48,246 4,441 1024 24,482 8,752 1468 ( 1MTU) 17,24312,426 2048 12,51717,118 2948 ( 2MTU) 8,91224,042 4428 ( 3MTU) 6,16734,744 4096 6,60832,425 5908 ( 4MTU) 4,84044,270 7388 ( 5MTU) 4,04253,010 8192 3,64158,848 8868 ( 6MTU) 3,42562,560 10348 ( 7MTU) 2,97472,047 11828 ( 8MTU) 2,73678,314 13308 ( 9MTU) 2,50885,433 14788 (10MTU) 2,28193,935 16000 2,23395,955 16268 (11MTU) 2,17498,559 So, that is 17-seconds (default - on the master), to 2 seconds. Wow - a 87% reduction! Doing the same with a larger image (default kernel + ext2 file system == 12Meg), gets similar results - goes from 928,688 ms / 12,493 bytes/second (yeah, that is 15 minutes with a tftpblocksize == 1468) to 107,626 ms / 107,866 bytes/second (that is under two minutes with a tftpblocksize == 16000)... Again - an 88% reduction in time spent waiting... So - I think this is well worth the effort for those people who want to use tftp outside of a local network. (on a local network - things do not change that drastically - An 18Meg file with default tftpblocksize (1468 bytes) took 5084ms to download, and with a tftpblocksize of 16268, it about half the time - 2625 ms... Thanks to Alessandro for putting it together. Feel free to add my Signed-off (once the docs have been updated explaining what this is all for). -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] minor debug cleanups in ./net
On Thu 6 Aug 2009 15:40, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200908061427.10961.rg...@blackfin.uclinux.org> you wrote: > > On Thu 23 Jul 2009 03:01, Robin Getz pondered: > > > OK - this is on > > > > > > git remote -v > > > origin git://git.denx.de/u-boot-net.git > > > > > > git log --max-count=1 > > > commit 97cfe86163505ea18e7ff7b71e78df5bb03dad57 > > > > > > (Is there a better way to tell if git is up to date?) > > > > Was there any problems with this one? > > Well, "git describe" needs less typing, and gives better information. Thanks for the tip. rg...@pinky:~/blackfin/mainline/u-boot/master> git remote -v origin git://git.denx.de/u-boot.git rg...@pinky:~/blackfin/mainline/u-boot/master> git describe --all HEAD^ warning: tag 'v2009.08-rc1' is really 'tags/v2009.08-rc1' here v2009.08-rc1-29-gc3fa4f0 when I switch to the net tree, I get: rg...@pinky:~/blackfin/mainline/u-boot/net> git remote -v origin git://git.denx.de/u-boot-net.git rg...@pinky:~/blackfin/mainline/u-boot/net> git describe --all HEAD^ warning: tag 'U-Boot-1_2_0' is really 'tags/U-Boot-1_2_0' here U-Boot-1_2_0-6291-g0b23fb3 That is because Ben hasn't done a git pull from the master since U-Boot-1_2_0? Or have I don't something wrong on my end? -robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] minor debug cleanups in ./net
On Thu 23 Jul 2009 03:01, Robin Getz pondered: > OK - this is on > > git remote -v > origin git://git.denx.de/u-boot-net.git > > git log --max-count=1 > commit 97cfe86163505ea18e7ff7b71e78df5bb03dad57 > > (Is there a better way to tell if git is up to date?) Was there any problems with this one? > --- > > From: Robin Getz > > Minor ./net cleanups - no functional changes > - change #ifdef DEBUG printf(); #endif to just debug() > - changed __FUNCTION__ to __func__ > - got rid of extra whitespace between function and opening brace > - removed unnecessary braces on if statements > > gcc dead code elimination should make this functionally/size equivalent > when DEBUG is not defined. (confirmed on Blackfin, with gcc 4.3.3). > > Signed-off-by: Robin Getz > > --- > > diff --git a/net/Makefile b/net/Makefile > index 835a04a..ff87d87 100644 > --- a/net/Makefile > +++ b/net/Makefile > @@ -23,7 +23,7 @@ > > include $(TOPDIR)/config.mk > > -# CFLAGS += -DET_DEBUG -DDEBUG > +# CFLAGS += -DDEBUG > > LIB = $(obj)libnet.a > > diff --git a/net/bootp.c b/net/bootp.c > index d5f9c4b..0799ae2 100644 > --- a/net/bootp.c > +++ b/net/bootp.c > @@ -8,17 +8,6 @@ > * Copyright 2000-2004 Wolfgang Denk, w...@denx.de > */ > > -#if 0 > -#define DEBUG1 /* general debug */ > -#define DEBUG_BOOTP_EXT 1/* Debug received vendor fields */ > -#endif > - > -#ifdef DEBUG_BOOTP_EXT > -#define debug_ext(fmt,args...) printf (fmt ,##args) > -#else > -#define debug_ext(fmt,args...) > -#endif > - > #include > #include > #include > @@ -107,7 +96,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, > unsigned src, unsigned len) > retval = -6; > } > > - debug ("Filtering pkt = %d\n", retval); > + debug("Filtering pkt = %d\n", retval); > > return retval; > } > @@ -129,7 +118,7 @@ static void BootpCopyNetParams(Bootp_t *bp) > if (strlen(bp->bp_file) > 0) > copy_filename (BootFile, bp->bp_file, sizeof(BootFile)); > > - debug ("Bootfile: %s\n", BootFile); > + debug("Bootfile: %s\n", BootFile); > > /* Propagate to environment: >* don't delete exising entry when BOOTP / DHCP reply does > @@ -156,7 +145,7 @@ static void BootpVendorFieldProcess (u8 * ext) > { > int size = *(ext + 1); > > - debug_ext ("[BOOTP] Processing extension %d... (%d bytes)\n", > *ext, > + debug("[BOOTP] Processing extension %d... (%d bytes)\n", *ext, > *(ext + 1)); > > NetBootFileSize = 0; > @@ -255,7 +244,7 @@ static void BootpVendorProcess (u8 * ext, int size) > { > u8 *end = ext + size; > > - debug_ext ("[BOOTP] Checking extension (%d bytes)...\n", size); > + debug("[BOOTP] Checking extension (%d bytes)...\n", size); > > while ((ext < end) && (*ext != 0xff)) { > if (*ext == 0) { > @@ -269,34 +258,27 @@ static void BootpVendorProcess (u8 * ext, int > size) > } > } > > -#ifdef DEBUG_BOOTP_EXT > - puts ("[BOOTP] Received fields: \n"); > + debug("[BOOTP] Received fields: \n"); > if (NetOurSubnetMask) > - printf ("NetOurSubnetMask : %pI4\n", &NetOurSubnetMask); > + debug("NetOurSubnetMask : %pI4\n", &NetOurSubnetMask); > > if (NetOurGatewayIP) > - printf ("NetOurGatewayIP: %pI4", > &NetOurGatewayIP); > + debug("NetOurGatewayIP : %pI4", &NetOurGatewayIP); > > - if (NetBootFileSize) { > - printf ("NetBootFileSize : %d\n", NetBootFileSize); > - } > + if (NetBootFileSize) > + debug("NetBootFileSize : %d\n", NetBootFileSize); > > - if (NetOurHostName[0]) { > - printf ("NetOurHostName : %s\n", NetOurHostName); > - } > + if (NetOurHostName[0]) > + debug("NetOurHostName : %s\n", NetOurHostName); > > - if (NetOurRootPath[0]) { > - printf ("NetOurRootPath : %s\n", NetOurRootPath); > - } > + if (NetOurRootPath[0]) > + debug("NetOurRootPath : %s\n", NetOurRootPath); > > - if (NetOurNISDomain[0]) { > - printf ("NetOurNISDomain : %s\n", NetOurNISDomain); > - } > + if (NetOurNISDomain[0]) > + debug("NetOurNISDomain : %s\n", Ne
Re: [U-Boot] [PATCH 1/3] net: defragment IP packets
On Fri 31 Jul 2009 03:46, Alessandro Rubini pondered: > Thanks for your comments. > > >> +#ifndef CONFIG_TFTP_MAXBLOCK > >> +#define CONFIG_TFTP_MAXBLOCK 16384 > > > > It is more than tftp - nfs could also use the same. > > Yes, I know. But most users are tftp ones. And if you want an even > number (like 16k) as a tftp packet you need to add the headers and the > sequence count. And I prefer to have the useful number in the config. > So I used "TFTP" in the name in order for NFS users to know they must > make some calculation. > > > How about CONFIG_NET_MAXDEFRAG instead? > > We could have MAXPAYLOAD if we count in NFS overhead as well (I don't > know how much it is, currently. Hope you see my point. Not really. IMHO - The protocol max payload should be taken care of on the protocol side, not the common network side. It then becomes: #define TFTP_MTU_BLOCKSIZE (CONFIG_NET_MAXDEFRAG - TFTP_OVERHEAD) #define NFS_READ_SIZE (CONFIG_NET_MAXDEFRAG - NFS_OVERHEAD) or something like that (since NFS likes to be power of two, 1024, 2048, etc - it would need to be tweaked a little), but you get the idea... > >> +static IP_t *__NetDefragment(IP_t *ip, int *lenp) > >> +{ > > > > I don't understand the purpose of the lenp. > > > > The calling function doesn't use the len var, except for > > ICMP_ECHO_REQUEST, which are not allowed to be fragmented. > > > > I eliminated it - and suffered no side effects. > > Well, since the caller has this "len" variable, I didn't want to leave > it corrupted. But if it's actually unused after this point, we can well > discard it. OK. > >> +#else /* !CONFIG_IP_DEFRAG */ > >> + > >> +static inline IP_t *NetDefragment(IP_t *ip, int *lenp) > >> +{ > >> + return ip; > >> +} > >> +#endif > > > > This needs to have the same logic (ip_off & (IP_OFFS | IP_FLAGS_MFRAG)) as > > the above function. See comment below. > > Yes, correct. Thanks. Were you going to send an update for Ben? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] net: defragment IP packets
On Fri 31 Jul 2009 03:46, Alessandro Rubini pondered: > > For some reason - why I'm ping flooding when tftping a large file (with > > large tftp block size) - things hang. If I set the block size to under > > the MTU - it works fine. Do you get the same? > > Didn't try, and I can't do that today. I suspect either your ping is > over-mtu, so each new fragment triggers the above code, or simply your > ether+uboot can't keep up with the data rate. I tried on a different network (tftp a 18M file) - and it worked without issues while ping flooding... Until I filled up the max number of packets on the target (and it did start loosing things). "sudo ping -l 3 -f targetip" worked fine while transferring the file... "sudo ping -l 4 -f targetip" made things fall over - but this is a function of the ethernet driver, not anything else. (We pre-allocate 4 packets worth of info, and and only allow 4 packets to stack up). Even when there is no fragmentation - this causes it to fail So, I'll have to see what is going on with my network at home - so it could just be the network couldn't keep up... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] net: defragment IP packets
On Fri 31 Jul 2009 10:02, Alessandro Rubini pondered: > >> Is the target replying to all pings? > > > > Yes. And I can see the same in wireshark. > > Ah. I see. Strange... > > >> > What is missing in the reassembly code (that is described in RFC815) > >> > is the timer. (quote from the RFC): > >> > -- > >> > The final part of the algorithm is some sort of timer based > >> > mechanism which decrements the time to live field of each > >> > partially reassembled datagram, so that incomplete datagrams > >> > which have outlived their usefulness can be detected and deleted. > >> > -- > >> > >> But I reassemble one packet only, so I don't need to timeout > >> partly-filled packets to recover memory. > > > > But it is for the state that you described - the user cntr-C a current > > transfer, and the reassembly algorithm doesn't know to throw away a > > partially accepted packet, when things are cancelled... > > No, it's not like that. Are you sure? That is how it is on my network/tftp server. Maybe we are talking about different things. What I'm talking about is: - start a tftp file transfer. - CNTR-C it, causing a partial reassembly to done. - start a new transfer. All I did was add this to the start of NetLoop() to ensure that things are OK. #ifdef CONFIG_IP_DEFRAG memset(pkt_buff, 0, IP_HDR_SIZE_NO_UDP); #endif > The old instance of the TFTP server resends the last packet of the > aborted xfer, I don't see how this can happen. the tftp server gets a request for a block of 2048 bytes (for example) - and whacks out all 2048 bytes at once, and waits for the ACK. If the ACK never comes - it doesn't send the packet again. The client times out, and the client needs to send another ACK of the last block before it. What tftp server are you using? imhotep:/home/rgetz # /usr/sbin/in.tftpd -V tftp-hpa 0.43, with remap, with tcpwrappers > while the new server sends the new > packet. Both packets are new, they just come as intermixed fragments. > And none survives as there is only one reassembly buffer. The only way this should happen - is a real attack - sending malformed packets to the U-Boot system while it is downloading things on the net - which I agree with your comments below - is outside the scope of what we are talking about. As a minimal test - since we are only talking about tftp and nfs (so far) - if we did not attempt to assemble packets which are not coming from the serverip, that might be an OK thing to do... if (NetReadIP(&ip->ip_src) != NetServerIP) > Or something > like that, but both are fresh fragmented packets, no timeout would solve > this sporadic problem. I still don't understand the use case. The server should only be sending packets in response to an ACK. The packets should be the entire UDP block size. the server should never mix packets. > It seems to me that if we want a secure defagment system (one that can > be use to net-boot production systems in hostile networks), it's > going to be too complex. It could work well, however, as a faster > tool for the interactive developer. I don't want something that is secure - security is beyond tftp's ability. For that -- you need to boot an OS and use https or sftp. All I'm asking for is something robust... :) -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] net: defragment IP packets
On Fri 31 Jul 2009 08:16, Alessandro Rubini pondered: > >> or simply your > >> ether+uboot can't keep up with the data rate. > > > > That doesn't explain, why does it work, when there is no fragmentation??? > > Well, with no fragmentation there is less traffic. Each tftp packet is > one patch, instead of a burst of packets (intermixed with pings). > > Is the target replying to all pings? Yes. And I can see the same in wireshark. > Or is it loosing some? No. > If it looses say 30%, I expect one fragment in 3 to be lost as well. > If your big-tftp is 4 fragments, 20% passes it loss is equally spread ((2/3)^4), > but I fear much less as the burst saturates the incoming queue. > > >> I'm pretty sure it's like this. > > > > I'm not convinced yet - but need to do some further poking on a different > > network... > > Thanks for these tests, mine is just guessing. No problem. I want to make sure it is robust as well. > > What is missing in the reassembly code (that is described in RFC815) > > is the timer. (quote from the RFC): > > -- > > The final part of the algorithm is some sort of timer based > > mechanism which decrements the time to live field of each partially > > reassembled datagram, so that incomplete datagrams which have outlived > > their usefulness can be detected and deleted. > > -- > > But I reassemble one packet only, so I don't need to timeout > partly-filled packets to recover memory. But it is for the state that you described - the user cntr-C a current transfer, and the reassembly algorithm doesn't know to throw away a partially accepted packet, when things are cancelled... > A soon as I have a fragment for a new packet, the old packet is > discarded (while unfragmented stuff flies intermixed). Which works when you receive a complete fragment. As you indicated - what really happens is it causes a timeout on the first packet. Up to you if you want to handle this or not... All that should be needed is just to clear things in the start of NetLoop() (before eth_init) - there are a few things in there for CONFIG_NET_MULTI already, so I don't think that is a big deal... (That becomes your timer - a one shot at the very beginning of the transfer :) -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] net: defragment IP packets
On Fri 31 Jul 2009 03:46, Alessandro Rubini pondered: > > For some reason - why I'm ping flooding when tftping a large file > > (with large tftp block size) - things hang. If I set the block size > > to under the MTU - it works fine. Do you get the same? > > Didn't try, and I can't do that today. I suspect either your ping is > over-mtu, so each new fragment triggers the above code, No ping is the default length. The default packet size is 56 bytes, which translates into 98 bytes on the wire (8 bytes of ICMP header data, 20 for the IP header, and another 14 for the MAC addresses) > or simply your > ether+uboot can't keep up with the data rate. That doesn't explain, why does it work, when there is no fragmentation??? What it looks like is there is something wrong with the main network loop - the system is so busy responding to pings, that it never sends the TFTP Block ACK when it should. > As eaplained in the cover letter > some fragments can be lost in high traffic, as polling mode doesn't allow > to enqueue packets. So I think you just loose some fragments, as target > CPU time is eaten by the ping packets, and you don't get the complete > reassembled packet any more. > > I'm pretty sure it's like this. I'm not convinced yet - but need to do some further poking on a different network... > On the other hand, I found a minor issue in this situation: > - start a tftp transfer > - ctrl-C it > - start another > > Server retransmissions for the first transfer go into the defrag > engine e that reset-defrag-data code is triggered, so a packet may be > lost, and I get a sporadic T in the receiving u-boot. I think it's > not a real problem, though --- or, now that I rethink about it, it > can be the same issue as above: my ether can't enqueue 8k of stuff > so a fragment is lost in that case. What is missing in the reassembly code (that is described in RFC815) is the timer. (quote from the RFC): -- The final part of the algorithm is some sort of timer based mechanism which decrements the time to live field of each partially reassembled datagram, so that incomplete datagrams which have outlived their usefulness can be detected and deleted. -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC 0/3] uboot-doc User's Manual Generation Tool
On Thu 30 Jul 2009 15:55, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200907301550.40651.rg...@blackfin.uclinux.org> you wrote: > > > > I assume - no Invariant Sections, no Front-Cover Texts, and no Back-Cover > > Texts? or did you desire something else? > > We don't have such detailed plans yet. Depending on the exact final plans, Analog Devices would be happy to donate anything from the U-Boot documentation we have created over the past few years https://docs.blackfin.uclinux.org/doku.php?id=bootloaders:u-boot While the examples are Blackfin specific, most should be generic enough to ensure that the reader should understand what to do on their architecture. We see maintaining something separate a duplication of effort, and wasted resources spent on documentation creation (which is a task most developers don't like anyways)... I think Mike brought this up awhile ago - http://www.mail-archive.com/u-boot-us...@lists.sourceforge.net/msg04491.html Which gets back to the original question... On Thu, 17 Apr 2008 11:02:18 -0700 Mike Frysinger pondered: > On Thursday 17 April 2008, Detlev Zundel wrote: > > Hi Mike, > > > > > On Monday 14 April 2008, Detlev Zundel wrote: > > >> > we maintain a Blackfin-specific u-boot wiki that goes into quite a > > >> > bit of detail, some of which is duplicated with the main u-boot > > >> > wiki. how do people feel about extending the u-boot wiki to allow > > >> > for arch-specific details ? > > >> > > >> What exactly do you have in mind? I surely don't see any principal > > >> problem here. > > >> > > >> It would certainly be valuable to get all U-Boot related info > > >> collected in a central place and have pointers wherever that make > > >> sense... > > > > > > from my reading of the wiki, it's more of a technical/command reference > > > than a guide. the wiki we maintain is geared to be more of a guide. i > > > think the two can be merged, i just dont want to convert things only to > > > find out people dont want to take it that direction. > > > > Just to be clear, we are discussing the DULG wiki, right? > > is there any other worth talking about :) > > > I agree that in the current state the documentation is more a reference > > but IIRC that wasn't really a conscious design decision. It simply > > turned out this way in the end. > > > > So I do not see any general problem in adding "guide style" sections in > > there. Maybe then most of the current documentation can then be shifted > > to a "commands reference" section. > > OK > > > One problem I see though is how to correctly adapt such sections to the > > board specific nature of the DULG. Hopefully we can get away with > > mostly generic text passages and only a few ifdefs. It would be very > > helpful to know more concrete plans (outline!) to think further about > > these implications. Are there any thoughts about this? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] net: defragment IP packets
On Thu 30 Jul 2009 05:02, Alessandro Rubini pondered: > The defragmenting code is enabled by CONFIG_IP_DEFRAG. The code > is useful for TFTP transfers, so the static reassembly buffer is sized > based on CONFIG_TFTP_MAXBLOCK (default is 16kB). > > The packet buffer is used as an array of "hole" structures, acting as > a double-linked list. Each new fragment can split a hole in two, > reduce a hole or fill a hole. No support is there for a fragment > overlapping two diffrent holes (i.e., thre new fragment is across an > already-received fragment). > > The code includes a number of suggestions by Robin Getz. > > Signed-off-by: Alessandro Rubini > --- > net/net.c | 172 > +++-- > 1 files changed, 167 insertions(+), 5 deletions(-) > > diff --git a/net/net.c b/net/net.c > index 641c37c..be382dd 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -1117,6 +1117,164 @@ static void CDPStart(void) > } > #endif > > +#ifdef CONFIG_IP_DEFRAG > +/* > + * This function collects fragments in a single packet, according > + * to the algorithm in RFC815. It returns NULL or the pointer to > + * a complete packet, in static storage > + */ > +#ifndef CONFIG_TFTP_MAXBLOCK > +#define CONFIG_TFTP_MAXBLOCK 16384 It is more than tftp - nfs could also use the same. How about CONFIG_NET_MAXDEFRAG instead? > +#endif > +#define IP_PAYLOAD (CONFIG_TFTP_MAXBLOCK + 4) > +#define IP_PKTSIZE (IP_PAYLOAD + IP_HDR_SIZE_NO_UDP) > + > +/* > + * this is the packet being assembled, either data or frag control. > + * Fragments go by 8 bytes, so this union must be 8 bytes long > + */ > +struct hole { > + /* first_byte is address of this structure */ > + u16 last_byte; /* last byte in this hole + 1 (begin of next hole) */ > + u16 next_hole; /* index of next (in 8-b blocks), 0 == none */ > + u16 prev_hole; /* index of prev, 0 == none */ > + u16 unused; > +}; > + > +static IP_t *__NetDefragment(IP_t *ip, int *lenp) > +{ I don't understand the purpose of the lenp. The calling function doesn't use the len var, except for ICMP_ECHO_REQUEST, which are not allowed to be fragmented. I eliminated it - and suffered no side effects. > + static uchar pkt_buff[IP_PKTSIZE] __attribute__((aligned(PKTALIGN))); > + static u16 first_hole, total_len; > + struct hole *payload, *thisfrag, *h, *newh; > + IP_t *localip = (IP_t *)pkt_buff; > + uchar *indata = (uchar *)ip; > + int offset8, start, len, done = 0; > + u16 ip_off = ntohs(ip->ip_off); > + > + /* payload starts after IP header, this fragment is in there */ > + payload = (struct hole *)(pkt_buff + IP_HDR_SIZE_NO_UDP); > + offset8 = (ip_off & IP_OFFS); > + thisfrag = payload + offset8; > + start = offset8 * 8; > + len = ntohs(ip->ip_len) - IP_HDR_SIZE_NO_UDP; > + > + if (start + len > IP_PAYLOAD) /* fragment extends too far */ > + return NULL; > + > + if (!total_len || localip->ip_id != ip->ip_id) { > + /* new (or different) packet, reset structs */ > + total_len = 0x; > + payload[0].last_byte = ~0; > + payload[0].next_hole = 0; > + payload[0].prev_hole = 0; > + first_hole = 0; > + /* any IP header will work, copy the first we received */ > + memcpy(localip, ip, IP_HDR_SIZE_NO_UDP); > + } I'm not sure the reset if we loose a packet, or get a bad one - start over is a great idea. For some reason - why I'm ping flooding when tftping a large file (with large tftp block size) - things hang. If I set the block size to under the MTU - it works fine. Do you get the same? I'm still poking to figure out why... > + /* > + * What follows is the reassembly algorithm. We use the payload > + * array as a linked list of hole descriptors, as each hole starts > + * at a multiple of 8 bytes. However, last byte can be whaever value, > + * so it is represented as byte count, not as 8-byte blocks. > + */ > + > + h = payload + first_hole; > + while (h->last_byte < start) { > + if (!h->next_hole) { > + /* no hole that far away */ > + return NULL; > + } > + h = payload + h->next_hole; > + } > + > + if (offset8 + (len / 8) <= h - payload) { > + /* no overlap with holes (dup fragment?) */ > + return NULL; > + } > + > + if (!(ip_off & IP_FLAGS_MFRAG)) { > + /* no more fragmentss: truncate this (last) hole */ > +
Re: [U-Boot] [RFC 0/3] uboot-doc User's Manual Generation Tool
On Thu 30 Jul 2009 14:45, Wolfgang DenkVersion 1.3 pondered: > Hi, > > In message Detlev Zundel wrote: > ... > > > Not meaning to interpret licensing, but to me that means I > > > can't copy/paste sections into end product documentation, and ship > > > the product for a fee... > > > > That's how it is currently, yes. > > > > > Was that the intent? > > > > Not really - but I'll better let Wolfgang answer that question as he > > came up with the original licence text. > > Originally this actualy has been my intent. But that was a long, long > time ago, back when I was writing all these documentation only by > myself. > > > It is on my priority list however to assemble a list of all contributors > > and ask them if they agree to relicence their contributions under the > > FDL. > > Consider my permission as granted :-) I assume - no Invariant Sections, no Front-Cover Texts, and no Back-Cover Texts? or did you desire something else? Since Front-Cover Text is limited to 5 words, and Back-Cover Text is limited to 25 words - how about (as Back-Cover Text) "This manual includes sections from the Das U-Boot documentation, which can be found at http://www.denx.de/wiki/U-Boot and is copyright it's authors. Included under the Free Documentation License (v1.3)" ? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC 0/3] uboot-doc User's Manual Generation Tool
On Tue 28 Jul 2009 17:27, Wolfgang Denk pondered: > Dear John, > > in message <1248813631.3915.102.ca...@johns> you wrote: > > > > It seems to me that DUTS is designed to test U-Boot and also automates > > the running of commands whose output can be put online in the DULG. I > > Correct. And the DULG itself is a wiki (TWiki at the moment, to be > converted to Foswiki ASAP) based framework which holds the "static" > parts of the documentation. While we are on the topics of wikis, and U-Boot docs, can we discuss the license? http://www.denx.de/wiki/view/DULG/Introduction#Section_2.1. Since it is "is not permitted to sell this document or the derivative work or to include it into any package or distribution that is not freely available to everybody." Not meaning to interpret licensing, but to me that means I can't copy/paste sections into end product documentation, and ship the product for a fee... Was that the intent? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH][Net] Convert SMC91111 Ethernet driver toCONFIG_NET_MULTI API
On Mon 27 Jul 2009 18:17, Ben Warren pondered: > I actually like to have them in the board C code. To the casual > observer, it is obvious that certain ethernet controllers are optional, > whereas if all they see is a string of initialization functions for > different chips they might say, "WTF?". Like I said - it is a style thing. less ifdefs is better in my opinion. Since you need to answer more questions about things - your way is OK too... Something like this is pretty easy to understand - and on modern compilers (anything over 3.x gcc) should compile to the same as yours (and I think is just as easy to understand for the casual reader) if (smc9_initialize(0, CONFIG_SMC9_BASE)) return 1; return 0; ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH][Net] Convert SMC91111 Ethernet driver to CONFIG_NET_MULTI API
On Mon 27 Jul 2009 17:47, Ben Warren pondered: > Ben Warren wrote: > > All in-tree boards that use this controller have CONFIG_NET_MULTI > > added > > Also: > > - changed CONFIG_DRIVER_SMC9 to CONFIG_SMC9 > > - cleaned up line lengths > > - modified all boards that override weak function in this driver > > - modified all eeprom standalone apps to work with new driver > > > > Signed-off-by: Ben Warren > This patch is pretty big (~98k) and should probably be split up in its > final incarnation. This is really meant as a test patch. > > Since I don't have boards with this chip family, I'm unable to test for > anything beyond clean compiling. I don't have a blackfin toolchain > installed, There are always tarballs and rpms (source and binaries) at: http://blackfin.uclinux.org/gf/project/toolchain/frs/?action=FrsReleaseBrowse&frs_package_id=127 > but it does build cleanly on 'MAKEALL ARM9' using ELDK 4.1 > for ARM. > > If anybody is able to test, please do and let me know the results. > > regards, > Ben > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH][Net] Convert SMC91111 Ethernet driver to CONFIG_NET_MULTI API
On Mon 27 Jul 2009 17:47, Ben Warren pondered: > Ben Warren wrote: > > All in-tree boards that use this controller have CONFIG_NET_MULTI > > added > > Also: > > - changed CONFIG_DRIVER_SMC9 to CONFIG_SMC9 > > - cleaned up line lengths > > - modified all boards that override weak function in this driver > > - modified all eeprom standalone apps to work with new driver > > > > Signed-off-by: Ben Warren > This patch is pretty big (~98k) and should probably be split up in its > final incarnation. This is really meant as a test patch. > > Since I don't have boards with this chip family, I'm unable to test for > anything beyond clean compiling. I don't have a blackfin toolchain > installed, but it does build cleanly on 'MAKEALL ARM9' using ELDK 4.1 > for ARM. > > If anybody is able to test, please do and let me know the results. I have a few boards in the office - I'll try tomorrow. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH][Net] Convert SMC91111 Ethernet driver toCONFIG_NET_MULTI API
On Mon 27 Jul 2009 17:43, Ben Warren pondered: > All in-tree boards that use this controller have CONFIG_NET_MULTI > added First - thanks. Second - It's a style thing, but... > --- > board/bf533-ezkit/bf533-ezkit.c | 12 + > include/netdev.h |1 + > 71 files changed, 888 insertions(+), 490 deletions(-) [snip] > diff --git a/board/bf533-ezkit/bf533-ezkit.c > b/board/bf533-ezkit/bf533-ezkit.c > index d5f0b7c..ff0e15e 100644 > --- a/board/bf533-ezkit/bf533-ezkit.c > +++ b/board/bf533-ezkit/bf533-ezkit.c > @@ -26,6 +26,7 @@ > */ > > #include > +#include > #include "psd4256.h" > #include "flash-defines.h" > > @@ -57,3 +58,14 @@ int misc_init_r(void) > > return 0; > } > + > +#ifdef CONFIG_CMD_NET > +int board_eth_init(bd_t *bis) > +{ > + int rc = 0; > +#ifdef CONFIG_SMC9 > + rc = smc9_initialize(0, CONFIG_SMC9_BASE); > +#endif > + return rc; > +} > +#endif [snip] > diff --git a/include/netdev.h b/include/netdev.h > index 3e66586..4636b57 100644 > --- a/include/netdev.h > +++ b/include/netdev.h > @@ -73,6 +73,7 @@ int rtl8169_initialize(bd_t *bis); > int scc_initialize(bd_t *bis); > int skge_initialize(bd_t *bis); > int smc911x_initialize(u8 dev_num, int base_addr); > +int smc9_initialize(u8 dev_num, int base_addr); > int tsi108_eth_initialize(bd_t *bis); > int uec_initialize(int index); > int uec_standard_init(bd_t *bis); would be alot less ifdefs if you put it in the header file... #ifdef CONFIG_SMC9 int smc9_initialize(u8 dev_num, int base_addr); #else #define smc9_initialize(dev_num, base_addr) 0 #endif that would remove all the "ifdef CONFIG_SMC9" in all the board files... also would not be required to set the initial value anymore either... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Mon 27 Jul 2009 08:13, Alessandro Rubini pondered: > Thanks for your comments. > > > Should have a CONFIG_ something - to make this conditional. > > This has been asked by Ben too. Will do, although I'm not very happy > about all those conditionals for every few lines of code. There are 22,250 #ifdef in the code base already - a few more isn't going to make thing any uglier than it already is. :) > Some of your remarks are just symptoms of this being a quick hack, > like the memcpy not checked, the missed getenv and so on. Yeah, comments were for completeness, not a comment on the (nice & quick) function you put together... > > and I was doing md5 or sha1 on things to make sure that things came over > > properly... > > Yes, me too. Besides booting the stuff I got. > > /alessandro > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Mon 27 Jul 2009 08:41, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200907262059.34188.rg...@blackfin.uclinux.org> you wrote: > > > ... > > and I was doing md5 or sha1 on things to make sure that things came over > > properly... > > Are you using FIT images, or reinventing the wheel? Neither - Blackfin does not support FIT yet - just doing a command line "sha1sum $(loadaddr) $(filesize)" (via the patch I sent yesterday). ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Mon 27 Jul 2009 01:08, Ben Warren pondered: > Hi Guys, > > This is great work. Thanks! If you follow these guidelines, I'll pull > it into the net repo: > > 1. Configurable block size (via a well-named CONFIG). Choose a good >default value. > 2. Handle out-of-order fragments, and some test results showing that it >works. Are you OK with something that only has been tested with TFTP? Looks like nfs.h also has: /* Block size used for NFS read accesses. A RPC reply packet (including all * headers) must fit within a single Ethernet frame to avoid fragmentation. * Chosen to be a power of two, as most NFS servers are optimized for this. */ #define NFS_READ_SIZE 1024 > 3. Make the feature configurable > 4. Test with a TFTP server that doesn't have blksize feature enabled It looks like the logic to do this is already there today - U-Boot uses a non-standard Block size (1468), and then falls back to 512 if it fails, or if the server doesn't ACK the option... > I'm not sure about how to handle the configurability of this feature. I think there are two issues: - core networks stuff (CONFIG_NET_FRAG_SIZE defines the size of a reassembled packet, if it is not set, no reassembly code..) - default block size (if CONFIG_NET_FRAG_SIZE is defined, just read it from an env var?) or do you want a separate fixed CONFIG (and checks to make sure it is going to fit?) > I > can see this being the default configuration in the future, but for now > it should probably be opt-in. Let's see how it goes. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Fri 24 Jul 2009 04:04, Alessandro Rubini pondered: [snip] > +/* This only reassembles fragments that come in proper order */ > +static inline IP_t *NetDefragment(IP_t *ip, int *lenp) > +{ > + static uchar pkt_buff[16384]; /*temporary arbitrary limit */ > + static int next_fragment; > + static ushort pkt_id; [snip] > + /* further fragment: skip ip header (we miss offset_of...) */ > + memcpy(pkt_buff + next_fragment, pkt, len); > + next_fragment += len; Also (forgot last night) - we need to make sure the length of the packet fits in the reassembly buffer before you do the memcpy(). Setting the tftp block size to 16384 is bad if the buffer is also set to 16384.. (since it has the IP header on it)... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add md5sum and sha1 commands...
From: Robin Getz Now that we have sha1 and md5 in lib_generic, allow people to use them on the command line, for checking downloaded files Signed-off-by: Robin Getz README |4 ++ common/cmd_mem.c | 68 + 2 files changed, 72 insertions(+) --- diff --git a/README b/README index 9071472..cf3867d 100644 --- a/README +++ b/README @@ -629,6 +629,8 @@ The following options need to be configured: CONFIG_CMD_KGDB * kgdb CONFIG_CMD_LOADB loadb CONFIG_CMD_LOADS loads + CONFIG_CMD_MD5SUM print md5 message digest + (requires CONFIG_CMD_MEMORY and CONFIG_MD5) CONFIG_CMD_MEMORY md, mm, nm, mw, cp, cmp, crc, base, loop, loopw, mtest CONFIG_CMD_MISC Misc functions like sleep etc @@ -652,6 +654,8 @@ The following options need to be configured: (requires CONFIG_CMD_I2C) CONFIG_CMD_SETGETDCR Support for DCR Register access (4xx only) + CONFIG_CMD_SHA1 print sha1 memory digest + (requires CONFIG_CMD_MEMORY) CONFIG_CMD_SOURCE "source" command Support CONFIG_CMD_SPI * SPI serial bus support CONFIG_CMD_USB * USB support diff --git a/common/cmd_mem.c b/common/cmd_mem.c index cdf8c79..9850800 100644 --- a/common/cmd_mem.c +++ b/common/cmd_mem.c @@ -34,6 +34,9 @@ #endif #include +#include +#include + #ifdef CMD_MEM_DEBUG #definePRINTF(fmt,args...) printf (fmt ,##args) #else @@ -1141,6 +1144,55 @@ int do_mem_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) } #endif /* CONFIG_CRC32_VERIFY */ +#ifdef CONFIG_CMD_MD5SUM +int do_md5sum(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned long addr, len; + unsigned int i; + u8 output[16]; + + if (argc < 3) { + cmd_usage(cmdtp); + return 1; + } + + addr = simple_strtoul(argv[1], NULL, 16); + len = simple_strtoul(argv[2], NULL, 16); + + md5((unsigned char *) addr, len, output); + printf("md5 for %08lx ... %08lx ==> ", addr, addr + len - 1); + for (i = 0; i < 16; i++) + printf("%02x", output[i]); + printf("\n"); + + return 0; +} +#endif + +#ifdef CONFIG_CMD_SHA1 +int do_sha1sum(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned long addr, len; + unsigned int i; + u8 output[20]; + + if (argc < 3) { + cmd_usage(cmdtp); + return 1; + } + + addr = simple_strtoul(argv[1], NULL, 16); + len = simple_strtoul(argv[2], NULL, 16); + + sha1_csum((unsigned char *) addr, len, output); + printf("SHA1 for %08lx ... %08lx ==> ", addr, addr + len - 1); + for (i = 0; i < 20; i++) + printf("%02x", output[i]); + printf("\n"); + + return 0; +} +#endif #ifdef CONFIG_CMD_UNZIP int gunzip (void *, int, unsigned char *, unsigned long *); @@ -1267,6 +1319,22 @@ U_BOOT_CMD( ); #endif /* CONFIG_MX_CYCLIC */ +#ifdef CONFIG_CMD_MD5SUM +U_BOOT_CMD( + md5sum, 3, 1, do_md5sum, + "compute MD5 message digest", + "address count" +); +#endif + +#ifdef CONFIG_CMD_SHA1SUM +U_BOOT_CMD( + sha1sum,3, 1, do_sha1sum, + "compute SHA1 message digest", + "address count" +); +#endif /* CONFIG_CMD_SHA1 */ + #ifdef CONFIG_CMD_UNZIP U_BOOT_CMD( unzip, 4, 1, do_unzip, ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 V3] new video driver for bus vcxkframebuffers
On Sun 26 Jul 2009 07:46, Anatolij Gustschin pondered: > Jens Scharsig wrote: > > This patch adds a new video driver > > > > * adds common bus_vcxk framebuffer driver > > > > Signed-off-by: Jens Scharsig > > Applied to u-boot-video. Thanks! Note that I had to fix lots > of style issues before applying. Next time please use > scripts/checkpatch.pl from the Linux source tree to > check your patches before submitting them and resolve > reported issues were it makes sence. Thanks. Is there a reason not to include the checkpatch.pl in http://www.denx.de/wiki/U-Boot/Patches or in the README (in the Submitting Patches section) if it is going to be a requirement? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
*/ if (!NetCksumOk((uchar *)ip, IP_HDR_SIZE_NO_UDP / 2)) { puts ("IP header checksum bad\n"); return; } /* Check the packet length */ if (len < ntohs(ip->ip_len)) { printf("len bad %d < %d\n", len, ntohs(ip->ip_len)); return; } /* If it is not for us, ignore it */ tmp = NetReadIP(&ip->ip_dst); if (NetOurIP && tmp != NetOurIP && tmp != 0x) { #ifdef CONFIG_MCAST_TFTP if (Mcast_addr != tmp) #endif return; } #ifdef CONFIG_NET_FRAGMENT /* If it is a IP fragment, try to combine it with it */ if (ntohs(ip->ip_off) & (IP_OFFS | IP_FLAGS_MFRAG)) { debug("received fragmented packet\n"); ip = NetDefragment(ip); if (!ip) return; } #endif len = ntohs(ip->ip_len); #ifdef ET_DEBUG printf("len=%d, v=%02x\n", len, ip->ip_hl_v & 0xff); #endif > @@ -1378,10 +1420,6 @@ NetReceive(volatile uchar * inpkt, int len) > if ((ip->ip_hl_v & 0xf0) != 0x40) { > return; > } > - /* Can't deal with fragments */ > - if (ip->ip_off & htons(IP_OFFS | IP_FLAGS_MFRAG)) { > - return; > - } > /* can't deal with headers > 20 bytes */ > if ((ip->ip_hl_v & 0x0f) > 0x05) { > return; I also had (for easier testing) @@ -478,8 +497,15 @@ TftpStart (void) #ifdef CONFIG_TFTP_PORT char *ep; /* Environment pointer */ #endif TftpServerIP = NetServerIP; +#ifdef CONFIG_NET_FRAGMENT + { + char *tmp; + tmp = getenv("tftpblocksize") ; + if (tmp) + TftpBlkSizeOption = simple_strtol(tmp, NULL, 10); + else + TftpBlkSizeOption = TFTP_MTU_BLOCKSIZE; + debug("using TftpBlkSizeOption = %d\n", TftpBlkSizeOption); + } +#endif if (BootFile[0] == '\0') { sprintf(default_filename, "%02lX%02lX%02lX%02lX.img", NetOurIP & 0xFF, and I was doing md5 or sha1 on things to make sure that things came over properly... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Sun 26 Jul 2009 16:23, Alessandro Rubini pondered: > >><http://www.faqs.org/rfcs/rfc815.html> > > > > Yeah, I had seen this - but didn't want to duplicate something > > that Alessandro might already working on... > > > > Alessandro - were you going to add out of order packets? > > If the code has chances to go mainline, I'll be happy to complete this > task. In the past - Wolfgang has normally said that as long as it doesn't negatively effect his platforms (i.e. is a compile option that doesn't effect the size of the normal build) he is mostly OK with anything (within reason). > So unless I get a nak earlier, I'm going to find a time slot in > the next few days (with your fixes, I suppose, or should they remain > separate patches?) Nah - roll them all together... I'll send some comments to your earlier patch. > > To make your host send out of order/delayed packets, which should be > > more "real world/long haul" try something like: > > # modprobe sch_netem (if it's not compiled into your kernel) > > # tc qdisc change dev eth0 root netem reorder 50% delay 10ms > > Thanks a lot, I was missing that. http://www.linuxfoundation.org/en/Net:Netem#Packet_re-ordering Some of the examples do not work, and the tc errors are pretty much meaningless - the man page is pretty thin, but the command line "tc qdisc change dev eth0 root netem help" might get you what you need to test things. -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Sat 25 Jul 2009 22:02, Jerry Van Baren pondered: > Robin Getz wrote: > > On Fri 24 Jul 2009 04:04, Alessandro Rubini pondered: > >> This patch add a quick and dirty defrag step in IP reception. > > I needed to modify your patch a little bit to get it working on my > > platform. > > > > If Ben/Wolfgang are interested in taking this - I'll fix up my mods, > > and send it back. > > FWIIW, RFC815 describes a reassembly algorithm that handles out-of-order > > reassembly directly in the receive buffer by keeping the "holes" > bookkeeping data in the holes themselves. ><http://www.faqs.org/rfcs/rfc815.html> Yeah, I had seen this - but didn't want to duplicate something that Alessandro might already working on... Alessandro - were you going to add out of order packets? Since we are only really interested in TFTP (which still requires an ACK for a complete block), we should not need to handle more than one packet ID, so it should be mostly trivial... To make your host send out of order/delayed packets, which should be more "real world/long haul" try something like: # modprobe sch_netem (if it's not compiled into your kernel) # tc qdisc change dev eth0 root netem reorder 50% delay 10ms -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Add md5sum and sha1 commands...
On Sat 25 Jul 2009 22:49, Mike Frysinger pondered: > On Saturday 25 July 2009 16:07:49 Robin Getz wrote: > > --- a/common/cmd_mem.c > > +++ b/common/cmd_mem.c > > @@ -34,6 +34,14 @@ > > #endif > > #include > > > > +#ifdef CONFIG_CMD_MD5SUM > > +#include > > +#endif > > + > > +#ifdef CONFIG_CMD_SHA1 > > +#include > > +#endif > > i dont think there would be a problem just including these all the time. > would make it easier to notice problems down the line if people moved files > and compile tested with boards that didnt enable these commands for example. I'm OK with either way. > > + for (i = 0; i < 16 ; i++) > > no space before that semicolon > > > + for (i = 0; i < 20 ; i++) > > same here Oops. > > +#ifdef CONFIG_CMD_MD5SUM > > +U_BOOT_CMD( > > + md5sum, 3, 1, do_md5sum, > > + "compute MD5 message digest", > > + "address count" > > +); > > +#endif > > + > > +#ifdef CONFIG_CMD_SHA1 > > +U_BOOT_CMD( > > + sha1, 3, 1, do_sha1, > > + "compute SHA1 message digest", > > + "address count" > > +); > > +#endif /* CONFIG_CMD_SHA1 */ > > there's no need for these to be at the bottom of the file. move the > U_BOOT_CMD() into the releated #ifdef block. I'm just doing the same as all the other things in the same file (which doesn't mean it is correct). What is the preferred style? > also, they should both have a "sum" suffix or neither. i'd lean towards the > former ... Will do. Since the standard Linux console commands are sha1sum & md5sum I'll make U-Boot do like-wise. I'll send a new version, when Wolfgang lets me know what the ifdef preference is... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH/RFC] net: defragment IP packets
On Fri 24 Jul 2009 04:04, Alessandro Rubini pondered: > This patch add a quick and dirty defrag step in IP reception. This > allows to increase the TFTP block size and get more performance in > slow links (but at that point it should be made configurable). > > The overhead is negligible, verified with an ARM9 CPU and 10MB data > file, changing the server MTU from 1500 to 800 and then 550. However, > on a LAN connection, I didn't see advantes with using a 4k block > size with default MTU. I do... Filesize : 4613551 bytes (4.3Mbytes) tftp modified, so it doesn't print out hashes (have have things slow down for the UART printing) - to make sure we are getting a true network measurement... +#ifndef CONFIG_TFTP_QUIET if (((TftpBlock - 1) % 10) == 0) { putc ('#'); } else if ((TftpBlock % (10 * HASHES_PER_LINE)) == 0) puts ("\n\t "); +#endif block size seconds 512 9.43 144.78% (+2.92 seconds) 1468 6.52 100.00% (default) 2048 6.27 96.20% (-0.25 seconds) 3072 5.69 87.40% (-0.82 seconds) 4096 5.46 83.81% (-1.05 seconds) 5120 5.26 80.76% (-1.25 seconds) 6144 5.13 78.79% (-1.38 seconds) 7168 5.09 78.13% (-1.42 seconds) 8192 5.01 76.92% (-1.50 seconds) 10240 4.94 75.83% (-1.58 seconds) 12288 4.87 74.74% (-1.65 seconds) 14336 4.85 74.49% (-1.66 seconds) 16384 4.82 73.95% (-1.70 seconds) Saving that 1.7 seconds is worth it to me... On a slower connection - I would guess that the difference is going to be more dramatic... I needed to modify your patch a little bit to get it working on my platform. If Ben/Wolfgang are interested in taking this - I'll fix up my mods, and send it back. -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Add md5sum and sha1 commands...
From: Robin Getz Now that we have sha1 and md5 in lib_generic, allow people to use them on the command line, for checking downloaded files Signed-off-by: Robin Getz README |4 ++ common/cmd_mem.c | 73 + 2 files changed, 77 insertions(+) --- diff --git a/README b/README index 9071472..cf3867d 100644 --- a/README +++ b/README @@ -629,6 +629,8 @@ The following options need to be configured: CONFIG_CMD_KGDB * kgdb CONFIG_CMD_LOADB loadb CONFIG_CMD_LOADS loads + CONFIG_CMD_MD5SUM print md5 message digest + (requires CONFIG_CMD_MEMORY and CONFIG_MD5) CONFIG_CMD_MEMORY md, mm, nm, mw, cp, cmp, crc, base, loop, loopw, mtest CONFIG_CMD_MISC Misc functions like sleep etc @@ -652,6 +654,8 @@ The following options need to be configured: (requires CONFIG_CMD_I2C) CONFIG_CMD_SETGETDCR Support for DCR Register access (4xx only) + CONFIG_CMD_SHA1 print sha1 memory digest + (requires CONFIG_CMD_MEMORY) CONFIG_CMD_SOURCE "source" command Support CONFIG_CMD_SPI * SPI serial bus support CONFIG_CMD_USB * USB support diff --git a/common/cmd_mem.c b/common/cmd_mem.c index cdf8c79..ac2492c 100644 --- a/common/cmd_mem.c +++ b/common/cmd_mem.c @@ -34,6 +34,14 @@ #endif #include +#ifdef CONFIG_CMD_MD5SUM +#include +#endif + +#ifdef CONFIG_CMD_SHA1 +#include +#endif + #ifdef CMD_MEM_DEBUG #definePRINTF(fmt,args...) printf (fmt ,##args) #else @@ -1141,6 +1149,55 @@ int do_mem_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) } #endif /* CONFIG_CRC32_VERIFY */ +#ifdef CONFIG_CMD_MD5SUM +int do_md5sum(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned long addr, len; + unsigned int i; + u8 output[16]; + + if (argc < 3) { + cmd_usage(cmdtp); + return 1; + } + + addr = simple_strtoul(argv[1], NULL, 16); + len = simple_strtoul(argv[2], NULL, 16); + + md5((unsigned char *) addr, len, output); + printf("md5 for %08lx ... %08lx ==> ", addr, addr + len - 1); + for (i = 0; i < 16 ; i++) + printf("%02x", output[i]); + printf("\n"); + + return 0; +} +#endif + +#ifdef CONFIG_CMD_SHA1 +int do_sha1(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned long addr, len; + unsigned int i; + u8 output[20]; + + if (argc < 3) { + cmd_usage(cmdtp); + return 1; + } + + addr = simple_strtoul(argv[1], NULL, 16); + len = simple_strtoul(argv[2], NULL, 16); + + sha1_csum((unsigned char *) addr, len, output); + printf("SHA1 for %08lx ... %08lx ==> ", addr, addr + len - 1); + for (i = 0; i < 20 ; i++) + printf("%02x", output[i]); + printf("\n"); + + return 0; +} +#endif #ifdef CONFIG_CMD_UNZIP int gunzip (void *, int, unsigned char *, unsigned long *); @@ -1267,6 +1324,22 @@ U_BOOT_CMD( ); #endif /* CONFIG_MX_CYCLIC */ +#ifdef CONFIG_CMD_MD5SUM +U_BOOT_CMD( + md5sum, 3, 1, do_md5sum, + "compute MD5 message digest", + "address count" +); +#endif + +#ifdef CONFIG_CMD_SHA1 +U_BOOT_CMD( + sha1, 3, 1, do_sha1, + "compute SHA1 message digest", + "address count" +); +#endif /* CONFIG_CMD_SHA1 */ + #ifdef CONFIG_CMD_UNZIP U_BOOT_CMD( unzip, 4, 1, do_unzip, ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
On Fri 24 Jul 2009 04:11, Alessandro Rubini pondered: > BTW: you (Jeffery) told you tried a blocksize of 16k, but how could > it work if u-boot refuses fragments? Did you add defragmenting > to the code first? I believe it depends on if it your host has gig support (for jumbo frames) Most Gig hosts/routers/gateways nowadays do... http://en.wikipedia.org/wiki/Jumbo_frame However - it looks like it might be a bug in the Linux networking stack (I'll ask on netdev) that if the Gig card is connected as 100, it should not be sending jumbo frames (from what I understand -- it is a spec violation). I tried on my subnet - and was able to transfer 4k packets without issue (which is as big as I tested). (dumb repeater/hub between U-Boot & the tftp server). However - my MAC running on U-Boot is 10/100 and does not support jumbo frames, and tosses the frame as a "Frame too long", and is truncated to 1556 (0x614) bytes in all cases, so if the host is not fragmenting things properly - I can't test it... :( ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] minor debug cleanups in ./net
On Thu 23 Jul 2009 02:27, Ben Warren pondered: > Robin, > > This won't apply: > > bwar...@bwarren-bldsrv:~/src/u-boot-net$ git am -s --whitespace=strip > ~/h_drive/patches/minor\ debug\ cleanups\ in\ ._net.eml > Applying minor debug cleanups in ./net > fatal: patch fragment without header at line 198: @@ -879,13 +856,13 @@ > DhcpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len) > Patch failed at 0001. > When you have resolved this problem run "git-am --resolved". > If you would prefer to skip this patch, instead run "git-am --skip". OK - this is on git remote -v origin git://git.denx.de/u-boot-net.git git log --max-count=1 commit 97cfe86163505ea18e7ff7b71e78df5bb03dad57 (Is there a better way to tell if git is up to date?) --- From: Robin Getz Minor ./net cleanups - no functional changes - change #ifdef DEBUG printf(); #endif to just debug() - changed __FUNCTION__ to __func__ - got rid of extra whitespace between function and opening brace - removed unnecessary braces on if statements gcc dead code elimination should make this functionally/size equivalent when DEBUG is not defined. (confirmed on Blackfin, with gcc 4.3.3). Signed-off-by: Robin Getz --- diff --git a/net/Makefile b/net/Makefile index 835a04a..ff87d87 100644 --- a/net/Makefile +++ b/net/Makefile @@ -23,7 +23,7 @@ include $(TOPDIR)/config.mk -# CFLAGS += -DET_DEBUG -DDEBUG +# CFLAGS += -DDEBUG LIB= $(obj)libnet.a diff --git a/net/bootp.c b/net/bootp.c index d5f9c4b..0799ae2 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -8,17 +8,6 @@ * Copyright 2000-2004 Wolfgang Denk, w...@denx.de */ -#if 0 -#define DEBUG 1 /* general debug */ -#define DEBUG_BOOTP_EXT 1 /* Debug received vendor fields */ -#endif - -#ifdef DEBUG_BOOTP_EXT -#define debug_ext(fmt,args...) printf (fmt ,##args) -#else -#define debug_ext(fmt,args...) -#endif - #include #include #include @@ -107,7 +96,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len) retval = -6; } - debug ("Filtering pkt = %d\n", retval); + debug("Filtering pkt = %d\n", retval); return retval; } @@ -129,7 +118,7 @@ static void BootpCopyNetParams(Bootp_t *bp) if (strlen(bp->bp_file) > 0) copy_filename (BootFile, bp->bp_file, sizeof(BootFile)); - debug ("Bootfile: %s\n", BootFile); + debug("Bootfile: %s\n", BootFile); /* Propagate to environment: * don't delete exising entry when BOOTP / DHCP reply does @@ -156,7 +145,7 @@ static void BootpVendorFieldProcess (u8 * ext) { int size = *(ext + 1); - debug_ext ("[BOOTP] Processing extension %d... (%d bytes)\n", *ext, + debug("[BOOTP] Processing extension %d... (%d bytes)\n", *ext, *(ext + 1)); NetBootFileSize = 0; @@ -255,7 +244,7 @@ static void BootpVendorProcess (u8 * ext, int size) { u8 *end = ext + size; - debug_ext ("[BOOTP] Checking extension (%d bytes)...\n", size); + debug("[BOOTP] Checking extension (%d bytes)...\n", size); while ((ext < end) && (*ext != 0xff)) { if (*ext == 0) { @@ -269,34 +258,27 @@ static void BootpVendorProcess (u8 * ext, int size) } } -#ifdef DEBUG_BOOTP_EXT - puts ("[BOOTP] Received fields: \n"); + debug("[BOOTP] Received fields: \n"); if (NetOurSubnetMask) - printf ("NetOurSubnetMask : %pI4\n", &NetOurSubnetMask); + debug("NetOurSubnetMask : %pI4\n", &NetOurSubnetMask); if (NetOurGatewayIP) - printf ("NetOurGatewayIP: %pI4", &NetOurGatewayIP); + debug("NetOurGatewayIP : %pI4", &NetOurGatewayIP); - if (NetBootFileSize) { - printf ("NetBootFileSize : %d\n", NetBootFileSize); - } + if (NetBootFileSize) + debug("NetBootFileSize : %d\n", NetBootFileSize); - if (NetOurHostName[0]) { - printf ("NetOurHostName : %s\n", NetOurHostName); - } + if (NetOurHostName[0]) + debug("NetOurHostName : %s\n", NetOurHostName); - if (NetOurRootPath[0]) { - printf ("NetOurRootPath : %s\n", NetOurRootPath); - } + if (NetOurRootPath[0]) + debug("NetOurRootPath : %s\n", NetOurRootPath); - if (NetOurNISDomain[0]) { - printf ("NetOurNISDomain : %s\n", NetOurNISDomain); - } + if (NetOurNISDomain[0]) + debug("NetOurNISDomain : %s\n", NetOurNISDomain); - if (NetBootFileSize) { -
[U-Boot] minor debug cleanups in ./net
From: Robin Getz Minor ./net cleanups - no functional changes - change #ifdef DEBUG printf(); #endif to just debug() - changed __FUNCTION__ to __func__ - got rid of extra whitespace between function and opening brace - removed unnecessary braces on if statements gcc dead code elimination should make this functionally/size equivalent when DEBUG is not defined. (confirmed on Blackfin, with gcc 4.3.3). Signed-off-by: Robin Getz --- Most changes are: -#ifdef DEBUG - printf("packet received\n"); -#endif + debug("packet received\n"); which is just plain nicer to read... Makefile |2 - bootp.c | 81 ++--- eth.c|8 ++--- net.c| 78 --- nfs.c| 42 ++- rarp.c |4 -- sntp.c |6 +-- tftp.c | 21 +++-- 8 files changed, 78 insertions(+), 164 deletions(-) --- diff --git a/net/Makefile b/net/Makefile index 835a04a..ff87d87 100644 --- a/net/Makefile +++ b/net/Makefile @@ -23,7 +23,7 @@ include $(TOPDIR)/config.mk -# CFLAGS += -DET_DEBUG -DDEBUG +# CFLAGS += -DDEBUG LIB= $(obj)libnet.a diff --git a/net/bootp.c b/net/bootp.c index d5f9c4b..0799ae2 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -8,17 +8,6 @@ * Copyright 2000-2004 Wolfgang Denk, w...@denx.de */ -#if 0 -#define DEBUG 1 /* general debug */ -#define DEBUG_BOOTP_EXT 1 /* Debug received vendor fields */ -#endif - -#ifdef DEBUG_BOOTP_EXT -#define debug_ext(fmt,args...) printf (fmt ,##args) -#else -#define debug_ext(fmt,args...) -#endif - #include #include #include @@ -107,7 +96,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len) retval = -6; } - debug ("Filtering pkt = %d\n", retval); + debug("Filtering pkt = %d\n", retval); return retval; } @@ -129,7 +118,7 @@ static void BootpCopyNetParams(Bootp_t *bp) if (strlen(bp->bp_file) > 0) copy_filename (BootFile, bp->bp_file, sizeof(BootFile)); - debug ("Bootfile: %s\n", BootFile); + debug("Bootfile: %s\n", BootFile); /* Propagate to environment: * don't delete exising entry when BOOTP / DHCP reply does @@ -156,7 +145,7 @@ static void BootpVendorFieldProcess (u8 * ext) { int size = *(ext + 1); - debug_ext ("[BOOTP] Processing extension %d... (%d bytes)\n", *ext, + debug("[BOOTP] Processing extension %d... (%d bytes)\n", *ext, *(ext + 1)); NetBootFileSize = 0; @@ -255,7 +244,7 @@ static void BootpVendorProcess (u8 * ext, int size) { u8 *end = ext + size; - debug_ext ("[BOOTP] Checking extension (%d bytes)...\n", size); + debug("[BOOTP] Checking extension (%d bytes)...\n", size); while ((ext < end) && (*ext != 0xff)) { if (*ext == 0) { @@ -269,34 +258,27 @@ static void BootpVendorProcess (u8 * ext, int size) } } -#ifdef DEBUG_BOOTP_EXT - puts ("[BOOTP] Received fields: \n"); + debug("[BOOTP] Received fields: \n"); if (NetOurSubnetMask) - printf ("NetOurSubnetMask : %pI4\n", &NetOurSubnetMask); + debug("NetOurSubnetMask : %pI4\n", &NetOurSubnetMask); if (NetOurGatewayIP) - printf ("NetOurGatewayIP: %pI4", &NetOurGatewayIP); + debug("NetOurGatewayIP : %pI4", &NetOurGatewayIP); - if (NetBootFileSize) { - printf ("NetBootFileSize : %d\n", NetBootFileSize); - } + if (NetBootFileSize) + debug("NetBootFileSize : %d\n", NetBootFileSize); - if (NetOurHostName[0]) { - printf ("NetOurHostName : %s\n", NetOurHostName); - } + if (NetOurHostName[0]) + debug("NetOurHostName : %s\n", NetOurHostName); - if (NetOurRootPath[0]) { - printf ("NetOurRootPath : %s\n", NetOurRootPath); - } + if (NetOurRootPath[0]) + debug("NetOurRootPath : %s\n", NetOurRootPath); - if (NetOurNISDomain[0]) { - printf ("NetOurNISDomain : %s\n", NetOurNISDomain); - } + if (NetOurNISDomain[0]) + debug("NetOurNISDomain : %s\n", NetOurNISDomain); - if (NetBootFileSize) { - printf ("NetBootFileSize: %d\n", NetBootFileSize); - } -#endif /* DEBUG_BOOTP_EXT */ + if (NetBootFileSize) + debug("NetBootFileSize: %d\n", NetBootFileSize); } /* * Handle a BOOTP received packet. @@ -307,7 +289,7 @@ BootpHandler(uchar * pkt, unsigned
Re: [U-Boot] http client?
On Wed 22 Jul 2009 18:00, Alessandro Rubini pondered: > > When I looked at the RFC data > > it appears that the overhead of IP fragmentation and reassembly (which > > does add overhead as the number of gateways increases) may be worth > > the time... > > Just tried it: U-Boot doesn't support reassembly, it seems. I don't think anyone said it did. > net.c confirms it: > > /* Can't deal with fragments */ > if (ip->ip_off & htons(IP_OFFS | IP_FLAGS_MFRAG)) { > return; > } > > I don't think it's worth adding. This is based on ... ? If it reduces download time by 1/2 (1432 byte block size == 13.70 seconds, 4096 byte block size == 6.85 seconds) it might be worth the complexity... At least worth it enough to give it a try, gather some results, and then make a decision. -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
On Wed 22 Jul 2009 16:53, Ben Warren pondered: > Robin Getz wrote: > > I see: > > > > #define TFTP_MTU_BLOCKSIZE 1468blksize > > static unsigned short TftpBlkSizeOption=TFTP_MTU_BLOCKSIZE; > > > > /* try for more effic. blk size */ > > pkt += sprintf((char *)pkt,"blksize%c%d%c", > > 0,TftpBlkSizeOption,0); > > > > but that is it... > > > > No CONFIG_ options for anything else? > > > > > Right, it's hard-coded to 1468 (maximum TFTP frame that will fit in a > 1500-byte Ethernet frame, due to UDP overhead). By default, TFTP > requests a blocksize that will fill the frame. If not, it uses the > default TFTP block size (512, I think). > > Is this not good enough? When I looked at the RFC data blocksize no gateway with gateway - -- 51223.85 37.05 102416.15 25.65 143213.70 23.10 204810.90 16.90 4096 6.85 9.65 8192 4.90 6.15 it appears that the overhead of IP fragmentation and reassembly (which does add overhead as the number of gateways increases) may be worth the time... -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
On Wed 22 Jul 2009 16:32, Ben Warren pondered:> Robin Getz wrote: > > On Wed 22 Jul 2009 10:04, jeffery palmer pondered: > > > >> We are looking for an http client now as well. Our major issue revolves > >> > > around the download times for tftp. > > > >> > >> Can Volkmar Uhlig kindly provide the patches? > >> > >> Our units automically update themselves inside of uboot giving us the most > >> control over our firmware. The issue is that it takes 20 minutes via a DSL > >> line in Africa to update our units. An http test showed that the same > >> firmware downloads in 30 seconds. We have also added things like the > >> blksize > >> parameter to the uboot tftp client to get it down to 20 minutes, our > >> original download times were ~50 minutes. > >> > > > > Hmm -- I'm assuming that is http://www.faqs.org/rfcs/rfc1783.html ? > > > > Do you have a patch to send - or that I can clean up and submit? > > > > > Requesting a bigger blocksize is already implemented and should work if > the server supports it. It's been a while since I used this, but it was > added along with support for multicast TFTP, probably about a year ago. I see: #define TFTP_MTU_BLOCKSIZE 1468blksize static unsigned short TftpBlkSizeOption=TFTP_MTU_BLOCKSIZE; /* try for more effic. blk size */ pkt += sprintf((char *)pkt,"blksize%c%d%c", 0,TftpBlkSizeOption,0); but that is it... No CONFIG_ options for anything else? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Less verbose output when loading vxworks 6.x images
On Wed 22 Jul 2009 15:32, Niklaus Giger pondered: > Loading vxWorks 5.x images resulted just into 3 or 4 lines of output. > With vxWorks 6.x and the new GCC it emits about 30 lines, which is > far too noisy in my opinion. > > Signed-off-by: Niklaus Giger > --- > common/cmd_elf.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/common/cmd_elf.c b/common/cmd_elf.c > index abec7dd..0842ce9 100644 > --- a/common/cmd_elf.c > +++ b/common/cmd_elf.c > @@ -286,6 +286,7 @@ unsigned long load_elf_image (unsigned long addr) > continue; > } > > +#ifdef DEBUG > if (strtab) { > printf ("%sing %s @ 0x%08lx (%ld bytes)\n", > (shdr->sh_type == SHT_NOBITS) ? > @@ -294,6 +295,7 @@ unsigned long load_elf_image (unsigned long addr) > (unsigned long) shdr->sh_addr, > (long) shdr->sh_size); > } > +#endif > > if (shdr->sh_type == SHT_NOBITS) { > memset ((void *)shdr->sh_addr, 0, > shdr->sh_size); its better to remove the ifdef, and replace the printf() with debug() (IMHO). ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
On Wed 22 Jul 2009 10:04, jeffery palmer pondered: > We are looking for an http client now as well. Our major issue revolves around the download times for tftp. > > Can Volkmar Uhlig kindly provide the patches? > > Our units automically update themselves inside of uboot giving us the most > control over our firmware. The issue is that it takes 20 minutes via a DSL > line in Africa to update our units. An http test showed that the same > firmware downloads in 30 seconds. We have also added things like the blksize > parameter to the uboot tftp client to get it down to 20 minutes, our > original download times were ~50 minutes. Hmm -- I'm assuming that is http://www.faqs.org/rfcs/rfc1783.html ? Do you have a patch to send - or that I can clean up and submit? > We would like to work to integrate or add the necessary http client > functions to achieve this. If there are no patches obtainable, is anyone > interested in working on this with us? Yes - Although it sounds like your schedule is more immediate than mine... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
On Tue 21 Jul 2009 17:09, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200907211400.21275.rg...@blackfin.uclinux.org> you wrote: > > > > > I know there have been discussions about adding wget to U-Boot, > > > which I agree is not something that is worthwhile to do. > > I am not so sure about that, but that's just my opinion. > > > > http://lists.denx.de/pipermail/u-boot/2006-March/013697.html > > > but a simple client would be helpful in many situations. > > There is such in IBM's port of U-Boot for the Blue Gene super- > computer, and Volkmar Uhlig promised to submit patches. But so far he > didn't. Hmm.. That sounds interesting. Do you know if there is a public repository? or Do I need to buy a Blue Gene to get access to the source :) According to: http://domino.research.ibm.com/comm/research_projects.nsf/pages/kittyhawk.index.html/$FILE/Kittyhawk%20OSR%2008.pdf >We added a simple HTTP server to U-Boot so that individual nodes can be > booted via a PUT command that pushes the desired boot images and kernel > command line. The command line is evaluated via the scripting environment > before it gets passed on to the OS kernel thereby allowing per node > customizations. With this simple extension to U-Boot we are able to > construct powerful scripted environments and bootstrap large numbers of > nodes in a controlled, flexible, and secure way. What is in RedBoot is a "HTTP GET". So -- while it shouldn't be hard to extend from there... > > > If a port of the redboot functionality is done - any thoughts on if > it would > > > be accepted? > > Well, you are aware that such a thing would need a TCP/IP stack, which > is far beyond what we currently have in U-Boot support? Yes - I'm aware. My thoughts were to do something like what IBM and other suggestions were - use lwip. -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] http client?
Sorry - first time I sent this -- I forgot to cc the list... On Tue 21 Jul 2009 12:37, Robin Getz pondered: > redboot supports (and has since 2002) a mini-http client: > > This is just a transfer data via the network using HTTP protocol, no > better or worse than tftp. (no https, no proxy, no other protocols). > > http://ecos.sourceware.org/cgi-bin/cvsweb.cgi/ecos/packages/redboot/current/src/net/http_client.c?rev=1.11&content-type=text/x-cvsweb-markup&cvsroot=ecos > > I know there have been discussions about adding wget to U-Boot, which > I agree is not something that is worthwhile to do. > > http://lists.denx.de/pipermail/u-boot/2006-March/013697.html > > but a simple client would be helpful in many situations. > > If a port of the redboot functionality is done - any thoughts on if it would > be accepted? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] - save the server's mac address...
On Tue 21 Jul 2009 02:37, Ben Warren pondered: > Can you please re-submit using git tools? From: Robin Getz Linux's netconsole works much better when you can pass it the MAC address of the server. (otherwise it just uses broadcast, which everyone else on my network complains about :) This sets the env var "serveraddr" (to match ethaddr), so that you can pass it to linux with whatever bootargs you want to addnetconsole=set bootargs $(bootargs) netconso...@$(ipaddr)/eth0,@$(serverip)/$(serveraddr) Signed-of-by: Robin Getz - diff --git a/README b/README index 4c74cb7..9071472 100644 --- a/README +++ b/README @@ -1184,6 +1184,11 @@ The following options need to be configured: Defines a default value for the IP address of a TFTP server to contact when using the "tftboot" command. + CONFIG_KEEP_SERVERADDR + + Keeps the server's MAC address, in the env 'serveraddr' + for passing to bootargs (like Linux's netconsole option) + - Multicast TFTP Mode: CONFIG_MCAST_TFTP diff --git a/net/net.c b/net/net.c index 7ce947d..641c37c 100644 --- a/net/net.c +++ b/net/net.c @@ -1287,6 +1287,15 @@ NetReceive(volatile uchar * inpkt, int len) /* are we waiting for a reply */ if (!NetArpWaitPacketIP || !NetArpWaitPacketMAC) break; + +#ifdef CONFIG_KEEP_SERVERADDR + if (NetServerIP == NetArpWaitPacketIP) { + char buf[20]; + sprintf(buf, "%pM", arp->ar_data); + setenv("serveraddr", buf); + } +#endif + #ifdef ET_DEBUG printf("Got ARP REPLY, set server/gtwy eth addr (%pM)\n", arp->ar_data); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] CONFIG naming convetion
On Mon 20 Jul 2009 16:33, Wolfgang Denk pondered: > Dear Robin Getz, [snip] > You seem to live on a different planet than me. That is a well though out point. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] v3 - add dns
On 04 Oct 2008 Pieter posted a dns implementation for U-Boot. http://www.mail-archive.com/u-boot-us...@lists.sourceforge.net/msg10216.html > > DNS can be enabled by setting CFG_CMD_DNS. After performing a query, > the serverip environment var is updated. > > Probably there are some cosmetic issues with the patch. Unfortunatly I > do not have the time to correct these. So if anybody else likes DNS > support in U-Boot and has the time, feel free to patch it in the main tree. Here it is again - slightly modified & smaller: - update to 2009-06 (Pieter's patch was for U-Boot 1.2.0) - README.dns is added - syntax is changed (now takes a third option, the env var to store the result in) - add a random port() function in net.c - sort Makefile in ./net/Makefile - dns just returns unless a env var is given - run through checkpatch, and clean up style issues - remove packet from stack - cleaned up some comments - failure returns much faster (if server responds, don't wait for timeout) - use built in functions (memcpy) rather than byte copy. Signed-off-by: Robin Getz Signed-off-by: Pieter Voorthuijsen common/cmd_net.c | 49 ++ doc/README.dns | 64 + include/net.h|5 + net/Makefile |7 - net/dns.c| 211 + net/dns.h| 39 net/net.c| 29 ++ 7 files changed, 401 insertions(+), 3 deletions(-) --- diff --git a/common/cmd_net.c b/common/cmd_net.c index 68183c4..ac706ae 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -353,3 +353,52 @@ U_BOOT_CMD( "[NTP server IP]\n" ); #endif + +#if defined(CONFIG_CMD_DNS) +int do_dns(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + if (argc == 1) { + cmd_usage(cmdtp); + return -1; + } + + /* +* We should check for a valid hostname: +* - Each label must be between 1 and 63 characters long +* - the entire hostname has a maximum of 255 characters +* - only the ASCII letters 'a' through 'z' (case-insensitive), +* the digits '0' through '9', and the hyphen +* - cannot begin or end with a hyphen +* - no other symbols, punctuation characters, or blank spaces are +* permitted +* but hey - this is a minimalist implmentation, so only check length +* and let the name server deal with things. +*/ + if (strlen(argv[1]) >= 255) { + printf("dns error: hostname too long\n"); + return 1; + } + + NetDNSResolve = argv[1]; + + if (argc == 3) + NetDNSenvvar = argv[2]; + else + NetDNSenvvar = NULL; + + if (NetLoop(DNS) < 0) { + printf("dns lookup of %s failed, check setup\n", argv[1]); + return 1; + } + + return 0; +} + +U_BOOT_CMD( + dns,3, 1, do_dns, + "lookup the IP of a hostname", + "hostname [envvar]" +); + +#endif /* CONFIG_CMD_DNS */ + diff --git a/doc/README.dns b/doc/README.dns new file mode 100644 index 000..deeccd7 --- /dev/null +++ b/doc/README.dns @@ -0,0 +1,64 @@ +Domain Name System +--- + +The Domain Name System (DNS) is a hierarchical naming system for computers, +services, or any resource participating in the Internet. It associates various +information with domain names assigned to each of the participants. Most +importantly, it translates domain names meaningful to humans into the numerical +(binary) identifiers associated with networking equipment for the purpose of +locating and addressing these devices world-wide. An often used analogy to +explain the Domain Name System is that it serves as the "phone book" for the +Internet by translating human-friendly computer hostnames into IP addresses. +For example, www.example.com translates to 208.77.188.166. + +For more information on DNS - http://en.wikipedia.org/wiki/Domain_Name_System + + + +U-Boot and DNS +-- + +CONFIG_CMD_DNS - controls if the 'dns' command is compiled in. If it is, it + will send name lookups to the dns server (env var 'dnsip') + Turning this option on will about abou 1k to U-Boot's size. + + Example: + +bfin> print dnsip +dnsip=192.168.0.1 + +bfin> dns www.google.com +66.102.1.104 + + By default, dns does nothing except print the IP number on + the default console - which by itself, would be pretty + useless. Adding a third argument to the dns command will + use that as the environment variable to be set. + + Example: + +bfin> print googleip +## Error: &qu
Re: [U-Boot] [RFC] CONFIG naming convetion
On Sat 18 Jul 2009 18:25, Wolfgang Denk pondered: > > > > I guess we could back up a step and look at the users, and defined things > > as things that should be changed by: > > - arch maintainers (Core/CPU specific) > > - SoC maintainer (SoC specific) > > - Board maintainer (PCB specific) > > - End user of the above (changes options, but nothing from the above > list?) > > Frankly, this makes no sense to me. I have yet to see any such clear > split of roles and functions. When you bring up a new board, you > usually have to check everything. > > The only split that made, and still makes, kind of sense to me is the > split into normal users (CONFIG_) versus "root" (CONFIG_SYS_) groups. I guess I still don't understand how you are making the distinction between "normal users" and "root" - I think there are more categories of users between those two... People responsible for the archicture/CPU core may set things up, and not want anyone to change things - on any SoC or Board. People responsible for SoC developments should be able to take what the arch provider delivers, write a few device drivers, make some specific choices that anyone who implements that SoC is going to have to live with. People responsible for Board porting, should be able to take what the SoC provider delivers, customise things for their platform, and move on. Then there are end users - which must live with the choices that all three have made, until they get their own hardware back, or in the case where the hardware is a module - just change some non-hardware related options. So maybe it is core, chip, PCB, and user. In some cases - all 4 categories are the same person - in many cases they are not. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] - add dns
On Sat 18 Jul 2009 21:15, Mike Frysinger pondered: > On Saturday 18 July 2009 20:27:00 Robin Getz wrote: > > On Sat 18 Jul 2009 18:11, Mike Frysinger pondered: > > > keep the modulus something with only 1 bit set so gcc will optimize into > > > a simple and operation. probably add a comment about it too: > > > /* make src port a little random, but use something trivial to compute > > > */ > > > > OK - So, this would give three different variations: > > So, these are definitality ugly... > > net/tftp.c: TftpOurPort = 1024 + (get_timer(0) % 3072); <_random_port>: 0: 78 05 [--SP] = (R7:7); 2: 67 01 [--SP] = RETS; 4: a6 6f SP += -0xc; /* (-12) */ 6: 00 60 R0 = 0x0 (X); /* R0=0x0( 0) */ 8: ff e3 fc ff CALL 0x0 <_random_port>; a: R_BFIN_PCREL24 _get_timer c: 41 e1 aa aa R1.H = 0x; /* (-21846) R1=0x000c(-1431699444) */ 10: 01 e1 ab aa R1.L = 0xaaab; /* (-21845) R1=0xaaab(-1431655765) */ 14: 38 30 R7 = R0; 16: ff e3 f5 ff CALL 0x0 <_random_port>; 18: R_BFIN_PCREL24 ___umulsi3_highpart 1a: 58 4e R0 >>= 0xb; 1c: 21 e1 00 0c R1 = 0xc00 (X); /* R1=0xc00(3072) */ 20: c8 40 R0 *= R1; 22: 66 6c SP += 0xc; /* ( 12) */ 24: c7 53 R7 = R7 - R0; 26: 20 e1 00 04 R0 = 0x400 (X); /* R0=0x400(1024) */ 2a: c7 51 R7 = R7 + R0; 2c: 27 01 RETS = [SP++]; 2e: 07 30 R0 = R7; 30: 38 05 (R7:7) = [SP++]; 32: 10 00 RTS; Disassembly of section .text.NetSetTimeout: > > net/nfs.c: NfsOurPort = 4096 + (get_ticks() % 3072); <_random_port>: 0: 67 01 [--SP] = RETS; 2: 86 6f SP += -0x10;/* (-16) */ 4: ff e3 fe ff CALL 0x0 <_random_port>; 6: R_BFIN_PCREL24 _get_ticks 8: 02 60 R2 = 0x0 (X); /* R2=0x0( 0) */ a: f2 b0 [SP + 0xc] = R2; c: 22 e1 00 0c R2 = 0xc00 (X); /* R2=0xc00(3072) */ 10: ff e3 f8 ff CALL 0x0 <_random_port>; 12: R_BFIN_PCREL24 ___umoddi3 14: 86 6c SP += 0x10; /* ( 16) */ 16: 08 30 R1 = R0; 18: 27 01 RETS = [SP++]; 1a: 20 e1 00 10 R0 = 0x1000 (X);/* R0=0x1000(4096) */ 1e: 01 50 R0 = R1 + R0; 20: 10 00 RTS; > > net/sntp.c: SntpOurPort = 1 + (get_timer(0) % 4096); <_random_port>: 0: 67 01 [--SP] = RETS; 2: a6 6f SP += -0xc; /* (-12) */ 4: 00 60 R0 = 0x0 (X); /* R0=0x0( 0) */ 6: ff e3 fd ff CALL 0x0 <_random_port>; 8: R_BFIN_PCREL24 _get_timer a: 21 e1 ff 0f R1 = 0xfff (X); /* R1=0xfff(4095) */ e: 66 6c SP += 0xc; /* ( 12) */ 10: 08 54 R0 = R0 & R1; 12: 27 01 RETS = [SP++]; 14: 21 e1 10 27 R1 = 0x2710 (X);/* R1=0x2710(1) */ 18: 08 50 R0 = R0 + R1; 1a: 10 00 RTS; Disassembly of section .text.NetSetTimeout: > 1024 + (get_timer(0) % 0x8000); <_random_port>: 0: 67 01 [--SP] = RETS; 2: a6 6f SP += -0xc; /* (-12) */ 4: 00 60 R0 = 0x0 (X); /* R0=0x0( 0) */ 6: ff e3 fd ff CALL 0x0 <_random_port>; 8: R_BFIN_PCREL24 _get_timer a: 21 e1 ff 7f R1 = 0x7fff (X);/* R1=0x7fff(32767) */ e: 66 6c SP += 0xc; /* ( 12) */ 10: 08 54 R0 = R0 & R1; 12: 27 01 RETS = [SP++]; 14: 21 e1 00 04 R1 = 0x400 (X); /* R1=0x400(1024) */ 18: 08 50 R0 = R0 + R1; 1a: 10 00 RTS; Disassembly of section .text.NetSetTimeout: > > Does it make sense to have 4 different ones? (not to me)... > > > > Or something new & common in ./net.c:random_port() > > i would make a new patch that adds a new random_port() function and converts > existing consumers to that, and then have the dns code rely on that. > > and you should adopt my implementation because the generated code is > much much nicer than the others ;) At least on Blackfin - the sntp version and yours are computationally equal - although I think yours ends up being more rando
Re: [U-Boot] [PATCH] - add dns
On Sun 19 Jul 2009 03:48, Wolfgang Denk pondered: > Dear Robin Getz, > In message <200907172120.50413.rg...@blackfin.uclinux.org> you wrote: > > On Fri 17 Jul 2009 16:55, Wolfgang Denk pondered: > > > Please keep list sorted. > > > > sorted how? What we have today is: > > > > COBJS-y += net.o > > COBJS-y += tftp.o > > COBJS-y += bootp.o > > COBJS-y += rarp.o > > COBJS-y += eth.o > > COBJS-y += nfs.o > > COBJS-$(CONFIG_CMD_SNTP) += sntp.o > > > > It is not sorted alphabetically... ??? > > I see. Sorry. Well, please use the opportunity to sort that list, > then. Alphabetically. > Ok -- what I have is this then. I'll wrap it up tomorrow and send to Ben, assuming he doesn't have anything else... diff --git a/net/Makefile b/net/Makefile index d341874..835a04a 100644 --- a/net/Makefile +++ b/net/Makefile @@ -27,13 +27,14 @@ include $(TOPDIR)/config.mk LIB= $(obj)libnet.a -COBJS-y += net.o -COBJS-y += tftp.o COBJS-y += bootp.o -COBJS-y += rarp.o +COBJS-$(CONFIG_CMD_DNS) += dns.o COBJS-y += eth.o +COBJS-y += net.o COBJS-y += nfs.o +COBJS-y += rarp.o COBJS-$(CONFIG_CMD_SNTP) += sntp.o +COBJS-y += tftp.o COBJS := $(COBJS-y) SRCS := $(COBJS:.o=.c) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] - add dns
On Sat 18 Jul 2009 18:11, Mike Frysinger pondered: > On Saturday 18 July 2009 01:14:25 Robin Getz wrote: > > + DnsOurPort = 1 + (get_timer(0) % 4096); > > 4096 port range seems kind of small. i dont think the requests really need > to > be greater than 1. not sure if services would get pissed about being > below the 1024 limit though, so this is probably better: > 1024 + (get_timer() % 0x8000); Sure. > keep the modulus something with only 1 bit set so gcc will optimize into a > simple and operation. probably add a comment about it too: > /* make src port a little random, but use something trivial to compute > */ OK - So, this would give three different variations: net/sntp.c: SntpOurPort = 1 + (get_timer(0) % 4096); net/tftp.c: TftpOurPort = 1024 + (get_timer(0) % 3072); net/nfs.c: NfsOurPort = 4096 + (get_ticks() % 3072); Does it make sense to have 4 different ones? (not to me)... Or something new & common in ./net.c:random_port() Ben? > > +void > > +DnsStart(void) > > +{ > > + NetSetTimeout(DNS_TIMEOUT, DnsTimeout); > > + NetSetHandler(DnsHandler); > > + memset(NetServerEther, 0, 6); > > is clearing the ether address really necessary ? if so, why should the dns > code care about it ? Nope - I can remove that... > > +/* http://en.wikipedia.org/wiki/List_of_DNS_record_types */ > > +enum dns_query_type { > > + DNS_A_RECORD = 0x01, > > + DNS_CNAME_RECORD = 0x05, > > + DNS_MX_RECORD = 0x0f }; > > that last }; should be on a line by itself, and the last entry should still > have a comma at the end Hmm - didn't notice that one from the orginal. Thanks (I'm surprised that checkpatch didn't complain). Since there aren't any functionality differences - I'll send out a new version on Monday for Ben - since he is away anyway (unless someone else comments tomorrow). -Robin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] CONFIG naming convetion
On Sat 18 Jul 2009 11:52, Jean-Christophe PLAGNIOL-VILLARD pondered: > On 11:15 Sat 18 Jul , Robin Getz wrote: > > It doesn't appear very "system" oriented to me... > it's as we had reorganise the drivers file place as example or > the common Makefile and continue to do it eveyday (take a look on Peter e-mail > about arch dir) > > > > It would be nice to come up with some list of namespaces, and what they > > they should be used for... > > > > For example, should it be: > > CONFIG_DRIVER_OMAP24XX_I2C > > or > > CONFIG_SYS_I2C_DRIVER_OMAP24XX > > or > > CONFIG_DRIVER_I2C_OMAP24XX > > DRIVER is really un-nessary IMHO It doesn't cost extra $ to keep 6 letters - and makes it alot more clear about what it does. I understand the point about having really _long_ names, and personally think things like "ThisVariableIsATemporaryCounter" are brain dead, but I don't think we have hit that yet with CONFIG_ yet. We already have lots (419) that are over 30 chars, and some over 40! 40 CONFIG_SYS_TFTP_BLINK_STATUS_ON_DATA_IN 41 CONFIG_SYS_MONAHANS_TURBO_RUN_MODE_RATIO 41 CONFIG_SYS_MPC8220_SYSPLL_VCO_MULTIPLIER 42 CONFIG_SYS_PCI_SUBSYS_DEVICEID_NONMONARCH ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC] CONFIG naming convetion
On Sat 18 Jul 2009 13:50, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200907181115.26404.rg...@blackfin.uclinux.org> you wrote: > > > > It would be nice to come up with some list of namespaces, and what they > > they should be used for... > > Agreed. Excellent - a common goal :) > > For example, should it be: > > CONFIG_DRIVER_OMAP24XX_I2C > > or > > CONFIG_SYS_I2C_DRIVER_OMAP24XX > > or > > CONFIG_DRIVER_I2C_OMAP24XX > > Well, the difference between CONFIG_ and CONFIG_SYS_ is well-defined. > > And the "DRIVER_" part makes not much sense to me in any of the > examples above. It's a namespace - controls if a driver is compile in or not. Should only be used in .config files, and Makefiles. Some (very few) already use it.. ./drivers/serial/Makefile:COBJS-$(CONFIG_DRIVER_S3C4510_UART) += s3c4510b_uart.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_3C589) += 3c589.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_AX88180) += ax88180.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_CS8900) += cs8900.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_DM9000) += dm9000x.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_KS8695ETH) += ks8695eth.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_LAN91C96) += lan91c96.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_NE2000) += ne2000.o ne2000_base.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_AX88796L) += ax88796.o ne2000_base.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_NETARMETH) += netarm_eth.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_NS7520_ETHERNET) += ns7520_eth.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_NS9750_ETHERNET) += ns9750_eth.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_RTL8019) += rtl8019.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_S3C4510_ETH) += s3c4510b_eth.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_SMC9) += smc9.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_SMC911X) += smc911x.o ./drivers/net/Makefile:COBJS-$(CONFIG_DRIVER_TI_EMAC) += davinci_emac.o ./drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o ./drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o ./drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o ./drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_OMAP34XX_I2C) += omap24xx_i2c.o ./drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_S3C24X0_I2C) += s3c24x0_i2c.o ./drivers/mtd/Makefile:COBJS-$(CONFIG_FLASH_CFI_DRIVER) += cfi_flash.o ./drivers/mtd/nand/Makefile:COBJS-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o ./board/micronas/vct/Makefile:COBJS-$(CONFIG_DRIVER_SMC911X) += ebi_smc911x.o smc_eeprom.o ./cpu/arm926ejs/davinci/Makefile:COBJS-$(CONFIG_DRIVER_TI_EMAC) += lxt972.o dp83848.o > My personal way of thinking about such options is usually CPU/archi- > tecture first, so I would probably chose CONFIG_OMAP24XX_I2C to en- > able/disable the I2C driver on a OMAP24XX based board, but I under- > stand that there are reasons to prefer CONFIG_I2C_OMAP24XX as well - > let's see if there is a clear majority of opiniions... If we are thinking of a "tree" type structure - it might make sense to start at the top? I guess the question is -- is it an I2C driver for the OMAP24xx or a OMAP24xx driver for I2C? Since the API is a I2C API - my thought it is an I2C driver, which happens to run on a specific SoC. The actual SoC doesn't really matter (and should be last) - so it is similar to the directory structure we have today. > > Again - which is only used in one place: > > drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o > > include/configs/omap2420h4.h:#define CONFIG_DRIVER_OMAP24XX_I2C > > > > Which is fine - since it is a driver, which I'm sure that people out of > > tree use. > > Well, if only out-of-tree ports use it, it probably should never have > been added in the first place. Then I can start sending patches for unused CONFIG_ from random config files, and no one will start complaining? >From what I can quickly tell - there seems to be about 472 different CONFIG_ options in ./include/config that are not actually used anywhere in the master tree. > > I would think should be CONFIG_DRIVERS_PATA_BFIN > > I dosagree, the "DRIVERS" part is just added line noise. It's a name space - making sure it is differentiated from an option. As you pointed out - this is what exists in the README today. (which I have read, but gained no clarity from it)... === * Configuration _OPTIONS_: These are selectable by the user and have names beginning with "CONFIG_". * Configuration _SETTINGS_: These depend on the hardware etc. and should not be meddled with if you don't know what you're doing; they have names beginning with "CONFIG_SYS_". ===
Re: [U-Boot] [RFC] CONFIG naming convetion
On Sat 18 Jul 2009 07:03, Jean-Christophe PLAGNIOL-VILLARD pondered: > Hi all, > > Currently we have a mess in the drivers CONFIG naming convention > as example on I2C we have > CONFIG_I2C_X > CONFIG_DRIVER_XXX_I2C > CONFIG__I2C > CONFIG_SYS_I2C_ > > I think it will good to have common and clean naming convention > so I'll propose we use this one > CONFIG_namespace_X > and > CONFIG_SYS_namespace_X > > so it will resutly for I2C to this > > CONFIG_I2C_X > CONFIG_SYS_I2C_ > > and the Makefile and KConfig it will be easier to sort and read I think this goes way beyond I2C There are ~4866 unique options in ./include/configs/* Most of which have no name spaces at all, some are not even used in any source files (that are in mainline anyway). We have 2815, which already start with CONFIG_SYS_xxx, like CONFIG_SYS_16M_MBMR Which is used in a single board: board/snmc/qs860t/qs860t.c: memctl->memc_mbmr = CONFIG_SYS_16M_MBMR; board/snmc/qs860t/qs860t.c: size = dram_size (CONFIG_SYS_16M_MBMR, (long *)SDRAM_BASE, SDRAM_16M_MAX_SIZE); include/configs/QS860T.h:#define CONFIG_SYS_16M_MBMR0x18802114 /* Mem Periodic Timer Prescaler */ It doesn't appear very "system" oriented to me... It would be nice to come up with some list of namespaces, and what they they should be used for... For example, should it be: CONFIG_DRIVER_OMAP24XX_I2C or CONFIG_SYS_I2C_DRIVER_OMAP24XX or CONFIG_DRIVER_I2C_OMAP24XX Again - which is only used in one place: drivers/i2c/Makefile:COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o include/configs/omap2420h4.h:#define CONFIG_DRIVER_OMAP24XX_I2C Which is fine - since it is a driver, which I'm sure that people out of tree use. we assumed that it was: CONFIG_DRIVER_NAND_BFIN but it depends on who added it... CONFIG_PATA_BFIN drivers/block/Makefile:COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o include/configs/bf548-ezkit.h:#define CONFIG_PATA_BFIN I would think should be CONFIG_DRIVERS_PATA_BFIN ? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] - add dns
On Fri 17 Jul 2009 18:01, Wolfgang Denk pondered: > Dear Robin Getz, > > In message <200907171745.36176.rg...@blackfin.uclinux.org> you wrote: > > > > > You probably should add a doc/README.* file to explain how that is > > > supposed to be used. > > > > Will do. > > > > What is the rule for when things go in ./doc/README.* vs ./README ? > > Size - anything that takes more than 5...10 lines ? > > > > Could you generate a git patch instead? > > > > Sure - I'll generate something from the net tree? > > Please use the mainline repo / "master" branch as reference. v2 - based on mainline repo "master" - implemented feedback from Wolfgang common/cmd_net.c | 47 doc/README.dns| 64 +++ include/net.h |5 net/Makefile |1 net/dns.c | 212 ++ net/dns.h | 38 ++ net/net.c | 19 +++ 8 files changed, 393 insertions(+) --- diff --git a/common/cmd_net.c b/common/cmd_net.c index 68183c4..8899143 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -353,3 +353,50 @@ U_BOOT_CMD( "[NTP server IP]\n" ); #endif + +#if defined(CONFIG_CMD_DNS) +int do_dns(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + if (argc == 1) { + cmd_usage(cmdtp); + return -1; + } + + /* +* We should check for a valid hostname: +* - Each label must be between 1 and 63 characters long +* - the entire hostname has a maximum of 255 characters +* - only the ASCII letters 'a' through 'z' (case-insensitive), +* the digits '0' through '9', and the hyphen +* - cannot begin or end with a hyphen +* - no other symbols, punctuation characters, or blank spaces are permitted +* but hey - this is a minimalist implmentation, so only check length +*/ + if (strlen(argv[1]) >= 255) { + printf("dns error: hostname too long\n"); + return 1; + } + + NetDNSResolve = argv[1]; + + if (argc == 3) + NetDNSenvvar = argv[2]; + else + NetDNSenvvar = NULL; + + if (NetLoop(DNS) < 0) { + printf("dns lookup of %s failed, check setup\n", argv[1]); + return 1; + } + + return 0; +} + +U_BOOT_CMD( + dns,3, 1, do_dns, + "lookup the IP of a hostname", + "hostname [envvar]" +); + +#endif /* CONFIG_CMD_DNS */ + diff --git a/doc/README.dns b/doc/README.dns new file mode 100644 index 000..deeccd7 --- /dev/null +++ b/doc/README.dns @@ -0,0 +1,64 @@ +Domain Name System +--- + +The Domain Name System (DNS) is a hierarchical naming system for computers, +services, or any resource participating in the Internet. It associates various +information with domain names assigned to each of the participants. Most +importantly, it translates domain names meaningful to humans into the numerical +(binary) identifiers associated with networking equipment for the purpose of +locating and addressing these devices world-wide. An often used analogy to +explain the Domain Name System is that it serves as the "phone book" for the +Internet by translating human-friendly computer hostnames into IP addresses. +For example, www.example.com translates to 208.77.188.166. + +For more information on DNS - http://en.wikipedia.org/wiki/Domain_Name_System + + + +U-Boot and DNS +-- + +CONFIG_CMD_DNS - controls if the 'dns' command is compiled in. If it is, it + will send name lookups to the dns server (env var 'dnsip') + Turning this option on will about abou 1k to U-Boot's size. + + Example: + +bfin> print dnsip +dnsip=192.168.0.1 + +bfin> dns www.google.com +66.102.1.104 + + By default, dns does nothing except print the IP number on + the default console - which by itself, would be pretty + useless. Adding a third argument to the dns command will + use that as the environment variable to be set. + + Example: + +bfin> print googleip +## Error: "googleip" not defined +bfin> dns www.google.com googleip +64.233.161.104 +bfin> print googleip +googleip=64.233.161.104 +bfin> ping ${googleip} +Using Blackfin EMAC device +host 64.233.161.104 is alive + + In this way, you can lookup, and set many more meaningful + things. + +bfin> sntp +ntpserverip not set +