Re: [PATCH v2] riscv: enable multi-range memory layout
On 9/14/2023 2:05 PM, Heinrich Schuchardt wrote:
>
>
> Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu :
>> In order to enable PCIe passthrough on qemu riscv, the physical memory
>> range between 3GB and 4GB is reserved. Therefore if guest has 4GB ram,
>> two ranges are created as [2G, 3G) and [4G, 7G), currently u-boot sets
>> ram_top to 4G - 1 if the gd->ram_top is above 4G in
>
> This should move to 7GiB - 1 in your example on riscv64.
>
I'm describing the current implementation of board_get_usable_ram_top()
in ./arch/riscv/cpu/generic/dram.c. Do you mean this function should be
changed? Is the comment about 32bit DMA device still valid?
phys_size_t board_get_usable_ram_top(phys_size_t total_size)
{
/*
* Ensure that we run from first 4GB so that all
* addresses used by U-Boot are 32bit addresses.
*
* This in-turn ensures that 32bit DMA capable
* devices work fine because DMA mapping APIs will
* provide 32bit DMA addresses only.
*/
if (gd->ram_top >= SZ_4G)
return SZ_4G - 1;
return gd->ram_top;
}
>> board_get_usable_ram_top(), but that address is not backed by ram. This
>> patch selects the lowest range instead.
>>
>> Signed-off-by: Fei Wu
>> ---
>> arch/riscv/cpu/generic/dram.c| 2 +-
>> configs/qemu-riscv64_defconfig | 2 +-
>> configs/qemu-riscv64_smode_defconfig | 2 +-
>> configs/qemu-riscv64_spl_defconfig | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
>> index 44e11bd56c..fb53a57b4e 100644
>> --- a/arch/riscv/cpu/generic/dram.c
>> +++ b/arch/riscv/cpu/generic/dram.c
>> @@ -13,7 +13,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>
>> int dram_init(void)
>> {
>> -return fdtdec_setup_mem_size_base();
>> +return fdtdec_setup_mem_size_base_lowest();
>
> Linaro is working on allowing to download a distro image via https and
> installing from a RAM disk.
>
> We should not artificially reduce the RAM size available for U-Boot by
> restricting ourselfs to 1 GiB.
>
> We must ensure that ram top is in the upper range.
>
How do they handle the case of >4GB? board_get_usable_ram_top() attached
above always returns <4GB. And it seems like
fdtdec_setup_mem_size_base() cannot ensure the upper one is picked, it
just picks up one, but which one is selected depending on fdt.
Thanks,
Fei.
> Best regards
>
> Heinrich
>
>> }
>>
>> int dram_init_banksize(void)
>> diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig
>> index 9a8bbef192..aa55317d26 100644
>> --- a/configs/qemu-riscv64_defconfig
>> +++ b/configs/qemu-riscv64_defconfig
>> @@ -1,6 +1,6 @@
>> CONFIG_RISCV=y
>> CONFIG_SYS_MALLOC_LEN=0x80
>> -CONFIG_NR_DRAM_BANKS=1
>> +CONFIG_NR_DRAM_BANKS=2
>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
>> CONFIG_ENV_SIZE=0x2
>> diff --git a/configs/qemu-riscv64_smode_defconfig
>> b/configs/qemu-riscv64_smode_defconfig
>> index 1d0f021ade..de08a49dab 100644
>> --- a/configs/qemu-riscv64_smode_defconfig
>> +++ b/configs/qemu-riscv64_smode_defconfig
>> @@ -1,6 +1,6 @@
>> CONFIG_RISCV=y
>> CONFIG_SYS_MALLOC_LEN=0x80
>> -CONFIG_NR_DRAM_BANKS=1
>> +CONFIG_NR_DRAM_BANKS=2
>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
>> CONFIG_ENV_SIZE=0x2
>> diff --git a/configs/qemu-riscv64_spl_defconfig
>> b/configs/qemu-riscv64_spl_defconfig
>> index bb10145e6e..66dc2a1dd9 100644
>> --- a/configs/qemu-riscv64_spl_defconfig
>> +++ b/configs/qemu-riscv64_spl_defconfig
>> @@ -1,6 +1,6 @@
>> CONFIG_RISCV=y
>> CONFIG_SYS_MALLOC_LEN=0x80
>> -CONFIG_NR_DRAM_BANKS=1
>> +CONFIG_NR_DRAM_BANKS=2
>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
>> CONFIG_ENV_SIZE=0x2
Re: [PATCH v2] riscv: enable multi-range memory layout
Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu :
>In order to enable PCIe passthrough on qemu riscv, the physical memory
>range between 3GB and 4GB is reserved. Therefore if guest has 4GB ram,
>two ranges are created as [2G, 3G) and [4G, 7G), currently u-boot sets
>ram_top to 4G - 1 if the gd->ram_top is above 4G in
This should move to 7GiB - 1 in your example on riscv64.
>board_get_usable_ram_top(), but that address is not backed by ram. This
>patch selects the lowest range instead.
>
>Signed-off-by: Fei Wu
>---
> arch/riscv/cpu/generic/dram.c| 2 +-
> configs/qemu-riscv64_defconfig | 2 +-
> configs/qemu-riscv64_smode_defconfig | 2 +-
> configs/qemu-riscv64_spl_defconfig | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
>index 44e11bd56c..fb53a57b4e 100644
>--- a/arch/riscv/cpu/generic/dram.c
>+++ b/arch/riscv/cpu/generic/dram.c
>@@ -13,7 +13,7 @@ DECLARE_GLOBAL_DATA_PTR;
>
> int dram_init(void)
> {
>- return fdtdec_setup_mem_size_base();
>+ return fdtdec_setup_mem_size_base_lowest();
Linaro is working on allowing to download a distro image via https and
installing from a RAM disk.
We should not artificially reduce the RAM size available for U-Boot by
restricting ourselfs to 1 GiB.
We must ensure that ram top is in the upper range.
Best regards
Heinrich
> }
>
> int dram_init_banksize(void)
>diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig
>index 9a8bbef192..aa55317d26 100644
>--- a/configs/qemu-riscv64_defconfig
>+++ b/configs/qemu-riscv64_defconfig
>@@ -1,6 +1,6 @@
> CONFIG_RISCV=y
> CONFIG_SYS_MALLOC_LEN=0x80
>-CONFIG_NR_DRAM_BANKS=1
>+CONFIG_NR_DRAM_BANKS=2
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
> CONFIG_ENV_SIZE=0x2
>diff --git a/configs/qemu-riscv64_smode_defconfig
>b/configs/qemu-riscv64_smode_defconfig
>index 1d0f021ade..de08a49dab 100644
>--- a/configs/qemu-riscv64_smode_defconfig
>+++ b/configs/qemu-riscv64_smode_defconfig
>@@ -1,6 +1,6 @@
> CONFIG_RISCV=y
> CONFIG_SYS_MALLOC_LEN=0x80
>-CONFIG_NR_DRAM_BANKS=1
>+CONFIG_NR_DRAM_BANKS=2
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
> CONFIG_ENV_SIZE=0x2
>diff --git a/configs/qemu-riscv64_spl_defconfig
>b/configs/qemu-riscv64_spl_defconfig
>index bb10145e6e..66dc2a1dd9 100644
>--- a/configs/qemu-riscv64_spl_defconfig
>+++ b/configs/qemu-riscv64_spl_defconfig
>@@ -1,6 +1,6 @@
> CONFIG_RISCV=y
> CONFIG_SYS_MALLOC_LEN=0x80
>-CONFIG_NR_DRAM_BANKS=1
>+CONFIG_NR_DRAM_BANKS=2
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
> CONFIG_ENV_SIZE=0x2
[PATCH v2] riscv: enable multi-range memory layout
In order to enable PCIe passthrough on qemu riscv, the physical memory
range between 3GB and 4GB is reserved. Therefore if guest has 4GB ram,
two ranges are created as [2G, 3G) and [4G, 7G), currently u-boot sets
ram_top to 4G - 1 if the gd->ram_top is above 4G in
board_get_usable_ram_top(), but that address is not backed by ram. This
patch selects the lowest range instead.
Signed-off-by: Fei Wu
---
arch/riscv/cpu/generic/dram.c| 2 +-
configs/qemu-riscv64_defconfig | 2 +-
configs/qemu-riscv64_smode_defconfig | 2 +-
configs/qemu-riscv64_spl_defconfig | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
index 44e11bd56c..fb53a57b4e 100644
--- a/arch/riscv/cpu/generic/dram.c
+++ b/arch/riscv/cpu/generic/dram.c
@@ -13,7 +13,7 @@ DECLARE_GLOBAL_DATA_PTR;
int dram_init(void)
{
- return fdtdec_setup_mem_size_base();
+ return fdtdec_setup_mem_size_base_lowest();
}
int dram_init_banksize(void)
diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig
index 9a8bbef192..aa55317d26 100644
--- a/configs/qemu-riscv64_defconfig
+++ b/configs/qemu-riscv64_defconfig
@@ -1,6 +1,6 @@
CONFIG_RISCV=y
CONFIG_SYS_MALLOC_LEN=0x80
-CONFIG_NR_DRAM_BANKS=1
+CONFIG_NR_DRAM_BANKS=2
CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
CONFIG_ENV_SIZE=0x2
diff --git a/configs/qemu-riscv64_smode_defconfig
b/configs/qemu-riscv64_smode_defconfig
index 1d0f021ade..de08a49dab 100644
--- a/configs/qemu-riscv64_smode_defconfig
+++ b/configs/qemu-riscv64_smode_defconfig
@@ -1,6 +1,6 @@
CONFIG_RISCV=y
CONFIG_SYS_MALLOC_LEN=0x80
-CONFIG_NR_DRAM_BANKS=1
+CONFIG_NR_DRAM_BANKS=2
CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
CONFIG_ENV_SIZE=0x2
diff --git a/configs/qemu-riscv64_spl_defconfig
b/configs/qemu-riscv64_spl_defconfig
index bb10145e6e..66dc2a1dd9 100644
--- a/configs/qemu-riscv64_spl_defconfig
+++ b/configs/qemu-riscv64_spl_defconfig
@@ -1,6 +1,6 @@
CONFIG_RISCV=y
CONFIG_SYS_MALLOC_LEN=0x80
-CONFIG_NR_DRAM_BANKS=1
+CONFIG_NR_DRAM_BANKS=2
CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
CONFIG_ENV_SIZE=0x2
--
2.34.1
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
Hi Simon,
On Wed, Sep 13, 2023 at 8:38 PM Simon Glass wrote:
>
> Hi Tom,
>
> On Wed, 13 Sept 2023 at 14:14, Tom Rini wrote:
> >
> > On Wed, Sep 13, 2023 at 12:56:53PM -0700, Tony Dinh wrote:
> > > Hi Tom,
> > >
> > > On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote:
> > > >
> > > > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote:
> > > >
> > > > > I've been testing the boostd for a few Marvell boards and seeing this
> > > > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The
> > > > > "bootflow scan scsi" command triggered the "CACHE: Misaligned
> > > > > operation at range" error. However, this error did not affect the
> > > > > result of the scan, i.e. the bootflow for scsi partition was created
> > > > > correctly, and u-boot is running normally.
> > > > >
> > > > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether.
> > > > > Perhaps this is a case where the DCACHE is not required and should be
> > > > > turned off?
> > > > >
> > > > > Please see the log after the break below.
> > > >
> > > > Can you please try -next ? There's at least one SCSI related cache
> > > > alignment fix there that's not in master, thanks.
> > >
> > > Unfortunately I got the same errors. This time the ranges are
> > > different, of course.
> > >
> > > master:
> > >
> > > N2350 > bootflow scan scsi
> > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > > ERROR: v7_outer_cache_inval_range - start address is not aligned -
> > > 0x3fb99f88
> > > ERROR: v7_outer_cache_inval_range - stop address is not aligned -
> > > 0x3fb9a388
> > >
> > > next:
> > >
> > > N2350 > bootflow scan scsi
> > > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > > ERROR: v7_outer_cache_inval_range - start address is not aligned -
> > > 0x3fb80388
> > > ERROR: v7_outer_cache_inval_range - stop address is not aligned -
> > > 0x3fb80788
> >
> > Can you debug to where these calls are so we can align these buffers?
> > See 02660defdc8a ("scsi: Cache align temporary buffer") for an example.
>
> I wonder if we need to use memalign() when allocating memory to read things
> from the media? But I am not sure which file time is being read, or which
> bootmeth it is.
Looks like we probably need to align the buffer tempbuff.
/drivers/scsi/scsi.c
static int scsi_detect_dev(struct udevice *dev, int target, int lun,
struct blk_desc *dev_desc)
{
unsigned char perq, modi;
lbaint_t capacity;
unsigned long blksz;
struct scsi_cmd *pccb = (struct scsi_cmd *)&tempccb;
int count, err;
pccb->target = target;
pccb->lun = lun;
pccb->pdata = (unsigned char *)&tempbuff;
pccb->datalen = 512;
If you look at the log I posted previously, this error shows up during
"bootflow scan scsi".
All the best,
Tony
>
> Regards,
> Simon
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
Hi Tom,
On Wed, 13 Sept 2023 at 14:14, Tom Rini wrote:
>
> On Wed, Sep 13, 2023 at 12:56:53PM -0700, Tony Dinh wrote:
> > Hi Tom,
> >
> > On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote:
> > >
> > > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote:
> > >
> > > > I've been testing the boostd for a few Marvell boards and seeing
this
> > > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The
> > > > "bootflow scan scsi" command triggered the "CACHE: Misaligned
> > > > operation at range" error. However, this error did not affect the
> > > > result of the scan, i.e. the bootflow for scsi partition was created
> > > > correctly, and u-boot is running normally.
> > > >
> > > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether.
> > > > Perhaps this is a case where the DCACHE is not required and should
be
> > > > turned off?
> > > >
> > > > Please see the log after the break below.
> > >
> > > Can you please try -next ? There's at least one SCSI related cache
> > > alignment fix there that's not in master, thanks.
> >
> > Unfortunately I got the same errors. This time the ranges are
> > different, of course.
> >
> > master:
> >
> > N2350 > bootflow scan scsi
> > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > ERROR: v7_outer_cache_inval_range - start address is not aligned -
0x3fb99f88
> > ERROR: v7_outer_cache_inval_range - stop address is not aligned -
0x3fb9a388
> >
> > next:
> >
> > N2350 > bootflow scan scsi
> > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > ERROR: v7_outer_cache_inval_range - start address is not aligned -
0x3fb80388
> > ERROR: v7_outer_cache_inval_range - stop address is not aligned -
0x3fb80788
>
> Can you debug to where these calls are so we can align these buffers?
> See 02660defdc8a ("scsi: Cache align temporary buffer") for an example.
I wonder if we need to use memalign() when allocating memory to read things
from the media? But I am not sure which file time is being read, or which
bootmeth it is.
Regards,
Simon
Re: [PATCH v4 01/13] scmi: refactor the code to hide a channel from devices
Hi Patrice,
On Wed, Sep 13, 2023 at 03:31:03PM +0200, Patrice CHOTARD wrote:
> On 9/12/23 08:19, AKASHI Takahiro wrote:
> > The commit 85dc58289238 ("firmware: scmi: prepare uclass to pass channel
> > reference") added an explicit parameter, channel, but it seems to make
> > the code complex.
> >
> > Hiding this parameter will allow for adding a generic (protocol-agnostic)
> > helper function, i.e. for PROTOCOL_VERSION, in a later patch.
> >
> > Signed-off-by: AKASHI Takahiro
> > Reviewed-by: Simon Glass
> > ---
> > v4
> > * revive scmi_bind_protocols which was accidentally removed
> > * remove .per_child_auto from the driver declaration as it is not needed
> > v3
> > * fix an issue on ST board (reported by Etienne)
> > by taking care of cases where probed devices are children of
> > SCMI protocol device (i.e. clock devices under CCF)
> > See find_scmi_protocol_device().
> > * move "per_device_plato_auto" to a succeeding right patch
> > v2
> > * new patch
> > ---
> > drivers/clk/clk_scmi.c| 27 ++
> > drivers/firmware/scmi/scmi_agent-uclass.c | 105 --
> > drivers/power/regulator/scmi_regulator.c | 26 ++
> > drivers/reset/reset-scmi.c| 19 +---
> > include/scmi_agent.h | 15 ++--
> > 5 files changed, 104 insertions(+), 88 deletions(-)
> >
> > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> > index d172fed24c9d..34a49363a51a 100644
> > --- a/drivers/clk/clk_scmi.c
> > +++ b/drivers/clk/clk_scmi.c
> > @@ -13,17 +13,8 @@
> > #include
> > #include
> >
> > -/**
> > - * struct scmi_clk_priv - Private data for SCMI clocks
> > - * @channel: Reference to the SCMI channel to use
> > - */
> > -struct scmi_clk_priv {
> > - struct scmi_channel *channel;
> > -};
> > -
> > static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(dev);
> > struct scmi_clk_protocol_attr_out out;
> > struct scmi_msg msg = {
> > .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > @@ -33,7 +24,7 @@ static int scmi_clk_get_num_clock(struct udevice *dev,
> > size_t *num_clocks)
> > };
> > int ret;
> >
> > - ret = devm_scmi_process_msg(dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(dev, &msg);
> > if (ret)
> > return ret;
> >
> > @@ -44,7 +35,6 @@ static int scmi_clk_get_num_clock(struct udevice *dev,
> > size_t *num_clocks)
> >
> > static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char
> > **name)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(dev);
> > struct scmi_clk_attribute_in in = {
> > .clock_id = clkid,
> > };
> > @@ -59,7 +49,7 @@ static int scmi_clk_get_attibute(struct udevice *dev, int
> > clkid, char **name)
> > };
> > int ret;
> >
> > - ret = devm_scmi_process_msg(dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(dev, &msg);
> > if (ret)
> > return ret;
> >
> > @@ -70,7 +60,6 @@ static int scmi_clk_get_attibute(struct udevice *dev, int
> > clkid, char **name)
> >
> > static int scmi_clk_gate(struct clk *clk, int enable)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> > struct scmi_clk_state_in in = {
> > .clock_id = clk->id,
> > .attributes = enable,
> > @@ -81,7 +70,7 @@ static int scmi_clk_gate(struct clk *clk, int enable)
> > in, out);
> > int ret;
> >
> > - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(clk->dev, &msg);
> > if (ret)
> > return ret;
> >
> > @@ -100,7 +89,6 @@ static int scmi_clk_disable(struct clk *clk)
> >
> > static ulong scmi_clk_get_rate(struct clk *clk)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> > struct scmi_clk_rate_get_in in = {
> > .clock_id = clk->id,
> > };
> > @@ -110,7 +98,7 @@ static ulong scmi_clk_get_rate(struct clk *clk)
> > in, out);
> > int ret;
> >
> > - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(clk->dev, &msg);
> > if (ret < 0)
> > return ret;
> >
> > @@ -123,7 +111,6 @@ static ulong scmi_clk_get_rate(struct clk *clk)
> >
> > static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> > {
> > - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> > struct scmi_clk_rate_set_in in = {
> > .clock_id = clk->id,
> > .flags = SCMI_CLK_RATE_ROUND_CLOSEST,
> > @@ -136,7 +123,7 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong
> > rate)
> > in, out);
> > int ret;
> >
> > - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> > + ret = devm_scmi_process_msg(clk->dev, &msg);
> > if (ret < 0)
> > return ret;
>
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
Hi Tom,
On Wed, Sep 13, 2023 at 1:13 PM Tom Rini wrote:
>
> On Wed, Sep 13, 2023 at 12:56:53PM -0700, Tony Dinh wrote:
> > Hi Tom,
> >
> > On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote:
> > >
> > > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote:
> > >
> > > > I've been testing the boostd for a few Marvell boards and seeing this
> > > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The
> > > > "bootflow scan scsi" command triggered the "CACHE: Misaligned
> > > > operation at range" error. However, this error did not affect the
> > > > result of the scan, i.e. the bootflow for scsi partition was created
> > > > correctly, and u-boot is running normally.
> > > >
> > > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether.
> > > > Perhaps this is a case where the DCACHE is not required and should be
> > > > turned off?
> > > >
> > > > Please see the log after the break below.
> > >
> > > Can you please try -next ? There's at least one SCSI related cache
> > > alignment fix there that's not in master, thanks.
> >
> > Unfortunately I got the same errors. This time the ranges are
> > different, of course.
> >
> > master:
> >
> > N2350 > bootflow scan scsi
> > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> > ERROR: v7_outer_cache_inval_range - start address is not aligned -
> > 0x3fb99f88
> > ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3fb9a388
> >
> > next:
> >
> > N2350 > bootflow scan scsi
> > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> > ERROR: v7_outer_cache_inval_range - start address is not aligned -
> > 0x3fb80388
> > ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3fb80788
>
> Can you debug to where these calls are so we can align these buffers?
> See 02660defdc8a ("scsi: Cache align temporary buffer") for an example.
I've added some printfs to the code. Looks like the last calls were
SCSI_READ16 (0x48) in this sequence:
scsi_exec() --> ahci_scsi_exec() --> ata_scsiop_read_write()
Note that the misaligned errors do not always occur.
I've also looked at the other recent commit by Marek and tried to
enable CONFIG_BOUNCE_BUFFER:
https://github.com/u-boot/u-boot/commit/4f543e82b9831333bc0effe9540d8e6a9dde3cb5
But I think it is a noop without a callback from the driver.
Please see the boot log below after the break.
Thanks,
Tony
U-Boot 2023.10-rc4-tld-1-00284-gce67ba1e30-dirty (Sep 13 2023 - 16:44:56 -0700)
Thecus N2350
SoC: MV88F6820-A0 at 1066 MHz
DRAM: 1 GiB (533 MHz, 32-bit, ECC not enabled)
Core: 65 devices, 23 uclasses, devicetree: separate
NAND: 512 MiB
MMC:
Loading Environment from SPIFlash... SF: Detected mx25l3205d with page
size 256 Bytes, erase size 4 KiB, total 4 MiB
*** Warning - bad CRC, using default environment
Model: Thecus N2350
Net:
Warning: ethernet@7 (eth0) using random MAC address - a2:6f:f1:4a:2e:51
eth0: ethernet@7
Hit any key to stop autoboot: 0
N2350 > env def -a
## Resetting to default environment
N2350 > bootdev l
Seq Probed Status UclassName
--- -- -- --
0 [ ] OK ethernet ethernet@7.bootdev
--- -- -- --
(1 bootdev)
N2350 > bootdev hunt scsi
Hunting with: scsi
pcie0.0: Link down
pcie1.0: Link down
scsi_scan
scanning bus for devices...
scsi_scan_dev
SATA link 0 timeout.
Target spinup took 0 ms.
AHCI 0001. 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
flags: 64bit ncq led only pmp fbss pio slum part sxs
do_scsi_scan_one
scsi_detect_dev
scsi_setup_inquiry
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x12
do_scsi_scan_one
scsi_detect_dev
scsi_setup_inquiry
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x12
scsi_ident_cpy
scsi_ident_cpy
scsi_ident_cpy
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x0
scsi_read_capacity
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x25
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
Device 0: (1:0) Vendor: ATA Prod.: ST750LX003-1AC15 Rev: SM12
Type: Hard Disk
Capacity: 715404.8 MB = 698.6 GB (1465149168 x 512)
N2350 > bootflow scan scsi
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x28
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x48
ata_scsiop_read_write
scsi_exec
ahci_scsi_exec
ahci_scsi_exec cmd = 0x48
ata_scsiop_read_w
Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties
On Fri, Sep 8, 2023 at 1:06 PM Mark Kettenis wrote: > > > From: Jassi Brar > > Date: Fri, 8 Sep 2023 09:37:12 -0500 > > > > Hi Simon, > > > > On Thu, Sep 7, 2023 at 3:08 PM Simon Glass wrote: > > > On Wed, 6 Sept 2023 at 23:20, Ilias Apalodimas > > > > > > > > > > I beg to differ. Devicetree is more than just hardware and always > > > > > > has > > > > > > been. See, for example the /chosen and /options nodes. > > > > > > > > > > There are exceptions... > > > > > > > > > > > > > We've been this over and over again and frankly it gets a bit annoying. > > > > It's called *DEVICE* tree for a reason. As Rob pointed out there are > > > > exceptions, but those made a lot of sense. Having arbitrary internal > > > > ABI > > > > stuff of various projects in the schema just defeats the definition of a > > > > spec. > > > > > > Our efforts should not just be about internal ABI, but working to > > > provide a consistent configuration system for all firmware elements. > > > > > Sure, programmatically we can pass any data/info via DT, however it is > > only meant to map hardware features onto data structures. > > > > devicetree.org landing page > > "The devicetree is a data structure for describing hardware." > > > > devicetree-specification-v0.3.pdf Chapter-2 Line-1 > >"DTSpec specifies a construct called a devicetree to describe > > system hardware." > > But it doesn't say that it describes *only* hardware. And it does > describe more than just hardware. There is /chosen to specify > firmware configuration and there are several examples of device tree > bindings that describe non-discoverable firmware interfaces in the > Linux tree. And it has been that way since the days of IEEE 1275. > There are also bindings describing things like the firmware partition > layout. Yes. The exact title for 1275[1] is: IEEE Standard for Boot (Initialization Configuration) Firmware: Core Requirements and Practices I see "configuration" in there. However, in the OF case, it's generally how firmware configured the hardware and what it discovered. That's a little different than telling the OS how to configure the hardware which is what we do with FDT. Then there's stuff that's how to configure Linux which we try to reject. Once we get into configuration of the OS/client (including u-boot), making that an ABI is a lot harder and if we use DT for that, I don't think it should be an ABI. > > If we want to digress from the spec, we need the majority of > > developers to switch sides :) which is unlikely to happen and rightly > > so, imho. > > It isn't even clear what the spec is. There is the document you > reference above, there are the yaml files at > https://github.com/devicetree-org/dt-schema and then there are > additional yaml files in the Linux tree. As far as I know it isn't > written down anywhere that those are the only places where device tree > bindings can live. There's not any restriction. My intention with dtschema schemas is to only have "spec level" schemas. (Stuff we'd add to DTSpec (but don't because no one wants to write specs).) Hardware specific stuff lives somewhere else. That happens to be the Linux tree because that is where all the h/w support is (nothing else is close (by far)). Last I checked, we've got ~3700 schemas and ~1500 unconverted bindings. > Anyway, let's face it, there is some consensus among developers that > what Simon has done in U-Boot is pushing the use of devicetree beyond > the point where a significant fraction of developers thinks it makes > sense. And I think I agree with that. But you can't beat him with > the spec to make your point. > > Now the devicetree is cleverly constructed such that it is possible to > define additional bindings without the risk of conflicting with > bindings developed by other parties. In particular if U-Boot is > augmenting a device tree with properties that are prefixed with > "u-boot," this isn't going to hurt an operating system that uses such > an augmented device tree. Until someone has some great idea to start using them. If the OS doesn't need something, then why pass it in the first place? What purpose does it serve? It's an invitation for someone somewhere to start using them. The downside of keeping the nodes is it creates an ABI where there doesn't need to be one necessarily. > The real problem is that some folks developed a certification program > that allegedly requires schema verification and now propose adding > code to U-Boot that doesn't really solve any problem. My proposed > solution would be to change said certification program to allow > firmware to augment the device tree with properties and nodes with > compatibles that are in the namespace controlled by the firmware. I don't think we should decide what to do here based on said certification program. It can adapt to what's decided. Probably, the /option nodes will be stripped out or ignored for certification. I accepted u-boot options node schema into dtsch
Re: [PATCH v2 3/3] phy: Refactor generic_{setup, shutdown}_phy() to reduce complexity
On Thu, Aug 31, 2023 at 11:07:11PM +, Jonas Karlman wrote:
> Refactor generic_{setup,shutdown}_phy() to reduce complexity and
> indentation. This have no intended functional change.
>
> Signed-off-by: Jonas Karlman
Applied to u-boot/next, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 2/3] phy: Return success from generic_setup_phy() when phy is not found
On Thu, Aug 31, 2023 at 11:07:10PM +, Jonas Karlman wrote:
> Restore the old behavior of ehci_setup_phy() and ohci_setup_phy() to
> return success when generic_phy_get_by_index() return -ENOENT.
>
> Fixes: 84e561407a5f ("phy: Add generic_{setup,shutdown}_phy() helpers")
> Fixes: 10005004db73 ("usb: ohci: Make usage of generic_{setup,shutdown}_phy()
> helpers")
> Fixes: 083f8aa978a8 ("usb: ehci: Make usage of generic_{setup,shutdown}_phy()
> helpers")
> Fixes: 75341e9c16aa ("usb: ehci: Remove unused ehci_{setup,shutdown}_phy()
> helpers")
> Signed-off-by: Jonas Karlman
Applied to u-boot/next, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 1/3] phy: Fix generic_setup_phy() return value on power on failure
On Thu, Aug 31, 2023 at 11:07:08PM +, Jonas Karlman wrote:
> generic_phy_exit() typically return 0 for a struct phy that has been
> initialized with a generic_phy_init() call.
>
> generic_setup_phy() returns the value from a generic_phy_exit() call
> when generic_phy_power_on() fails. This hides the failed state of the
> power_on ops from the caller of generic_setup_phy().
>
> Fix this by ignoring the return value of the generic_phy_exit() call and
> return the value from the generic_phy_power_on() call.
>
> Fixes: 84e561407a5f ("phy: Add generic_{setup,shutdown}_phy() helpers")
> Signed-off-by: Jonas Karlman
Applied to u-boot/next, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails
On Thu, Aug 31, 2023 at 10:16:33PM +, Jonas Karlman wrote:
> generic_phy_get_by_name() does not initialize phy->dev to NULL before
> returning when dev_read_stringlist_search() fails. This can lead to an
> uninitialized or reused struct phy erroneously be report as valid by
> generic_phy_valid().
>
> Fix this issue by initializing phy->dev to NULL, also extend the
> dm_test_phy_base test with calls to generic_phy_valid().
>
> Fixes: b9688df3cbf4 ("drivers: phy: Set phy->dev to NULL when
> generic_phy_get_by_index() fails")
> Fixes: 868d58f69c7c ("usb: dwc3: Fix non-usb3 configurations")
> Signed-off-by: Jonas Karlman
For the series, applied to u-boot/next, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2] net: phy: broadcom: add support for BCM54210E
On Sat, Aug 05, 2023 at 04:10:08PM +0200, Marek Vasut wrote:
> It's Broadcom PHY simply described as single-port
> RGMII 10/100/1000BASE-T PHY. It requires disabling
> delay skew and GTXCLK bits.
>
> BCM54210E support ported from Linux kernel commit
> 0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E")
> AUX/SHD/bcm54xx_config_clock_delay update ported from Linux 6.5-rc4 commit
> 28e219aea0b9e ("net: phy: broadcom: drop brcm_phy_setbits() and use
> phy_set_bits() instead")
>
> Signed-off-by: Marek Vasut
> Reviewed-by: Rafał Miłecki
Applied to u-boot/next, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] net: phy: motorcomm: Add support for YT8511 PHY
On Sat, Aug 05, 2023 at 12:35:01PM +0200, Nicolas Frattaroli wrote: > The YT8511 ethernet PHYs can be found on e.g. the SOQuartz or > the Quartz64. Add rudimentary support for them. > > Signed-off-by: Nicolas Frattaroli Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
[PATCH] spi: nxp_fspi: reset the FLSHxCR1 registers
Reset the FLSHxCR1 registers to default value. ROM may set the register value and it affects the SPI NAND normal functions. Signed-off-by: Han Xu --- drivers/spi/nxp_fspi.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/spi/nxp_fspi.c b/drivers/spi/nxp_fspi.c index 579d6bac9b..5db27f9ae2 100644 --- a/drivers/spi/nxp_fspi.c +++ b/drivers/spi/nxp_fspi.c @@ -927,6 +927,13 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f) fspi_writel(f, FSPI_AHBCR_PREF_EN | FSPI_AHBCR_RDADDROPT, base + FSPI_AHBCR); + /* Reset the flashx control1 registers */ + reg = FSPI_FLSHXCR1_TCSH(0x3) | FSPI_FLSHXCR1_TCSS(0x3); + fspi_writel(f, reg, base + FSPI_FLSHA1CR1); + fspi_writel(f, reg, base + FSPI_FLSHA2CR1); + fspi_writel(f, reg, base + FSPI_FLSHB1CR1); + fspi_writel(f, reg, base + FSPI_FLSHB2CR1); + /* AHB Read - Set lut sequence ID for all CS. */ fspi_writel(f, SEQID_LUT, base + FSPI_FLSHA1CR2); fspi_writel(f, SEQID_LUT, base + FSPI_FLSHA2CR2); -- 2.34.1
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
On Wed, Sep 13, 2023 at 12:56:53PM -0700, Tony Dinh wrote:
> Hi Tom,
>
> On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote:
> >
> > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote:
> >
> > > I've been testing the boostd for a few Marvell boards and seeing this
> > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The
> > > "bootflow scan scsi" command triggered the "CACHE: Misaligned
> > > operation at range" error. However, this error did not affect the
> > > result of the scan, i.e. the bootflow for scsi partition was created
> > > correctly, and u-boot is running normally.
> > >
> > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether.
> > > Perhaps this is a case where the DCACHE is not required and should be
> > > turned off?
> > >
> > > Please see the log after the break below.
> >
> > Can you please try -next ? There's at least one SCSI related cache
> > alignment fix there that's not in master, thanks.
>
> Unfortunately I got the same errors. This time the ranges are
> different, of course.
>
> master:
>
> N2350 > bootflow scan scsi
> CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> CACHE: Misaligned operation at range [3fb99f88, 3fb9a388]
> ERROR: v7_outer_cache_inval_range - start address is not aligned - 0x3fb99f88
> ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3fb9a388
>
> next:
>
> N2350 > bootflow scan scsi
> CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> CACHE: Misaligned operation at range [3fb80388, 3fb80788]
> ERROR: v7_outer_cache_inval_range - start address is not aligned - 0x3fb80388
> ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3fb80788
Can you debug to where these calls are so we can align these buffers?
See 02660defdc8a ("scsi: Cache align temporary buffer") for an example.
--
Tom
signature.asc
Description: PGP signature
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
Hi Tom, On Wed, Sep 13, 2023 at 9:22 AM Tom Rini wrote: > > On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote: > > > I've been testing the boostd for a few Marvell boards and seeing this > > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The > > "bootflow scan scsi" command triggered the "CACHE: Misaligned > > operation at range" error. However, this error did not affect the > > result of the scan, i.e. the bootflow for scsi partition was created > > correctly, and u-boot is running normally. > > > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether. > > Perhaps this is a case where the DCACHE is not required and should be > > turned off? > > > > Please see the log after the break below. > > Can you please try -next ? There's at least one SCSI related cache > alignment fix there that's not in master, thanks. Unfortunately I got the same errors. This time the ranges are different, of course. master: N2350 > bootflow scan scsi CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] CACHE: Misaligned operation at range [3fb99f88, 3fb9a388] ERROR: v7_outer_cache_inval_range - start address is not aligned - 0x3fb99f88 ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3fb9a388 next: N2350 > bootflow scan scsi CACHE: Misaligned operation at range [3fb80388, 3fb80788] CACHE: Misaligned operation at range [3fb80388, 3fb80788] CACHE: Misaligned operation at range [3fb80388, 3fb80788] ERROR: v7_outer_cache_inval_range - start address is not aligned - 0x3fb80388 ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x3fb80788 Thanks, Tony > > -- > Tom
[PATCH] USB: gadget: atmel: fix transfer of queued requests
In the existing implementation, multiple requests queued up on an
endpoint are subject to getting evicted without transmission.
For both control and bulk endpoints, their respective logic found in
usba_control_irq()/usba_ep_irq() guarantees that TX FIFO is empty before
data is sent out, and that request_complete() gets called once the
transaction has been finished. At this point however, if any additional
requests are found on the endpoint queue, they will be processed by
submit_next_request(), which makes no checks against the above
conditions, trashing data on a busy FIFO and neglecting completion
handlers.
Fix the above issues by removing the calls to submit_next_request(),
and thus forcing the pending requests to be processed on the next pass
of the respective endpoint logic. While at it, remove a DBG message, as
that branch becomes part of regular flow.
This restores mass storage mode operation on Microchip ATSAMA5D27 SoC.
Signed-off-by: Artur Rojek
---
drivers/usb/gadget/atmel_usba_udc.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 7d51821497b4..e6b458d940d8 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -57,13 +57,9 @@ static void submit_request(struct usba_ep *ep, struct
usba_request *req)
req->submitted = 1;
next_fifo_transaction(ep, req);
- if (req->last_transaction) {
- usba_ep_writel(ep, CTL_DIS, USBA_TX_PK_RDY);
- usba_ep_writel(ep, CTL_ENB, USBA_TX_COMPLETE);
- } else {
+ if (ep_is_control(ep))
usba_ep_writel(ep, CTL_DIS, USBA_TX_COMPLETE);
- usba_ep_writel(ep, CTL_ENB, USBA_TX_PK_RDY);
- }
+ usba_ep_writel(ep, CTL_ENB, USBA_TX_PK_RDY);
}
static void submit_next_request(struct usba_ep *ep)
@@ -889,7 +885,6 @@ restart:
if (req) {
list_del_init(&req->queue);
request_complete(ep, req, 0);
- submit_next_request(ep);
}
usba_ep_writel(ep, CTL_DIS, USBA_TX_COMPLETE);
ep->state = WAIT_FOR_SETUP;
@@ -1036,7 +1031,6 @@ static void usba_ep_irq(struct usba_udc *udc, struct
usba_ep *ep)
DBG(DBG_BUS, "%s: TX PK ready\n", ep->ep.name);
if (list_empty(&ep->queue)) {
- DBG(DBG_INT, "ep_irq: queue empty\n");
usba_ep_writel(ep, CTL_DIS, USBA_TX_PK_RDY);
return;
}
@@ -1050,7 +1044,6 @@ static void usba_ep_irq(struct usba_udc *udc, struct
usba_ep *ep)
if (req->last_transaction) {
list_del_init(&req->queue);
- submit_next_request(ep);
request_complete(ep, req, 0);
}
--
2.42.0
Re: [PATCH v1] CI: allow jobs to be run in merge requests
On 13/09/2023 15:21, Tom Rini wrote: > > This seems fine but please rebase on top of current -next, thanks. > Thanks for your quick review. Done in V2: https://lore.kernel.org/all/20230913141536.109844-1-andrejs.cainik...@gmail.com/ -- Best regards, Andrejs Cainikovs
Re: [PATCH 1/8] tools: mkeficapsule: Add support for parsing capsule params from config file
On Fri, Sep 08, 2023 at 05:29:55PM +0530, Sughosh Ganu wrote: > Add support for specifying the parameters needed for capsule > generation through a config file, instead of passing them through > command-line. Parameters for more than a single capsule file can be > specified, resulting in generation of multiple capsules through a > single invocation of the command. > > Signed-off-by: Sughosh Ganu Is this config file format used in any other project / form? [snip] > +config EFI_CAPSULE_CFG_FILE > + string "Path to the EFI Capsule Config File" > + default "" No empty defaults. -- Tom signature.asc Description: PGP signature
Re: [PATCH 0/8] Add some more EFI capsule tooling support
On Fri, Sep 08, 2023 at 05:29:54PM +0530, Sughosh Ganu wrote: > Recently, a set of patches were merged in next, which were adding > support for generating capsules as part of the U-Boot build. Mid way > through the review of those patches, it was decided to drop the > patches for generating capsules through a config file. That was > primarily due to the use of absolute paths in binman for testing the > capsule genertion through config file. Now that the base set of > patches have been merged, this series is picking up the remaining > patches for review. This series addresses the concern that Simon Glass > had with the use of absolute paths. > > The first set of patches are adding support for generating capsules by > parsing the capsule parameters through a config file, and adding a > binman entry type for this. These are patches 1-5. > > The other set of patches is for generating empty accept and revert > capsules through binman. These capsules are needed for the FWU A/B > update functionality. I think what we really need next is to be able to implement and document how to use the capsule (and FWU!) spec / mechanisms on another platform and I think the Synquacer DeveloperBox would be a good target here as I believe you have access to one and so does Simon to we can deal with feedback on a real platform that both of you have access to. -- Tom signature.asc Description: PGP signature
Re: [PATCH] am33xx: ignore return value from usb_ether_init()
On Wed, Sep 13, 2023 at 2:32 PM Tom Rini wrote: > On Wed, Sep 13, 2023 at 08:50:49AM -0400, Trevor Woerner wrote: > > > Can this get added to the next release? I don't see it in -next. > > I was going to pick this up for v2024.01 (i.e. the next branch) > soon'ish. > Awesome, thanks! :-)
Re: [PATCH] am33xx: ignore return value from usb_ether_init()
On Wed, Sep 13, 2023 at 08:50:49AM -0400, Trevor Woerner wrote:
> Can this get added to the next release? I don't see it in -next.
I was going to pick this up for v2024.01 (i.e. the next branch)
soon'ish.
>
> On Thu, Aug 31, 2023 at 6:15 AM Michal Suchánek wrote:
>
> > Hello,
> >
> > On Wed, Aug 30, 2023 at 10:49:50PM -0400, Trevor Woerner wrote:
> > > In 2cb43ef1c223 ("usb: ether: Fix error handling in usb_ether_init") the
> > error
> > > handling of usb_ether_init() was changed. Not a single other call site
> > of this
> > > function checks its return value, therefore follow suit in the am33xx
> > code.
> >
> > then there is the question what point is there in having a return value
> > in this function at all.
> >
> > Anyway, it's fine to not check the return value in the caller if there
> > is no use for the error.
> >
> > Reviewed-by: Michal Suchánek
> >
> > >
> > > Do not cause the boot to halt if the usb gadget ethernet initialization
> > fails:
> > >
> > > initcall sequence 9ffdbd84 failed at call 808024b9 (err=-19)
> > > ### ERROR ### Please RESET the board ###
> > >
> > > Signed-off-by: Trevor Woerner
> > > ---
> > > arch/arm/mach-omap2/am33xx/board.c | 6 +-
> > > 1 file changed, 1 insertion(+), 5 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-omap2/am33xx/board.c
> > b/arch/arm/mach-omap2/am33xx/board.c
> > > index ecc0a592e993..8f772310a1a7 100644
> > > --- a/arch/arm/mach-omap2/am33xx/board.c
> > > +++ b/arch/arm/mach-omap2/am33xx/board.c
> > > @@ -270,11 +270,7 @@ int arch_misc_init(void)
> > > return ret;
> > >
> > > #if defined(CONFIG_DM_ETH) && defined(CONFIG_USB_ETHER)
> > > - ret = usb_ether_init();
> > > - if (ret) {
> > > - pr_err("USB ether init failed\n");
> > > - return ret;
> > > - }
> > > + usb_ether_init();
> > > #endif
> > >
> > > return 0;
> > > --
> > > 2.41.0.327.gaa9166bcc0ba
> > >
> >
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 3/8] binman: capsule: Generate capsules through config file
Hi Sughosh, On Mon, 11 Sept 2023 at 08:13, Sughosh Ganu wrote: > > hi Simon, > > On Mon, 11 Sept 2023 at 00:44, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Fri, 8 Sept 2023 at 06:00, Sughosh Ganu wrote: > > > > > > Add support in binman for generating capsules by reading the capsule > > > parameters through a config file. Also add a test case in binman for > > > this mode of capsule generation. > > > > > > Signed-off-by: Sughosh Ganu > > > --- > > > > > > tools/binman/entries.rst | 35 > > > tools/binman/etype/efi_capsule_cfg_file.py | 66 ++ > > > tools/binman/ftest.py | 29 ++ > > > tools/binman/test/319_capsule_cfg.dts | 15 + > > > 4 files changed, 145 insertions(+) > > > create mode 100644 tools/binman/etype/efi_capsule_cfg_file.py > > > create mode 100644 tools/binman/test/319_capsule_cfg.dts > > > > I think we discussed this some time ago. The problem I have with this > > is that it adds another way of specifying the image description, > > separate from the binman definition. This introduces all sorts of > > problems: > > > > - the question of where the filenames are located, something which > > binman tries to handle cohesively > > - binman has no knowledge of what is being put in the capsules > > - the resulting fdtmap lacks any information about what is in the capsules > > - it seems to create two ways of doing the same thing > > > > Binman can presumably already create multiple capsule files, just by > > adding to the description. > > > > What need does this feature meet? > > We already had this discussion earlier about the need for generating > capsules through the config file, where I had explained the need for > this [1]. This is functionality which is supported by the > specification, and we have folks who are interested in having the > capsules generated through config file, and also in generation of a > single capsule file consisting of multiple input payloads. > > I understand that this model does not fit with the usual way in binman > of generating an output image by specifying it's inputs as DT nodes. > But just because this does not fit or align with the design of the > tool does not mean that we do not add support for the functionality. > Why can't we support this in binman as long as it is properly > documented as to what is being done. Moreover, my initial versions of > this patchset were attempting to achieve this by adding a make target, > but you had shot it down [2]. So it either has to be done through > binman, or through a make target. We have a lot of balls in the air right now...I would like to pause this one until we get the other things sorted out: - simple capsule update in sandbox (see my other email) - example of how to use all this on a real board (Tom is going to make a proposal there) - tidy up the testing so that it is easier to understand and fits better with the test framework - figure out the DT story, since we are still struggling to align there (Binman bindings, removing nodes, etc) We don't need to get all those done, but we do need to at least agree on the direction. Regards, Simon > > -sughosh > > [1] - https://lists.denx.de/pipermail/u-boot/2023-July/524849.html > [2] - https://lists.denx.de/pipermail/u-boot/2023-June/521103.html
Re: UEFI update files missing on sandbox?
Hi Sughosh, On Sun, 10 Sept 2023 at 13:13, Simon Glass wrote: > > Hi Sughosh, > > On Sun, 10 Sept 2023 at 04:54, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Sun, 10 Sept 2023 at 04:10, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > I thought that all your patches were merged but now when I build > > > sandbox I don't see the capsule-update files generated. Is there > > > something missing? > > > > The building of the EFI capsules was moved to the corresponding test > > setup. This was based on feedback from Tom Rini [1], where he had > > suggested that instead of having the binman nodes for the capsules as > > part of the sandbox dtb, it should rather be highlighted in the > > document. > > Hmmm that was not how I was hoping it would work. > > Is there a way to make the build create the files? > > ...I just found efi_capsule_data() > > I really don't like this at all...it is running a part of the build > system in the test. We have to do this in some cases, e.g. for tracing > we must run with a special build of sandbox, but it should not be > necessary for EFI capsules. > > I would really like the sandbox build to produce the capsules needed > for tests, without any extra effort. As it is, there is no way to try > this out. How about we add a single capsule update to the sandbox build, so that it produces an update file containing U-Boot? Is that easy to do? Regards, Simon
Re: bootstd: CACHE Misaligned operation errors (Marvell Armada 385)
On Tue, Sep 12, 2023 at 12:38:00PM -0700, Tony Dinh wrote: > I've been testing the boostd for a few Marvell boards and seeing this > error on the Thecus N2350 (Marvell Armada 385, dual-core CPU). The > "bootflow scan scsi" command triggered the "CACHE: Misaligned > operation at range" error. However, this error did not affect the > result of the scan, i.e. the bootflow for scsi partition was created > correctly, and u-boot is running normally. > > Enabling CONFIG_SYS_DCACHE_OFF got rid of the errors altogether. > Perhaps this is a case where the DCACHE is not required and should be > turned off? > > Please see the log after the break below. Can you please try -next ? There's at least one SCSI related cache alignment fix there that's not in master, thanks. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] CI: allow jobs to be run in merge requests
On Wed, Sep 13, 2023 at 04:15:36PM +0200, Andrejs Cainikovs wrote: > From: Andrejs Cainikovs > > Out-of-tree users could run an out-of-tree CI with limited coverage, > however it is convenient to be able to run the upstream CI from time > to time. To enable that we would need to change job rules to be able > to run on any GitLab event. Excerpt from GitLab documentation: > > > Jobs with no rules default to except: merge_requests > > Signed-off-by: Andrejs Cainikovs Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 0/7] Port gen_compile_commands.py from Linux to U-Boot
On Wed, Sep 13, 2023 at 03:28:07PM +0200, João Marcos Costa wrote: > Hello, > > Em sáb., 2 de set. de 2023 às 16:53, Joao Marcos Costa > escreveu: > > > Hello U-Boot community, > > > > I'm submitting a patch series that ports the gen_compile_commands.py > > script from the Linux kernel's sources to U-Boot. This script, originally > > located in scripts/clang-tools/gen_compile_commands.py, enables the > > generation of compile_commands.json file for improved code navigation > > and analysis. The series consists of the initial script import, the > > necessary modifications for U-Boot compatibility, and finally some > > documentation. > > > > Your feedback on these contributions would be greatly appreciated. > > > > Best regards, > > > > Changes in v3: > > - Add documentation to index and fix syntax issues > > - Add reference to documentation in doc/build/tools > > Changes in v2: > > - Add compile_commands.json to gitignore > > - Add documentation > > > > Joao Marcos Costa (7): > > scripts: Port Linux's gen_compile_commands.py to U-Boot > > scripts/gen_compile_commands.py: adapt _LINE_PATTERN > > scripts/gen_compile_commands.py: fix docstring > > scripts/gen_compile_commands.py: add acknowledgments > > .gitignore: add compile_commands.json > > doc: add documentation for gen_compile_commands.py > > doc: add new section to build/tools > > > > .gitignore | 3 + > > doc/build/gen_compile_commands.rst | 47 ++ > > doc/build/index.rst| 1 + > > doc/build/tools.rst| 13 ++ > > scripts/gen_compile_commands.py| 230 + > > 5 files changed, 294 insertions(+) > > create mode 100644 doc/build/gen_compile_commands.rst > > create mode 100755 scripts/gen_compile_commands.py > > > > -- > > 2.41.0 > > > > > Any updates? Heinrich, were you happy with the doc patches? -- Tom signature.asc Description: PGP signature
[PATCH v2] binman: doc: fix reference tag placement for Logging section
Move BinmanLogging reference tag after section "Signing FIT container
with private key in an image" and just before section "Logging".
Remove meaningless line with incomplete sentence.
Fixes: 0f40e23fd22 ("binman: add documentation for binman sign option")
Signed-off-by: Massimo Pegorer
Reviewed-by: Heinrich Schuchardt
---
Changes in v2:
- Removed incomplete sentence (confirmed by Simon)
tools/binman/binman.rst | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index aeea33fddb..020988d955 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1480,9 +1480,6 @@ as set in stone, so Binman will ensure it doesn't change.
Without this feature,
repacking an entry might cause it to disobey the original constraints provided
when it was created.
- Repacking an image involves
-
-.. _`BinmanLogging`:
Signing FIT container with private key in an image
--
@@ -1501,6 +1498,7 @@ If you want to sign and replace FIT container in place::
which will sign FIT container with private key and replace it immediately
inside your image.
+.. _`BinmanLogging`:
Logging
---
--
2.34.1
[PATCH v2] CI: allow jobs to be run in merge requests
From: Andrejs Cainikovs
Out-of-tree users could run an out-of-tree CI with limited coverage,
however it is convenient to be able to run the upstream CI from time
to time. To enable that we would need to change job rules to be able
to run on any GitLab event. Excerpt from GitLab documentation:
> Jobs with no rules default to except: merge_requests
Signed-off-by: Andrejs Cainikovs
---
.gitlab-ci.yml | 38 +-
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 6efbd8021c8..981b95c00d8 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -21,6 +21,8 @@ stages:
.buildman_and_testpy_template: &buildman_and_testpy_dfn
stage: test.py
retry: 2 # QEMU may be too slow, etc.
+ rules:
+- when: always
before_script:
# Clone uboot-test-hooks
- git config --global --add safe.directory "${CI_PROJECT_DIR}"
@@ -93,8 +95,13 @@ stages:
- "*.css"
expire_in: 1 week
-build all 32bit ARM platforms:
+.world_build:
stage: world build
+ rules:
+- when: always
+
+build all 32bit ARM platforms:
+ extends: .world_build
script:
- ret=0;
git config --global --add safe.directory "${CI_PROJECT_DIR}";
@@ -106,7 +113,7 @@ build all 32bit ARM platforms:
fi;
build all 64bit ARM platforms:
- stage: world build
+ extends: .world_build
script:
- virtualenv -p /usr/bin/python3 /tmp/venv
- . /tmp/venv/bin/activate
@@ -120,7 +127,7 @@ build all 64bit ARM platforms:
fi;
build all PowerPC platforms:
- stage: world build
+ extends: .world_build
script:
- ret=0;
git config --global --add safe.directory "${CI_PROJECT_DIR}";
@@ -131,7 +138,7 @@ build all PowerPC platforms:
fi;
build all other platforms:
- stage: world build
+ extends: .world_build
script:
- ret=0;
git config --global --add safe.directory "${CI_PROJECT_DIR}";
@@ -141,8 +148,13 @@ build all other platforms:
exit $ret;
fi;
-check for new CONFIG symbols outside Kconfig:
+.testsuites:
stage: testsuites
+ rules:
+- when: always
+
+check for new CONFIG symbols outside Kconfig:
+ extends: .testsuites
script:
- git config --global --add safe.directory "${CI_PROJECT_DIR}"
# If grep succeeds and finds a match the test fails as we should
@@ -153,7 +165,7 @@ check for new CONFIG symbols outside Kconfig:
# build documentation
docs:
- stage: testsuites
+ extends: .testsuites
script:
- virtualenv -p /usr/bin/python3 /tmp/venvhtml
- . /tmp/venvhtml/bin/activate
@@ -163,20 +175,20 @@ docs:
# ensure all configs have MAINTAINERS entries
Check for configs without MAINTAINERS entry:
- stage: testsuites
+ extends: .testsuites
script:
- ./tools/buildman/buildman --maintainer-check || exit 0
# Ensure host tools build
Build tools-only and envtools:
- stage: testsuites
+ extends: .testsuites
script:
- make tools-only_config tools-only -j$(nproc);
make mrproper;
make tools-only_config envtools -j$(nproc)
Run binman, buildman, dtoc, Kconfig and patman testsuites:
- stage: testsuites
+ extends: .testsuites
script:
- git config --global user.name "GitLab CI Runner";
git config --global user.email tr...@konsulko.com;
@@ -200,7 +212,7 @@ Run binman, buildman, dtoc, Kconfig and patman testsuites:
make testconfig
Run tests for Nokia RX-51 (aka N900):
- stage: testsuites
+ extends: .testsuites
script:
- mkdir nokia_rx51_tmp;
ln -s /opt/nokia/u-boot-gen-combined nokia_rx51_tmp/;
@@ -214,7 +226,7 @@ Run tests for Nokia RX-51 (aka N900):
# Check for any pylint regressions
Run pylint:
- stage: testsuites
+ extends: .testsuites
script:
- git config --global --add safe.directory "${CI_PROJECT_DIR}"
- pip install -r test/py/requirements.txt
@@ -234,7 +246,7 @@ Run pylint:
# Check for pre-schema driver model tags
Check for pre-schema tags:
- stage: testsuites
+ extends: .testsuites
script:
- git config --global --add safe.directory "${CI_PROJECT_DIR}";
# If grep succeeds and finds a match the test fails as we should
@@ -243,7 +255,7 @@ Check for pre-schema tags:
# Check we can package the Python tools
Check packing of Python tools:
- stage: testsuites
+ extends: .testsuites
script:
- make pip
--
2.34.1
Re: [PATCHv8 03/15] net/lwip: implement dns cmd
On Wed, 13 Sept 2023 at 14:32, Simon Goldschmidt wrote:
>
>
> On 13.09.2023 07:56, Ilias Apalodimas wrote:
> > On Fri, Sep 08, 2023 at 07:53:08PM +0600, Maxim Uvarov wrote:
> >> U-Boot recently got support for an alternative network stack using LWIP.
> >> Replace dns command with the LWIP variant while keeping the output and
> >> error messages identical.
> >>
> >> Signed-off-by: Maxim Uvarov
> >> ---
> >> include/net/lwip.h | 19 +++
> >> net/lwip/Makefile| 2 ++
> >> net/lwip/apps/dns/lwip-dns.c | 63
> >> 3 files changed, 84 insertions(+)
> >> create mode 100644 include/net/lwip.h
> >> create mode 100644 net/lwip/apps/dns/lwip-dns.c
> >>
> >> diff --git a/include/net/lwip.h b/include/net/lwip.h
> >> new file mode 100644
> >> index 00..ab3db1a214
> >> --- /dev/null
> >> +++ b/include/net/lwip.h
> >> @@ -0,0 +1,19 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +
> >> +int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
> >> +char *const argv[]);
> >> +
> >> +/**
> >> + * ulwip_dns() - creates the DNS request to resolve a domain host name
> >> + *
> >> + * This function creates the DNS request to resolve a domain host
> name. Function
> >> + * can return immediately if previous request was cached or it might
> require
> >> + * entering the polling loop for a request to a remote server.
> >> + *
> >> + * @name:dns name to resolve
> >> + * @varname: (optional) U-Boot variable name to store the result
> >> + * Returns: ERR_OK(0) for fetching entry from the cache
> >> + * -EINPROGRESS success, can go to the polling loop
> >> + * Other value < 0, if error
> >> + */
> >> +int ulwip_dns(char *name, char *varname);
> >> diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> >> index 3fd5d34564..5d8d5527c6 100644
> >> --- a/net/lwip/Makefile
> >> +++ b/net/lwip/Makefile
> >> @@ -62,3 +62,5 @@ obj-$(CONFIG_NET) +=
> lwip-external/src/netif/ethernet.o
> >>
> >> obj-$(CONFIG_NET) += port/if.o
> >> obj-$(CONFIG_NET) += port/sys-arch.o
> >> +
> >> +obj-y += apps/dns/lwip-dns.o
> >> diff --git a/net/lwip/apps/dns/lwip-dns.c b/net/lwip/apps/dns/lwip-dns.c
> >> new file mode 100644
> >> index 00..b340302f2c
> >> --- /dev/null
> >> +++ b/net/lwip/apps/dns/lwip-dns.c
> >> @@ -0,0 +1,63 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +/*
> >> + * (C) Copyright 2023 Linaro Ltd.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include
> >> +#include
> >> +
> >> +#include
> >> +
> >> +static void dns_found_cb(const char *name, const ip_addr_t *ipaddr,
> void *callback_arg)
> >> +{
> >> +char *varname = (char *)callback_arg;
> >> +char *ipstr = ip4addr_ntoa(ipaddr);
> >> +
> >> +if (varname)
> >> +env_set(varname, ipstr);
> >> +log_info("resolved %s to %s\n", name, ipstr);
> >> +ulwip_exit(0);
> >> +}
> >> +
> >> +int ulwip_dns(char *name, char *varname)
> >> +{
> >> +int err;
> >> +ip_addr_t ipaddr; /* not used */
> >> +ip_addr_t dns1;
> >> +ip_addr_t dns2;
> >> +char *dnsenv = env_get("dnsip");
> >> +char *dns2env = env_get("dnsip2");
> >> +
> >> +if (!dnsenv && !dns2env) {
> >> +log_err("nameserver is not set with dnsip and dnsip2
> vars\n");
> >> +return -ENOENT;
> >> +}
> >> +
> >> +if (!dnsenv)
> >> +log_warning("dnsip var is not set\n");
> >> +if (!dns2env)
> >> +log_warning("dnsip2 var is not set\n");
> >> +
> >> +dns_init();
> >> +
> >> +if (ipaddr_aton(dnsenv, &dns1))
> >> +dns_setserver(0, &dns1);
> >> +
> >> +if (ipaddr_aton(dns2env, &dns2))
> >> +dns_setserver(1, &dns2);
> >
> > env_get will return NULL if any of these is not set. Looking at
> > ipaddr_aton() of lwip that might lead to a NULL deref in ip_2_ip6()
>
> Looking at the NULL checks in ipaddr_aton(), you found a bug in lwIP.
> I'd vote to leave the above code as is and rely on the bug being fixed
> in lwIP before U-Boot enables IPv6 (this is only a bug in dual-stack
> mode where IPv4 and IPv6 is enabled).
>
> Regards,
> Simon
>
>
Yes, I looked for an ipv4 case with null check. But I think here we can go
with:
if (dns2env && ipaddr_aton(dns2env, &dns2))
> >
> >> +
> >> +err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
> >> +if (err == ERR_OK)
> >> +dns_found_cb(name, &ipaddr, varname);
> >> +
> >> +/* convert lwIP ERR_INPROGRESS to U-Boot -EINPROGRESS */
> >> +if (err == ERR_INPROGRESS)
> >> +err = -EINPROGRESS;
> >> +
> >> +return err;
> >> +}
> >> --
> >> 2.30.2
> >>
>
Re: [PATCHv8 05/15] net/lwip: implement tftp cmd
On Wed, 13 Sept 2023 at 14:38, Simon Goldschmidt wrote:
>
>
> On 13.09.2023 08:15, Ilias Apalodimas wrote:
> >> +
> >> +/*
> >> + * (C) Copyright 2023 Linaro Ltd.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include "tftp_client.h"
> >> +#include "tftp_server.h"
> >> +#include
> >> +
> >> +#include
> >> +
> >> +#include
> >> +
> >> +static ulong daddr;
> >> +static ulong size;
> >> +
> >> +static void *tftp_open(const char *fname, const char *mode, u8_t
> is_write)
> >> +{
> >> +return NULL;
> >> +}
> >> +
> >> +static void tftp_close(void *handle)
> >> +{
> >> +log_info("\ndone\n");
> >> +log_info("Bytes transferred = %ld (0x%lx hex)\n", size, size);
> >> +
> >> +bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
> >> +if (env_set_hex("filesize", size)) {
> >> +log_err("filesize not updated\n");
> >> +ulwip_exit(-1);
> >> +return;
> >> +}
> >> +ulwip_exit(0);
> >> +}
> >> +
> >> +static int tftp_read(void *handle, void *buf, int bytes)
> >> +{
> >> +return 0;
> >> +}
> >> +
> >> +static int tftp_write(void *handle, struct pbuf *p)
> >> +{
> >> +struct pbuf *q;
> >> +
> >> +for (q = p; q != NULL; q = q->next) {
> >> +memcpy((void *)daddr, q->payload, q->len);
> >> +daddr += q->len;
> >> +size += q->len;
> >> +log_info("#");
> >> +}
> >> +
> >> +return 0;
> >> +}
> >> +
> >> +static void tftp_error(void *handle, int err, const char *msg, int
> size)
> >> +{
> >> +char message[100];
> >> +
> >> +memset(message, 0, sizeof(message));
> >> +memcpy(message, msg, LWIP_MIN(sizeof(message) - 1, (size_t)size));
> >> +
> >> +log_info("TFTP error: %d (%s)", err, message);
> >> +}
> >> +
> >> +static const struct tftp_context tftp = {
> >> +tftp_open,
> >> +tftp_close,
> >> +tftp_read,
> >> +tftp_write,
> >> +tftp_error
> >> +};
> >> +
> >> +int ulwip_tftp(ulong addr, char *fname)
> >> +{
> >> +void *f = (void *)0x1; /* unused fake file handle*/
> >
> > I haven't dug into lwip details yet, but I am not sure this is safe to
> use.
> > Simon, since you maintain the lwip part can you elaborate a bit more?
>
> From the lwIP design, using an invalid pointer here is ok: that pointer
> is only used as a client handle which is never dereferenced internally.
> The only requirement is that it is not NULL, as that is the validity
> check for the tftp client to know the handle is valid (or e.g. open
> failed etc.).
>
> So while we can leave 0x1 here, using a static uint8_t would probably be
> better, at the expense of using a byte for nothing.
>
> Regards,
> Simon
>
U-Boot does not like statics due to the fact that it can relocate itself to
another address.
BR,
Maxim.
>
> >
> >> +err_t err;
> >> +ip_addr_t srv;
> >> +int ret;
> >> +char *server_ip;
> >> +
> >> +if (!fname || addr == 0)
> >> +return CMD_RET_FAILURE;
> >> +
> >> +size = 0;
> >> +daddr = addr;
> >> +server_ip = env_get("serverip");
> >> +if (!server_ip) {
> >> +log_err("error: serverip variable has to be set\n");
> >> +return CMD_RET_FAILURE;
> >> +}
> >> +
> >> +ret = ipaddr_aton(server_ip, &srv);
> >> +if (!ret) {
> >> +log_err("error: ipaddr_aton\n");
> >> +return CMD_RET_FAILURE;
> >> +}
> >> +
> >> +log_info("TFTP from server %s; our IP address is %s\n",
> >> + server_ip, env_get("ipaddr"));
> >> +log_info("Filename '%s'.\n", fname);
> >> +log_info("Load address: 0x%lx\n", daddr);
> >> +log_info("Loading:");
> >> +
> >> +bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
> >> +
> >> +err = tftp_init_client(&tftp);
> >> +if (!(err == ERR_OK || err == ERR_USE))
> >> +log_err("tftp_init_client err: %d\n", err);
> >> +
> >> +err = tftp_get(f, &srv, TFTP_PORT, fname, TFTP_MODE_OCTET);
> >> +/* might return different errors, like routing problems */
> >> +if (err != ERR_OK) {
> >> +log_err("tftp_get err=%d\n", err);
> >> +return CMD_RET_FAILURE;
> >> +}
> >> +
> >> +if (env_set_hex("fileaddr", addr)) {
> >> +log_err("fileaddr not updated\n");
> >> +return CMD_RET_FAILURE;
> >> +}
> >> +
> >> +return CMD_RET_SUCCESS;
> >> +}
> >> --
> >> 2.30.2
> >>
> >
> > Thanks
> > /Ilias
>
Re: [PATCHv8 00/15] net/lwip: add lwip library for the network stack
On Wed, 13 Sept 2023 at 19:14, Tom Rini wrote: > On Wed, Sep 13, 2023 at 11:06:13AM +0100, Peter Robinson wrote: > > > >> Then if for development you need to pull he history of lwip, you > can do it with: > > > >> git pull -s subtree lwip master --allow-unrelated-histories > > > >> (but I think nobody will need this.) > > > >> > > > >> New update of the lwip net/lwip/lwip-external dir will be done with: > > > >> git pull -s subtree lwip master --allow-unrelated-histories > --squash > > > >> Squash commit also has to be git format-patch friendly. > > > >> > > > >> If you are ok with that proposal I will send v9 with the first > patch created with steps above. > > > > > > > > We've gone through this before. The whole purpose of this is not > > > > having to maintain patches. > > > > Simon, instead of "I had problems in the past", can you elaborate a > bit more? > > > > > > > > Tom said he is fine with subtrees instead of submodules and I know > for > > > > a fact EDK2 doesn't have too many issues with submodules. > > > > Their documentation is pretty clear on building and requires > > > > > > > > git clone https://github.com/tianocore/edk2.git > > > > cd edk2 > > > > git submodule update --init > > > > > > > > Perhaps the situation has improved since you had issues? > > > > > > While I don't really care how you solve this technically, I'd > *strongly* > > > be interested for U-Boot to use *unmodified* lwIP sources where an > > > explicit reference to an lwIP commit is used. I'd rather integrate > > > bugfixes for U-Boot into lwIP than having the sources drift apart. > > > > Strongly agree here, we want to use upstream and all the combined > > development and reviews etc rather than forking off and ending up with > > yet another slightly different IP stack. The whole advantage of > > adopting LWIP is the advantage of combined security, features and bugs > > from a wide range of projects :-) > > Yes, this is what I want as well, and why I'm perhaps more agreeable > with the approaches where it's a lot harder for us to start forking > things unintentionally. I gather submodule rather than subtree would be > better for that case? > > -- > Tom > Yes, submodule will be a much better solution for us. And I also don't think that today there are any issues with submodules. It works well of OE, RPM and DEB builds, distributions should not have problems with it. BR, Maxim.
Re: [PATCH v4 01/13] scmi: refactor the code to hide a channel from devices
On 9/12/23 08:19, AKASHI Takahiro wrote:
> The commit 85dc58289238 ("firmware: scmi: prepare uclass to pass channel
> reference") added an explicit parameter, channel, but it seems to make
> the code complex.
>
> Hiding this parameter will allow for adding a generic (protocol-agnostic)
> helper function, i.e. for PROTOCOL_VERSION, in a later patch.
>
> Signed-off-by: AKASHI Takahiro
> Reviewed-by: Simon Glass
> ---
> v4
> * revive scmi_bind_protocols which was accidentally removed
> * remove .per_child_auto from the driver declaration as it is not needed
> v3
> * fix an issue on ST board (reported by Etienne)
> by taking care of cases where probed devices are children of
> SCMI protocol device (i.e. clock devices under CCF)
> See find_scmi_protocol_device().
> * move "per_device_plato_auto" to a succeeding right patch
> v2
> * new patch
> ---
> drivers/clk/clk_scmi.c| 27 ++
> drivers/firmware/scmi/scmi_agent-uclass.c | 105 --
> drivers/power/regulator/scmi_regulator.c | 26 ++
> drivers/reset/reset-scmi.c| 19 +---
> include/scmi_agent.h | 15 ++--
> 5 files changed, 104 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> index d172fed24c9d..34a49363a51a 100644
> --- a/drivers/clk/clk_scmi.c
> +++ b/drivers/clk/clk_scmi.c
> @@ -13,17 +13,8 @@
> #include
> #include
>
> -/**
> - * struct scmi_clk_priv - Private data for SCMI clocks
> - * @channel: Reference to the SCMI channel to use
> - */
> -struct scmi_clk_priv {
> - struct scmi_channel *channel;
> -};
> -
> static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
> {
> - struct scmi_clk_priv *priv = dev_get_priv(dev);
> struct scmi_clk_protocol_attr_out out;
> struct scmi_msg msg = {
> .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> @@ -33,7 +24,7 @@ static int scmi_clk_get_num_clock(struct udevice *dev,
> size_t *num_clocks)
> };
> int ret;
>
> - ret = devm_scmi_process_msg(dev, priv->channel, &msg);
> + ret = devm_scmi_process_msg(dev, &msg);
> if (ret)
> return ret;
>
> @@ -44,7 +35,6 @@ static int scmi_clk_get_num_clock(struct udevice *dev,
> size_t *num_clocks)
>
> static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name)
> {
> - struct scmi_clk_priv *priv = dev_get_priv(dev);
> struct scmi_clk_attribute_in in = {
> .clock_id = clkid,
> };
> @@ -59,7 +49,7 @@ static int scmi_clk_get_attibute(struct udevice *dev, int
> clkid, char **name)
> };
> int ret;
>
> - ret = devm_scmi_process_msg(dev, priv->channel, &msg);
> + ret = devm_scmi_process_msg(dev, &msg);
> if (ret)
> return ret;
>
> @@ -70,7 +60,6 @@ static int scmi_clk_get_attibute(struct udevice *dev, int
> clkid, char **name)
>
> static int scmi_clk_gate(struct clk *clk, int enable)
> {
> - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> struct scmi_clk_state_in in = {
> .clock_id = clk->id,
> .attributes = enable,
> @@ -81,7 +70,7 @@ static int scmi_clk_gate(struct clk *clk, int enable)
> in, out);
> int ret;
>
> - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> + ret = devm_scmi_process_msg(clk->dev, &msg);
> if (ret)
> return ret;
>
> @@ -100,7 +89,6 @@ static int scmi_clk_disable(struct clk *clk)
>
> static ulong scmi_clk_get_rate(struct clk *clk)
> {
> - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> struct scmi_clk_rate_get_in in = {
> .clock_id = clk->id,
> };
> @@ -110,7 +98,7 @@ static ulong scmi_clk_get_rate(struct clk *clk)
> in, out);
> int ret;
>
> - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> + ret = devm_scmi_process_msg(clk->dev, &msg);
> if (ret < 0)
> return ret;
>
> @@ -123,7 +111,6 @@ static ulong scmi_clk_get_rate(struct clk *clk)
>
> static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> {
> - struct scmi_clk_priv *priv = dev_get_priv(clk->dev);
> struct scmi_clk_rate_set_in in = {
> .clock_id = clk->id,
> .flags = SCMI_CLK_RATE_ROUND_CLOSEST,
> @@ -136,7 +123,7 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong
> rate)
> in, out);
> int ret;
>
> - ret = devm_scmi_process_msg(clk->dev, priv->channel, &msg);
> + ret = devm_scmi_process_msg(clk->dev, &msg);
> if (ret < 0)
> return ret;
>
> @@ -149,12 +136,11 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong
> rate)
>
> static int scmi_clk_probe(struct udevice *dev)
> {
> - struct scmi_clk_priv *priv = dev_get_priv(dev);
> struct clk *clk;
> size_t nu
Re: [PATCH v3 0/7] Port gen_compile_commands.py from Linux to U-Boot
Hello, Em sáb., 2 de set. de 2023 às 16:53, Joao Marcos Costa escreveu: > Hello U-Boot community, > > I'm submitting a patch series that ports the gen_compile_commands.py > script from the Linux kernel's sources to U-Boot. This script, originally > located in scripts/clang-tools/gen_compile_commands.py, enables the > generation of compile_commands.json file for improved code navigation > and analysis. The series consists of the initial script import, the > necessary modifications for U-Boot compatibility, and finally some > documentation. > > Your feedback on these contributions would be greatly appreciated. > > Best regards, > > Changes in v3: > - Add documentation to index and fix syntax issues > - Add reference to documentation in doc/build/tools > Changes in v2: > - Add compile_commands.json to gitignore > - Add documentation > > Joao Marcos Costa (7): > scripts: Port Linux's gen_compile_commands.py to U-Boot > scripts/gen_compile_commands.py: adapt _LINE_PATTERN > scripts/gen_compile_commands.py: fix docstring > scripts/gen_compile_commands.py: add acknowledgments > .gitignore: add compile_commands.json > doc: add documentation for gen_compile_commands.py > doc: add new section to build/tools > > .gitignore | 3 + > doc/build/gen_compile_commands.rst | 47 ++ > doc/build/index.rst| 1 + > doc/build/tools.rst| 13 ++ > scripts/gen_compile_commands.py| 230 + > 5 files changed, 294 insertions(+) > create mode 100644 doc/build/gen_compile_commands.rst > create mode 100755 scripts/gen_compile_commands.py > > -- > 2.41.0 > > Any updates? Best regards, www.linkedin.com/in/jmarcoscosta/ https://github.com/jmarcoscosta
Re: [PATCH v1] CI: allow jobs to be run in merge requests
On Tue, Sep 12, 2023 at 01:00:22PM +0200, Andrejs Cainikovs wrote: > Out-of-tree users could run an out-of-tree CI with limited coverage, > however it is convenient to be able to run the upstream CI from time > to time. To enable that we would need to change job rules to be able > to run on any GitLab event. Excerpt from GitLab documentation: > > > Jobs with no rules default to except: merge_requests > > Signed-off-by: Andrejs Cainikovs > --- > .gitlab-ci.yml | 46 +- > 1 file changed, 29 insertions(+), 17 deletions(-) This seems fine but please rebase on top of current -next, thanks. -- Tom signature.asc Description: PGP signature
Re: PSCI: prefetch abort with Mainline linux (even with rockchip u-boot)
Hi Jagan, On 10/23/22 01:35, Jagan Teki wrote: Hi Kever and Heiko, Rockchip 32-bit SoC, like rv1126 seems to depend on PSCI to bring SMP in linux. With rockchip u-boot and Mainline U-Boot the psci in linux-next triggers the abort. (note that I have added rockchip_smcc and enabled PSCI in u-boot) Did you ever find a solution to this? I am hitting the same issue building 6.1 kernel for rv1126. [0.00] Booting Linux on physical CPU 0xf00 [0.00] Linux version 6.1.0-rc1-00029-gb09e57e6064a (j@j-ThinkPad-E14-Gen-2) (arm-linux-gnueabihf-gcc (GCC) 11.0.1 20210310 (experimental) [master revision 5987d8a79cda1069c774e5c302d5597310270026], GNU ld (Linaro_Binutils-2021.03) 2.36.50.20210310) #12 SMP Sat Oct 22 19:44:56 IST 2022 [0.00] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d [0.00] CPU: div instructions available: patching division code [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] OF: fdt: Machine model: Edgeble Neu2 IO Board [0.00] earlycon: uart8250 at MMIO32 0xff57 (options '') [0.00] printk: bootconsole [uart8250] enabled [0.00] Memory policy: Data cache writealloc [0.00] efi: UEFI not found. [0.00] cma: Reserved 64 MiB at 0x7c00 [0.00] Zone ranges: [0.00] DMA [mem 0x-0x2fff] [0.00] Normal empty [0.00] HighMem [mem 0x3000-0x7fff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x7fff] [0.00] Initmem setup node 0 [mem 0x-0x7fff] [0.00] psci: probing for conduit method from DT. [0.00] Bad mode in prefetch abort handler detected [0.00] Internal error: Oops - bad mode: 0 [#1] SMP ARM [0.00] Modules linked in: [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc1-00029-gb09e57e6064a #12 [0.00] Hardware name: Generic DT based system [0.00] PC is at 0x133fd468 [0.00] LR is at __invoke_psci_fn_smc+0x38/0x58 [0.00] pc : [<133fd468>]lr : []psr: 61d6 [0.00] sp : c1e01e68 ip : c1e01ec0 fp : 8200 [0.00] r10: c1e01f58 r9 : 7fff r8 : c1e09410 [0.00] r7 : r6 : r5 : r4 : [0.00] r3 : r2 : r1 : r0 : 8400 [0.00] Flags: nZCv IRQs off FIQs off Mode MON_32 ISA ARM Segment none [0.00] Control: 10c5387d Table: 0020406a DAC: c1e09980 [0.00] Register r0 information: non-paged memory [0.00] Register r1 information: NULL pointer [0.00] Register r2 information: NULL pointer [0.00] Register r3 information: NULL pointer [0.00] Register r4 information: NULL pointer [0.00] Register r5 information: NULL pointer [0.00] Register r6 information: NULL pointer [0.00] Register r7 information: NULL pointer [0.00] Register r8 information: non-slab/vmalloc memory [0.00] Register r9 information: non-paged memory [0.00] Register r10 information: non-slab/vmalloc memory [0.00] Register r11 information: non-paged memory [0.00] Register r12 information: non-slab/vmalloc memory [0.00] Process swapper (pid: 0, stack limit = 0x(ptrval)) [0.00] Stack: (0xc1e01e68 to 0xc1e02000) [0.00] 1e60: 8400 [0.00] 1e80: c1e09410 7fff c1e01f58 8200 c1e01ec0 c1e01e68 [0.00] 1ea0: c0eb20f0 133fd468 61d6 c1e09980 c20891c0 c1b1c168 [0.00] 1ec0: c1e01edc c20891c0 c1245ac8 [0.00] 1ee0: ff8005a8 c1d2fca0 eefea2fc c1c93594 eefea2fc c1e09980 [0.00] 1f00: c20891c0 c1b1c168 c1e09410 c1c93a10 eefea2fc c1c939d8 c1d2fbdc [0.00] 1f20: 7fff c1caf854 c1e0afa0 c1c04c44 c1e01f4c c1e01f50 [0.00] 1f40: c1e09980 c198cfec c1e01f9c 8000 c1e09980 [0.00] 1f60: c2089064 0830 410fc075 10c5387d c039de88 c1e01f9c [0.00] 1f80: c1e09980 c1c00420 c1e04ec0 c1e09980 c2089064 0830 410fc075 10c5387d [0.00] 1fa0: c1c00b08 [0.00] 1fc0: c1cc8a6c c1c00420 0051 10c0387d [0.00] 1fe0: 0830 410fc075 10c5387d [0.00] __invoke_psci_fn_smc from 0xc1e01e68 [0.00] Code: bad PC value Any comments? Thanks, Jagan.
Re: [PATCHv8 00/15] net/lwip: add lwip library for the network stack
On Wed, Sep 13, 2023 at 11:06:13AM +0100, Peter Robinson wrote: > > >> Then if for development you need to pull he history of lwip, you can do > > >> it with: > > >> git pull -s subtree lwip master --allow-unrelated-histories > > >> (but I think nobody will need this.) > > >> > > >> New update of the lwip net/lwip/lwip-external dir will be done with: > > >> git pull -s subtree lwip master --allow-unrelated-histories --squash > > >> Squash commit also has to be git format-patch friendly. > > >> > > >> If you are ok with that proposal I will send v9 with the first patch > > >> created with steps above. > > > > > > We've gone through this before. The whole purpose of this is not > > > having to maintain patches. > > > Simon, instead of "I had problems in the past", can you elaborate a bit > > > more? > > > > > > Tom said he is fine with subtrees instead of submodules and I know for > > > a fact EDK2 doesn't have too many issues with submodules. > > > Their documentation is pretty clear on building and requires > > > > > > git clone https://github.com/tianocore/edk2.git > > > cd edk2 > > > git submodule update --init > > > > > > Perhaps the situation has improved since you had issues? > > > > While I don't really care how you solve this technically, I'd *strongly* > > be interested for U-Boot to use *unmodified* lwIP sources where an > > explicit reference to an lwIP commit is used. I'd rather integrate > > bugfixes for U-Boot into lwIP than having the sources drift apart. > > Strongly agree here, we want to use upstream and all the combined > development and reviews etc rather than forking off and ending up with > yet another slightly different IP stack. The whole advantage of > adopting LWIP is the advantage of combined security, features and bugs > from a wide range of projects :-) Yes, this is what I want as well, and why I'm perhaps more agreeable with the approaches where it's a lot harder for us to start forking things unintentionally. I gather submodule rather than subtree would be better for that case? -- Tom signature.asc Description: PGP signature
Re: [PATCH] am33xx: ignore return value from usb_ether_init()
Can this get added to the next release? I don't see it in -next.
On Thu, Aug 31, 2023 at 6:15 AM Michal Suchánek wrote:
> Hello,
>
> On Wed, Aug 30, 2023 at 10:49:50PM -0400, Trevor Woerner wrote:
> > In 2cb43ef1c223 ("usb: ether: Fix error handling in usb_ether_init") the
> error
> > handling of usb_ether_init() was changed. Not a single other call site
> of this
> > function checks its return value, therefore follow suit in the am33xx
> code.
>
> then there is the question what point is there in having a return value
> in this function at all.
>
> Anyway, it's fine to not check the return value in the caller if there
> is no use for the error.
>
> Reviewed-by: Michal Suchánek
>
> >
> > Do not cause the boot to halt if the usb gadget ethernet initialization
> fails:
> >
> > initcall sequence 9ffdbd84 failed at call 808024b9 (err=-19)
> > ### ERROR ### Please RESET the board ###
> >
> > Signed-off-by: Trevor Woerner
> > ---
> > arch/arm/mach-omap2/am33xx/board.c | 6 +-
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/am33xx/board.c
> b/arch/arm/mach-omap2/am33xx/board.c
> > index ecc0a592e993..8f772310a1a7 100644
> > --- a/arch/arm/mach-omap2/am33xx/board.c
> > +++ b/arch/arm/mach-omap2/am33xx/board.c
> > @@ -270,11 +270,7 @@ int arch_misc_init(void)
> > return ret;
> >
> > #if defined(CONFIG_DM_ETH) && defined(CONFIG_USB_ETHER)
> > - ret = usb_ether_init();
> > - if (ret) {
> > - pr_err("USB ether init failed\n");
> > - return ret;
> > - }
> > + usb_ether_init();
> > #endif
> >
> > return 0;
> > --
> > 2.41.0.327.gaa9166bcc0ba
> >
>
Re: [PATCH v3 3/3] config: j7200: remove not needed power config
On 10:09-20230913, Udit Kumar wrote:
> For J7200, R5/SPL TI_SCI_POWER_DOMAIN should be
> disabled as DM is a separate binary like other
> SOC of J7* family.
>
> Fixes: 02dff65efe70 ("configs: j7200_evm_r5: Add initial support")
>
> Signed-off-by: Udit Kumar
> ---
> configs/j7200_evm_r5_defconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig
> index 6d240b16cb..9e744ba434 100644
> --- a/configs/j7200_evm_r5_defconfig
> +++ b/configs/j7200_evm_r5_defconfig
> @@ -126,7 +126,6 @@ CONFIG_SPL_PINCTRL=y
> # CONFIG_SPL_PINCTRL_GENERIC is not set
> CONFIG_PINCTRL_SINGLE=y
> CONFIG_POWER_DOMAIN=y
> -CONFIG_TI_SCI_POWER_DOMAIN=y
> CONFIG_TI_POWER_DOMAIN=y
> CONFIG_DM_PMIC=y
> CONFIG_PMIC_TPS65941=y
> --
> 2.34.1
>
Reviewed-by: Nishanth Menon
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5
849D 1736 249D
Re: [PATCH 1/1] configs: NVMe/USB target boot devices on VisionFive 2
On Thu, Sep 07, 2023 at 03:53:36PM +0200, Heinrich Schuchardt wrote: > Make NVMe and USB target boot devices on the StarFive VisionFive 2 board. > The boot devices are sorted by decreasing device speed. > > CONFIG_PCI_INIT_R=y is set via [1]. 'start usb' is added to CONFIG_PREBOOT > by the same patch. > > [1] [PATCH v1 1/2] configs: starfive: Enable PCIE auto enum and NVME/USB > stuff for Starfive Visionfive 2 > > https://lore.kernel.org/u-boot/ty3p286mb2611c9ad6e5bb3756a959e8998...@ty3p286mb2611.jpnp286.prod.outlook.com/ > > Signed-off-by: Heinrich Schuchardt > --- > include/configs/starfive-visionfive2.h | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Leo Yu-Chi Liang
Re: [PATCH 1/1] riscv: set fdtfile on VisionFive 2
On Thu, Sep 07, 2023 at 01:21:28PM +0200, Heinrich Schuchardt wrote: > Multiple revisions of the StarFive VisionFive 2 board exist. They can be > identified by reading their EEPROM. > > Linux uses two differently named device-tree files. To load the correct > device-tree we need to set $fdtfile to the device-tree file name that > matches the board revision. > > Signed-off-by: Heinrich Schuchardt > --- > arch/riscv/Kconfig| 1 + > .../visionfive2/starfive_visionfive2.c| 43 ++- > 2 files changed, 42 insertions(+), 2 deletions(-) Reviewed-by: Leo Yu-Chi Liang
Re: [PATCH v1] rockchip: include: asm: fix entering download mode rk3066
Hi,
Maybe put this Rockchip rk3066 patch on hold, if we can find a better solution.
Any insider help is appreciated here.
The idea to test BOOT_BROM_DOWNLOAD comes from 30_LPDDR2_300MHz_DD.bin
Setting location 0x10080028 only breaks the inner loop.
BOOT_BROM_DOWNLOAD flag is not reset.
It continues testing for boot blocks on NAND till nothing is left.
For NAND up to 50 erase blocks are tested
struct func_array function_table[4] =
{
{ &NAND_INIT, &NAND_READ_BLK, &NAND_READ_HEADER, &NAND_GET_PAGE, 50 },
{ &SPI0_INIT, &SPI_READ_BLK, &SPI_READ_HEADER, &SPI_GET_PAGE, 13 },
{ &SPI1_INIT, &SPI_READ_BLK, &SPI_READ_HEADER, &SPI_GET_PAGE, 13 },
{ &EMMC_INIT, &EMMC_READ_BLK, &EMMC_READ_HEADER, &EMMC_GET_PAGE, 1 }
};
Given below part of rk3066 BROM main read loop.
Proposal 1:
In case of BOOT_BROM_DOWNLOAD return to 0x1100 instead of 0x1058.
Also reset flag.
//
In case this TPL binary is reused to upload and to store this TPL entry function
can be called from 3 to 4 BROM functions. Only when data comes from
SPI, NAND or EMMC it needs to early return to BROM.
In download mode this section is skipped when the lr register
address is not in the functions that handles USB OTG and UART0.
Proposal 2:
Instead of a retry_counter test LR: 0x1058.
In all other cases skip early return
Let me know your ideas/advice!
How rk3188 handle return flag?
Johan
/
//main read loop rk3066 bootrom
db4: e92d4ff3push{r0, r1, r4, r5, r6, r7, r8, r9, sl,
fp, lr}
[..]
1050: 02840004addeq r0, r4, #4
1054: 012fff30blxeq r0
// Branch to U-boot TPL early
// LR: 0x1058
// In case of BOOT_BROM_DOWNLOAD return to 0x1100 instead of 0x1058.
1058: e3a01c02mov r1, #512@ 0x200
105c: e2840b01add r0, r4, #1024 @ 0x400
1060: ebfffc49bl 0x18c
1064: e3a01c02mov r1, #512@ 0x200
1068: e2840c06add r0, r4, #1536 @ 0x600
106c: ebfffc46bl 0x18c
1070: e2490004sub r0, r9, #4
1074: e156cmp r6, r0
1078: 12844b02addne r4, r4, #2048 @ 0x800
107c: 1a12bne 0x10cc
1080: e59f10ccldr r1, [pc, #204] @ 0x1154
1084: e59f00d0ldr r0, [pc, #208] @ 0x115c
1088: e5d13000ldrbr3, [r1]
108c: e5d02000ldrbr2, [r0]
1090: e1530002cmp r3, r2
1094: 05d12001ldrbeq r2, [r1, #1]
1098: 05d03001ldrbeq r3, [r0, #1]
109c: 01520003cmpeq r2, r3
10a0: 1a0dbne 0x10dc
10a4: e5d12002ldrbr2, [r1, #2]
10a8: e5d03002ldrbr3, [r0, #2]
10ac: e1520003cmp r2, r3
10b0: 05d3ldrbeq r0, [r0, #3]
10b4: 05d12003ldrbeq r2, [r1, #3]
10b8: 0152cmpeq r2, r0
10bc: 1a06bne 0x10dc
10c0: e2810004add r0, r1, #4
10c4: e12fff30blx r0
// Branch to U-boot TPL full
// LR2: 0x10c8
10c8: e3a04206mov r4, #1610612736 @ 0x6000
10cc: e598ldr r0, [r8]
10d0: e371cmn r0, #1
10d4: 12866004addne r6, r6, #4
10d8: 1ab6bne 0xfb8
10dc: e1570006cmp r7, r6
10e0: 93a00206movls r0, #1610612736 @ 0x6000
10e4: 912fff30blxls r0
// Branch to U-boot SPL
10e8: e28bb001add fp, fp, #1
10ec: e59a0010ldr r0, [sl, #16]
10f0: e15bcmp r0, fp
10f4: 92855001addls r5, r5, #1
10f8: 9a35bls 0xdd4
10fc: ea88b 0xf24
// End off read loop
// In case of BOOT_BROM_DOWNLOAD return to 0x1100 instead of 0x1058 to prevent
further testing.
1100: e8bd8ffepop {r1, r2, r3, r4, r5, r6, r7, r8, r9,
sl, fp, pc}
On 9/11/23 17:37, Johan Jonker wrote:
> When a Rockchip rk3066 board download key is pressed it hangs.
> The rk3066 BROM doesn't have support to check the return to BROM,
> so when a key is pressed the loop that reads data must be broken
> by writing a "-1" to the variable that points to the next page address.
> It then goes in download mode and waits for data on USB OTG and UART0.
>
> Signed-off-by: Johan Jonker
> ---
> arch/arm/include/asm/arch-rockchip/boot0.h | 32 +++---
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h
> b/arch/arm/include/asm/arch-rockchip/boot0.h
> index 0c375e543a5e..305461ce3751 100644
> --- a/arch/arm/include/asm/arch-rockchip/boot0.h
> +++ b/arch/arm/include/a
Re: [PATCHv8 00/15] net/lwip: add lwip library for the network stack
> >> Then if for development you need to pull he history of lwip, you can do > >> it with: > >> git pull -s subtree lwip master --allow-unrelated-histories > >> (but I think nobody will need this.) > >> > >> New update of the lwip net/lwip/lwip-external dir will be done with: > >> git pull -s subtree lwip master --allow-unrelated-histories --squash > >> Squash commit also has to be git format-patch friendly. > >> > >> If you are ok with that proposal I will send v9 with the first patch > >> created with steps above. > > > > We've gone through this before. The whole purpose of this is not > > having to maintain patches. > > Simon, instead of "I had problems in the past", can you elaborate a bit > > more? > > > > Tom said he is fine with subtrees instead of submodules and I know for > > a fact EDK2 doesn't have too many issues with submodules. > > Their documentation is pretty clear on building and requires > > > > git clone https://github.com/tianocore/edk2.git > > cd edk2 > > git submodule update --init > > > > Perhaps the situation has improved since you had issues? > > While I don't really care how you solve this technically, I'd *strongly* > be interested for U-Boot to use *unmodified* lwIP sources where an > explicit reference to an lwIP commit is used. I'd rather integrate > bugfixes for U-Boot into lwIP than having the sources drift apart. Strongly agree here, we want to use upstream and all the combined development and reviews etc rather than forking off and ending up with yet another slightly different IP stack. The whole advantage of adopting LWIP is the advantage of combined security, features and bugs from a wide range of projects :-)
Re: [PATCHv8 00/15] net/lwip: add lwip library for the network stack
On 13.09.2023 09:53, Ilias Apalodimas wrote: Hi Maxim, On Wed, 13 Sept 2023 at 10:32, Maxim Uvarov wrote: On Wed, 13 Sept 2023 at 01:27, Simon Glass wrote: Hi Maxim, On Tue, 12 Sept 2023 at 05:42, Maxim Uvarov wrote: On Fri, 8 Sept 2023 at 19:59, Tom Rini wrote: On Fri, Sep 08, 2023 at 07:53:05PM +0600, Maxim Uvarov wrote: Before apply these patches it is needed to create lwIP merge into U-Boot: git subtree add --prefix net/lwip/lwip-external https://git.savannah.nongnu.org/git/lwip.git master --squash or create git submodule, depends how it's more easy to maintain external library. So, I think we're going to go with subtree. Please work out how to integrate the above in to the build process automatically (and such that we can maintain it via upgrades moving forward). -- Tom I did not find a good way to friend git format-patch, git am and subtree. And now with using subtree I can provide my thoughts, in general I do not see any big advantages with maintaining subtree code. Problem is that: 1. subtree does some root reset. So rebase looks like: label onto # Branch acbc0469a49de7055141cc730aa9c728e61b6de2-2 reset [new root] pick acbc0469a4 Squashed 'net/lwip/lwip-external/' content from commit 84fde1ebbf label acbc0469a49de7055141cc730aa9c728e61b6de2-2 reset onto merge -C ec4a128c8d acbc0469a49de7055141cc730aa9c728e61b6de2-2 # Merge commit 'acbc0469a49de7055141cc730aa9c728e61b6de2' as 'net/lwip/lwip-external' pick 739681a6f5 net/lwip: add doc/develop/net_lwip.rst pick f0ecab85e0 net/lwip: integrate lwIP library 2. if --rebase-merges option was not provided to rebase, then rebase will omit subtree directory and try to apply rebase patches to root directory. I.e. in current case squashed commit for lwip, will be applied to uboot root directory instead of ./net/lwip/lwip-external. 3. due to broken rebases without --rebase-merges more likely things like git bisect also will not work. 4. change in subtree code ./net/lwip/lwip-external/../something.c will create a git commit which looks like a standard U-Boot commit. I.e. path starts with ./net/lwip/lwip-external/ I don't really understand most of the above, but I take it that subtree has some problems...I did find an article about subtree [1] 5. lwip maintains code with a mailing list. So we don't need to push subtree git somewhere to create a pull request. 6. I rechecked the latest edk2 and they use submodules now. (openssl, libfdt, berkeley-softfloat-3 and others). 7. dynamic download also looks horrible for me. I.e. create subtree in Makefile on compilation process. I think maintaining that will give you a bunch of problems. I think we should not touch git structure after cloning. So what I can here suggest: 1. lwip source code is 9.4M. If we compare all code it will be 564M in total. So just having 1 commit witn copy of lwip library will work here. So we add a 9.4MB patch for the code we need? I suppose that is OK, although it is much larger than net/ today (0.5MB). What is the churn on lwip? E.g. would it be easy to add a commit every few months to bring in upstream changes? or 2. use git submodules. Size of the project will be lower. Submodule will not allow you to use local changes. I.e. change needs to be merged into the upstream project and then you can update git HEAD for the submodule. I really don't want to work with a submodule project. I've just had too many problems. or 3. inside u-boot.git create branch lib-lwip and clone lwip repo there. Then use git submoule to connect this branch as a folder to the main U-Boot code. It really needs to be properly part of U-Boot. Ok. Then more likely we don't need all the git history of lwip inside uboot.git. Then the option with a single commit is more preferable. Then we can use part 2 of this article, how to go with standard git commands: 1. git remote add -f lwip https://git.savannah.nongnu.org/git/lwip.git git read-tree --prefix=net/lwip/lwip-external -u lwip/master git commit -m "lwip merge sha: " this will create a git format-patch friendly commit. Then we send it to the mailing list and apply. I hope the mailing list will allow us to send a 7.8 MB patch. Then if for development you need to pull he history of lwip, you can do it with: git pull -s subtree lwip master --allow-unrelated-histories (but I think nobody will need this.) New update of the lwip net/lwip/lwip-external dir will be done with: git pull -s subtree lwip master --allow-unrelated-histories --squash Squash commit also has to be git format-patch friendly. If you are ok with that proposal I will send v9 with the first patch created with steps above. We've gone through this before. The whole purpose of this is not having to maintain patches. Simon, instead of "I had problems in the past", can you elaborate a bit more? Tom said he is fine with subtrees instead of submodules and I know for a fact EDK2 doesn't have too many issues with submo
Re: [PATCHv8 05/15] net/lwip: implement tftp cmd
On 13.09.2023 08:15, Ilias Apalodimas wrote:
+
+/*
+ * (C) Copyright 2023 Linaro Ltd.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "tftp_client.h"
+#include "tftp_server.h"
+#include
+
+#include
+
+#include
+
+static ulong daddr;
+static ulong size;
+
+static void *tftp_open(const char *fname, const char *mode, u8_t is_write)
+{
+ return NULL;
+}
+
+static void tftp_close(void *handle)
+{
+ log_info("\ndone\n");
+ log_info("Bytes transferred = %ld (0x%lx hex)\n", size, size);
+
+ bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
+ if (env_set_hex("filesize", size)) {
+ log_err("filesize not updated\n");
+ ulwip_exit(-1);
+ return;
+ }
+ ulwip_exit(0);
+}
+
+static int tftp_read(void *handle, void *buf, int bytes)
+{
+ return 0;
+}
+
+static int tftp_write(void *handle, struct pbuf *p)
+{
+ struct pbuf *q;
+
+ for (q = p; q != NULL; q = q->next) {
+ memcpy((void *)daddr, q->payload, q->len);
+ daddr += q->len;
+ size += q->len;
+ log_info("#");
+ }
+
+ return 0;
+}
+
+static void tftp_error(void *handle, int err, const char *msg, int size)
+{
+ char message[100];
+
+ memset(message, 0, sizeof(message));
+ memcpy(message, msg, LWIP_MIN(sizeof(message) - 1, (size_t)size));
+
+ log_info("TFTP error: %d (%s)", err, message);
+}
+
+static const struct tftp_context tftp = {
+ tftp_open,
+ tftp_close,
+ tftp_read,
+ tftp_write,
+ tftp_error
+};
+
+int ulwip_tftp(ulong addr, char *fname)
+{
+ void *f = (void *)0x1; /* unused fake file handle*/
I haven't dug into lwip details yet, but I am not sure this is safe to use.
Simon, since you maintain the lwip part can you elaborate a bit more?
From the lwIP design, using an invalid pointer here is ok: that pointer
is only used as a client handle which is never dereferenced internally.
The only requirement is that it is not NULL, as that is the validity
check for the tftp client to know the handle is valid (or e.g. open
failed etc.).
So while we can leave 0x1 here, using a static uint8_t would probably be
better, at the expense of using a byte for nothing.
Regards,
Simon
+ err_t err;
+ ip_addr_t srv;
+ int ret;
+ char *server_ip;
+
+ if (!fname || addr == 0)
+ return CMD_RET_FAILURE;
+
+ size = 0;
+ daddr = addr;
+ server_ip = env_get("serverip");
+ if (!server_ip) {
+ log_err("error: serverip variable has to be set\n");
+ return CMD_RET_FAILURE;
+ }
+
+ ret = ipaddr_aton(server_ip, &srv);
+ if (!ret) {
+ log_err("error: ipaddr_aton\n");
+ return CMD_RET_FAILURE;
+ }
+
+ log_info("TFTP from server %s; our IP address is %s\n",
+server_ip, env_get("ipaddr"));
+ log_info("Filename '%s'.\n", fname);
+ log_info("Load address: 0x%lx\n", daddr);
+ log_info("Loading:");
+
+ bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
+
+ err = tftp_init_client(&tftp);
+ if (!(err == ERR_OK || err == ERR_USE))
+ log_err("tftp_init_client err: %d\n", err);
+
+ err = tftp_get(f, &srv, TFTP_PORT, fname, TFTP_MODE_OCTET);
+ /* might return different errors, like routing problems */
+ if (err != ERR_OK) {
+ log_err("tftp_get err=%d\n", err);
+ return CMD_RET_FAILURE;
+ }
+
+ if (env_set_hex("fileaddr", addr)) {
+ log_err("fileaddr not updated\n");
+ return CMD_RET_FAILURE;
+ }
+
+ return CMD_RET_SUCCESS;
+}
--
2.30.2
Thanks
/Ilias
Re: [PATCHv8 04/15] net/lwip: implement dhcp cmd
On 13.09.2023 08:07, Ilias Apalodimas wrote:
On Fri, Sep 08, 2023 at 07:53:09PM +0600, Maxim Uvarov wrote:
+
+#include
+#include
+#include
+
+#include
+#include
+#include "lwip/timeouts.h"
+
+#include
+#include
+
+#define DHCP_WAIT_MS 2000
Is this the time we wait for a dhcp reply? If so we should bump it to
something higher
+
+static void dhcp_tmo(void *arg)
+{
+ struct netif *netif = (struct netif *)arg;
+ struct dhcp *dhcp;
+ int err = 0;
+
+ dhcp = netif_get_client_data(netif, LWIP_NETIF_CLIENT_DATA_INDEX_DHCP);
+ if (!dhcp)
+ return;
+
+ switch (dhcp->state) {
+ case DHCP_STATE_BOUND:
+ err += env_set("bootfile", dhcp->boot_file_name);
+ err += env_set("ipaddr", ip4addr_ntoa(&dhcp->offered_ip_addr));
+ err += env_set("netmask", ip4addr_ntoa(&dhcp->offered_sn_mask));
+ err += env_set("serverip", ip4addr_ntoa(&dhcp->server_ip_addr));
+ if (err)
+ log_err("error update envs\n");
+ log_info("DHCP client bound to address %s\n",
ip4addr_ntoa(&dhcp->offered_ip_addr));
+ break;
+ default:
+ return;
+ }
+}
+
+int ulwip_dhcp(void)
+{
+ struct netif *netif;
+ int eth_idx;
+
+ eth_idx = eth_get_dev_index();
+ if (eth_idx < 0)
+ return -EPERM;
+
+ netif = netif_get_by_index(eth_idx + 1);
Why is the +1 needed here?
Netif index is driven by posix design and is 1-based in contrast to
U-Boot's 0-based dev index. A comment noting that would probably help
the ones not knowing lwIP.
Regards,
Simon
+ if (!netif)
+ return -ENOENT;
+
+ sys_timeout(DHCP_WAIT_MS, dhcp_tmo, netif);
+
+ return dhcp_start(netif) ? 0 : -EPERM;
+}
--
2.30.2
Re: [PATCHv8 03/15] net/lwip: implement dns cmd
On 13.09.2023 07:56, Ilias Apalodimas wrote:
On Fri, Sep 08, 2023 at 07:53:08PM +0600, Maxim Uvarov wrote:
U-Boot recently got support for an alternative network stack using LWIP.
Replace dns command with the LWIP variant while keeping the output and
error messages identical.
Signed-off-by: Maxim Uvarov
---
include/net/lwip.h | 19 +++
net/lwip/Makefile| 2 ++
net/lwip/apps/dns/lwip-dns.c | 63
3 files changed, 84 insertions(+)
create mode 100644 include/net/lwip.h
create mode 100644 net/lwip/apps/dns/lwip-dns.c
diff --git a/include/net/lwip.h b/include/net/lwip.h
new file mode 100644
index 00..ab3db1a214
--- /dev/null
+++ b/include/net/lwip.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[]);
+
+/**
+ * ulwip_dns() - creates the DNS request to resolve a domain host name
+ *
+ * This function creates the DNS request to resolve a domain host name.
Function
+ * can return immediately if previous request was cached or it might require
+ * entering the polling loop for a request to a remote server.
+ *
+ * @name:dns name to resolve
+ * @varname: (optional) U-Boot variable name to store the result
+ * Returns: ERR_OK(0) for fetching entry from the cache
+ * -EINPROGRESS success, can go to the polling loop
+ * Other value < 0, if error
+ */
+int ulwip_dns(char *name, char *varname);
diff --git a/net/lwip/Makefile b/net/lwip/Makefile
index 3fd5d34564..5d8d5527c6 100644
--- a/net/lwip/Makefile
+++ b/net/lwip/Makefile
@@ -62,3 +62,5 @@ obj-$(CONFIG_NET) += lwip-external/src/netif/ethernet.o
obj-$(CONFIG_NET) += port/if.o
obj-$(CONFIG_NET) += port/sys-arch.o
+
+obj-y += apps/dns/lwip-dns.o
diff --git a/net/lwip/apps/dns/lwip-dns.c b/net/lwip/apps/dns/lwip-dns.c
new file mode 100644
index 00..b340302f2c
--- /dev/null
+++ b/net/lwip/apps/dns/lwip-dns.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * (C) Copyright 2023 Linaro Ltd.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+static void dns_found_cb(const char *name, const ip_addr_t *ipaddr, void
*callback_arg)
+{
+ char *varname = (char *)callback_arg;
+ char *ipstr = ip4addr_ntoa(ipaddr);
+
+ if (varname)
+ env_set(varname, ipstr);
+ log_info("resolved %s to %s\n", name, ipstr);
+ ulwip_exit(0);
+}
+
+int ulwip_dns(char *name, char *varname)
+{
+ int err;
+ ip_addr_t ipaddr; /* not used */
+ ip_addr_t dns1;
+ ip_addr_t dns2;
+ char *dnsenv = env_get("dnsip");
+ char *dns2env = env_get("dnsip2");
+
+ if (!dnsenv && !dns2env) {
+ log_err("nameserver is not set with dnsip and dnsip2 vars\n");
+ return -ENOENT;
+ }
+
+ if (!dnsenv)
+ log_warning("dnsip var is not set\n");
+ if (!dns2env)
+ log_warning("dnsip2 var is not set\n");
+
+ dns_init();
+
+ if (ipaddr_aton(dnsenv, &dns1))
+ dns_setserver(0, &dns1);
+
+ if (ipaddr_aton(dns2env, &dns2))
+ dns_setserver(1, &dns2);
env_get will return NULL if any of these is not set. Looking at
ipaddr_aton() of lwip that might lead to a NULL deref in ip_2_ip6()
Looking at the NULL checks in ipaddr_aton(), you found a bug in lwIP.
I'd vote to leave the above code as is and rely on the bug being fixed
in lwIP before U-Boot enables IPv6 (this is only a bug in dual-stack
mode where IPv4 and IPv6 is enabled).
Regards,
Simon
+
+ err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
+ if (err == ERR_OK)
+ dns_found_cb(name, &ipaddr, varname);
+
+ /* convert lwIP ERR_INPROGRESS to U-Boot -EINPROGRESS */
+ if (err == ERR_INPROGRESS)
+ err = -EINPROGRESS;
+
+ return err;
+}
--
2.30.2
Re: [PATCHv8 00/15] net/lwip: add lwip library for the network stack
Hi Maxim, On Wed, 13 Sept 2023 at 10:32, Maxim Uvarov wrote: > > > > On Wed, 13 Sept 2023 at 01:27, Simon Glass wrote: >> >> Hi Maxim, >> >> On Tue, 12 Sept 2023 at 05:42, Maxim Uvarov wrote: >> > >> > On Fri, 8 Sept 2023 at 19:59, Tom Rini wrote: >> > >> > > On Fri, Sep 08, 2023 at 07:53:05PM +0600, Maxim Uvarov wrote: >> > > >> > > > Before apply these patches it is needed to create lwIP merge into >> > > U-Boot: >> > > > git subtree add --prefix net/lwip/lwip-external >> > > https://git.savannah.nongnu.org/git/lwip.git master --squash >> > > > or >> > > > create git submodule, depends how it's more easy to maintain external >> > > > library. >> > > >> > > So, I think we're going to go with subtree. Please work out how to >> > > integrate the above in to the build process automatically (and such that >> > > we can maintain it via upgrades moving forward). >> > > >> > > -- >> > > Tom >> > > >> > >> > I did not find a good way to friend git format-patch, git am and subtree. >> > And now with using subtree I can provide my thoughts, in general I do not >> > see any big advantages >> > with maintaining subtree code. >> > >> > Problem is that: >> > >> > 1. subtree does some root reset. So rebase looks like: >> > label onto >> > >> > >> > >> > # Branch acbc0469a49de7055141cc730aa9c728e61b6de2-2 >> > >> > reset [new root] >> > >> > pick acbc0469a4 Squashed 'net/lwip/lwip-external/' content from commit >> > 84fde1ebbf >> > label acbc0469a49de7055141cc730aa9c728e61b6de2-2 >> > >> > >> > >> > reset onto >> > >> > merge -C ec4a128c8d acbc0469a49de7055141cc730aa9c728e61b6de2-2 # Merge >> > commit 'acbc0469a49de7055141cc730aa9c728e61b6de2' as >> > 'net/lwip/lwip-external' >> > pick 739681a6f5 net/lwip: add doc/develop/net_lwip.rst >> > >> > pick f0ecab85e0 net/lwip: integrate lwIP library >> > >> > 2. if --rebase-merges option was not provided to rebase, then rebase will >> > omit subtree directory and try to apply rebase patches to root directory. >> > I.e. in current case squashed commit for lwip, will be applied to uboot >> > root directory instead of ./net/lwip/lwip-external. >> > >> > 3. due to broken rebases without --rebase-merges more likely things like >> > git bisect also will not work. >> > >> > 4. change in subtree code ./net/lwip/lwip-external/../something.c will >> > create a git commit which looks like a standard U-Boot commit. I.e. path >> > starts with ./net/lwip/lwip-external/ >> >> I don't really understand most of the above, but I take it that >> subtree has some problems...I did find an article about subtree [1] >> >> > >> > 5. lwip maintains code with a mailing list. So we don't need to push >> > subtree git somewhere to create a pull request. >> > >> > 6. I rechecked the latest edk2 and they use submodules now. (openssl, >> > libfdt, berkeley-softfloat-3 and others). >> > >> > 7. dynamic download also looks horrible for me. I.e. create subtree in >> > Makefile on compilation process. I think maintaining that will give you a >> > bunch of problems. I think we should not touch git structure after cloning. >> > >> > So what I can here suggest: >> > 1. lwip source code is 9.4M. If we compare all code it will be 564M in >> > total. So just having 1 commit witn copy of lwip library will work here. >> >> So we add a 9.4MB patch for the code we need? I suppose that is OK, >> although it is much larger than net/ today (0.5MB). >> >> What is the churn on lwip? E.g. would it be easy to add a commit every >> few months to bring in upstream changes? >> >> > >> > or >> > >> > 2. use git submodules. Size of the project will be lower. Submodule will >> > not allow you to use local changes. I.e. change needs to be merged into the >> > upstream project and then you can update git HEAD for the submodule. >> >> I really don't want to work with a submodule project. I've just had >> too many problems. >> >> > >> > or >> > >> > 3. inside u-boot.git create branch lib-lwip and clone lwip repo there. Then >> > use git submoule to connect this branch as a folder to the main U-Boot >> > code. >> >> It really needs to be properly part of U-Boot. >> > > Ok. Then more likely we don't need all the git history of lwip inside > uboot.git. Then the option with a single commit is more preferable. > Then we can use part 2 of this article, how to go with standard git commands: > > 1. > > git remote add -f lwip https://git.savannah.nongnu.org/git/lwip.git > git read-tree --prefix=net/lwip/lwip-external -u lwip/master > git commit -m "lwip merge sha: " > > this will create a git format-patch friendly commit. Then we send it to the > mailing list and apply. > I hope the mailing list will allow us to send a 7.8 MB patch. > > > Then if for development you need to pull he history of lwip, you can do it > with: > git pull -s subtree lwip master --allow-unrelated-histories > (but I think nobody will need this.) > > New update of the lwip net/lwip/lwip-external dir will be done with: > git pull -s subtree lwip ma
Re: [PATCHv8 00/15] net/lwip: add lwip library for the network stack
On Wed, 13 Sept 2023 at 01:27, Simon Glass wrote: > Hi Maxim, > > On Tue, 12 Sept 2023 at 05:42, Maxim Uvarov > wrote: > > > > On Fri, 8 Sept 2023 at 19:59, Tom Rini wrote: > > > > > On Fri, Sep 08, 2023 at 07:53:05PM +0600, Maxim Uvarov wrote: > > > > > > > Before apply these patches it is needed to create lwIP merge into > > > U-Boot: > > > > git subtree add --prefix net/lwip/lwip-external > > > https://git.savannah.nongnu.org/git/lwip.git master --squash > > > > or > > > > create git submodule, depends how it's more easy to maintain external > > > > library. > > > > > > So, I think we're going to go with subtree. Please work out how to > > > integrate the above in to the build process automatically (and such > that > > > we can maintain it via upgrades moving forward). > > > > > > -- > > > Tom > > > > > > > I did not find a good way to friend git format-patch, git am and subtree. > > And now with using subtree I can provide my thoughts, in general I do not > > see any big advantages > > with maintaining subtree code. > > > > Problem is that: > > > > 1. subtree does some root reset. So rebase looks like: > > label onto > > > > > > > > # Branch acbc0469a49de7055141cc730aa9c728e61b6de2-2 > > > > reset [new root] > > > > pick acbc0469a4 Squashed 'net/lwip/lwip-external/' content from commit > > 84fde1ebbf > > label acbc0469a49de7055141cc730aa9c728e61b6de2-2 > > > > > > > > reset onto > > > > merge -C ec4a128c8d acbc0469a49de7055141cc730aa9c728e61b6de2-2 # Merge > > commit 'acbc0469a49de7055141cc730aa9c728e61b6de2' as > > 'net/lwip/lwip-external' > > pick 739681a6f5 net/lwip: add doc/develop/net_lwip.rst > > > > pick f0ecab85e0 net/lwip: integrate lwIP library > > > > 2. if --rebase-merges option was not provided to rebase, then rebase > will > > omit subtree directory and try to apply rebase patches to root directory. > > I.e. in current case squashed commit for lwip, will be applied to uboot > > root directory instead of ./net/lwip/lwip-external. > > > > 3. due to broken rebases without --rebase-merges more likely things like > > git bisect also will not work. > > > > 4. change in subtree code ./net/lwip/lwip-external/../something.c will > > create a git commit which looks like a standard U-Boot commit. I.e. path > > starts with ./net/lwip/lwip-external/ > > I don't really understand most of the above, but I take it that > subtree has some problems...I did find an article about subtree [1] > > > > > 5. lwip maintains code with a mailing list. So we don't need to push > > subtree git somewhere to create a pull request. > > > > 6. I rechecked the latest edk2 and they use submodules now. (openssl, > > libfdt, berkeley-softfloat-3 and others). > > > > 7. dynamic download also looks horrible for me. I.e. create subtree in > > Makefile on compilation process. I think maintaining that will give you a > > bunch of problems. I think we should not touch git structure after > cloning. > > > > So what I can here suggest: > > 1. lwip source code is 9.4M. If we compare all code it will be 564M in > > total. So just having 1 commit witn copy of lwip library will work here. > > So we add a 9.4MB patch for the code we need? I suppose that is OK, > although it is much larger than net/ today (0.5MB). > > What is the churn on lwip? E.g. would it be easy to add a commit every > few months to bring in upstream changes? > > > > > or > > > > 2. use git submodules. Size of the project will be lower. Submodule will > > not allow you to use local changes. I.e. change needs to be merged into > the > > upstream project and then you can update git HEAD for the submodule. > > I really don't want to work with a submodule project. I've just had > too many problems. > > > > > or > > > > 3. inside u-boot.git create branch lib-lwip and clone lwip repo there. > Then > > use git submoule to connect this branch as a folder to the main U-Boot > code. > > It really needs to be properly part of U-Boot. > > Ok. Then more likely we don't need all the git history of lwip inside uboot.git. Then the option with a single commit is more preferable. Then we can use part 2 of this article, how to go with standard git commands: 1. git remote add -f lwip https://git.savannah.nongnu.org/git/lwip.git git read-tree --prefix=net/lwip/lwip-external -u lwip/master git commit -m "lwip merge sha: " this will create a git format-patch friendly commit. Then we send it to the mailing list and apply. I hope the mailing list will allow us to send a 7.8 MB patch. Then if for development you need to pull he history of lwip, you can do it with: git pull -s subtree lwip master --allow-unrelated-histories (but I think nobody will need this.) New update of the lwip net/lwip/lwip-external dir will be done with: git pull -s subtree lwip master --allow-unrelated-histories --squash Squash commit also has to be git format-patch friendly. If you are ok with that proposal I will send v9 with the first patch created with steps above. Thanks, Maxim.
