Re: [PATCH 00/17] video: dw_hdmi: Support Vendor PHY

2023-12-18 Thread Robin Murphy

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

2023-09-06 Thread Robin Murphy

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

2023-09-06 Thread Robin Murphy

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

2021-12-15 Thread Robin Murphy
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

2020-07-03 Thread Robin Murphy

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

2020-07-02 Thread Robin Murphy

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

2020-06-09 Thread Robin Murphy

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

2020-04-27 Thread Robin Murphy

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

2020-04-08 Thread Robin Randhawa
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

2020-01-09 Thread Robin Randhawa
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

2020-01-09 Thread Robin Randhawa
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

2020-01-01 Thread Robin Randhawa
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

2019-10-18 Thread Robin Murphy

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

2019-05-08 Thread Robin Murphy

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

2019-04-17 Thread Robin Murphy

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

2018-12-19 Thread Robin Polak
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

2018-12-17 Thread Robin Polak
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

2018-12-08 Thread Robin Polak
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

2018-12-06 Thread Robin Polak
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

2016-09-14 Thread Robin Randhawa
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

2016-09-13 Thread Robin Randhawa
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

2015-02-13 Thread Robin Gong
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

2015-02-13 Thread Robin Gong
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

2015-02-12 Thread Robin Gong
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

2013-10-16 Thread Robin Fernandes

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

2011-10-16 Thread Robin Getz
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

2011-08-26 Thread Robin Theunis
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

2011-08-25 Thread Robin Theunis
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

2011-07-15 Thread Philippe Robin
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

2010-07-28 Thread Robin Getz
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

2010-04-27 Thread robin
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

2010-04-26 Thread robin
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

2010-03-08 Thread Robin Getz

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

2009-12-18 Thread Robin Getz
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..."

2009-12-10 Thread Robin Randhawa
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.

2009-08-24 Thread Robin Getz
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

2009-08-20 Thread Robin Getz
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

2009-08-17 Thread Robin Getz
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

2009-08-17 Thread Robin Getz
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

2009-08-17 Thread Robin Getz
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

2009-08-17 Thread Robin Getz
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

2009-08-13 Thread Robin Getz
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

2009-08-13 Thread Robin Getz
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

2009-08-12 Thread Robin Getz
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

2009-08-12 Thread Robin Getz
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

2009-08-12 Thread Robin Getz
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...

2009-08-12 Thread Robin Getz
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?

2009-08-12 Thread Robin Getz
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...

2009-08-12 Thread Robin Getz
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

2009-08-10 Thread Robin Getz
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

2009-08-10 Thread Robin Getz
>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

2009-08-10 Thread Robin Getz
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

2009-08-06 Thread Robin Getz
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

2009-08-06 Thread Robin Getz
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

2009-08-05 Thread Robin Getz
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

2009-07-31 Thread Robin Getz
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

2009-07-31 Thread Robin Getz
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

2009-07-31 Thread Robin Getz
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

2009-07-31 Thread Robin Getz
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

2009-07-30 Thread Robin Getz
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

2009-07-30 Thread Robin Getz
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

2009-07-30 Thread Robin Getz
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

2009-07-28 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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

2009-07-27 Thread Robin Getz
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...

2009-07-26 Thread Robin Getz
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

2009-07-26 Thread Robin Getz
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

2009-07-26 Thread Robin Getz
*/
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

2009-07-26 Thread Robin Getz
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

2009-07-25 Thread Robin Getz
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...

2009-07-25 Thread Robin Getz
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

2009-07-25 Thread Robin Getz
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...

2009-07-25 Thread Robin Getz
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?

2009-07-24 Thread Robin Getz
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

2009-07-22 Thread Robin Getz
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

2009-07-22 Thread Robin Getz
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?

2009-07-22 Thread Robin Getz
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?

2009-07-22 Thread Robin Getz
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?

2009-07-22 Thread Robin Getz
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

2009-07-22 Thread Robin Getz
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?

2009-07-22 Thread Robin Getz
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?

2009-07-22 Thread Robin Getz
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?

2009-07-21 Thread Robin Getz
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...

2009-07-21 Thread Robin Getz
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

2009-07-21 Thread Robin Getz
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

2009-07-20 Thread Robin Getz
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

2009-07-19 Thread Robin Getz
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

2009-07-19 Thread Robin Getz
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

2009-07-19 Thread Robin Getz
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

2009-07-18 Thread Robin Getz
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

2009-07-18 Thread Robin Getz
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

2009-07-18 Thread Robin Getz
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

2009-07-18 Thread Robin Getz
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

2009-07-17 Thread Robin Getz
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
+

  1   2   >