Re: [PATCH v2] i2c: eeprom: Use reg property instead of offset and size

[Heiko Schocher]

Hi Michal,

Am 17.06.2020 um 05:12 schrieb Simon Glass:

Hi Michal,

On Tue, 16 Jun 2020 at 07:53, Michal Simek  wrote:




On 16. 06. 20 15:43, Simon Glass wrote:

On Mon, 15 Jun 2020 at 07:41, Michal Simek  wrote:


Remove adhoc dt binding for fixed-partition definition for i2c eeprom.
fixed-partition are using reg property instead of offset/size pair.

Signed-off-by: Michal Simek 
---

Changes in v2:
- Bootcount tested on zynqmp zcu104
- Add missing address/size cells
- Use dev_read_addr_size_index
- Check parameters

Just build tested - ge_bx50v3_defconfig
Definitely please retest on hardware.

---
  arch/arm/dts/imx53-ppd-uboot.dtsi| 15 +--
  arch/arm/dts/imx6q-bx50v3-uboot.dtsi | 12 +++-
  drivers/misc/i2c_eeprom.c| 20 ++--
  3 files changed, 26 insertions(+), 21 deletions(-)



We have a sandbox I2C EEPROM, so you should be able to use the
existing test, right?


The way how I have tested it was via drivers/bootcount/i2c-eeprom.c
driver which define which eeprom stores it.
Do you have any existing tests for bootcount done via sandbox?

If bootcount is not the right way to go then doing this code should be
better way. It means just define some partitions (0 size - for failure,
then proper range, proper write, write behind size for failure).


Can you use drivers/misc/i2c_eeprom.c?

See test/dm/bootcount.c for the sandbox tests for bootcount.


Any updates on this?

@Robert: May you find time to test this change and give us feedback?

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

Re: [PATCH v2 19/25] x86: mtrr: Update MTRRs on all CPUs

[Simon Glass]

Hi Bin,

On Sun, 28 Jun 2020 at 02:07, Bin Meng  wrote:
>
> Hi Simon,
>
> On Mon, Jun 15, 2020 at 1:00 AM Simon Glass  wrote:
> >
> > When the boot CPU MTRRs are updated, perform the same update on all
other
> > CPUs so they are kept in sync.
> >
> > This avoids kernel warnings about mismatched MTRRs.
> >
> > Signed-off-by: Simon Glass 
> > Reviewed-by: Wolfgang Wallner 
> > ---
> >
> > Changes in v2:
> > - Rename function to mtrr_write_all()
> >
> >  arch/x86/cpu/mtrr.c | 57 +
> >  1 file changed, 57 insertions(+)
> >
> > diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
> > index c9b4e7d06e..5c567551e5 100644
> > --- a/arch/x86/cpu/mtrr.c
> > +++ b/arch/x86/cpu/mtrr.c
> > @@ -74,10 +74,61 @@ void mtrr_read_all(struct mtrr_info *info)
> > }
> >  }
> >
> > +void mtrr_write_all(struct mtrr_info *info)
> > +{
> > +   struct mtrr_state state;
> > +   int i;
> > +
> > +   for (i = 0; i < MTRR_COUNT; i++) {
> > +   mtrr_open(, true);
> > +   wrmsrl(MTRR_PHYS_BASE_MSR(i), info->mtrr[i].base);
> > +   wrmsrl(MTRR_PHYS_MASK_MSR(i), info->mtrr[i].mask);
> > +   mtrr_close(, true);
> > +   }
> > +}
> > +
> > +static void write_mtrrs(void *arg)
> > +{
> > +   struct mtrr_info *info = arg;
> > +
> > +   mtrr_write_all(info);
> > +}
> > +
> > +static void read_mtrrs(void *arg)
> > +{
> > +   struct mtrr_info *info = arg;
> > +
> > +   mtrr_read_all(info);
> > +}
> > +
> > +/**
> > + * mtrr_copy_to_aps() - Copy the MTRRs from the boot CPU to other CPUs
> > + *
> > + * @return 0 on success, -ve on failure
> > + */
> > +static int mtrr_copy_to_aps(void)
> > +{
> > +   struct mtrr_info info;
> > +   int ret;
> > +
> > +   ret = mp_run_on_cpus(MP_SELECT_BSP, read_mtrrs, );
> > +   if (ret == -ENXIO)
> > +   return 0;
> > +   else if (ret)
> > +   return log_msg_ret("bsp", ret);
> > +
> > +   ret = mp_run_on_cpus(MP_SELECT_APS, write_mtrrs, );
> > +   if (ret)
> > +   return log_msg_ret("bsp", ret);
> > +
> > +   return 0;
> > +}
> > +
> >  int mtrr_commit(bool do_caches)
> >  {
> > struct mtrr_request *req = gd->arch.mtrr_req;
> > struct mtrr_state state;
> > +   int ret;
> > int i;
> >
> > debug("%s: enabled=%d, count=%d\n", __func__, gd->arch.has_mtrr,
> > @@ -99,6 +150,12 @@ int mtrr_commit(bool do_caches)
> > mtrr_close(, do_caches);
> > debug("mtrr done\n");
> >
> > +   if (gd->flags & GD_FLG_RELOC) {
> > +   ret = mtrr_copy_to_aps();
> > +   if (ret)
> > +   return log_msg_ret("copy", ret);
> > +   }
> > +
> > return 0;
> >  }
> >
> > --
>
> We need to find a place where mtrr in BSP is duplicated in AP.
>
> Currently this is done in init_cache_f_r() but at that time SMP is not
> ready yet.

I don't think we can. I thought about this quite a bit.

We need mtrr to be set up in init_cache_f_r(), otherwise U-Boot becomes
very slow after relocation.

We want to set up mp_init() while setting up the CPUs.

The video driver decides the address of the MTRR for the display, but it
will not be probed until used.

For example, for a board that uses serial for stdout but has video and
wants to use it later, 'setenv stdout vidconsole' on the command line will
probe the video and add the MTRR.

So I think we have to sync over the MTRRs whenever there is a change.

Regards,
Simon

Re: [PATCH v2 15/25] x86: mp: Add iterators for CPUs

[Simon Glass]

Hi Bin,

On Sun, 28 Jun 2020 at 01:35, Bin Meng  wrote:
>
> Hi Simon,
>
> On Mon, Jun 15, 2020 at 1:00 AM Simon Glass  wrote:
> >
> > It is convenient to iterate through the CPUs performing work on each one
> > and processing the result. Add a few iterator functions which handle
this.
> > These can be used by any client code. It can call mp_run_on_cpus() on
> > each CPU that is returned, handling them one at a time.
> >
> > Signed-off-by: Simon Glass 
> > Reviewed-by: Wolfgang Wallner 
> > ---
> >
> > (no changes since v1)
> >
> >  arch/x86/cpu/mp_init.c| 62 +++
> >  arch/x86/include/asm/mp.h | 40 +
> >  2 files changed, 102 insertions(+)
> >
> > diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> > index 9970b51c8d..c708c3e3c0 100644
> > --- a/arch/x86/cpu/mp_init.c
> > +++ b/arch/x86/cpu/mp_init.c
> > @@ -675,6 +675,68 @@ int mp_park_aps(void)
> > return get_timer(start);
> >  }
> >
> > +int mp_first_cpu(int cpu_select)
> > +{
> > +   struct udevice *dev;
> > +   int num_cpus;
> > +   int ret;
> > +
> > +   /*
> > +* This assumes that CPUs are numbered from 0. This function
tries to
> > +* avoid assuming the CPU 0 is the boot CPU
>
> So CPU 0 is not BSP ..

It does seem to be, but I worry that we might hit an architecture where it
is not?

>
> > +*/
> > +   if (cpu_select == MP_SELECT_ALL)
> > +   return 0;   /* start with the first one */
> > +
> > +   ret = get_bsp(, _cpus);
> > +   if (ret < 0)
> > +   return log_msg_ret("bsp", ret);
> > +
> > +   /* Return boot CPU if requested */
> > +   if (cpu_select == MP_SELECT_BSP)
> > +   return ret;
> > +
> > +   /* Return something other than the boot CPU, if APs requested */
> > +   if (cpu_select == MP_SELECT_APS && num_cpus > 1)
> > +   return ret == 0 ? 1 : 0;
> > +
> > +   /* Try to check for an invalid value */
> > +   if (cpu_select < 0 || cpu_select >= num_cpus)
>
> The logic (cpu_select >= num_cpus) assumes that cpu number is consecutive.

Yes, 0...n-1 as set up by mp_init() using values from the device tree.

>
> > +   return -EINVAL;
> > +
> > +   return cpu_select;  /* return the only selected one */
> > +}
> > +
> > +int mp_next_cpu(int cpu_select, int prev_cpu)
> > +{
> > +   struct udevice *dev;
> > +   int num_cpus;
> > +   int ret;
> > +   int bsp;
> > +
> > +   /* If we selected the BSP or a particular single CPU, we are
done */
> > +   if (cpu_select == MP_SELECT_BSP || cpu_select >= 0)
>
> Why stops on MP_SELECT_BSP?
>
> So if I call the 2 APIs with the following sequence, is this allowed?
>
> int cpu = mp_first_cpu(MP_SELECT_ALL); // this will return zero
> cpu = mp_next_cpu(MP_SELECT_BSP, cpu);  // then I got -EFBIG

No, you must provide the same value for cpu_select to both calls.

It does say that in the header file but I will add another note there.

>
> > +   return -EFBIG;
> > +
> > +   /* Must be doing MP_SELECT_ALL or MP_SELECT_APS; return the
next CPU */
> > +   ret = get_bsp(, _cpus);
> > +   if (ret < 0)
> > +   return log_msg_ret("bsp", ret);
> > +   bsp = ret;
> > +
> > +   /* Move to the next CPU */
> > +   assert(prev_cpu >= 0);
> > +   ret = prev_cpu + 1;
> > +
> > +   /* Skip the BSP if needed */
> > +   if (cpu_select == MP_SELECT_APS && ret == bsp)
> > +   ret++;
> > +   if (ret >= num_cpus)
> > +   return -EFBIG;
> > +
> > +   return ret;
> > +}
> > +
> >  int mp_init(void)
> >  {
> > int num_aps, num_cpus;
> > diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
> > index 38961ca44b..9f4223ae8c 100644
> > --- a/arch/x86/include/asm/mp.h
> > +++ b/arch/x86/include/asm/mp.h
> > @@ -115,6 +115,31 @@ int mp_run_on_cpus(int cpu_select, mp_run_func
func, void *arg);
> >   * @return 0 on success, -ve on error
> >   */
> >  int mp_park_aps(void);
> > +
> > +/**
> > + * mp_first_cpu() - Get the first CPU to process, from a selection
> > + *
> > + * This is used to iterate through selected CPUs. Call this function
first, then
> > + * call mp_next_cpu() repeatedly until it returns -EFBIG.
>
> So how does specify the cpu_select of these 2 APIs? Do they have to be
the same?
>
> For example, how about the following code sequence:
>
> int cpu = mp_first_cpu(MP_SELECT_APS);
> cpu = mp_next_cpu(MP_SELECT_BSP, cpu);
> cpu = mp_next_cpu(MP_SELECT_APS, cpu);
>
> It's quite ambiguous API design.

I will beef up the comments. cpu_select must be the same for all calls for
the iteration to work.

>
> > + *
> > + * @cpu_select: Selected CPUs (either a CPU number or MP_SELECT_...)
> > + * @return next CPU number to run on (e.g. 0)
> > + */
> > +int mp_first_cpu(int cpu_select);
> > +
> > +/**
> > + * mp_next_cpu() - Get the next CPU to process, from a selection
> > + *
> > + * This is 

Re: [PATCH v2 10/25] x86: mp: Support APs waiting for instructions

[Simon Glass]

Hi Bin,

On Sun, 28 Jun 2020 at 00:25, Bin Meng  wrote:
>
> Hi Simon,
>
> On Mon, Jun 15, 2020 at 1:00 AM Simon Glass  wrote:
> >
> > At present the APs (non-boot CPUs) are inited once and then parked ready
> > for the OS to use them. However in some cases we want to send new
requests
> > through, such as to change MTRRs and keep them consistent across CPUs.
> >
> > Change the last state of the flight plan to go into a wait loop,
accepting
> > instructions from the main CPU.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v2:
> > - Add more comments
> >
> >  arch/x86/cpu/mp_init.c| 126 +++---
> >  arch/x86/include/asm/mp.h |  11 
> >  2 files changed, 128 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> > index cd4cae559d..673fdc9628 100644
> > --- a/arch/x86/cpu/mp_init.c
> > +++ b/arch/x86/cpu/mp_init.c
> > @@ -43,13 +43,38 @@ struct mp_flight_plan {
> > struct mp_flight_record *records;
> >  };
> >
> > +/**
> > + * struct mp_callback - Callback information for APs
> > + *
> > + * @func: Function to run
> > + * @arg: Argument to pass to the function
> > + * @logical_cpu_number: Either a CPU number (i.e. dev->req_seq) or a
special
> > + * value like MP_SELECT_BSP. It tells the AP whether it should
process this
> > + * callback
> > + */
> > +struct mp_callback {
> > +   /**
> > +* func() - Function to call on the AP
> > +*
> > +* @arg: Argument to pass
> > +*/
> > +   void (*func)(void *arg);
> > +   void *arg;
> > +   int logical_cpu_number;
> > +};
> > +
> >  static struct mp_flight_plan mp_info;
> >
> > -struct cpu_map {
> > -   struct udevice *dev;
> > -   int apic_id;
> > -   int err_code;
> > -};
> > +/*
> > + * ap_callbacks - Callback mailbox array
> > + *
> > + * Array of callback, one entry for each available CPU, indexed by the
CPU
> > + * number, which is dev->req_seq. The entry for the main CPU is never
used.
> > + * When this is NULL, there is no pending work for the CPU to run. When
> > + * non-NULL it points to the mp_callback structure. This is shared
between all
> > + * CPUs, so should only be written by the main CPU.
> > + */
> > +static struct mp_callback **ap_callbacks;
> >
> >  static inline void barrier_wait(atomic_t *b)
> >  {
> > @@ -147,11 +172,9 @@ static void ap_init(unsigned int cpu_index)
> > debug("AP: slot %d apic_id %x, dev %s\n", cpu_index, apic_id,
> >   dev ? dev->name : "(apic_id not found)");
> >
> > -   /* Walk the flight plan */
> > +   /* Walk the flight plan, which never returns */
> > ap_do_flight_plan(dev);
> > -
> > -   /* Park the AP */
> > -   debug("parking\n");
> > +   debug("Unexpected return\n");
> >  done:
> > stop_this_cpu();
> >  }
> > @@ -455,6 +478,86 @@ static int get_bsp(struct udevice **devp, int
*cpu_countp)
> > return dev->req_seq;
> >  }
> >
> > +/**
> > + * read_callback() - Read the pointer in a callback slot
> > + *
> > + * This is called by APs to read their callback slow to see if there
is a
> > + * pointer to new instructions
> > + *
> > + * @slot: Pointer to the AP's callback slot
> > + * @return value of that pointer
> > + */
> > +static struct mp_callback *read_callback(struct mp_callback **slot)
> > +{
> > +   struct mp_callback *ret;
> > +
> > +   asm volatile ("mov  %1, %0\n"
> > +   : "=r" (ret)
> > +   : "m" (*slot)
> > +   : "memory"
> > +   );
>
> Why do we need inline assembly here? Prevent compiler reordering the
> codes ("memory")? Can we use C and additional memory barrier to do
> this?

Yes that should work. Will fix.

>
> > +   return ret;
> > +}
> > +
> > +/**
> > + * store_callback() - Store a pointer to the callback slot
> > + *
> > + * This is called by APs to write NULL into the callback slot when
they have
> > + * finished the work requested by the BSP.
> > + *
> > + * @slot: Pointer to the AP's callback slot
> > + * @val: Value to write (e.g. NULL)
> > + */
> > +static void store_callback(struct mp_callback **slot, struct
mp_callback *val)
> > +{
> > +   asm volatile ("mov  %1, %0\n"
> > +   : "=m" (*slot)
> > +   : "r" (val)
> > +   : "memory"
> > +   );
> > +}
> > +
> > +/**
> > + * ap_wait_for_instruction() - Wait for and process requests from the
main CPU
> > + *
> > + * This is called by APs (here, everything other than the main boot
CPU) to
> > + * await instructions. They arrive in the form of a function call and
argument,
> > + * which is then called. This uses a simple mailbox with atomic
read/set
> > + *
> > + * @cpu: CPU that is waiting
> > + * @unused: Optional argument provided by struct mp_flight_record, not
used here
> > + * @return Does not return
> > + */
> > +static int ap_wait_for_instruction(struct udevice *cpu, void *unused)
> > +{
> > +   struct 

Re: [PATCH v3 07/15] dt-bindings: memory: ns3: update GIC LPI address

[Simon Glass]

Hi Rayagonda,

On Sun, 5 Jul 2020 at 12:21, Rayagonda Kokatanur <
rayagonda.kokata...@broadcom.com> wrote:
>
> Hi Simon,
>
> On Sun, Jul 5, 2020 at 12:20 AM Rayagonda Kokatanur
>  wrote:
> >
> > Hi Simon,
> >
> > On Sat, Jul 4, 2020 at 1:20 PM Rayagonda Kokatanur
> >  wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass  wrote:
> > > >
> > > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > > >  wrote:
> > > > >
> > > > > Update NS3 GIC LPI address.
> > > > >
> > > > > Signed-off-by: Rayagonda Kokatanur <
rayagonda.kokata...@broadcom.com>
> > > > > ---
> > > > >  include/dt-bindings/memory/bcm-ns3-mc.h | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > >
> > > > Reviewed-by: Simon Glass 
> > > >
> > > > Lower case hex
> > > >
> > > > Can these be in the device tree and use a driver, like syscon maybe?
>
> I will use dt and syscon to get the LPI base address.
> This will require change in arch/arm/lib/gic-v3-its.c driver.
>
> Hope this is okay. Please let me know.

Yes of course, change whatever you like.

Regards,
SImon

Re: [PATCH v2] arm64: issue ISB after updating system registers

[Masahiro Yamada]

Hi.

On Mon, Jun 29, 2020 at 12:29 AM Masahiro Yamada  wrote:
>
> On Wed, Jun 24, 2020 at 11:07 AM Volodymyr Babchuk
>  wrote:
> >
> > ARM Architecture reference manual clearly states that PE pipeline
> > should be flushed after any change to system registers. Refer to
> > paragraph "B2.3.5 Memory Barriers" at page B2-92 of "Arm Architecture
> > Reference Manual ARMv8 for ARMv8-A Architecture Profile" (ARM DDI
> > 0487B.a).
> >
> > Failing to issue instruction memory synchronization barrier can lead
> > to spurious errors, like synchronous exception when accessing FPU
> > registers. This is very prominent on CPUs with long instruction
> > pipeline, like ARM Cortex A72.
> >
> > This change fixes the following U-Boot panic:
> >
> >  "Synchronous Abort" handler, esr 0x1fe0
> >  elr: 800948cc lr : 80091e04
> >  x0 : 801ffdc8 x1 : 00c8
> >  x2 : 800979d4 x3 : 801ffc60
> >  x4 : 801ffd40 x5 : ff80ffd8
> >  x6 : 801ffd70 x7 : 801ffd70
> >  x8 : 000a x9 : 
> >  x10: 0044 x11: 
> >  x12:  x13: 
> >  x14:  x15: 
> >  x16: 8008b2e0 x17: 
> >  x18: 801ffec0 x19: 800957b0
> >  x20: 00c8 x21: 801ffdc8
> >  x22: 8009909e x23: 
> >  x24:  x25: 
> >  x26:  x27: 
> >  x28:  x29: 801ffc50
> >
> >  Code: a94417e4 a90217e4 a9051fe6 a90617e4 (3d801fe0)
> >
> > While executing instruction
> >
> >  str q0, [sp, #112]
> >
> > in vsnprintf() prologue. This panic was observed only on Cortex A72 so
> > far.
> >
> > This patch places ISBs on other strategic places as well.
> >
> > Also, this probably is the right fix for the issue workarounded in the
> > commit 45f41c13 ("ARM: uniphier: add weird workaround code for LD20")
>
>
> Thanks for addressing this issue.
> Currently, I do not have a board in hand to test this.
> (I do not commute to the office due to COVID-19 these days...)
>
> I have another SoC board, but it does not integrate CA72.
> I have ever seen this problem only on CA72.
>
> Eventually, I will go to the office, and I can test this.
> But, you do not need to wait for my test if other people
> review it.
>
> Thank you.
>


Today I tested this patch.

Yes, it fixes the CA72 problem on my board.

Tested-by: Masahiro Yamada 



After this patch is picked up,
I will revert the ugly workaround:

http://patchwork.ozlabs.org/project/uboot/patch/20200706042304.15853-1-yamada.masah...@socionext.com/

Interestingly, I observe this problem
only on U-Boot running at EL1.

Anyway, this fix makes it work at
any exception level.

--
Best Regards
Masahiro Yamada

[PATCH] Revert "ARM: uniphier: add weird workaround code for LD20"

[Masahiro Yamada]

This reverts commit 45f41c134baf5ff1bbf59d33027f6c79884fa4d9.

This weird workaround was the best I came up with at that time
to boot U-Boot from TF-A.

I noticed U-Boot successfully boots on LD20 (i.e. CA72 CPU) by using
the latest TF-A.

Specifically, since the following TF-A commit, U-Boot runs at EL2
instead of EL1, and this issue does not happen:

|commit f998a052fd94ea082833109f25b94ed5bfa24e8b
|Author: Masahiro Yamada 
|Date:   Thu Jul 25 10:57:38 2019 +0900
|
|uniphier: run BL33 at EL2
|
|All the SoCs in 64-bit UniPhier SoC family support EL2.
|
|Just hard-code MODE_EL2 instead of using el_implemented() helper.
|
|Change-Id: I7ab48002c5205bc8c013e1b46313b57d6c431db0
|Signed-off-by: Masahiro Yamada 

However, if I revert that, U-Boot hangs on LD20, presumably because
some EL1 code in U-Boot triggers this issue.

Now that commit "arm64: issue ISB after updating system registers"
fixes this issue properly, this weird workaround is unneeded
irrespective of the exception level at which U-Boot runs.

Signed-off-by: Masahiro Yamada 
---

 arch/arm/mach-uniphier/arm64/Makefile|  1 -
 arch/arm/mach-uniphier/arm64/lowlevel_init.S | 13 -
 2 files changed, 14 deletions(-)
 delete mode 100644 arch/arm/mach-uniphier/arm64/lowlevel_init.S

diff --git a/arch/arm/mach-uniphier/arm64/Makefile 
b/arch/arm/mach-uniphier/arm64/Makefile
index c569551120c7..750c4f756edb 100644
--- a/arch/arm/mach-uniphier/arm64/Makefile
+++ b/arch/arm/mach-uniphier/arm64/Makefile
@@ -1,4 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 obj-y += mem_map.o
-obj-$(CONFIG_ARCH_UNIPHIER_LD20) += lowlevel_init.o
diff --git a/arch/arm/mach-uniphier/arm64/lowlevel_init.S 
b/arch/arm/mach-uniphier/arm64/lowlevel_init.S
deleted file mode 100644
index f4e5cbbbd1cc..
--- a/arch/arm/mach-uniphier/arm64/lowlevel_init.S
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Copyright (C) 2017 Socionext Inc.
- */
-
-#include 
-
-ENTRY(lowlevel_init)
-   /* LD20 needs the following code to boot.  I do not know why. */
-   mrs x0, sctlr_el1
-   msr sctlr_el1, x0
-   ret
-ENDPROC(lowlevel_init)
-- 
2.17.1

[RFC PATCH 16/16] patman: Support listing comments from patchwork

[Simon Glass]

While reviewing feedback it is helpful to see the review comments on the
command line to check that each has been addressed. Add an option to
support that.

Update the workflow documentation to describe the new features.

Signed-off-by: Simon Glass 
---

 tools/patman/README |  36 +-
 tools/patman/control.py |  11 +-
 tools/patman/main.py|   5 +-
 tools/patman/status.py  | 278 +++-
 4 files changed, 227 insertions(+), 103 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 595a0d616b..0ce299d2c0 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -14,6 +14,7 @@ This tool is a Python script which:
 It also has some Patchwork features:
 - shows review tags from Patchwork so you can update your local patches
 - pulls these down into a new branch on request
+- lists comments received on a series
 
 It is intended to automate patch creation and make it a less
 error-prone process. It is useful for U-Boot and Linux work so far,
@@ -391,6 +392,8 @@ but has the new review tags in it. You can check that this 
worked with:
 
 which should show that there are no new responses compared to this new branch.
 
+There is also a -C option to list the comments received for each patch.
+
 
 Example Work Flow
 =
@@ -475,17 +478,33 @@ people on the list don't see your secret info.
 Of course patches often attract comments and you need to make some updates.
 Let's say one person sent comments and you get an Acked-by: on one patch.
 Also, the patch on the list that you were waiting for has been merged,
-so you can drop your wip commit. So you resync with upstream:
+so you can drop your wip commit.
+
+Take a look on patchwork and find out the URL of the series. This will be
+something like http://patchwork.ozlabs.org/project/uboot/list/?series=187331
+Add this to a tag in your top commit:
+
+   Series-link: http://patchwork.ozlabs.org/project/uboot/list/?series=187331
+
+You can use then patman to collect the Acked-by tag to the correct commit,
+creating a new 'version 2' branch for us-cmd:
+
+patman status -d us-cmd2
+git checkout us-cmd2
+
+You can look at the comments in Patchwork or with:
+
+patman status -C
+
+Then you can resync with upstream:
 
 git fetch origin   (or whatever upstream is called)
 git rebase origin/master
 
-and use git rebase -i to edit the commits, dropping the wip one. You add
-the ack tag to one commit:
-
-Acked-by: Heiko Schocher 
+and use git rebase -i to edit the commits, dropping the wip one.
 
-update the Series-cc: in the top commit:
+Then update the Series-cc: in the top commit to add the person who reviewed
+the v1 series:
 
 Series-cc: bfin, marex, Heiko Schocher 
 
@@ -524,7 +543,9 @@ so to send them:
 
 and it will create and send the version 2 series.
 
-General points:
+
+General points
+==
 
 1. When you change back to the us-cmd branch days or weeks later all your
 information is still there, safely stored in the commits. You don't need
@@ -606,3 +627,4 @@ a bad thing.
 Simon Glass 
 v1, v2, 19-Oct-11
 revised v3 24-Nov-11
+revised v4 Independence Day 2020, with Patchwork integration
diff --git a/tools/patman/control.py b/tools/patman/control.py
index b6ba0a56c0..b7d23b58c1 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -174,11 +174,13 @@ def send(args):
 args.limit, args.dry_run, args.in_reply_to, args.thread,
 args.smtp_server)
 
-def patchwork_status(branch, count, start, end, dest_branch, force):
+def patchwork_status(branch, count, start, end, dest_branch, force,
+ show_comments):
 """Check the status of patches in patchwork
 
 This finds the series in patchwork using the Series-link tag, checks for 
new
-review tags, displays then and creates a new branch with the review tags.
+comments and review tags, displays then and creates a new branch with the
+review tags.
 
 Args:
 branch (str): Branch to create patches from (None = current)
@@ -190,6 +192,8 @@ def patchwork_status(branch, count, start, end, 
dest_branch, force):
 dest_branch (str): Name of new branch to create with the updated tags
 (None to not create a branch)
 force (bool): With dest_branch, force overwriting an existing branch
+show_comments (bool): True to display snippets from the comments
+provided by reviewers
 
 Raises:
 ValueError: if the branch has no Series-link value
@@ -206,4 +210,5 @@ def patchwork_status(branch, count, start, end, 
dest_branch, force):
 # Import this here to avoid failing on other commands if the dependencies
 # are not present
 from patman import status
-status.check_patchwork_status(series, link, branch, dest_branch, force)
+status.check_patchwork_status(series, link, branch, dest_branch, force,
+show_comments)
diff --git a/tools/patman/main.py b/tools/patman/main.py

[RFC PATCH 15/16] patman: Support updating a branch with review tags

[Simon Glass]

It is tedious to add review tags into the local branch and errors can
sometimes be made. Add an option to create a new branch with the review
tags obtained from patchwork.

Signed-off-by: Simon Glass 
---

 tools/patman/README | 17 +++--
 tools/patman/control.py |  9 ---
 tools/patman/main.py|  7 +-
 tools/patman/status.py  | 54 ++---
 4 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index a85974740f..595a0d616b 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -11,11 +11,13 @@ This tool is a Python script which:
 - Runs the patches through checkpatch.pl and its own checks
 - Optionally emails them out to selected people
 
-It also shows review tags from Patchwork so you can update your local patches.
+It also has some Patchwork features:
+- shows review tags from Patchwork so you can update your local patches
+- pulls these down into a new branch on request
 
 It is intended to automate patch creation and make it a less
 error-prone process. It is useful for U-Boot and Linux work so far,
-since it uses the checkpatch.pl script.
+since they use the checkpatch.pl script.
 
 It is configured almost entirely by tags it finds in your commits.
 This means that you can work on a number of different branches at
@@ -378,6 +380,17 @@ attracted another review each. If the series needs 
changes, you can update
 these commits with the new review tag before sending the next version of the
 series.
 
+To automatically pull into these tags into a new branch, use the -d option:
+
+patman status -d mtrr4
+
+This will create a new 'mtrr4' branch which is the same as your current branch
+but has the new review tags in it. You can check that this worked with:
+
+patman -b mtrr4 status
+
+which should show that there are no new responses compared to this new branch.
+
 
 Example Work Flow
 =
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 3bb3c236e4..b6ba0a56c0 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -174,11 +174,11 @@ def send(args):
 args.limit, args.dry_run, args.in_reply_to, args.thread,
 args.smtp_server)
 
-def patchwork_status(branch, count, start, end):
+def patchwork_status(branch, count, start, end, dest_branch, force):
 """Check the status of patches in patchwork
 
 This finds the series in patchwork using the Series-link tag, checks for 
new
-comments / review tags and displays them
+review tags, displays then and creates a new branch with the review tags.
 
 Args:
 branch (str): Branch to create patches from (None = current)
@@ -187,6 +187,9 @@ def patchwork_status(branch, count, start, end):
 start (int): Start partch to use (0=first / top of branch)
 end (int): End patch to use (0=last one in series, 1=one before that,
 etc.)
+dest_branch (str): Name of new branch to create with the updated tags
+(None to not create a branch)
+force (bool): With dest_branch, force overwriting an existing branch
 
 Raises:
 ValueError: if the branch has no Series-link value
@@ -203,4 +206,4 @@ def patchwork_status(branch, count, start, end):
 # Import this here to avoid failing on other commands if the dependencies
 # are not present
 from patman import status
-status.check_patchwork_status(series, link, branch)
+status.check_patchwork_status(series, link, branch, dest_branch, force)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 227eb3b228..c8fc035c10 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -90,6 +90,10 @@ AddCommonArgs(test_parser)
 
 status = subparsers.add_parser('status',
help='Check status of patches in patchwork')
+status.add_argument('-d', '--dest-branch', type=str,
+help='Name of branch to create with collected responses')
+status.add_argument('-f', '--force', action='store_true',
+help='Force overwriting an existing branch')
 AddCommonArgs(status)
 
 # Parse options twice: first to get the project and second to handle
@@ -156,7 +160,8 @@ elif args.cmd == 'send':
 elif args.cmd == 'status':
 ret_code = 0
 try:
-control.patchwork_status(args.branch, args.count, args.start, args.end)
+control.patchwork_status(args.branch, args.count, args.start, args.end,
+ args.dest_branch, args.force)
 except Exception as e:
 print('patman: %s: %s' % (type(e).__name__, e))
 if args.debug:
diff --git a/tools/patman/status.py b/tools/patman/status.py
index c2e98a5d11..71124d6e5e 100644
--- a/tools/patman/status.py
+++ b/tools/patman/status.py
@@ -304,7 +304,48 @@ def ShowResponses(rtags, indent, is_new):
 count += 1
 return count
 
-def check_patchwork_status(series, url, branch):
+def 

[RFC PATCH 13/16] patchstream: Support parsing of review snippets

[Simon Glass]

Add support for parsing the contents of a patchwork 'patch' web page
containing comments received from reviewers. This allows patman to show
these comments in a simple 'snippets' format.

A snippet is some quoted code plus some unquoted comments below it. Each
review is from a unique person/email and can produce multiple snippets,
one for each part of the code that attracts a comment.

Signed-off-by: Simon Glass 
---

 tools/patman/patchstream.py | 53 +
 1 file changed, 53 insertions(+)

diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 0c68c86156..5e2b8e3986 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -2,10 +2,12 @@
 # Copyright (c) 2011 The Chromium OS Authors.
 #
 
+import collections
 import datetime
 import math
 import os
 import re
+import queue
 import shutil
 import tempfile
 
@@ -80,6 +82,10 @@ class PatchStream:
 self.state = STATE_MSG_HEADER# What state are we in?
 self.signoff = []# Contents of signoff line
 self.commit = None   # Current commit
+self.snippets = []   # List of unquoted test blocks
+self.recent_quoted = collections.deque([], 5)
+self.recent_unquoted = queue.Queue()
+self.was_quoted = None
 
 def AddToSeries(self, line, name, value):
 """Add a new Series-xxx tag.
@@ -165,6 +171,40 @@ class PatchStream:
 self.commit.AddChange(self.change_version, change)
 self.change_lines = []
 
+def FinaliseSnippet(self):
+"""Finish off a snippet and add it to the list
+
+This is called when we get to the end of a snippet, i.e. the we enter
+the next block of quoted text:
+
+This is a comment from someone.
+
+Something else
+
+> Now we have some code  <- end of snippet
+> more code
+
+Now a comment about the above code
+
+This adds the snippet to our list
+"""
+quoted_lines = []
+while len(self.recent_quoted):
+quoted_lines.append(self.recent_quoted.popleft())
+unquoted_lines = []
+valid = False
+while not self.recent_unquoted.empty():
+text = self.recent_unquoted.get()
+unquoted_lines.append(text)
+if text:
+valid = True
+if valid:
+lines = quoted_lines + unquoted_lines
+if lines[0].startswith('On ') and lines[0].endswith('wrote:'):
+lines = lines[1:]
+if lines:
+self.snippets.append(lines)
+
 def ProcessLine(self, line):
 """Process a single line of a patch file or commit log
 
@@ -384,6 +424,18 @@ class PatchStream:
 out = [line]
 self.linenum += 1
 self.skip_blank = False
+
+# If this is quoted, keep recent lines
+if self.linenum > 1 and line:
+if line.startswith('>'):
+if not self.was_quoted:
+self.FinaliseSnippet()
+self.recent_quoted.append(line)
+self.was_quoted = True
+else:
+self.recent_unquoted.put(line)
+self.was_quoted = False
+
 if self.state == STATE_DIFFS:
 pass
 
@@ -407,6 +459,7 @@ class PatchStream:
 
 def Finalize(self):
 """Close out processing of this patch stream"""
+self.FinaliseSnippet()
 self.FinalizeChange()
 self.CloseCommit()
 if self.lines_after_test:
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 12/16] patman: Add a -D option to enable debugging

[Simon Glass]

Most users don't want to see traceback errors. Add an option to enable
them for debugging. Disable them by default.

Signed-off-by: Simon Glass 
---

 tools/patman/main.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tools/patman/main.py b/tools/patman/main.py
index 77f187e769..b96000807e 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -10,6 +10,7 @@ from argparse import ArgumentParser
 import os
 import re
 import sys
+import traceback
 import unittest
 
 if __name__ == "__main__":
@@ -34,6 +35,8 @@ def AddCommonArgs(parser):
 default=-1, help='Automatically create patches from top n commits')
 parser.add_argument('-e', '--end', type=int, default=0,
 help='Commits to skip at end of patch list')
+parser.add_argument('-D', '--debug', action='store_true',
+help='Enabling debugging (provides a full traceback on error)')
 parser.add_argument('-s', '--start', dest='start', type=int,
 default=0, help='Commit to start creating patches from (0 = HEAD)')
 
@@ -98,6 +101,9 @@ if hasattr(args, 'project'):
 if __name__ != "__main__":
 pass
 
+if not args.debug:
+sys.tracebacklimit = 0
+
 # Run our meagre tests
 if args.cmd == 'test':
 import doctest
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 10/16] patman: Support collecting response tags in Patchstream

[Simon Glass]

Collect response tags such as 'Reviewed-by' while parsing the stream.
This allows us to see what tags are present.

Add a new 'Fixes' tag also, since this is now quite common.

Signed-off-by: Simon Glass 
---

 tools/patman/commit.py  | 14 ++
 tools/patman/patchstream.py | 21 -
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/tools/patman/commit.py b/tools/patman/commit.py
index 48d0529c53..8d583c4ed3 100644
--- a/tools/patman/commit.py
+++ b/tools/patman/commit.py
@@ -2,6 +2,7 @@
 # Copyright (c) 2011 The Chromium OS Authors.
 #
 
+import collections
 import re
 
 # Separates a tag: at the beginning of the subject from the rest of it
@@ -23,6 +24,9 @@ class Commit:
 notes: List of lines in the commit (not series) notes
 change_id: the Change-Id: tag that was stripped from this commit
 and can be used to generate the Message-Id.
+rtags: Response tags (e.g. Reviewed-by) collected by the commit, dict:
+key: rtag type (e.g. 'Reviewed-by')
+value: Set of people who gave that rtag, each a name/email string
 """
 def __init__(self, hash):
 self.hash = hash
@@ -33,6 +37,7 @@ class Commit:
 self.signoff_set = set()
 self.notes = []
 self.change_id = None
+self.rtags = collections.defaultdict(set)
 
 def AddChange(self, version, info):
 """Add a new change line to the change list for a version.
@@ -88,3 +93,12 @@ class Commit:
   return False
 self.signoff_set.add(signoff)
 return True
+
+def AddRtag(self, rtag_type, who):
+"""Add a response tag to a commit
+
+Args:
+key: rtag type (e.g. 'Reviewed-by')
+who: Person who gave that rtag, e.g. 'Fred Bloggs 
'
+"""
+self.rtags[rtag_type].add(who)
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 2ea8ebcc3f..0c68c86156 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -37,7 +37,7 @@ re_change_id = re.compile('^Change-Id: *(.*)')
 re_commit_tag = re.compile('^Commit-([a-z-]*): *(.*)')
 
 # Commit tags that we want to collect and keep
-re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc): (.*)')
+re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc|Fixes): (.*)')
 
 # The start of a new commit in the git log
 re_commit = re.compile('^commit ([0-9a-f]*)$')
@@ -112,6 +112,15 @@ class PatchStream:
 self.in_section = 'commit-' + name
 self.skip_blank = False
 
+def AddCommitRtag(self, rtag_type, who):
+"""Add a response tag to the current commit
+
+Args:
+key: rtag type (e.g. 'Reviewed-by')
+who: Person who gave that rtag, e.g. 'Fred Bloggs 
'
+"""
+self.commit.AddRtag(rtag_type, who)
+
 def CloseCommit(self):
 """Save the current commit into our commit list, and reset our state"""
 if self.commit and self.is_log:
@@ -346,12 +355,14 @@ class PatchStream:
 
 # Detect tags in the commit message
 elif tag_match:
+rtag_type, who = tag_match.groups()
+self.AddCommitRtag(rtag_type, who)
 # Remove Tested-by self, since few will take much notice
-if (tag_match.group(1) == 'Tested-by' and
-tag_match.group(2).find(os.getenv('USER') + '@') != -1):
+if (rtag_type == 'Tested-by' and
+who.find(os.getenv('USER') + '@') != -1):
 self.warn.append("Ignoring %s" % line)
-elif tag_match.group(1) == 'Patch-cc':
-self.commit.AddCc(tag_match.group(2).split(','))
+elif rtag_type == 'Patch-cc':
+self.commit.AddCc(who.split(','))
 else:
 out = [line]
 
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 14/16] patman: Support checking for review tags in patchwork

[Simon Glass]

Before sending out a new version of a series for review, it is important
to add any review tags (e.g. Reviewed-by, Acked-by) collected by
patchwork. Otherwise people waste time reviewing the same patch
repeatedly, become frustrated and stop reviewing your patches.

To help with this, add a new 'status' subcommand that checks patchwork
for review tags, showing those which are not present in the local branch.

This allows users to see what new review tags have been received and then
add them.

Signed-off-by: Simon Glass 
---

 tools/patman/README |  34 
 tools/patman/control.py |  31 
 tools/patman/main.py|  17 ++
 tools/patman/status.py  | 334 
 4 files changed, 416 insertions(+)
 create mode 100644 tools/patman/status.py

diff --git a/tools/patman/README b/tools/patman/README
index 7291e47d0c..a85974740f 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -11,6 +11,8 @@ This tool is a Python script which:
 - Runs the patches through checkpatch.pl and its own checks
 - Optionally emails them out to selected people
 
+It also shows review tags from Patchwork so you can update your local patches.
+
 It is intended to automate patch creation and make it a less
 error-prone process. It is useful for U-Boot and Linux work so far,
 since it uses the checkpatch.pl script.
@@ -345,6 +347,38 @@ These people will get the cover letter even if they are 
not on the To/Cc
 list for any of the patches.
 
 
+Patchwork Integration
+=
+
+Patman has a very basic integration with Patchwork. If you point patman to
+your series on patchwork it can show you what new reviews have appears since
+you sent your series.
+
+To set this up, add a Series-link tag to one of the commits in your series
+(see above).
+
+Then you can type
+
+patman status
+
+and patman will show you each patch and what review tags have been collected,
+for example:
+
+...
+ 21 x86: mtrr: Update the command to use the new mtrr
+Reviewed-by: Wolfgang Wallner 
+  + Reviewed-by: Bin Meng 
+ 22 x86: mtrr: Restructure so command execution is in
+Reviewed-by: Wolfgang Wallner 
+  + Reviewed-by: Bin Meng 
+...
+
+This shows that patch 21 and 22 were sent out with one review but have since
+attracted another review each. If the series needs changes, you can update
+these commits with the new review tag before sending the next version of the
+series.
+
+
 Example Work Flow
 =
 
diff --git a/tools/patman/control.py b/tools/patman/control.py
index e67867b845..3bb3c236e4 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -173,3 +173,34 @@ def send(args):
 its_a_go, args.ignore_bad_tags, args.add_maintainers,
 args.limit, args.dry_run, args.in_reply_to, args.thread,
 args.smtp_server)
+
+def patchwork_status(branch, count, start, end):
+"""Check the status of patches in patchwork
+
+This finds the series in patchwork using the Series-link tag, checks for 
new
+comments / review tags and displays them
+
+Args:
+branch (str): Branch to create patches from (None = current)
+count (int): Number of patches to produce, or -1 to produce patches for
+the current branch back to the upstream commit
+start (int): Start partch to use (0=first / top of branch)
+end (int): End patch to use (0=last one in series, 1=one before that,
+etc.)
+
+Raises:
+ValueError: if the branch has no Series-link value
+"""
+if count == -1:
+# Work out how many patches to send if we can
+count = (gitutil.CountCommitsToBranch(branch) - start)
+
+series = patchstream.GetMetaData(branch, start, count - end)
+link = series.get('link')
+if not link:
+raise ValueError("Branch has no Series-link value")
+
+# Import this here to avoid failing on other commands if the dependencies
+# are not present
+from patman import status
+status.check_patchwork_status(series, link, branch)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index b96000807e..227eb3b228 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -88,6 +88,10 @@ send.add_argument('patchfiles', nargs='*')
 test_parser = subparsers.add_parser('test', help='Run tests')
 AddCommonArgs(test_parser)
 
+status = subparsers.add_parser('status',
+   help='Check status of patches in patchwork')
+AddCommonArgs(status)
+
 # Parse options twice: first to get the project and second to handle
 # defaults properly (which depends on project).
 argv = sys.argv[1:]
@@ -147,3 +151,16 @@ elif args.cmd == 'send':
 
 else:
 control.send(args)
+
+# Check status of patches in patchwork
+elif args.cmd == 'status':
+ret_code = 0
+try:
+control.patchwork_status(args.branch, args.count, args.start, args.end)
+except Exception as e:
+print('patman: %s: %s' % (type(e).__name__, e))
+

[RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output

[Simon Glass]

At present all text is marked bright, which makes it stand out on the
terminal. Add a way to disable that, as is done with the Color class.

Signed-off-by: Simon Glass 
---

 tools/patman/terminal.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py
index c709438bdc..60dbce3ce1 100644
--- a/tools/patman/terminal.py
+++ b/tools/patman/terminal.py
@@ -122,7 +122,7 @@ def TrimAsciiLen(text, size):
 return out
 
 
-def Print(text='', newline=True, colour=None, limit_to_line=False):
+def Print(text='', newline=True, colour=None, limit_to_line=False, 
bright=True):
 """Handle a line of output to the terminal.
 
 In test mode this is recorded in a list. Otherwise it is output to the
@@ -140,7 +140,7 @@ def Print(text='', newline=True, colour=None, 
limit_to_line=False):
 else:
 if colour:
 col = Color()
-text = col.Color(colour, text)
+text = col.Color(colour, text, bright=bright)
 if newline:
 print(text)
 last_print_len = None
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 11/16] patman: Allow linking a series with patchwork

[Simon Glass]

Add a new Series-link tag to tell patman how to find the series in
patchwork.

Signed-off-by: Simon Glass 
---

 tools/patman/README   | 8 
 tools/patman/func_test.py | 1 +
 tools/patman/series.py| 2 +-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/patman/README b/tools/patman/README
index 52b2cf70bd..7291e47d0c 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -187,6 +187,14 @@ Series-name: name
patman does not yet use it, but it is convenient to put the branch
name here to help you keep track of multiple upstreaming efforts.
 
+Series-link: url
+   Set the URL of the series in patchwork. You can set this after you send
+   out the series and look in patchwork for the resulting series. The
+   URL you want is the one for the series itself, not any particular patch.
+   E.g. http://patchwork.ozlabs.org/project/uboot/list/?series=187331
+   When this is set, patman can compare your local branch against patchwork
+   to see what new reviews your series has collected.
+
 Cover-letter:
 This is the patch set title
 blah blah
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 810af9c604..5eb777054a 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -392,6 +392,7 @@ Series for my board
 This series implements support
 for my glorious board.
 END
+Series-link: http://patchwork.ozlabs.org/project/uboot/list/?series=183237
 ''', 'serial.c', '''The code for the
 serial driver is here''')
 self.make_commit_with_file('bootm: Make it boot', '''
diff --git a/tools/patman/series.py b/tools/patman/series.py
index 9f885c8987..edb1141fa8 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -16,7 +16,7 @@ from patman import tools
 
 # Series-xxx tags that we understand
 valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name',
-'cover_cc', 'process_log']
+'cover_cc', 'process_log', 'link']
 
 class Series(dict):
 """Holds information about a patch series, including all tags.
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 05/16] patman: Allow skipping patches at the end

[Simon Glass]

The -s option allows skipping patches at the top of the branch. Sometimes
there are commits at the bottom that need to be skipped. At present it is
necessary to count the number of commits and then use -c to tell patman
how many to process.

Add a -e option to easily skip a number of commits at the bottom of the
branch.

Signed-off-by: Simon Glass 
---

 tools/patman/control.py   |  8 +---
 tools/patman/func_test.py | 13 +++--
 tools/patman/main.py  |  2 ++
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/patman/control.py b/tools/patman/control.py
index b48eac41fd..b481ff6b27 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -20,7 +20,7 @@ def setup():
 """Do required setup before doing anything"""
 gitutil.Setup()
 
-def prepare_patches(col, branch, count, start, ignore_binary):
+def prepare_patches(col, branch, count, start, end, ignore_binary):
 """Figure out what patches to generate, then generate them
 
 The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -32,6 +32,8 @@ def prepare_patches(col, branch, count, start, ignore_binary):
 count (int): Number of patches to produce, or -1 to produce patches for
 the current branch back to the upstream commit
 start (int): Start partch to use (0=first / top of branch)
+end (int): End patch to use (0=last one in series, 1=one before that,
+etc.)
 ignore_binary (bool): Don't generate patches for binary files
 
 Returns:
@@ -50,7 +52,7 @@ def prepare_patches(col, branch, count, start, ignore_binary):
'No commits found to process - please use -c flag'))
 
 # Read the metadata from the commits
-to_do = count
+to_do = count - end
 series = patchstream.GetMetaData(branch, start, to_do)
 cover_fname, patch_files = gitutil.CreatePatches(
 branch, start, to_do, ignore_binary, series)
@@ -159,7 +161,7 @@ def send(options):
 setup()
 col = terminal.Color()
 series, cover_fname, patch_files = prepare_patches(
-col, options.branch, options.count, options.start,
+col, options.branch, options.count, options.start, options.end,
 options.ignore_binary)
 ok = check_patches(series, patch_files, options.check_patch,
options.verbose)
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 588be73ef4..810af9c604 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -430,7 +430,8 @@ complicated as possible''')
 col = terminal.Color()
 with capture_sys_output() as _:
 _, cover_fname, patch_files = control.prepare_patches(
-col, branch=None, count=-1, start=0, ignore_binary=False)
+col, branch=None, count=-1, start=0, end=0,
+ignore_binary=False)
 self.assertIsNone(cover_fname)
 self.assertEqual(2, len(patch_files))
 
@@ -438,9 +439,17 @@ complicated as possible''')
 self.assertEqual(3, gitutil.CountCommitsToBranch('second'))
 with capture_sys_output() as _:
 _, cover_fname, patch_files = control.prepare_patches(
-col, branch='second', count=-1, start=0,
+col, branch='second', count=-1, start=0, end=0,
 ignore_binary=False)
 self.assertIsNotNone(cover_fname)
 self.assertEqual(3, len(patch_files))
+
+# Check that it can skip patches at the end
+with capture_sys_output() as _:
+_, cover_fname, patch_files = control.prepare_patches(
+col, branch='second', count=-1, start=0, end=1,
+ignore_binary=False)
+self.assertIsNotNone(cover_fname)
+self.assertEqual(2, len(patch_files))
 finally:
 os.chdir(orig_dir)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 066754196e..4d7a3044ea 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -35,6 +35,8 @@ parser.add_option('-b', '--branch', type='str',
   help="Branch to process (by default, the current branch)")
 parser.add_option('-c', '--count', dest='count', type='int',
default=-1, help='Automatically create patches from top n commits')
+parser.add_option('-e', '--end', type='int', default=0,
+  help='Commits to skip at end of patch list')
 parser.add_option('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 07/16] patman: Allow different commands

[Simon Glass]

At present patman only does one thing so does not have any comments. We
want to add a few more command, so create a sub-parser for the default
command ('send').

Signed-off-by: Simon Glass 
---

 tools/patman/main.py | 77 +++-
 1 file changed, 41 insertions(+), 36 deletions(-)

diff --git a/tools/patman/main.py b/tools/patman/main.py
index 34cad9a562..fee9bc848b 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -31,60 +31,65 @@ epilog = '''Create patches from commits in a branch, check 
them and email them
 as specified by tags you place in the commits. Use -n to do a dry run first.'''
 
 parser = ArgumentParser(epilog=epilog)
-parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
+subparsers = parser.add_subparsers(dest='cmd')
+send = subparsers.add_parser('send')
+send.add_argument('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
-parser.add_argument('-b', '--branch', type=str,
+send.add_argument('-b', '--branch', type=str,
   help="Branch to process (by default, the current branch)")
-parser.add_argument('-c', '--count', dest='count', type=int,
+send.add_argument('-c', '--count', dest='count', type=int,
default=-1, help='Automatically create patches from top n commits')
-parser.add_argument('-e', '--end', type=int, default=0,
+send.add_argument('-e', '--end', type=int, default=0,
   help='Commits to skip at end of patch list')
-parser.add_argument('-i', '--ignore-errors', action='store_true',
+send.add_argument('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
-parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
-   help='Limit the cc list to LIMIT entries [default: %(default)]')
-parser.add_argument('-m', '--no-maintainers', action='store_false',
+send.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
+   help='Limit the cc list to LIMIT entries [default: %(default)s]')
+send.add_argument('-m', '--no-maintainers', action='store_false',
dest='add_maintainers', default=True,
help="Don't cc the file maintainers automatically")
-parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
+send.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help="Do a dry run (create but don't email patches)")
-parser.add_argument('-p', '--project', default=project.DetectProject(),
-help="Project name; affects default option values and "
-"aliases [default: %(default)]")
-parser.add_argument('-r', '--in-reply-to', type=str, action='store',
+send.add_argument('-p', '--project', default=project.DetectProject(),
+  help="Project name; affects default option values and "
+  "aliases [default: %(default)s]")
+send.add_argument('-r', '--in-reply-to', type=str, action='store',
   help="Message ID that this series is in reply to")
-parser.add_argument('-s', '--start', dest='start', type=int,
+send.add_argument('-s', '--start', dest='start', type=int,
default=0, help='Commit to start creating patches from (0 = HEAD)')
-parser.add_argument('-t', '--ignore-bad-tags', action='store_true',
-default=False, help='Ignore bad tags / aliases')
-parser.add_argument('-v', '--verbose', action='store_true', dest='verbose',
+send.add_argument('-t', '--ignore-bad-tags', action='store_true',
+  default=False, help='Ignore bad tags / aliases')
+send.add_argument('-v', '--verbose', action='store_true', dest='verbose',
default=False, help='Verbose output of errors and warnings')
-parser.add_argument('-T', '--thread', action='store_true', dest='thread',
-default=False, help='Create patches as a single thread')
-parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store',
+send.add_argument('-T', '--thread', action='store_true', dest='thread',
+  default=False, help='Create patches as a single thread')
+send.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store',
default=None, help='Output cc list for patch file (used by git)')
-parser.add_argument('--no-binary', action='store_true', dest='ignore_binary',
-default=False,
-help="Do not output contents of changes in binary files")
-parser.add_argument('--no-check', action='store_false', dest='check_patch',
-default=True,
-help="Don't check for patch compliance")
-parser.add_argument('--no-tags', action='store_false', dest='process_tags',
-default=True, help="Don't process subject tags as aliases")
-parser.add_argument('--smtp-server', type=str,
-help="Specify the SMTP 

[RFC PATCH 08/16] patman: Add a 'test' subcommand

[Simon Glass]

At present we use --test to indicate that tests should be run. It is
better to use a subcommand for list, like binman. Change it and adjust
the existing code to fit under a 'send' subcommand, the default.

Give this subcommand the same default arguments as the others.

Signed-off-by: Simon Glass 
---

 .azure-pipelines.yml  |  2 +-
 .gitlab-ci.yml|  2 +-
 .travis.yml   |  2 +-
 test/run  |  2 +-
 tools/patman/main.py  | 75 +--
 tools/patman/test_util.py |  2 +-
 6 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 9f88a539c0..45f0fabd6d 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -158,7 +158,7 @@ jobs:
   ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test
   ./tools/buildman/buildman -t
   ./tools/dtoc/dtoc -t
-  ./tools/patman/patman --test
+  ./tools/patman/patman test
   make O=${UBOOT_TRAVIS_BUILD_DIR} testconfig
   EOF
   cat build.sh
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a685a7879d..c3d8ef6543 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -167,7 +167,7 @@ Run binman, buildman, dtoc, Kconfig and patman testsuites:
   ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test;
   ./tools/buildman/buildman -t;
   ./tools/dtoc/dtoc -t;
-  ./tools/patman/patman --test;
+  ./tools/patman/patman test;
   make testconfig
 
 Run tests for Nokia RX-51 (aka N900):
diff --git a/.travis.yml b/.travis.yml
index a042aa2c7d..4ce760d938 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -257,7 +257,7 @@ script:
export PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt";
export PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}";
./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test &&
-   ./tools/patman/patman --test &&
+   ./tools/patman/patman test &&
./tools/buildman/buildman -t &&
./tools/dtoc/dtoc -t &&
make testconfig;
diff --git a/test/run b/test/run
index 27331a8e40..de87e7530b 100755
--- a/test/run
+++ b/test/run
@@ -48,7 +48,7 @@ export DTC=${DTC_DIR}/dtc
 TOOLS_DIR=build-sandbox_spl/tools
 
 run_test "binman" ./tools/binman/binman --toolpath ${TOOLS_DIR} test
-run_test "patman" ./tools/patman/patman --test
+run_test "patman" ./tools/patman/patman test
 
 run_test "buildman" ./tools/buildman/buildman -t ${skip}
 run_test "fdt" ./tools/dtoc/test_fdt -t
diff --git a/tools/patman/main.py b/tools/patman/main.py
index fee9bc848b..77f187e769 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -27,6 +27,16 @@ from patman import terminal
 from patman import test_util
 from patman import test_checkpatch
 
+def AddCommonArgs(parser):
+parser.add_argument('-b', '--branch', type=str,
+help="Branch to process (by default, the current branch)")
+parser.add_argument('-c', '--count', dest='count', type=int,
+default=-1, help='Automatically create patches from top n commits')
+parser.add_argument('-e', '--end', type=int, default=0,
+help='Commits to skip at end of patch list')
+parser.add_argument('-s', '--start', dest='start', type=int,
+default=0, help='Commit to start creating patches from (0 = HEAD)')
+
 epilog = '''Create patches from commits in a branch, check them and email them
 as specified by tags you place in the commits. Use -n to do a dry run first.'''
 
@@ -35,12 +45,6 @@ subparsers = parser.add_subparsers(dest='cmd')
 send = subparsers.add_parser('send')
 send.add_argument('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
-send.add_argument('-b', '--branch', type=str,
-  help="Branch to process (by default, the current branch)")
-send.add_argument('-c', '--count', dest='count', type=int,
-   default=-1, help='Automatically create patches from top n commits')
-send.add_argument('-e', '--end', type=int, default=0,
-  help='Commits to skip at end of patch list')
 send.add_argument('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
@@ -56,8 +60,6 @@ send.add_argument('-p', '--project', 
default=project.DetectProject(),
   "aliases [default: %(default)s]")
 send.add_argument('-r', '--in-reply-to', type=str, action='store',
   help="Message ID that this series is in reply to")
-send.add_argument('-s', '--start', dest='start', type=int,
-   default=0, help='Commit to start creating patches from (0 = HEAD)')
 send.add_argument('-t', '--ignore-bad-tags', action='store_true',
   default=False, help='Ignore bad tags / aliases')
 send.add_argument('-v', '--verbose', action='store_true', dest='verbose',
@@ -76,11 +78,13 @@ 

[RFC PATCH 04/16] patman: Allow creating patches for another branch

[Simon Glass]

Add a -b option to allow patches to be created from a branch other than
the current one.

Signed-off-by: Simon Glass 
---

 tools/patman/control.py | 13 -
 tools/patman/func_test.py   | 13 +++--
 tools/patman/gitutil.py | 19 ++-
 tools/patman/main.py|  2 ++
 tools/patman/patchstream.py |  8 +---
 5 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/tools/patman/control.py b/tools/patman/control.py
index a896c924b5..b48eac41fd 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -20,7 +20,7 @@ def setup():
 """Do required setup before doing anything"""
 gitutil.Setup()
 
-def prepare_patches(col, count, start, ignore_binary):
+def prepare_patches(col, branch, count, start, ignore_binary):
 """Figure out what patches to generate, then generate them
 
 The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -28,6 +28,7 @@ def prepare_patches(col, count, start, ignore_binary):
 
 Args:
 col (terminal.Color): Colour output object
+branch (str): Branch to create patches from (None = current)
 count (int): Number of patches to produce, or -1 to produce patches for
 the current branch back to the upstream commit
 start (int): Start partch to use (0=first / top of branch)
@@ -42,7 +43,7 @@ def prepare_patches(col, count, start, ignore_binary):
 """
 if count == -1:
 # Work out how many patches to send if we can
-count = (gitutil.CountCommitsToBranch() - start)
+count = (gitutil.CountCommitsToBranch(branch) - start)
 
 if not count:
 sys.exit(col.Color(col.RED,
@@ -50,9 +51,9 @@ def prepare_patches(col, count, start, ignore_binary):
 
 # Read the metadata from the commits
 to_do = count
-series = patchstream.GetMetaData(start, to_do)
+series = patchstream.GetMetaData(branch, start, to_do)
 cover_fname, patch_files = gitutil.CreatePatches(
-start, to_do, ignore_binary, series)
+branch, start, to_do, ignore_binary, series)
 
 # Fix up the patch files to our liking, and insert the cover letter
 patchstream.FixPatches(series, patch_files)
@@ -158,9 +159,11 @@ def send(options):
 setup()
 col = terminal.Color()
 series, cover_fname, patch_files = prepare_patches(
-col, options.count, options.start, options.ignore_binary)
+col, options.branch, options.count, options.start,
+options.ignore_binary)
 ok = check_patches(series, patch_files, options.check_patch,
options.verbose)
+
 its_a_go = ok or options.ignore_errors
 if its_a_go:
 email_patches(
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 211952154a..588be73ef4 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -426,12 +426,21 @@ complicated as possible''')
 os.chdir(self.gitdir)
 
 # Check that it can detect the current branch
-self.assertEqual(2, gitutil.CountCommitsToBranch())
+self.assertEqual(2, gitutil.CountCommitsToBranch(None))
 col = terminal.Color()
 with capture_sys_output() as _:
 _, cover_fname, patch_files = control.prepare_patches(
-col, count=-1, start=0, ignore_binary=False)
+col, branch=None, count=-1, start=0, ignore_binary=False)
 self.assertIsNone(cover_fname)
 self.assertEqual(2, len(patch_files))
+
+# Check that it can detect a different branch
+self.assertEqual(3, gitutil.CountCommitsToBranch('second'))
+with capture_sys_output() as _:
+_, cover_fname, patch_files = control.prepare_patches(
+col, branch='second', count=-1, start=0,
+ignore_binary=False)
+self.assertIsNotNone(cover_fname)
+self.assertEqual(3, len(patch_files))
 finally:
 os.chdir(orig_dir)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 29444bf8e9..b683481a57 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -49,17 +49,24 @@ def LogCmd(commit_range, git_dir=None, oneline=False, 
reverse=False,
 cmd.append('--')
 return cmd
 
-def CountCommitsToBranch():
+def CountCommitsToBranch(branch):
 """Returns number of commits between HEAD and the tracking branch.
 
 This looks back to the tracking branch and works out the number of commits
 since then.
 
+Args:
+branch: Branch to count from (None for current branch)
+
 Return:
 Number of patches that exist on top of the branch
 """
-pipe = [LogCmd('@{upstream}..', oneline=True),
-['wc', '-l']]
+if branch:
+us, msg = GetUpstream('.git', branch)
+rev_range = '%s..%s' % (us, branch)
+else:
+rev_range = '@{upstream}..'
+pipe = 

[RFC PATCH 06/16] patman: Convert to ArgumentParser

[Simon Glass]

Convert from OptionParser to ArgumentParser to match binman. With this we
can easily add sub-commands.

Signed-off-by: Simon Glass 
---

 tools/patman/control.py  | 22 -
 tools/patman/main.py | 97 
 tools/patman/settings.py | 10 +++--
 3 files changed, 65 insertions(+), 64 deletions(-)

diff --git a/tools/patman/control.py b/tools/patman/control.py
index b481ff6b27..e67867b845 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -152,24 +152,24 @@ def email_patches(col, series, cover_fname, patch_files, 
process_tags, its_a_go,
 
 os.remove(cc_file)
 
-def send(options):
+def send(args):
 """Create, check and send patches by email
 
 Args:
-options (optparse.Values): Arguments to patman
+args (argparse.Namespace): Arguments to patman
 """
 setup()
 col = terminal.Color()
 series, cover_fname, patch_files = prepare_patches(
-col, options.branch, options.count, options.start, options.end,
-options.ignore_binary)
-ok = check_patches(series, patch_files, options.check_patch,
-   options.verbose)
+col, args.branch, args.count, args.start, args.end,
+args.ignore_binary)
+ok = check_patches(series, patch_files, args.check_patch,
+   args.verbose)
 
-its_a_go = ok or options.ignore_errors
+its_a_go = ok or args.ignore_errors
 if its_a_go:
 email_patches(
-col, series, cover_fname, patch_files, options.process_tags,
-its_a_go, options.ignore_bad_tags, options.add_maintainers,
-options.limit, options.dry_run, options.in_reply_to, 
options.thread,
-options.smtp_server)
+col, series, cover_fname, patch_files, args.process_tags,
+its_a_go, args.ignore_bad_tags, args.add_maintainers,
+args.limit, args.dry_run, args.in_reply_to, args.thread,
+args.smtp_server)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 4d7a3044ea..34cad9a562 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -6,7 +6,7 @@
 
 """See README for more information"""
 
-from optparse import OptionParser
+from argparse import ArgumentParser
 import os
 import re
 import sys
@@ -27,71 +27,70 @@ from patman import terminal
 from patman import test_util
 from patman import test_checkpatch
 
+epilog = '''Create patches from commits in a branch, check them and email them
+as specified by tags you place in the commits. Use -n to do a dry run first.'''
 
-parser = OptionParser()
-parser.add_option('-H', '--full-help', action='store_true', dest='full_help',
+parser = ArgumentParser(epilog=epilog)
+parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
-parser.add_option('-b', '--branch', type='str',
+parser.add_argument('-b', '--branch', type=str,
   help="Branch to process (by default, the current branch)")
-parser.add_option('-c', '--count', dest='count', type='int',
+parser.add_argument('-c', '--count', dest='count', type=int,
default=-1, help='Automatically create patches from top n commits')
-parser.add_option('-e', '--end', type='int', default=0,
+parser.add_argument('-e', '--end', type=int, default=0,
   help='Commits to skip at end of patch list')
-parser.add_option('-i', '--ignore-errors', action='store_true',
+parser.add_argument('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
-parser.add_option('-l', '--limit-cc', dest='limit', type='int',
-   default=None, help='Limit the cc list to LIMIT entries [default: 
%default]')
-parser.add_option('-m', '--no-maintainers', action='store_false',
+parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
+   help='Limit the cc list to LIMIT entries [default: %(default)]')
+parser.add_argument('-m', '--no-maintainers', action='store_false',
dest='add_maintainers', default=True,
help="Don't cc the file maintainers automatically")
-parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
+parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help="Do a dry run (create but don't email patches)")
-parser.add_option('-p', '--project', default=project.DetectProject(),
-  help="Project name; affects default option values and "
-  "aliases [default: %default]")
-parser.add_option('-r', '--in-reply-to', type='string', action='store',
+parser.add_argument('-p', '--project', default=project.DetectProject(),
+help="Project name; affects default option values and "
+"aliases [default: %(default)]")
+parser.add_argument('-r', '--in-reply-to', type=str, action='store',
   

[RFC PATCH 01/16] patman: Use test_util to show test results

[Simon Glass]

This handles skipped tests correctly, so use it instead of the existing
code.

Signed-off-by: Simon Glass 
---

 tools/patman/main.py  | 8 ++--
 tools/patman/test_util.py | 6 +++---
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/patman/main.py b/tools/patman/main.py
index 28a9a26087..03668d1bb8 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -25,6 +25,7 @@ from patman import patchstream
 from patman import project
 from patman import settings
 from patman import terminal
+from patman import test_util
 from patman import test_checkpatch
 
 
@@ -101,12 +102,7 @@ elif options.test:
 suite = doctest.DocTestSuite(module)
 suite.run(result)
 
-# TODO: Surely we can just 'print' result?
-print(result)
-for test, err in result.errors:
-print(err)
-for test, err in result.failures:
-print(err)
+sys.exit(test_util.ReportResult('patman', None, result))
 
 # Called from git with a patch filename as argument
 # Printout a list of additional CC recipients for this patch
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index aac58fb72f..0827488f33 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: 
unittest.TestResult):
 for test, err in result.failures:
 print(err, result.failures)
 if result.skipped:
-print('%d binman test%s SKIPPED:' %
-  (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
+print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
+'s' if len(result.skipped) > 1 else ''))
 for skip_info in result.skipped:
 print('%s: %s' % (skip_info[0], skip_info[1]))
 if result.errors or result.failures:
-print('binman tests FAILED')
+print('%s tests FAILED' % toolname)
 return 1
 return 0
 
-- 
2.27.0.212.ge8ba1cc988-goog

[RFC PATCH 03/16] patman: Add a test that uses gitpython

[Simon Glass]

It is convenient to use gitpython to create a real git repo for testing
patman's operation. Add a test for this. So far it just checks that patman
produces the right number of patches for a branch.

Signed-off-by: Simon Glass 
---

 tools/patman/func_test.py | 151 +-
 tools/patman/tools.py |   4 +-
 2 files changed, 152 insertions(+), 3 deletions(-)

diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index dc30078cce..211952154a 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -14,15 +14,23 @@ import unittest
 
 from io import StringIO
 
+from patman import control
 from patman import gitutil
 from patman import patchstream
 from patman import settings
+from patman import terminal
 from patman import tools
+from patman.test_util import capture_sys_output
+
+try:
+import pygit2
+HAVE_PYGIT2= True
+except ModuleNotFoundError:
+HAVE_PYGIT2 = False
 
 
 @contextlib.contextmanager
 def capture():
-import sys
 oldout,olderr = sys.stdout, sys.stderr
 try:
 out=[StringIO(), StringIO()]
@@ -37,6 +45,8 @@ def capture():
 class TestFunctional(unittest.TestCase):
 def setUp(self):
 self.tmpdir = tempfile.mkdtemp(prefix='patman.')
+self.gitdir = os.path.join(self.tmpdir, 'git')
+self.repo = None
 
 def tearDown(self):
 shutil.rmtree(self.tmpdir)
@@ -286,3 +296,142 @@ Changes in v2:
 if expected:
 expected = expected.splitlines()
 self.assertEqual(expected, lines[start:(start+len(expected))])
+
+def make_commit_with_file(self, subject, body, fname, text):
+"""Create a file and add it to the git repo with a new commit
+
+Args:
+subject (str): Subject for the commit
+body (str): Body text of the commit
+fname (str): Filename of file to create
+text (str): Text to put into the file
+"""
+path = os.path.join(self.gitdir, fname)
+tools.WriteFile(path, text, binary=False)
+index = self.repo.index
+index.add(fname)
+author =  pygit2.Signature('Test user', 't...@email.com')
+committer = author
+tree = index.write_tree()
+message = subject + '\n' + body
+self.repo.create_commit('HEAD', author, committer, message, tree,
+[self.repo.head.target])
+
+def make_git_tree(self):
+"""Make a simple git tree suitable for testing
+
+It has three branches:
+'base' has two commits: PCI, main
+'first' has base as upstream and two more commits: I2C, SPI
+'second' has base as upstream and three more: video, serial, bootm
+
+Returns:
+pygit2 repository
+"""
+repo = pygit2.init_repository(self.gitdir)
+self.repo = repo
+new_tree = repo.TreeBuilder().write()
+
+author = pygit2.Signature('Test user', 't...@email.com')
+committer = author
+commit = repo.create_commit('HEAD', author, committer,
+ 'Created master', new_tree, [])
+
+self.make_commit_with_file('Initial commit', '''
+Add a README
+
+''', 'README', '''This is the README file
+describing this project
+in very little detail''')
+
+self.make_commit_with_file('pci: PCI implementation', '''
+Here is a basic PCI implementation
+
+''', 'pci.c', '''This is a file
+it has some contents
+and some more things''')
+self.make_commit_with_file('main: Main program', '''
+Hello here is the second commit.
+''', 'main.c', '''This is the main file
+there is very little here
+but we can always add more later
+if we want to
+
+Series-to: u-boot
+Series-cc: Barry Crump 
+''')
+base_target = repo.revparse_single('HEAD')
+self.make_commit_with_file('i2c: I2C things', '''
+This has some stuff to do with I2C
+''', 'i2c.c', '''And this is the file contents
+with some I2C-related things in it''')
+self.make_commit_with_file('spi: SPI fixes', '''
+SPI needs some fixes
+and here they are
+''', 'spi.c', '''Some fixes for SPI in this
+file to make SPI work
+better than before''')
+first_target = repo.revparse_single('HEAD')
+
+target = repo.revparse_single('HEAD~2')
+repo.reset(target.oid, pygit2.GIT_CHECKOUT_FORCE)
+self.make_commit_with_file('video: Some video improvements', '''
+Fix up the video so that
+it looks more purple. Purple is
+a very nice colour.
+''', 'video.c', '''More purple here
+Purple and purple
+Even more purple
+Could not be any more purple''')
+self.make_commit_with_file('serial: Add a serial driver', '''
+Here is the serial driver
+for my chip.
+
+Cover-letter:
+Series for my board
+This series implements support
+for my glorious board.
+END
+''', 'serial.c', '''The code for the
+serial driver is here''')
+self.make_commit_with_file('bootm: Make it boot', '''

[RFC PATCH 02/16] patman: Move main code out to a control module

[Simon Glass]

To make testing easier, move the code out from main into a separate
'control' module and split it into four parts: setup, preparing patches,
checking patches and emailing patches.

Add comments and fix a few code-style issues while we are here.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py |   6 ++
 tools/patman/control.py| 170 +
 tools/patman/gitutil.py|   4 +-
 tools/patman/main.py   |  57 +
 tools/patman/series.py |   2 +-
 5 files changed, 182 insertions(+), 57 deletions(-)
 create mode 100644 tools/patman/control.py

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 07c3e2739a..263bac3fc9 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -41,6 +41,12 @@ def FindCheckPatch():
 def CheckPatch(fname, verbose=False, show_types=False):
 """Run checkpatch.pl on a file.
 
+Args:
+fname: Filename to check
+verbose: True to print out every line of the checkpatch output as it is
+parsed
+show_types: Tell checkpatch to show the type (number) of each message
+
 Returns:
 namedtuple containing:
 ok: False=failure, True=ok
diff --git a/tools/patman/control.py b/tools/patman/control.py
new file mode 100644
index 00..a896c924b5
--- /dev/null
+++ b/tools/patman/control.py
@@ -0,0 +1,170 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2020 Google LLC
+#
+"""Handles the main control logic of patman
+
+This module provides various functions called by the main program to implement
+the features of patman.
+"""
+
+import os
+import sys
+
+from patman import checkpatch
+from patman import gitutil
+from patman import patchstream
+from patman import terminal
+
+def setup():
+"""Do required setup before doing anything"""
+gitutil.Setup()
+
+def prepare_patches(col, count, start, ignore_binary):
+"""Figure out what patches to generate, then generate them
+
+The patch files are written to the current directory, e.g. 0001_xxx.patch
+0002_yyy.patch
+
+Args:
+col (terminal.Color): Colour output object
+count (int): Number of patches to produce, or -1 to produce patches for
+the current branch back to the upstream commit
+start (int): Start partch to use (0=first / top of branch)
+ignore_binary (bool): Don't generate patches for binary files
+
+Returns:
+Tuple:
+Series object for this series (set of patches)
+Filename of the cover letter as a string (None if none)
+patch_files: List of patch filenames, each a string, e.g.
+['0001_xxx.patch', '0002_yyy.patch']
+"""
+if count == -1:
+# Work out how many patches to send if we can
+count = (gitutil.CountCommitsToBranch() - start)
+
+if not count:
+sys.exit(col.Color(col.RED,
+   'No commits found to process - please use -c flag'))
+
+# Read the metadata from the commits
+to_do = count
+series = patchstream.GetMetaData(start, to_do)
+cover_fname, patch_files = gitutil.CreatePatches(
+start, to_do, ignore_binary, series)
+
+# Fix up the patch files to our liking, and insert the cover letter
+patchstream.FixPatches(series, patch_files)
+if cover_fname and series.get('cover'):
+patchstream.InsertCoverLetter(cover_fname, series, to_do)
+return series, cover_fname, patch_files
+
+def check_patches(series, patch_files, run_checkpatch, verbose):
+"""Run some checks on a set of patches
+
+This santiy-checks the patman tags like Series-version and runs the patches
+through checkpatch
+
+Args:
+series (Series): Series object for this series (set of patches)
+patch_files (list): List of patch filenames, each a string, e.g.
+['0001_xxx.patch', '0002_yyy.patch']
+run_checkpatch (bool): True to run checkpatch.pl
+verbose (bool): True to print out every line of the checkpatch output 
as
+it is parsed
+
+Returns:
+bool: True if the patches had no errors, False if they did
+"""
+# Do a few checks on the series
+series.DoChecks()
+
+# Check the patches, and run them through 'git am' just to be sure
+if run_checkpatch:
+ok = checkpatch.CheckPatches(verbose, patch_files)
+else:
+ok = True
+return ok
+
+
+def email_patches(col, series, cover_fname, patch_files, process_tags, 
its_a_go,
+  ignore_bad_tags, add_maintainers, limit, dry_run, 
in_reply_to,
+  thread, smtp_server):
+"""Email patches to the recipients
+
+This emails out the patches and cover letter using 'git send-email'. Each
+patch is copied to recipients identified by the patch tag and output from
+the get_maintainer.pl script. The cover letter is copied to all recipients
+of any patch.
+
+To make this work a CC file 

[RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork

[Simon Glass]

Patman is a useful tool for creating, checking sending out patches. It
automates the creation of patches, simplifies checking them and handles
cover letters and change logs.

However once patches are sent and reviewers add Reviewed-by tags, etc.,
these must be manually added into the commits using git before the next
version of the series is sent. Also, the only way to look at patches is
in the web interface.

It would be nice if patman could talk to Patchwork and collect the review
tags itself. Even better if it could show the review comments in a
command-line view patch-by-patch to speed up addressing comments.

This series adds these features to patman, talking directly to the web
URLs. While pwclient seems like it could handle some of this, or at least
provide a library to patman, the documentation[1] doesn't really explain
what the commands do and it doesn't seem to work with the current
Patchwork (e.g. pwclient git-am requires a patch number but Patchwork
seems to use an ID now).

With these changes, which currently lack tests, it is possible to type
'patman status' and see a list of new review tags. It is also possible
to create a new branch with those tags and list out the review comments
as small snippets on the command line.

I have been using these features for a short while and they seem useful,
so herewith a series.

This series depends on u-boot-dm/next so you might find it easier to get
it from u-boot-dm/patman-working[2]

[1] https://patchwork.readthedocs.io/projects/pwclient/en/latest/usage/
[2] https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/patman-working


Simon Glass (16):
  patman: Use test_util to show test results
  patman: Move main code out to a control module
  patman: Add a test that uses gitpython
  patman: Allow creating patches for another branch
  patman: Allow skipping patches at the end
  patman: Convert to ArgumentParser
  patman: Allow different commands
  patman: Add a 'test' subcommand
  patman: Allow disabling 'bright' mode with Print output
  patman: Support collecting response tags in Patchstream
  patman: Allow linking a series with patchwork
  patman: Add a -D option to enable debugging
  patchstream: Support parsing of review snippets
  patman: Support checking for review tags in patchwork
  patman: Support updating a branch with review tags
  patman: Support listing comments from patchwork

 .azure-pipelines.yml|   2 +-
 .gitlab-ci.yml  |   2 +-
 .travis.yml |   2 +-
 test/run|   2 +-
 tools/patman/README |  93 ++-
 tools/patman/checkpatch.py  |   6 +
 tools/patman/commit.py  |  14 ++
 tools/patman/control.py | 214 
 tools/patman/func_test.py   | 170 -
 tools/patman/gitutil.py |  23 +-
 tools/patman/main.py| 219 -
 tools/patman/patchstream.py |  82 ++-
 tools/patman/series.py  |   4 +-
 tools/patman/settings.py|  10 +-
 tools/patman/status.py  | 476 
 tools/patman/terminal.py|   4 +-
 tools/patman/test_util.py   |   8 +-
 tools/patman/tools.py   |   4 +-
 18 files changed, 1178 insertions(+), 157 deletions(-)
 create mode 100644 tools/patman/control.py
 create mode 100644 tools/patman/status.py

-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 24/25] x86: mp: Add more comments to the module

[Simon Glass]

Add a description of how this module works and also some missing function
comments.

Reviewed-by: Wolfgang Wallner 
Signed-off-by: Simon Glass 
Reviewed-by: Bin Meng 
---

Changes in v3:
- Remove stray asterisk from comments
- Drop mention of cpu_map which was handled in a previous patch

Changes in v2:
- Add a new patch with more comments

 arch/x86/cpu/mp_init.c| 91 ++-
 arch/x86/include/asm/mp.h | 14 +-
 2 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index d054563722..0f99a405bb 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -32,13 +32,99 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/*
+ * Setting up multiprocessing
+ *
+ * See 
https://www.intel.com/content/www/us/en/intelligent-systems/intel-boot-loader-development-kit/minimal-intel-architecture-boot-loader-paper.html
+ *
+ * Note that this file refers to the boot CPU (the one U-Boot is running on) as
+ * the BSP (BootStrap Processor) and the others as APs (Application 
Processors).
+ *
+ * This module works by loading some setup code into RAM at AP_DEFAULT_BASE and
+ * telling each AP to execute it. The code that each AP runs is in
+ * sipi_vector.S (see ap_start16) which includes a struct sipi_params at the
+ * end of it. Those parameters are set up by the C code.
+
+ * Setting up is handled by load_sipi_vector(). It inits the common block of
+ * parameters (sipi_params) which tell the APs what to do. This block includes
+ * microcode and the MTTRs (Memory-Type-Range Registers) from the main CPU.
+ * There is also an ap_count which each AP increments as it starts up, so the
+ * BSP can tell how many checked in.
+ *
+ * The APs are started with a SIPI (Startup Inter-Processor Interrupt) which
+ * tells an AP to start executing at a particular address, in this case
+ * AP_DEFAULT_BASE which contains the code copied from ap_start16. This 
protocol
+ * is handled by start_aps().
+ *
+ * After being started, each AP runs the code in ap_start16, switches to 32-bit
+ * mode, runs the code at ap_start, then jumps to c_handler which is ap_init().
+ * This runs a very simple 'flight plan' described in mp_steps(). This sets up
+ * the CPU and waits for further instructions by looking at its entry in
+ * ap_callbacks[]. Note that the flight plan is only actually run for each CPU
+ * in bsp_do_flight_plan(): once the BSP completes each flight record, it sets
+ * mp_flight_record->barrier to 1 to allow the APs to executed the record one
+ * by one.
+ *
+ * CPUS are numbered sequentially from 0 using the device tree:
+ *
+ * cpus {
+ * u-boot,dm-pre-reloc;
+ * #address-cells = <1>;
+ * #size-cells = <0>;
+ *
+ * cpu@0 {
+ * u-boot,dm-pre-reloc;
+ * device_type = "cpu";
+ * compatible = "intel,apl-cpu";
+ * reg = <0>;
+ * intel,apic-id = <0>;
+ * };
+ *
+ * cpu@1 {
+ * device_type = "cpu";
+ * compatible = "intel,apl-cpu";
+ * reg = <1>;
+ * intel,apic-id = <2>;
+ * };
+ *
+ * Here the 'reg' property is the CPU number and then is placed in dev->req_seq
+ * so that we can index into ap_callbacks[] using that. The APIC ID is 
different
+ * and may not be sequential (it typically is if hyperthreading is supported).
+ *
+ * Once APs are inited they wait in ap_wait_for_instruction() for instructions.
+ * Instructions come in the form of a function to run. This logic is in
+ * mp_run_on_cpus() which supports running on any one AP, all APs, just the BSP
+ * or all CPUs. The BSP logic is handled directly in mp_run_on_cpus(), by
+ * calling the function. For the APs, callback information is stored in a
+ * single, common struct mp_callback and a pointer to this is written to each
+ * AP's slot in ap_callbacks[] by run_ap_work(). All APs get the message even
+ * if it is only for one of them. When an AP notices a message it checks 
whether
+ * it should call the function (see check in ap_wait_for_instruction()) and 
then
+ * does so if needed. After that it sets its slot to NULL to indicate it is
+ * done.
+ *
+ * While U-Boot is running it can use mp_run_on_cpus() to run code on the APs.
+ * An example of this is the 'mtrr' command which allows reading and changing
+ * the MTRRs on all CPUs.
+ *
+ * Before U-Boot exits it calls mp_park_aps() which tells all CPUs to halt by
+ * executing a 'hlt' instruction. That allows them to be used by Linux when it
+ * starts up.
+ */
+
 /* This also needs to match the sipi.S assembly code for saved MSR encoding */
-struct saved_msr {
+struct __packed saved_msr {
uint32_t index;
uint32_t lo;
uint32_t hi;
-} __packed;
+};
 
+/**
+ * struct mp_flight_plan - Holds the flight plan
+ *
+ * @num_records: Number of flight records
+ * 

[PATCH v3 23/25] x86: mtrr: Update 'mtrr' to allow setting MTRRs on any CPU

[Simon Glass]

Add a -c option to mtrr to allow any CPU to be updated with this command.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

Changes in v3:
- Mention that the CPU number is in hex

 cmd/x86/mtrr.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index b51b1cd7e2..d8a7e56d5a 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -104,6 +104,17 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
int ret;
 
cpu_select = MP_SELECT_BSP;
+   if (argc >= 3 && !strcmp("-c", argv[1])) {
+   const char *cpustr;
+
+   cpustr = argv[2];
+   if (*cpustr == 'a')
+   cpu_select = MP_SELECT_ALL;
+   else
+   cpu_select = simple_strtol(cpustr, NULL, 16);
+   argc -= 2;
+   argv += 2;
+   }
argc--;
argv++;
cmd = argv[0] ? *argv[0] : 0;
@@ -145,11 +156,14 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
 }
 
 U_BOOT_CMD(
-   mtrr,   6,  1,  do_mtrr,
+   mtrr,   8,  1,  do_mtrr,
"Use x86 memory type range registers (32-bit only)",
"[list]- list current registers\n"
"set   - set a register\n"
"\t is Uncacheable, Combine, Through, Protect, Back\n"
"disable   - disable a register\n"
-   "enable- enable a register"
+   "enable- enable a register\n"
+   "\n"
+   "Precede command with '-c |all' to access a particular hex CPU, 
e.g.\n"
+   "   mtrr -c all list; mtrr -c 2e list"
 );
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 20/25] x86: mtrr: Add support for writing to MTRRs on any CPU

[Simon Glass]

To enable support for the 'mtrr' command, add a way to perform MTRR
operations on selected CPUs.

This works by setting up a little 'operation' structure and sending it
around the CPUs for action.

Signed-off-by: Simon Glass 
Reviewed-by: Bin Meng 
Reviewed-by: Wolfgang Wallner 
---

(no changes since v2)

Changes in v2:
- Keep things building by temporarily renaming the function in cmd/

 arch/x86/cpu/mtrr.c | 81 +
 arch/x86/include/asm/mtrr.h | 21 ++
 cmd/x86/mtrr.c  |  6 +--
 3 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
index 5c567551e5..2468d88a80 100644
--- a/arch/x86/cpu/mtrr.c
+++ b/arch/x86/cpu/mtrr.c
@@ -221,3 +221,84 @@ int mtrr_set_next_var(uint type, uint64_t start, uint64_t 
size)
 
return 0;
 }
+
+/** enum mtrr_opcode - supported operations for mtrr_do_oper() */
+enum mtrr_opcode {
+   MTRR_OP_SET,
+   MTRR_OP_SET_VALID,
+};
+
+/**
+ * struct mtrr_oper - An MTRR operation to perform on a CPU
+ *
+ * @opcode: Indicates operation to perform
+ * @reg: MTRR reg number to select (0-7, -1 = all)
+ * @valid: Valid value to write for MTRR_OP_SET_VALID
+ * @base: Base value to write for MTRR_OP_SET
+ * @mask: Mask value to write for MTRR_OP_SET
+ */
+struct mtrr_oper {
+   enum mtrr_opcode opcode;
+   int reg;
+   bool valid;
+   u64 base;
+   u64 mask;
+};
+
+static void mtrr_do_oper(void *arg)
+{
+   struct mtrr_oper *oper = arg;
+   u64 mask;
+
+   switch (oper->opcode) {
+   case MTRR_OP_SET_VALID:
+   mask = native_read_msr(MTRR_PHYS_MASK_MSR(oper->reg));
+   if (oper->valid)
+   mask |= MTRR_PHYS_MASK_VALID;
+   else
+   mask &= ~MTRR_PHYS_MASK_VALID;
+   wrmsrl(MTRR_PHYS_MASK_MSR(oper->reg), mask);
+   break;
+   case MTRR_OP_SET:
+   wrmsrl(MTRR_PHYS_BASE_MSR(oper->reg), oper->base);
+   wrmsrl(MTRR_PHYS_MASK_MSR(oper->reg), oper->mask);
+   break;
+   }
+}
+
+static int mtrr_start_op(int cpu_select, struct mtrr_oper *oper)
+{
+   struct mtrr_state state;
+   int ret;
+
+   mtrr_open(, true);
+   ret = mp_run_on_cpus(cpu_select, mtrr_do_oper, oper);
+   mtrr_close(, true);
+   if (ret)
+   return log_msg_ret("run", ret);
+
+   return 0;
+}
+
+int mtrr_set_valid(int cpu_select, int reg, bool valid)
+{
+   struct mtrr_oper oper;
+
+   oper.opcode = MTRR_OP_SET_VALID;
+   oper.reg = reg;
+   oper.valid = valid;
+
+   return mtrr_start_op(cpu_select, );
+}
+
+int mtrr_set(int cpu_select, int reg, u64 base, u64 mask)
+{
+   struct mtrr_oper oper;
+
+   oper.opcode = MTRR_OP_SET;
+   oper.reg = reg;
+   oper.base = base;
+   oper.mask = mask;
+
+   return mtrr_start_op(cpu_select, );
+}
diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
index e1f1a44643..48db1dd82f 100644
--- a/arch/x86/include/asm/mtrr.h
+++ b/arch/x86/include/asm/mtrr.h
@@ -159,6 +159,27 @@ int mtrr_set_next_var(uint type, uint64_t base, uint64_t 
size);
  */
 void mtrr_read_all(struct mtrr_info *info);
 
+/**
+ * mtrr_set_valid() - Set the valid flag for a selected MTRR and CPU(s)
+ *
+ * @cpu_select: Selected CPUs (either a CPU number or MP_SELECT_...)
+ * @reg: MTRR register to write (0-7)
+ * @valid: Valid flag to write
+ * @return 0 on success, -ve on error
+ */
+int mtrr_set_valid(int cpu_select, int reg, bool valid);
+
+/**
+ * mtrr_set() - Set the valid flag for a selected MTRR and CPU(s)
+ *
+ * @cpu_select: Selected CPUs (either a CPU number or MP_SELECT_...)
+ * @reg: MTRR register to write (0-7)
+ * @base: Base address and MTRR_BASE_TYPE_MASK
+ * @mask: Mask and MTRR_PHYS_MASK_VALID
+ * @return 0 on success, -ve on error
+ */
+int mtrr_set(int cpu_select, int reg, u64 base, u64 mask);
+
 #endif
 
 #if ((CONFIG_XIP_ROM_SIZE & (CONFIG_XIP_ROM_SIZE - 1)) != 0)
diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index f357f58767..46ef6a2830 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -96,7 +96,7 @@ static int do_mtrr_set(uint reg, int argc, char *const argv[])
return 0;
 }
 
-static int mtrr_set_valid(int reg, bool valid)
+static int mtrr_set_valid_(int reg, bool valid)
 {
struct mtrr_state state;
uint64_t mask;
@@ -134,9 +134,9 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
return CMD_RET_USAGE;
}
if (*cmd == 'e')
-   return mtrr_set_valid(reg, true);
+   return mtrr_set_valid_(reg, true);
else if (*cmd == 'd')
-   return mtrr_set_valid(reg, false);
+   return mtrr_set_valid_(reg, false);
else if (*cmd == 's')
return do_mtrr_set(reg, argc - 1, argv + 1);
else
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 22/25] x86: mtrr: Restructure so command execution is in one place

[Simon Glass]

At present do_mtrr() does the 'list' subcommand at the top and the rest
below. Update it to do them all in the same place so we can (in a later
patch) add parsing of the CPU number for all subcommands.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 cmd/x86/mtrr.c | 55 +-
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index b047a9897c..b51b1cd7e2 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -98,31 +98,48 @@ static int do_mtrr_set(int cpu_select, uint reg, int argc, 
char *const argv[])
 static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int argc,
   char *const argv[])
 {
-   const char *cmd;
+   int cmd;
int cpu_select;
uint reg;
+   int ret;
 
cpu_select = MP_SELECT_BSP;
-   cmd = argv[1];
-   if (argc < 2 || *cmd == 'l')
+   argc--;
+   argv++;
+   cmd = argv[0] ? *argv[0] : 0;
+   if (argc < 1 || !cmd) {
+   cmd = 'l';
+   reg = 0;
+   } else {
+   if (argc < 2)
+   return CMD_RET_USAGE;
+   reg = simple_strtoul(argv[1], NULL, 16);
+   if (reg >= MTRR_COUNT) {
+   printf("Invalid register number\n");
+   return CMD_RET_USAGE;
+   }
+   }
+   if (cmd == 'l') {
return do_mtrr_list(cpu_select);
-   argc -= 2;
-   argv += 2;
-   if (argc <= 0)
-   return CMD_RET_USAGE;
-   reg = simple_strtoul(argv[0], NULL, 16);
-   if (reg >= MTRR_COUNT) {
-   printf("Invalid register number\n");
-   return CMD_RET_USAGE;
+   } else {
+   switch (cmd) {
+   case 'e':
+   ret = mtrr_set_valid(cpu_select, reg, true);
+   break;
+   case 'd':
+   ret = mtrr_set_valid(cpu_select, reg, false);
+   break;
+   case 's':
+   ret = do_mtrr_set(cpu_select, reg, argc - 2, argv + 2);
+   break;
+   default:
+   return CMD_RET_USAGE;
+   }
+   if (ret) {
+   printf("Operation failed (err=%d)\n", ret);
+   return CMD_RET_FAILURE;
+   }
}
-   if (*cmd == 'e')
-   return mtrr_set_valid(cpu_select, reg, true);
-   else if (*cmd == 'd')
-   return mtrr_set_valid(cpu_select, reg, false);
-   else if (*cmd == 's')
-   return do_mtrr_set(cpu_select, reg, argc - 1, argv + 1);
-   else
-   return CMD_RET_USAGE;
 
return 0;
 }
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 25/25] x86: mtrr: Enhance 'mtrr' command to list MTRRs on any CPU

[Simon Glass]

Update this command so it can list the MTRRs on a selected CPU. If
'-c all' is used, then all CPUs are listed.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

(no changes since v1)

 cmd/x86/mtrr.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index d8a7e56d5a..e118bba5a2 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -131,7 +131,27 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
}
}
if (cmd == 'l') {
-   return do_mtrr_list(cpu_select);
+   bool first;
+   int i;
+
+   i = mp_first_cpu(cpu_select);
+   if (i < 0) {
+   printf("Invalid CPU (err=%d)\n", i);
+   return CMD_RET_FAILURE;
+   }
+   first = true;
+   for (; i >= 0; i = mp_next_cpu(cpu_select, i)) {
+   if (!first)
+   printf("\n");
+   printf("CPU %d:\n", i);
+   ret = do_mtrr_list(i);
+   if (ret) {
+   printf("Failed to read CPU %d (err=%d)\n", i,
+  ret);
+   return CMD_RET_FAILURE;
+   }
+   first = false;
+   }
} else {
switch (cmd) {
case 'e':
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 21/25] x86: mtrr: Update the command to use the new mtrr calls

[Simon Glass]

Use the multi-CPU calls to set the MTRR values. This still supports only
the boot CPU for now.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v2)

Changes in v2:
- Drop the renamed mtrr_set_valid_() instead of mtrr_set_valid()

 cmd/x86/mtrr.c | 34 --
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index 46ef6a2830..b047a9897c 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -59,14 +59,14 @@ static int do_mtrr_list(int cpu_select)
return 0;
 }
 
-static int do_mtrr_set(uint reg, int argc, char *const argv[])
+static int do_mtrr_set(int cpu_select, uint reg, int argc, char *const argv[])
 {
const char *typename = argv[0];
-   struct mtrr_state state;
uint32_t start, size;
uint64_t base, mask;
int i, type = -1;
bool valid;
+   int ret;
 
if (argc < 3)
return CMD_RET_USAGE;
@@ -88,27 +88,9 @@ static int do_mtrr_set(uint reg, int argc, char *const 
argv[])
if (valid)
mask |= MTRR_PHYS_MASK_VALID;
 
-   mtrr_open(, true);
-   wrmsrl(MTRR_PHYS_BASE_MSR(reg), base);
-   wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask);
-   mtrr_close(, true);
-
-   return 0;
-}
-
-static int mtrr_set_valid_(int reg, bool valid)
-{
-   struct mtrr_state state;
-   uint64_t mask;
-
-   mtrr_open(, true);
-   mask = native_read_msr(MTRR_PHYS_MASK_MSR(reg));
-   if (valid)
-   mask |= MTRR_PHYS_MASK_VALID;
-   else
-   mask &= ~MTRR_PHYS_MASK_VALID;
-   wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask);
-   mtrr_close(, true);
+   ret = mtrr_set(cpu_select, reg, base, mask);
+   if (ret)
+   return CMD_RET_FAILURE;
 
return 0;
 }
@@ -134,11 +116,11 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
return CMD_RET_USAGE;
}
if (*cmd == 'e')
-   return mtrr_set_valid_(reg, true);
+   return mtrr_set_valid(cpu_select, reg, true);
else if (*cmd == 'd')
-   return mtrr_set_valid_(reg, false);
+   return mtrr_set_valid(cpu_select, reg, false);
else if (*cmd == 's')
-   return do_mtrr_set(reg, argc - 1, argv + 1);
+   return do_mtrr_set(cpu_select, reg, argc - 1, argv + 1);
else
return CMD_RET_USAGE;
 
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 19/25] x86: mtrr: Update MTRRs on all CPUs

[Simon Glass]

When the boot CPU MTRRs are updated, perform the same update on all other
CPUs so they are kept in sync.

This avoids kernel warnings about mismatched MTRRs.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

(no changes since v2)

Changes in v2:
- Rename function to mtrr_write_all()

 arch/x86/cpu/mtrr.c | 57 +
 1 file changed, 57 insertions(+)

diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
index c9b4e7d06e..5c567551e5 100644
--- a/arch/x86/cpu/mtrr.c
+++ b/arch/x86/cpu/mtrr.c
@@ -74,10 +74,61 @@ void mtrr_read_all(struct mtrr_info *info)
}
 }
 
+void mtrr_write_all(struct mtrr_info *info)
+{
+   struct mtrr_state state;
+   int i;
+
+   for (i = 0; i < MTRR_COUNT; i++) {
+   mtrr_open(, true);
+   wrmsrl(MTRR_PHYS_BASE_MSR(i), info->mtrr[i].base);
+   wrmsrl(MTRR_PHYS_MASK_MSR(i), info->mtrr[i].mask);
+   mtrr_close(, true);
+   }
+}
+
+static void write_mtrrs(void *arg)
+{
+   struct mtrr_info *info = arg;
+
+   mtrr_write_all(info);
+}
+
+static void read_mtrrs(void *arg)
+{
+   struct mtrr_info *info = arg;
+
+   mtrr_read_all(info);
+}
+
+/**
+ * mtrr_copy_to_aps() - Copy the MTRRs from the boot CPU to other CPUs
+ *
+ * @return 0 on success, -ve on failure
+ */
+static int mtrr_copy_to_aps(void)
+{
+   struct mtrr_info info;
+   int ret;
+
+   ret = mp_run_on_cpus(MP_SELECT_BSP, read_mtrrs, );
+   if (ret == -ENXIO)
+   return 0;
+   else if (ret)
+   return log_msg_ret("bsp", ret);
+
+   ret = mp_run_on_cpus(MP_SELECT_APS, write_mtrrs, );
+   if (ret)
+   return log_msg_ret("bsp", ret);
+
+   return 0;
+}
+
 int mtrr_commit(bool do_caches)
 {
struct mtrr_request *req = gd->arch.mtrr_req;
struct mtrr_state state;
+   int ret;
int i;
 
debug("%s: enabled=%d, count=%d\n", __func__, gd->arch.has_mtrr,
@@ -99,6 +150,12 @@ int mtrr_commit(bool do_caches)
mtrr_close(, do_caches);
debug("mtrr done\n");
 
+   if (gd->flags & GD_FLG_RELOC) {
+   ret = mtrr_copy_to_aps();
+   if (ret)
+   return log_msg_ret("copy", ret);
+   }
+
return 0;
 }
 
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 16/25] x86: mtrr: Use MP calls to list the MTRRs

[Simon Glass]

Update the mtrr command to use mp_run_on_cpus() to obtain its information.
Since the selected CPU is the boot CPU this does not change the result,
but it sets the stage for supporting other CPUs.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v2)

Changes in v2:
- Rename mtrr_save_all() to mtrr_read_all()

 arch/x86/cpu/mtrr.c | 11 +++
 arch/x86/include/asm/mtrr.h | 30 ++
 cmd/x86/mtrr.c  | 25 +
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
index 7ec077..c9b4e7d06e 100644
--- a/arch/x86/cpu/mtrr.c
+++ b/arch/x86/cpu/mtrr.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -63,6 +64,16 @@ static void set_var_mtrr(uint reg, uint type, uint64_t 
start, uint64_t size)
wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask | MTRR_PHYS_MASK_VALID);
 }
 
+void mtrr_read_all(struct mtrr_info *info)
+{
+   int i;
+
+   for (i = 0; i < MTRR_COUNT; i++) {
+   info->mtrr[i].base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
+   info->mtrr[i].mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
+   }
+}
+
 int mtrr_commit(bool do_caches)
 {
struct mtrr_request *req = gd->arch.mtrr_req;
diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
index 212a699c1b..e1f1a44643 100644
--- a/arch/x86/include/asm/mtrr.h
+++ b/arch/x86/include/asm/mtrr.h
@@ -70,6 +70,26 @@ struct mtrr_state {
bool enable_cache;
 };
 
+/**
+ * struct mtrr - Information about a single MTRR
+ *
+ * @base: Base address and MTRR_BASE_TYPE_MASK
+ * @mask: Mask and MTRR_PHYS_MASK_VALID
+ */
+struct mtrr {
+   u64 base;
+   u64 mask;
+};
+
+/**
+ * struct mtrr_info - Information about all MTRRs
+ *
+ * @mtrr: Information about each mtrr
+ */
+struct mtrr_info {
+   struct mtrr mtrr[MTRR_COUNT];
+};
+
 /**
  * mtrr_open() - Prepare to adjust MTRRs
  *
@@ -129,6 +149,16 @@ int mtrr_commit(bool do_caches);
  */
 int mtrr_set_next_var(uint type, uint64_t base, uint64_t size);
 
+/**
+ * mtrr_read_all() - Save all the MTRRs
+ *
+ * This reads all MTRRs from the boot CPU into a struct so they can be loaded
+ * onto other CPUs
+ *
+ * @info: Place to put the MTRR info
+ */
+void mtrr_read_all(struct mtrr_info *info);
+
 #endif
 
 #if ((CONFIG_XIP_ROM_SIZE & (CONFIG_XIP_ROM_SIZE - 1)) != 0)
diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index 5d25c5802a..f357f58767 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -5,7 +5,9 @@
 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 
 static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
@@ -18,19 +20,32 @@ static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
"Back",
 };
 
-static int do_mtrr_list(void)
+static void read_mtrrs(void *arg)
 {
+   struct mtrr_info *info = arg;
+
+   mtrr_read_all(info);
+}
+
+static int do_mtrr_list(int cpu_select)
+{
+   struct mtrr_info info;
+   int ret;
int i;
 
printf("Reg Valid Write-type   %-16s %-16s %-16s\n", "Base   ||",
   "Mask   ||", "Size   ||");
+   memset(, '\0', sizeof(info));
+   ret = mp_run_on_cpus(cpu_select, read_mtrrs, );
+   if (ret)
+   return log_msg_ret("run", ret);
for (i = 0; i < MTRR_COUNT; i++) {
const char *type = "Invalid";
uint64_t base, mask, size;
bool valid;
 
-   base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
-   mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
+   base = info.mtrr[i].base;
+   mask = info.mtrr[i].mask;
size = ~mask & ((1ULL << CONFIG_CPU_ADDR_BITS) - 1);
size |= (1 << 12) - 1;
size += 1;
@@ -102,11 +117,13 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int 
argc,
   char *const argv[])
 {
const char *cmd;
+   int cpu_select;
uint reg;
 
+   cpu_select = MP_SELECT_BSP;
cmd = argv[1];
if (argc < 2 || *cmd == 'l')
-   return do_mtrr_list();
+   return do_mtrr_list(cpu_select);
argc -= 2;
argv += 2;
if (argc <= 0)
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 18/25] x86: coral: Update the memory map

[Simon Glass]

This currently excludes the temporary memory used to start up the APs.
Add it.

Signed-off-by: Simon Glass 
Reviewed-by: Bin Meng 
Reviewed-by: Wolfgang Wallner 
---

(no changes since v2)

Changes in v2:
- Add new patch to add AP_DEFAULT_BASE to coral's memory map

 doc/board/google/chromebook_coral.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/board/google/chromebook_coral.rst 
b/doc/board/google/chromebook_coral.rst
index 40bd9397d4..c39f1e310c 100644
--- a/doc/board/google/chromebook_coral.rst
+++ b/doc/board/google/chromebook_coral.rst
@@ -188,6 +188,7 @@ Partial memory map
 fef0  1000 CONFIG_BOOTSTAGE_STASH_ADDR
 fef0   Base of CAR region
 
+   3   AP_DEFAULT_BASE (used to start up additional CPUs)
f   CONFIG_ROM_TABLE_ADDR
   12   BSS (defined in u-boot-spl.lds)
   20   FSP-S (which is run after U-Boot is relocated)
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 17/25] x86: Don't enable SMP in SPL

[Simon Glass]

SMP should be set up in U-Boot where possible, not SPL. Disable it in SPL.
For 64-bit U-Boot we should find a way to allow SMP operations in U-Boot,
but this is somewhat more complicated. For now that is disabled too.

Signed-off-by: Simon Glass 
Reviewed-by: Bin Meng 
Reviewed-by: Wolfgang Wallner 
---

(no changes since v2)

Changes in v2:
- Add a new patch to avoid enabling SMP in SPL

 arch/x86/cpu/Makefile | 2 +-
 arch/x86/include/asm/mp.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile
index ee0499f5d7..16e67e3da2 100644
--- a/arch/x86/cpu/Makefile
+++ b/arch/x86/cpu/Makefile
@@ -60,7 +60,7 @@ ifndef CONFIG_SYS_COREBOOT
 obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += irq.o
 endif
 ifndef CONFIG_$(SPL_)X86_64
-obj-$(CONFIG_SMP) += mp_init.o
+obj-$(CONFIG_$(SPL_)SMP) += mp_init.o
 endif
 obj-y += mtrr.o
 obj-$(CONFIG_PCI) += pci.o
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index cec790b32b..6c7f1ebe3f 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -93,7 +93,7 @@ int x86_mp_init(void);
  */
 typedef void (*mp_run_func)(void *arg);
 
-#if defined(CONFIG_SMP) && !CONFIG_IS_ENABLED(X86_64)
+#if CONFIG_IS_ENABLED(SMP) && !CONFIG_IS_ENABLED(X86_64)
 /**
  * mp_run_on_cpus() - Run a function on one or all CPUs
  *
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 15/25] x86: mp: Add iterators for CPUs

[Simon Glass]

It is convenient to iterate through the CPUs performing work on each one
and processing the result. Add a few iterator functions which handle this.
These can be used by any client code. It can call mp_run_on_cpus() on
each CPU that is returned, handling them one at a time.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

Changes in v3:
- Add more comments on how the iterators work

 arch/x86/cpu/mp_init.c| 62 +++
 arch/x86/include/asm/mp.h | 42 ++
 2 files changed, 104 insertions(+)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index 0bf325ae88..d054563722 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -669,6 +669,68 @@ int mp_park_aps(void)
return get_timer(start);
 }
 
+int mp_first_cpu(int cpu_select)
+{
+   struct udevice *dev;
+   int num_cpus;
+   int ret;
+
+   /*
+* This assumes that CPUs are numbered from 0. This function tries to
+* avoid assuming the CPU 0 is the boot CPU
+*/
+   if (cpu_select == MP_SELECT_ALL)
+   return 0;   /* start with the first one */
+
+   ret = get_bsp(, _cpus);
+   if (ret < 0)
+   return log_msg_ret("bsp", ret);
+
+   /* Return boot CPU if requested */
+   if (cpu_select == MP_SELECT_BSP)
+   return ret;
+
+   /* Return something other than the boot CPU, if APs requested */
+   if (cpu_select == MP_SELECT_APS && num_cpus > 1)
+   return ret == 0 ? 1 : 0;
+
+   /* Try to check for an invalid value */
+   if (cpu_select < 0 || cpu_select >= num_cpus)
+   return -EINVAL;
+
+   return cpu_select;  /* return the only selected one */
+}
+
+int mp_next_cpu(int cpu_select, int prev_cpu)
+{
+   struct udevice *dev;
+   int num_cpus;
+   int ret;
+   int bsp;
+
+   /* If we selected the BSP or a particular single CPU, we are done */
+   if (cpu_select == MP_SELECT_BSP || cpu_select >= 0)
+   return -EFBIG;
+
+   /* Must be doing MP_SELECT_ALL or MP_SELECT_APS; return the next CPU */
+   ret = get_bsp(, _cpus);
+   if (ret < 0)
+   return log_msg_ret("bsp", ret);
+   bsp = ret;
+
+   /* Move to the next CPU */
+   assert(prev_cpu >= 0);
+   ret = prev_cpu + 1;
+
+   /* Skip the BSP if needed */
+   if (cpu_select == MP_SELECT_APS && ret == bsp)
+   ret++;
+   if (ret >= num_cpus)
+   return -EFBIG;
+
+   return ret;
+}
+
 int mp_init(void)
 {
int num_aps, num_cpus;
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index 4acce55b8c..cec790b32b 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -115,6 +115,33 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, void 
*arg);
  * @return time taken to park the APs on success (in microseconds), -ve on 
error
  */
 int mp_park_aps(void);
+
+/**
+ * mp_first_cpu() - Get the first CPU to process, from a selection
+ *
+ * This is used to iterate through selected CPUs. Call this function first, 
then
+ * call mp_next_cpu() repeatedly (with the same @cpu_select) until it returns
+ * -EFBIG.
+ *
+ * @cpu_select: Selected CPUs (either a CPU number or MP_SELECT_...)
+ * @return next CPU number to run on (e.g. 0)
+ */
+int mp_first_cpu(int cpu_select);
+
+/**
+ * mp_next_cpu() - Get the next CPU to process, from a selection
+ *
+ * This is used to iterate through selected CPUs. After first calling
+ * mp_first_cpu() once, call this function repeatedly until it returns -EFBIG.
+ *
+ * The value of @cpu_select must be the same for all calls and must match the
+ * value passed to mp_first_cpu(), otherwise the behaviour is undefined.
+ *
+ * @cpu_select: Selected CPUs (either a CPU number or MP_SELECT_...)
+ * @prev_cpu: Previous value returned by mp_first_cpu()/mp_next_cpu()
+ * @return next CPU number to run on (e.g. 0)
+ */
+int mp_next_cpu(int cpu_select, int prev_cpu);
 #else
 static inline int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
 {
@@ -131,6 +158,21 @@ static inline int mp_park_aps(void)
return 0;
 }
 
+static inline int mp_first_cpu(int cpu_select)
+{
+   /* We cannot run on any APs, nor a selected CPU */
+   return cpu_select == MP_SELECT_APS ? -EFBIG : MP_SELECT_BSP;
+}
+
+static inline int mp_next_cpu(int cpu_select, int prev_cpu)
+{
+   /*
+* When MP is not enabled, there is only one CPU and we did it in
+* mp_first_cpu()
+*/
+   return -EFBIG;
+}
+
 #endif
 
 #endif /* _X86_MP_H_ */
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 12/25] x86: Set the SMP flag when MP init is complete

[Simon Glass]

Set this flag so we can track when it is safe to use CPUs other than the
main one.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

Changes in v3:
- Rename flag to GD_FLG_SMP_READY

 arch/x86/cpu/mp_init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index ef4e8ea3af..c3c3683d5a 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -636,6 +636,7 @@ int mp_init(void)
debug("CPU init failed: err=%d\n", ret);
return ret;
}
+   gd->flags |= GD_FLG_SMP_READY;
 
return 0;
 }
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 13/25] x86: mp: Allow running functions on multiple CPUs

[Simon Glass]

Add a way to run a function on a selection of CPUs. This supports either
a single CPU, all CPUs, just the main CPU or just the 'APs', in Intel
terminology.

It works by writing into a mailbox and then waiting for the CPUs to notice
it, take action and indicate they are done.

When SMP is not yet enabled, this just calls the function on the main CPU.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

Changes in v3:
- Add a comment to run_ap_work()
- Rename flag to GD_FLG_SMP_READY
- Update the comment for run_ap_work() to explain logical_cpu_number
- Clarify meaning of @cpu_select in mp_run_on_cpus() comment

 arch/x86/cpu/mp_init.c| 97 ---
 arch/x86/include/asm/mp.h | 30 
 2 files changed, 121 insertions(+), 6 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index c3c3683d5a..bcb7513084 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -54,12 +54,7 @@ struct mp_flight_plan {
  * callback
  */
 struct mp_callback {
-   /**
-* func() - Function to call on the AP
-*
-* @arg: Argument to pass
-*/
-   void (*func)(void *arg);
+   mp_run_func func;
void *arg;
int logical_cpu_number;
 };
@@ -510,6 +505,65 @@ static void store_callback(struct mp_callback **slot, 
struct mp_callback *val)
dmb();
 }
 
+/**
+ * run_ap_work() - Run a callback on selected APs
+ *
+ * This writes @callback to all APs and waits for them all to acknowledge it,
+ * Note that whether each AP actually calls the callback depends on the value
+ * of logical_cpu_number (see struct mp_callback). The logical CPU number is
+ * the CPU device's req->seq value.
+ *
+ * @callback: Callback information to pass to all APs
+ * @bsp: CPU device for the BSP
+ * @num_cpus: The number of CPUs in the system (= number of APs + 1)
+ * @expire_ms: Timeout to wait for all APs to finish, in milliseconds, or 0 for
+ * no timeout
+ * @return 0 if OK, -ETIMEDOUT if one or more APs failed to respond in time
+ */
+static int run_ap_work(struct mp_callback *callback, struct udevice *bsp,
+  int num_cpus, uint expire_ms)
+{
+   int cur_cpu = bsp->req_seq;
+   int num_aps = num_cpus - 1; /* number of non-BSPs to get this message */
+   int cpus_accepted;
+   ulong start;
+   int i;
+
+   /* Signal to all the APs to run the func. */
+   for (i = 0; i < num_cpus; i++) {
+   if (cur_cpu != i)
+   store_callback(_callbacks[i], callback);
+   }
+   mfence();
+
+   /* Wait for all the APs to signal back that call has been accepted. */
+   start = get_timer(0);
+
+   do {
+   mdelay(1);
+   cpus_accepted = 0;
+
+   for (i = 0; i < num_cpus; i++) {
+   if (cur_cpu == i)
+   continue;
+   if (!read_callback(_callbacks[i]))
+   cpus_accepted++;
+   }
+
+   if (expire_ms && get_timer(start) >= expire_ms) {
+   log(UCLASS_CPU, LOGL_CRIT,
+   "AP call expired; %d/%d CPUs accepted\n",
+   cpus_accepted, num_aps);
+   return -ETIMEDOUT;
+   }
+   } while (cpus_accepted != num_aps);
+
+   /* Make sure we can see any data written by the APs */
+   mfence();
+
+   return 0;
+}
+
 /**
  * ap_wait_for_instruction() - Wait for and process requests from the main CPU
  *
@@ -566,6 +620,37 @@ static struct mp_flight_record mp_steps[] = {
MP_FR_BLOCK_APS(ap_wait_for_instruction, NULL, NULL, NULL),
 };
 
+int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
+{
+   struct mp_callback lcb = {
+   .func = func,
+   .arg = arg,
+   .logical_cpu_number = cpu_select,
+   };
+   struct udevice *dev;
+   int num_cpus;
+   int ret;
+
+   if (!(gd->flags & GD_FLG_SMP_READY))
+   return -ENXIO;
+
+   ret = get_bsp(, _cpus);
+   if (ret < 0)
+   return log_msg_ret("bsp", ret);
+   if (cpu_select == MP_SELECT_ALL || cpu_select == MP_SELECT_BSP ||
+   cpu_select == ret) {
+   /* Run on BSP first */
+   func(arg);
+   }
+
+   /* Allow up to 1 second for all APs to finish */
+   ret = run_ap_work(, dev, num_cpus, 1000 /* ms */);
+   if (ret)
+   return log_msg_ret("aps", ret);
+
+   return 0;
+}
+
 int mp_init(void)
 {
int num_aps, num_cpus;
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index 41b1575f4b..3f1d70663d 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -86,4 +86,34 @@ int mp_init(void);
 /* Set up additional CPUs */
 int x86_mp_init(void);
 
+/**
+ * mp_run_func() - Function to call on the AP
+ *
+ * @arg: Argument to pass
+ 

[PATCH v3 11/25] global_data: Add a generic global_data flag for SMP state

[Simon Glass]

Allow keeping track of whether all CPUs have been enabled yet. This allows
us to know whether other CPUs need to be considered when updating
CPU-specific settings such as MTRRs on x86.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

Changes in v3:
- Rename flag to GD_FLG_SMP_READY

 include/asm-generic/global_data.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/asm-generic/global_data.h 
b/include/asm-generic/global_data.h
index 8c78792cc9..d4a4e2215d 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -167,5 +167,6 @@ typedef struct global_data {
 #define GD_FLG_LOG_READY   0x08000 /* Log system is ready for use */
 #define GD_FLG_WDT_READY   0x1 /* Watchdog is ready for use   */
 #define GD_FLG_SKIP_LL_INIT0x2 /* Don't perform low-level init*/
+#define GD_FLG_SMP_READY   0x4 /* SMP init is complete*/
 
 #endif /* __ASM_GENERIC_GBL_DATA_H */
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 10/25] x86: mp: Support APs waiting for instructions

[Simon Glass]

At present the APs (non-boot CPUs) are inited once and then parked ready
for the OS to use them. However in some cases we want to send new requests
through, such as to change MTRRs and keep them consistent across CPUs.

Change the last state of the flight plan to go into a wait loop, accepting
instructions from the main CPU.

Drop cpu_map since it is not used.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

Changes in v3:
- s/slow/slot/
- Use C code instead of assembler to read/write callback pointers
- Update commit message to mention dropping of cpu_map

Changes in v2:
- Add more comments

 arch/x86/cpu/mp_init.c| 119 +++---
 arch/x86/include/asm/mp.h |  11 
 2 files changed, 121 insertions(+), 9 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index eef7dac809..ef4e8ea3af 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,13 +44,38 @@ struct mp_flight_plan {
struct mp_flight_record *records;
 };
 
+/**
+ * struct mp_callback - Callback information for APs
+ *
+ * @func: Function to run
+ * @arg: Argument to pass to the function
+ * @logical_cpu_number: Either a CPU number (i.e. dev->req_seq) or a special
+ * value like MP_SELECT_BSP. It tells the AP whether it should process this
+ * callback
+ */
+struct mp_callback {
+   /**
+* func() - Function to call on the AP
+*
+* @arg: Argument to pass
+*/
+   void (*func)(void *arg);
+   void *arg;
+   int logical_cpu_number;
+};
+
 static struct mp_flight_plan mp_info;
 
-struct cpu_map {
-   struct udevice *dev;
-   int apic_id;
-   int err_code;
-};
+/*
+ * ap_callbacks - Callback mailbox array
+ *
+ * Array of callback, one entry for each available CPU, indexed by the CPU
+ * number, which is dev->req_seq. The entry for the main CPU is never used.
+ * When this is NULL, there is no pending work for the CPU to run. When
+ * non-NULL it points to the mp_callback structure. This is shared between all
+ * CPUs, so should only be written by the main CPU.
+ */
+static struct mp_callback **ap_callbacks;
 
 static inline void barrier_wait(atomic_t *b)
 {
@@ -147,11 +173,9 @@ static void ap_init(unsigned int cpu_index)
debug("AP: slot %d apic_id %x, dev %s\n", cpu_index, apic_id,
  dev ? dev->name : "(apic_id not found)");
 
-   /* Walk the flight plan */
+   /* Walk the flight plan, which never returns */
ap_do_flight_plan(dev);
-
-   /* Park the AP */
-   debug("parking\n");
+   debug("Unexpected return\n");
 done:
stop_this_cpu();
 }
@@ -455,6 +479,78 @@ static int get_bsp(struct udevice **devp, int *cpu_countp)
return dev->req_seq;
 }
 
+/**
+ * read_callback() - Read the pointer in a callback slot
+ *
+ * This is called by APs to read their callback slot to see if there is a
+ * pointer to new instructions
+ *
+ * @slot: Pointer to the AP's callback slot
+ * @return value of that pointer
+ */
+static struct mp_callback *read_callback(struct mp_callback **slot)
+{
+   dmb();
+
+   return *slot;
+}
+
+/**
+ * store_callback() - Store a pointer to the callback slot
+ *
+ * This is called by APs to write NULL into the callback slot when they have
+ * finished the work requested by the BSP.
+ *
+ * @slot: Pointer to the AP's callback slot
+ * @val: Value to write (e.g. NULL)
+ */
+static void store_callback(struct mp_callback **slot, struct mp_callback *val)
+{
+   *slot = val;
+   dmb();
+}
+
+/**
+ * ap_wait_for_instruction() - Wait for and process requests from the main CPU
+ *
+ * This is called by APs (here, everything other than the main boot CPU) to
+ * await instructions. They arrive in the form of a function call and argument,
+ * which is then called. This uses a simple mailbox with atomic read/set
+ *
+ * @cpu: CPU that is waiting
+ * @unused: Optional argument provided by struct mp_flight_record, not used 
here
+ * @return Does not return
+ */
+static int ap_wait_for_instruction(struct udevice *cpu, void *unused)
+{
+   struct mp_callback lcb;
+   struct mp_callback **per_cpu_slot;
+
+   per_cpu_slot = _callbacks[cpu->req_seq];
+
+   while (1) {
+   struct mp_callback *cb = read_callback(per_cpu_slot);
+
+   if (!cb) {
+   asm ("pause");
+   continue;
+   }
+
+   /* Copy to local variable before using the value */
+   memcpy(, cb, sizeof(lcb));
+   mfence();
+   if (lcb.logical_cpu_number == MP_SELECT_ALL ||
+   lcb.logical_cpu_number == MP_SELECT_APS ||
+   cpu->req_seq == lcb.logical_cpu_number)
+   lcb.func(lcb.arg);
+
+   /* Indicate we are finished */
+   store_callback(per_cpu_slot, 

[PATCH v3 14/25] x86: mp: Park CPUs before running the OS

[Simon Glass]

With the new MP features the CPUs are no-longer parked when the OS is run.
Fix this by calling a special function to park them, just before the OS is
started.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
---

Changes in v3:
- Update the comment for mp_park_aps()

 arch/x86/cpu/cpu.c|  5 +
 arch/x86/cpu/mp_init.c| 18 ++
 arch/x86/include/asm/mp.h | 17 +
 3 files changed, 40 insertions(+)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index d0720fb7fb..baa7dae172 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -66,6 +66,11 @@ static const char *const x86_vendor_name[] = {
 
 int __weak x86_cleanup_before_linux(void)
 {
+   int ret;
+
+   ret = mp_park_aps();
+   if (ret)
+   return log_msg_ret("park", ret);
bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
CONFIG_BOOTSTAGE_STASH_SIZE);
 
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index bcb7513084..0bf325ae88 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -651,6 +651,24 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, void 
*arg)
return 0;
 }
 
+static void park_this_cpu(void *unused)
+{
+   stop_this_cpu();
+}
+
+int mp_park_aps(void)
+{
+   unsigned long start;
+   int ret;
+
+   start = get_timer(0);
+   ret = mp_run_on_cpus(MP_SELECT_APS, park_this_cpu, NULL);
+   if (ret)
+   return ret;
+
+   return get_timer(start);
+}
+
 int mp_init(void)
 {
int num_aps, num_cpus;
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index 3f1d70663d..4acce55b8c 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -106,6 +106,15 @@ typedef void (*mp_run_func)(void *arg);
  * @return 0 on success, -ve on error
  */
 int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg);
+
+/**
+ * mp_park_aps() - Park the APs ready for the OS
+ *
+ * This halts all CPUs except the main one, ready for the OS to use them
+ *
+ * @return time taken to park the APs on success (in microseconds), -ve on 
error
+ */
+int mp_park_aps(void);
 #else
 static inline int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
 {
@@ -114,6 +123,14 @@ static inline int mp_run_on_cpus(int cpu_select, 
mp_run_func func, void *arg)
 
return 0;
 }
+
+static inline int mp_park_aps(void)
+{
+   /* No APs to park */
+
+   return 0;
+}
+
 #endif
 
 #endif /* _X86_MP_H_ */
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 09/25] x86: cpu: Remove unnecessary #ifdefs

[Simon Glass]

Drop some #ifdefs that are not needed or can be converted to compile-time
checks.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 arch/x86/cpu/cpu.c  | 58 -
 arch/x86/cpu/i386/cpu.c |  2 --
 2 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 23a4d633d2..d0720fb7fb 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -66,10 +66,8 @@ static const char *const x86_vendor_name[] = {
 
 int __weak x86_cleanup_before_linux(void)
 {
-#ifdef CONFIG_BOOTSTAGE_STASH
bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
CONFIG_BOOTSTAGE_STASH_SIZE);
-#endif
 
return 0;
 }
@@ -200,18 +198,19 @@ int last_stage_init(void)
 
write_tables();
 
-#ifdef CONFIG_GENERATE_ACPI_TABLE
-   fadt = acpi_find_fadt();
+   if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) {
+   fadt = acpi_find_fadt();
 
-   /* Don't touch ACPI hardware on HW reduced platforms */
-   if (fadt && !(fadt->flags & ACPI_FADT_HW_REDUCED_ACPI)) {
-   /*
-* Other than waiting for OSPM to request us to switch to ACPI
-* mode, do it by ourselves, since SMI will not be triggered.
-*/
-   enter_acpi_mode(fadt->pm1a_cnt_blk);
+   /* Don't touch ACPI hardware on HW reduced platforms */
+   if (fadt && !(fadt->flags & ACPI_FADT_HW_REDUCED_ACPI)) {
+   /*
+* Other than waiting for OSPM to request us to switch
+* to ACPI * mode, do it by ourselves, since SMI will
+* not be triggered.
+*/
+   enter_acpi_mode(fadt->pm1a_cnt_blk);
+   }
}
-#endif
 
return 0;
 }
@@ -219,19 +218,20 @@ int last_stage_init(void)
 
 static int x86_init_cpus(void)
 {
-#ifdef CONFIG_SMP
-   debug("Init additional CPUs\n");
-   x86_mp_init();
-#else
-   struct udevice *dev;
+   if (IS_ENABLED(CONFIG_SMP)) {
+   debug("Init additional CPUs\n");
+   x86_mp_init();
+   } else {
+   struct udevice *dev;
 
-   /*
-* This causes the cpu-x86 driver to be probed.
-* We don't check return value here as we want to allow boards
-* which have not been converted to use cpu uclass driver to boot.
-*/
-   uclass_first_device(UCLASS_CPU, );
-#endif
+   /*
+* This causes the cpu-x86 driver to be probed.
+* We don't check return value here as we want to allow boards
+* which have not been converted to use cpu uclass driver to
+* boot.
+*/
+   uclass_first_device(UCLASS_CPU, );
+   }
 
return 0;
 }
@@ -269,13 +269,11 @@ int cpu_init_r(void)
 #ifndef CONFIG_EFI_STUB
 int reserve_arch(void)
 {
-#ifdef CONFIG_ENABLE_MRC_CACHE
-   mrccache_reserve();
-#endif
+   if (IS_ENABLED(CONFIG_ENABLE_MRC_CACHE))
+   mrccache_reserve();
 
-#ifdef CONFIG_SEABIOS
-   high_table_reserve();
-#endif
+   if (IS_ENABLED(CONFIG_SEABIOS))
+   high_table_reserve();
 
if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
acpi_s3_reserve();
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
index 9809ac5111..fca3f79b69 100644
--- a/arch/x86/cpu/i386/cpu.c
+++ b/arch/x86/cpu/i386/cpu.c
@@ -626,7 +626,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
return -EFAULT;
 }
 
-#ifdef CONFIG_SMP
 int x86_mp_init(void)
 {
int ret;
@@ -639,4 +638,3 @@ int x86_mp_init(void)
 
return 0;
 }
-#endif
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 07/25] x86: mp_init: Set up the CPU numbers at the start

[Simon Glass]

At present each CPU is given a number when it starts itself up. While this
saves a tiny amount of time by doing the device-tree read in parallel, it
is confusing that the numbering happens on the fly.

Move this code into mp_init() and do it at the start.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 arch/x86/cpu/mp_init.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index df43f71572..6f6de49df0 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -444,12 +444,6 @@ static int mp_init_cpu(struct udevice *cpu, void *unused)
 {
struct cpu_platdata *plat = dev_get_parent_platdata(cpu);
 
-   /*
-* Multiple APs are brought up simultaneously and they may get the same
-* seq num in the uclass_resolve_seq() during device_probe(). To avoid
-* this, set req_seq to the reg number in the device tree in advance.
-*/
-   cpu->req_seq = dev_read_u32_default(cpu, "reg", -1);
plat->ucode_version = microcode_read_rev();
plat->device_id = gd->arch.x86_device;
 
@@ -465,13 +459,8 @@ int mp_init(void)
int num_aps, num_cpus;
atomic_t *ap_count;
struct udevice *cpu;
-   int ret;
-
-   /* This will cause the CPUs devices to be bound */
struct uclass *uc;
-   ret = uclass_get(UCLASS_CPU, );
-   if (ret)
-   return ret;
+   int ret;
 
if (IS_ENABLED(CONFIG_QFW)) {
ret = qemu_cpu_fixup();
@@ -479,6 +468,14 @@ int mp_init(void)
return ret;
}
 
+   /*
+* Multiple APs are brought up simultaneously and they may get the same
+* seq num in the uclass_resolve_seq() during device_probe(). To avoid
+* this, set req_seq to the reg number in the device tree in advance.
+*/
+   uclass_id_foreach_dev(UCLASS_CPU, cpu, uc)
+   cpu->req_seq = dev_read_u32_default(cpu, "reg", -1);
+
ret = init_bsp();
if (ret) {
debug("Cannot init boot CPU: err=%d\n", ret);
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 04/25] x86: mp_init: Switch parameter names in start_aps()

[Simon Glass]

These parameters are named differently from elsewhere in this file. Switch
them to avoid confusion.

Also add comments to this function.

Signed-off-by: Simon Glass 
Reviewed-by: Bin Meng 
Reviewed-by: Wolfgang Wallner 
---

(no changes since v2)

Changes in v2:
- Add comments to explain what start_aps() does

 arch/x86/cpu/mp_init.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index e77d7f2cd6..8b00d57c88 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -308,13 +308,26 @@ static int apic_wait_timeout(int total_delay, const char 
*msg)
return 0;
 }
 
-static int start_aps(int ap_count, atomic_t *num_aps)
+/**
+ * start_aps() - Start up the APs and count how many we find
+ *
+ * This is called on the boot processor to start up all the other processors
+ * (here called APs).
+ *
+ * @num_aps: Number of APs we expect to find
+ * @ap_count: Initially zero. Incremented by this function for each AP found
+ * @return 0 if all APs were set up correctly or there are none to set up,
+ * -ENOSPC if the SIPI vector is too high in memory,
+ * -ETIMEDOUT if the ICR is busy or the second SIPI fails to complete
+ * -EIO if not all APs check in correctly
+ */
+static int start_aps(int num_aps, atomic_t *ap_count)
 {
int sipi_vector;
/* Max location is 4KiB below 1MiB */
const int max_vector_loc = ((1 << 20) - (1 << 12)) >> 12;
 
-   if (ap_count == 0)
+   if (num_aps == 0)
return 0;
 
/* The vector is sent as a 4k aligned address in one byte */
@@ -326,7 +339,7 @@ static int start_aps(int ap_count, atomic_t *num_aps)
return -ENOSPC;
}
 
-   debug("Attempting to start %d APs\n", ap_count);
+   debug("Attempting to start %d APs\n", num_aps);
 
if (apic_wait_timeout(1000, "ICR not to be busy"))
return -ETIMEDOUT;
@@ -349,7 +362,7 @@ static int start_aps(int ap_count, atomic_t *num_aps)
return -ETIMEDOUT;
 
/* Wait for CPUs to check in up to 200 us */
-   wait_for_aps(num_aps, ap_count, 200, 15);
+   wait_for_aps(ap_count, num_aps, 200, 15);
 
/* Send 2nd SIPI */
if (apic_wait_timeout(1000, "ICR not to be busy"))
@@ -362,9 +375,9 @@ static int start_aps(int ap_count, atomic_t *num_aps)
return -ETIMEDOUT;
 
/* Wait for CPUs to check in */
-   if (wait_for_aps(num_aps, ap_count, 1, 50)) {
+   if (wait_for_aps(ap_count, num_aps, 1, 50)) {
debug("Not all APs checked in: %d/%d\n",
- atomic_read(num_aps), ap_count);
+ atomic_read(ap_count), num_aps);
return -EIO;
}
 
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 06/25] x86: mtrr: Fix 'ensable' typo

[Simon Glass]

Fix a typo in the command help.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 cmd/x86/mtrr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index 084d7315f4..5d25c5802a 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -135,5 +135,5 @@ U_BOOT_CMD(
"set   - set a register\n"
"\t is Uncacheable, Combine, Through, Protect, Back\n"
"disable   - disable a register\n"
-   "ensable   - enable a register"
+   "enable- enable a register"
 );
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 08/25] x86: mp_init: Adjust bsp_init() to return more information

[Simon Glass]

This function is misnamed since it does not actually init the BSP. Also
it is convenient to adjust it to return a little more information.

Rename and update the function, to allow it to return the BSP CPU device
and number, as well as the total number of CPUs.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v2)

Changes in v2:
- Drop change to include/dm/uclass.h
- Mention error return in get_bsp()

 arch/x86/cpu/mp_init.c | 35 ++-
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index 6f6de49df0..eef7dac809 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -421,9 +421,17 @@ static int bsp_do_flight_plan(struct udevice *cpu, struct 
mp_flight_plan *plan,
return ret;
 }
 
-static int init_bsp(struct udevice **devp)
+/**
+ * get_bsp() - Get information about the bootstrap processor
+ *
+ * @devp: If non-NULL, returns CPU device corresponding to the BSP
+ * @cpu_countp: If non-NULL, returns the total number of CPUs
+ * @return CPU number of the BSP, or -ve on error
+ */
+static int get_bsp(struct udevice **devp, int *cpu_countp)
 {
char processor_name[CPU_MAX_NAME_LEN];
+   struct udevice *dev;
int apic_id;
int ret;
 
@@ -431,13 +439,20 @@ static int init_bsp(struct udevice **devp)
debug("CPU: %s\n", processor_name);
 
apic_id = lapicid();
-   ret = find_cpu_by_apic_id(apic_id, devp);
-   if (ret) {
+   ret = find_cpu_by_apic_id(apic_id, );
+   if (ret < 0) {
printf("Cannot find boot CPU, APIC ID %d\n", apic_id);
return ret;
}
+   ret = cpu_get_count(dev);
+   if (ret < 0)
+   return log_msg_ret("count", ret);
+   if (devp)
+   *devp = dev;
+   if (cpu_countp)
+   *cpu_countp = ret;
 
-   return 0;
+   return dev->req_seq;
 }
 
 static int mp_init_cpu(struct udevice *cpu, void *unused)
@@ -476,24 +491,18 @@ int mp_init(void)
uclass_id_foreach_dev(UCLASS_CPU, cpu, uc)
cpu->req_seq = dev_read_u32_default(cpu, "reg", -1);
 
-   ret = init_bsp();
-   if (ret) {
+   ret = get_bsp(, _cpus);
+   if (ret < 0) {
debug("Cannot init boot CPU: err=%d\n", ret);
return ret;
}
 
-   num_cpus = cpu_get_count(cpu);
-   if (num_cpus < 0) {
-   debug("Cannot get number of CPUs: err=%d\n", num_cpus);
-   return num_cpus;
-   }
-
if (num_cpus < 2)
debug("Warning: Only 1 CPU is detected\n");
 
ret = check_cpu_devices(num_cpus);
if (ret)
-   debug("Warning: Device tree does not describe all CPUs. Extra 
ones will not be started correctly\n");
+   log_warning("Warning: Device tree does not describe all CPUs. 
Extra ones will not be started correctly\n");
 
/* Copy needed parameters so that APs have a reference to the plan */
mp_info.num_records = ARRAY_SIZE(mp_steps);
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 03/25] x86: mp_init: Avoid declarations in header files

[Simon Glass]

The functions used by the flight plan are declared in the header file but
are not used in any other file.

Move the flight plan steps down to just above where it is used so that we
can make these function static.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 arch/x86/cpu/mp_init.c| 40 +++
 arch/x86/include/asm/mp.h |  3 ---
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index 831fd7035d..e77d7f2cd6 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -41,10 +41,6 @@ struct saved_msr {
uint32_t hi;
 } __packed;
 
-static struct mp_flight_record mp_steps[] = {
-   MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
-};
-
 struct mp_flight_plan {
int num_records;
struct mp_flight_record *records;
@@ -423,6 +419,26 @@ static int init_bsp(struct udevice **devp)
return 0;
 }
 
+static int mp_init_cpu(struct udevice *cpu, void *unused)
+{
+   struct cpu_platdata *plat = dev_get_parent_platdata(cpu);
+
+   /*
+* Multiple APs are brought up simultaneously and they may get the same
+* seq num in the uclass_resolve_seq() during device_probe(). To avoid
+* this, set req_seq to the reg number in the device tree in advance.
+*/
+   cpu->req_seq = dev_read_u32_default(cpu, "reg", -1);
+   plat->ucode_version = microcode_read_rev();
+   plat->device_id = gd->arch.x86_device;
+
+   return device_probe(cpu);
+}
+
+static struct mp_flight_record mp_steps[] = {
+   MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
+};
+
 int mp_init(void)
 {
int num_aps;
@@ -495,19 +511,3 @@ int mp_init(void)
 
return 0;
 }
-
-int mp_init_cpu(struct udevice *cpu, void *unused)
-{
-   struct cpu_platdata *plat = dev_get_parent_platdata(cpu);
-
-   /*
-* Multiple APs are brought up simultaneously and they may get the same
-* seq num in the uclass_resolve_seq() during device_probe(). To avoid
-* this, set req_seq to the reg number in the device tree in advance.
-*/
-   cpu->req_seq = dev_read_u32_default(cpu, "reg", -1);
-   plat->ucode_version = microcode_read_rev();
-   plat->device_id = gd->arch.x86_device;
-
-   return device_probe(cpu);
-}
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index db02904ecb..94af819ad9 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -72,9 +72,6 @@ struct mp_flight_record {
  */
 int mp_init(void);
 
-/* Probes the CPU device */
-int mp_init_cpu(struct udevice *cpu, void *unused);
-
 /* Set up additional CPUs */
 int x86_mp_init(void);
 
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 02/25] x86: Move MP code into mp_init

[Simon Glass]

At present the 'flight plan' for CPUs is passed into mp_init. But it is
always the same. Move it into the mp_init file so everything is in one
place. Also drop the SMI function since it does nothing. If we implement
SMIs, more refactoring will be needed anyway.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 arch/x86/cpu/i386/cpu.c   | 24 +---
 arch/x86/cpu/mp_init.c| 22 ++
 arch/x86/include/asm/mp.h | 17 +
 3 files changed, 16 insertions(+), 47 deletions(-)

diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
index d27324cb4e..9809ac5111 100644
--- a/arch/x86/cpu/i386/cpu.c
+++ b/arch/x86/cpu/i386/cpu.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -626,29 +627,14 @@ int cpu_jump_to_64bit_uboot(ulong target)
 }
 
 #ifdef CONFIG_SMP
-static int enable_smis(struct udevice *cpu, void *unused)
-{
-   return 0;
-}
-
-static struct mp_flight_record mp_steps[] = {
-   MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
-   /* Wait for APs to finish initialization before proceeding */
-   MP_FR_BLOCK_APS(NULL, NULL, enable_smis, NULL),
-};
-
 int x86_mp_init(void)
 {
-   struct mp_params mp_params;
-
-   mp_params.parallel_microcode_load = 0,
-   mp_params.flight_plan = _steps[0];
-   mp_params.num_records = ARRAY_SIZE(mp_steps);
-   mp_params.microcode_pointer = 0;
+   int ret;
 
-   if (mp_init(_params)) {
+   ret = mp_init();
+   if (ret) {
printf("Warning: MP init failure\n");
-   return -EIO;
+   return log_ret(ret);
}
 
return 0;
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index c25d17c647..831fd7035d 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -41,6 +41,9 @@ struct saved_msr {
uint32_t hi;
 } __packed;
 
+static struct mp_flight_record mp_steps[] = {
+   MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),
+};
 
 struct mp_flight_plan {
int num_records;
@@ -372,7 +375,7 @@ static int start_aps(int ap_count, atomic_t *num_aps)
return 0;
 }
 
-static int bsp_do_flight_plan(struct udevice *cpu, struct mp_params *mp_params)
+static int bsp_do_flight_plan(struct udevice *cpu, struct mp_flight_plan *plan)
 {
int i;
int ret = 0;
@@ -380,8 +383,8 @@ static int bsp_do_flight_plan(struct udevice *cpu, struct 
mp_params *mp_params)
const int step_us = 100;
int num_aps = num_cpus - 1;
 
-   for (i = 0; i < mp_params->num_records; i++) {
-   struct mp_flight_record *rec = _params->flight_plan[i];
+   for (i = 0; i < plan->num_records; i++) {
+   struct mp_flight_record *rec = >records[i];
 
/* Wait for APs if the record is not released */
if (atomic_read(>barrier) == 0) {
@@ -420,7 +423,7 @@ static int init_bsp(struct udevice **devp)
return 0;
 }
 
-int mp_init(struct mp_params *p)
+int mp_init(void)
 {
int num_aps;
atomic_t *ap_count;
@@ -445,11 +448,6 @@ int mp_init(struct mp_params *p)
return ret;
}
 
-   if (p == NULL || p->flight_plan == NULL || p->num_records < 1) {
-   printf("Invalid MP parameters\n");
-   return -EINVAL;
-   }
-
num_cpus = cpu_get_count(cpu);
if (num_cpus < 0) {
debug("Cannot get number of CPUs: err=%d\n", num_cpus);
@@ -464,8 +462,8 @@ int mp_init(struct mp_params *p)
debug("Warning: Device tree does not describe all CPUs. Extra 
ones will not be started correctly\n");
 
/* Copy needed parameters so that APs have a reference to the plan */
-   mp_info.num_records = p->num_records;
-   mp_info.records = p->flight_plan;
+   mp_info.num_records = ARRAY_SIZE(mp_steps);
+   mp_info.records = mp_steps;
 
/* Load the SIPI vector */
ret = load_sipi_vector(_count, num_cpus);
@@ -489,7 +487,7 @@ int mp_init(struct mp_params *p)
}
 
/* Walk the flight plan for the BSP */
-   ret = bsp_do_flight_plan(cpu, p);
+   ret = bsp_do_flight_plan(cpu, _info);
if (ret) {
debug("CPU init failed: err=%d\n", ret);
return ret;
diff --git a/arch/x86/include/asm/mp.h b/arch/x86/include/asm/mp.h
index 9dddf88b5a..db02904ecb 100644
--- a/arch/x86/include/asm/mp.h
+++ b/arch/x86/include/asm/mp.h
@@ -51,21 +51,6 @@ struct mp_flight_record {
 #define MP_FR_NOBLOCK_APS(ap_func, ap_arg, bsp_func, bsp_arg) \
MP_FLIGHT_RECORD(1, ap_func, ap_arg, bsp_func, bsp_arg)
 
-/*
- * The mp_params structure provides the arguments to the mp subsystem
- * for bringing up APs.
- *
- * At present this is overkill for U-Boot, but it may make it easier to add
- * SMM support.
- */
-struct mp_params {
-   int parallel_microcode_load;
-   const 

[PATCH v3 05/25] x86: mp_init: Drop the num_cpus static variable

[Simon Glass]

This does not need to be global across all functions in this file. Pass a
parameter instead.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

Changes in v3:
- Update bsp_do_flight_plan() to say 'on the BSP'

 arch/x86/cpu/mp_init.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index 8b00d57c88..df43f71572 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -31,9 +31,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-/* Total CPUs include BSP */
-static int num_cpus;
-
 /* This also needs to match the sipi.S assembly code for saved MSR encoding */
 struct saved_msr {
uint32_t index;
@@ -384,13 +381,23 @@ static int start_aps(int num_aps, atomic_t *ap_count)
return 0;
 }
 
-static int bsp_do_flight_plan(struct udevice *cpu, struct mp_flight_plan *plan)
+/**
+ * bsp_do_flight_plan() - Do the flight plan on the BSP
+ *
+ * This runs the flight plan on the main CPU used to boot U-Boot
+ *
+ * @cpu: Device for the main CPU
+ * @plan: Flight plan to run
+ * @num_aps: Number of APs (CPUs other than the BSP)
+ * @returns 0 on success, -ETIMEDOUT if an AP failed to come up
+ */
+static int bsp_do_flight_plan(struct udevice *cpu, struct mp_flight_plan *plan,
+ int num_aps)
 {
int i;
int ret = 0;
const int timeout_us = 10;
const int step_us = 100;
-   int num_aps = num_cpus - 1;
 
for (i = 0; i < plan->num_records; i++) {
struct mp_flight_record *rec = >records[i];
@@ -410,6 +417,7 @@ static int bsp_do_flight_plan(struct udevice *cpu, struct 
mp_flight_plan *plan)
 
release_barrier(>barrier);
}
+
return ret;
 }
 
@@ -454,7 +462,7 @@ static struct mp_flight_record mp_steps[] = {
 
 int mp_init(void)
 {
-   int num_aps;
+   int num_aps, num_cpus;
atomic_t *ap_count;
struct udevice *cpu;
int ret;
@@ -516,7 +524,7 @@ int mp_init(void)
}
 
/* Walk the flight plan for the BSP */
-   ret = bsp_do_flight_plan(cpu, _info);
+   ret = bsp_do_flight_plan(cpu, _info, num_aps);
if (ret) {
debug("CPU init failed: err=%d\n", ret);
return ret;
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 01/25] x86: mp_init: Switch to livetree

[Simon Glass]

Update this code to use livetree calls instead of flat-tree.

Signed-off-by: Simon Glass 
Reviewed-by: Wolfgang Wallner 
Reviewed-by: Bin Meng 
---

(no changes since v1)

 arch/x86/cpu/mp_init.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index 7fde4ff7e1..c25d17c647 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -507,8 +507,7 @@ int mp_init_cpu(struct udevice *cpu, void *unused)
 * seq num in the uclass_resolve_seq() during device_probe(). To avoid
 * this, set req_seq to the reg number in the device tree in advance.
 */
-   cpu->req_seq = fdtdec_get_int(gd->fdt_blob, dev_of_offset(cpu), "reg",
- -1);
+   cpu->req_seq = dev_read_u32_default(cpu, "reg", -1);
plat->ucode_version = microcode_read_rev();
plat->device_id = gd->arch.x86_device;
 
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v3 00/25] x86: Enhance MTRR functionality to support multiple CPUs

[Simon Glass]

At present MTRRs are mirrored to the secondary CPUs only once, as those
CPUs are started up. But U-Boot may add more MTRRs later, e.g. if it
decides that a video console must be set up.

This series enhances the x86 multi-processor support to allow MTRRs to
be updated at any time. It also updates the 'mtrr' command to support
setting the MTRRs on CPUs other than the boot CPU.

Changes in v3:
- Update bsp_do_flight_plan() to say 'on the BSP'
- s/slow/slot/
- Use C code instead of assembler to read/write callback pointers
- Update commit message to mention dropping of cpu_map
- Rename flag to GD_FLG_SMP_READY
- Rename flag to GD_FLG_SMP_READY
- Add a comment to run_ap_work()
- Rename flag to GD_FLG_SMP_READY
- Update the comment for run_ap_work() to explain logical_cpu_number
- Clarify meaning of @cpu_select in mp_run_on_cpus() comment
- Update the comment for mp_park_aps()
- Add more comments on how the iterators work
- Mention that the CPU number is in hex
- Remove stray asterisk from comments
- Drop mention of cpu_map which was handled in a previous patch

Changes in v2:
- Add comments to explain what start_aps() does
- Drop change to include/dm/uclass.h
- Mention error return in get_bsp()
- Add more comments
- Rename mtrr_save_all() to mtrr_read_all()
- Add a new patch to avoid enabling SMP in SPL
- Add new patch to add AP_DEFAULT_BASE to coral's memory map
- Rename function to mtrr_write_all()
- Keep things building by temporarily renaming the function in cmd/
- Drop the renamed mtrr_set_valid_() instead of mtrr_set_valid()
- Add a new patch with more comments

Simon Glass (25):
  x86: mp_init: Switch to livetree
  x86: Move MP code into mp_init
  x86: mp_init: Avoid declarations in header files
  x86: mp_init: Switch parameter names in start_aps()
  x86: mp_init: Drop the num_cpus static variable
  x86: mtrr: Fix 'ensable' typo
  x86: mp_init: Set up the CPU numbers at the start
  x86: mp_init: Adjust bsp_init() to return more information
  x86: cpu: Remove unnecessary #ifdefs
  x86: mp: Support APs waiting for instructions
  global_data: Add a generic global_data flag for SMP state
  x86: Set the SMP flag when MP init is complete
  x86: mp: Allow running functions on multiple CPUs
  x86: mp: Park CPUs before running the OS
  x86: mp: Add iterators for CPUs
  x86: mtrr: Use MP calls to list the MTRRs
  x86: Don't enable SMP in SPL
  x86: coral: Update the memory map
  x86: mtrr: Update MTRRs on all CPUs
  x86: mtrr: Add support for writing to MTRRs on any CPU
  x86: mtrr: Update the command to use the new mtrr calls
  x86: mtrr: Restructure so command execution is in one place
  x86: mtrr: Update 'mtrr' to allow setting MTRRs on any CPU
  x86: mp: Add more comments to the module
  x86: mtrr: Enhance 'mtrr' command to list MTRRs on any CPU

 arch/x86/cpu/Makefile |   2 +-
 arch/x86/cpu/cpu.c|  63 ++--
 arch/x86/cpu/i386/cpu.c   |  26 +-
 arch/x86/cpu/mp_init.c| 514 ++
 arch/x86/cpu/mtrr.c   | 149 
 arch/x86/include/asm/mp.h | 134 ++-
 arch/x86/include/asm/mtrr.h   |  51 +++
 cmd/x86/mtrr.c| 148 +---
 doc/board/google/chromebook_coral.rst |   1 +
 include/asm-generic/global_data.h |   1 +
 10 files changed, 900 insertions(+), 189 deletions(-)

-- 
2.27.0.212.ge8ba1cc988-goog

RE: [PATCH 0/4] Move eSDHC adapter card code to board files

[Y.b. Lu]

Hi Priyanka,

Any comments on this patch-set?
Thanks a lot.

Best regards,
Yangbo Lu

> -Original Message-
> From: Yangbo Lu 
> Sent: Wednesday, June 17, 2020 6:09 PM
> To: u-boot@lists.denx.de; Priyanka Jain ; Peng Fan
> 
> Cc: Y.b. Lu 
> Subject: [PATCH 0/4] Move eSDHC adapter card code to board files
> 
> The eSDHC adapter card identification and multiplexing configuration
> through FPGA had been implemented in both common mmc driver and
> fsl_esdhc driver. However it is proper to move these code to board
> files and do it during board initialization. The FPGA registers are
> also board specific.
> 
> This patch-set is to move eSDHC adapter card identification and
> multiplexing configuration from mmc driver to specific board files.
> Add eSDHC adapter card identification for LX2 QDS.
> And the option CONFIG_FSL_ESDHC_ADAPTER_IDENT is no longer needed.
> 
> CI build result
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci
> .org%2Fgithub%2Fyangbolu1991%2Fu-boot-test%2Fbuilds%2F699180417
> mp;data=02%7C01%7Cyangbo.lu%40nxp.com%7C2351c69f76e142be1c4108d
> 812a73fd6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637279
> 856821151607sdata=doJR9jInaSGNQHqZlOar%2BKb3kY%2F7xuQzNgA
> 88ADf8B0%3Dreserved=0
> 
> Yangbo Lu (4):
>   Drop global data sdhc_adapter for powerpc
>   Move eSDHC adapter card identification to board files
>   board: fsl: lx2160aqds: identify SDHC adapter during board init
>   configs: lx2160aqds: enable CONFIG_BOARD_EARLY_INIT_R
> 
>  arch/powerpc/include/asm/global_data.h   |  4 +--
>  board/freescale/common/qixis.h   | 14 +++---
>  board/freescale/lx2160a/lx2160a.c| 36
> +++--
>  board/freescale/t1040qds/t1040qds.c  | 29
> -
>  board/freescale/t208xqds/t208xqds.c  | 29
> -
>  configs/lx2160aqds_tfa_SECURE_BOOT_defconfig |  1 +
>  configs/lx2160aqds_tfa_defconfig |  1 +
>  doc/README.fsl-esdhc | 14 --
>  drivers/mmc/fsl_esdhc.c  | 39 
> 
>  drivers/mmc/mmc-uclass.c |  4 +--
>  drivers/mmc/mmc.c|  7 +
>  drivers/mmc/mmc_legacy.c |  7 +
>  drivers/mmc/mmc_private.h|  4 +--
>  include/configs/T1040QDS.h   |  1 -
>  include/configs/T208xQDS.h   |  1 -
>  include/fsl_esdhc.h  |  4 ---
>  scripts/config_whitelist.txt |  1 -
>  17 files changed, 108 insertions(+), 88 deletions(-)
> 
> --
> 2.7.4

[PATCH v2 0/6] Add support MediaTek USB3 DRD driver

[Chunfeng Yun]

These patches introduce the MediaTek USB3 Dual-Role Controller
driver.
The driver can be configured as Dual-Role Device, Peripheral only
and Host only(xHCI) modes, and it's ported from Linux Kernel 5.7-rc1

v2 changes:
1. simplify QMU operations

Chunfeng Yun (6):
  dt-binding: usb: add bindings for some common properties
  dt-bindings: usb: mtu3: add bindings for MediaTek USB3 DRD
  usb: add USB_SPEED_SUPER_PLUS
  usb: add MediaTek USB3 DRD driver
  arm: dts: mt8512: add usb related nodes
  configs: mt8512: enable fastboot

 Makefile   |   1 +
 arch/arm/dts/mt8512-bm1-emmc.dts   |  10 +
 arch/arm/dts/mt8512.dtsi   |  41 +-
 configs/mt8512_bm1_emmc_defconfig  |  19 +
 doc/device-tree-bindings/usb/generic.txt   |  31 +
 doc/device-tree-bindings/usb/mediatek,mtu3.txt |  74 ++
 drivers/usb/Kconfig|   2 +
 drivers/usb/mtu3/Kconfig   |  45 ++
 drivers/usb/mtu3/Makefile  |  11 +
 drivers/usb/mtu3/mtu3.h| 404 +++
 drivers/usb/mtu3/mtu3_core.c   | 863 +++
 drivers/usb/mtu3/mtu3_dr.h |  59 ++
 drivers/usb/mtu3/mtu3_gadget.c | 705 +++
 drivers/usb/mtu3/mtu3_gadget_ep0.c | 933 +
 drivers/usb/mtu3/mtu3_host.c   | 170 +
 drivers/usb/mtu3/mtu3_hw_regs.h| 514 ++
 drivers/usb/mtu3/mtu3_plat.c   | 251 +++
 drivers/usb/mtu3/mtu3_qmu.c| 505 +
 drivers/usb/mtu3/mtu3_qmu.h|  37 +
 include/linux/usb/ch9.h|   1 +
 20 files changed, 4675 insertions(+), 1 deletion(-)
 create mode 100644 doc/device-tree-bindings/usb/generic.txt
 create mode 100644 doc/device-tree-bindings/usb/mediatek,mtu3.txt
 create mode 100644 drivers/usb/mtu3/Kconfig
 create mode 100644 drivers/usb/mtu3/Makefile
 create mode 100644 drivers/usb/mtu3/mtu3.h
 create mode 100644 drivers/usb/mtu3/mtu3_core.c
 create mode 100644 drivers/usb/mtu3/mtu3_dr.h
 create mode 100644 drivers/usb/mtu3/mtu3_gadget.c
 create mode 100644 drivers/usb/mtu3/mtu3_gadget_ep0.c
 create mode 100644 drivers/usb/mtu3/mtu3_host.c
 create mode 100644 drivers/usb/mtu3/mtu3_hw_regs.h
 create mode 100644 drivers/usb/mtu3/mtu3_plat.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.c
 create mode 100644 drivers/usb/mtu3/mtu3_qmu.h

-- 
1.9.1

[PATCH v2 6/6] configs: mt8512: enable fastboot

[Chunfeng Yun]

Enable fastboot to support download image from usb, also enable
usb related drivers, such as usb phy etc.

Signed-off-by: Chunfeng Yun 
---
v2: no changes
---
 configs/mt8512_bm1_emmc_defconfig | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/configs/mt8512_bm1_emmc_defconfig 
b/configs/mt8512_bm1_emmc_defconfig
index e7659d7..bdcd06a 100644
--- a/configs/mt8512_bm1_emmc_defconfig
+++ b/configs/mt8512_bm1_emmc_defconfig
@@ -13,6 +13,7 @@ CONFIG_DEFAULT_FDT_FILE="mt8512-bm1-emmc.dtb"
 CONFIG_SYS_PROMPT="MT8512> "
 CONFIG_CMD_BOOTMENU=y
 CONFIG_CMD_MMC=y
+CONFIG_EFI_PARTITION=y
 CONFIG_DEFAULT_DEVICE_TREE="mt8512-bm1-emmc"
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
@@ -25,6 +26,15 @@ CONFIG_PINCONF=y
 CONFIG_PINCTRL_MT8512=y
 CONFIG_RAM=y
 CONFIG_BAUDRATE=921600
+CONFIG_USB_FUNCTION_FASTBOOT=y
+CONFIG_FASTBOOT_BUF_ADDR=0x5600
+CONFIG_FASTBOOT_BUF_SIZE=0x1e0
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT=y
+CONFIG_PHY=y
+CONFIG_PHY_MTK_TPHY=y
+CONFIG_DM_REGULATOR=y
 CONFIG_DM_SERIAL=y
 CONFIG_MTK_SERIAL=y
 CONFIG_TIMER=y
@@ -32,3 +42,12 @@ CONFIG_MTK_TIMER=y
 CONFIG_WDT=y
 CONFIG_WDT_MTK=y
 CONFIG_LZO=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_DM_USB_GADGET=y
+CONFIG_USB_MTU3=y
+CONFIG_USB_MTU3_GADGET=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="MediaTek"
+CONFIG_USB_GADGET_VENDOR_NUM=0x0e8d
+CONFIG_USB_GADGET_PRODUCT_NUM=0x201c
-- 
1.9.1

[PATCH v2 3/6] usb: add USB_SPEED_SUPER_PLUS

[Chunfeng Yun]

Add enum USB_SPEED_SUPER_PLUS for USB3.1

Signed-off-by: Chunfeng Yun 
---
v2: no changes
---
 include/linux/usb/ch9.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
index 989a5fc..7d225ee 100644
--- a/include/linux/usb/ch9.h
+++ b/include/linux/usb/ch9.h
@@ -956,6 +956,7 @@ enum usb_device_speed {
USB_SPEED_HIGH, /* usb 2.0 */
USB_SPEED_WIRELESS, /* wireless (usb 2.5) */
USB_SPEED_SUPER,/* usb 3.0 */
+   USB_SPEED_SUPER_PLUS,   /* usb 3.1 */
 };
 
 #ifdef __KERNEL__
-- 
1.9.1

[PATCH v2 5/6] arm: dts: mt8512: add usb related nodes

[Chunfeng Yun]

Add usb, usb phy nodes

Signed-off-by: Chunfeng Yun 
---
v2: no changes
---
 arch/arm/dts/mt8512-bm1-emmc.dts | 10 ++
 arch/arm/dts/mt8512.dtsi | 41 +++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/mt8512-bm1-emmc.dts b/arch/arm/dts/mt8512-bm1-emmc.dts
index 296ed93..0788e05 100644
--- a/arch/arm/dts/mt8512-bm1-emmc.dts
+++ b/arch/arm/dts/mt8512-bm1-emmc.dts
@@ -95,6 +95,16 @@
};
 };
 
+ {
+   dr_mode = "peripheral";
+   maximum-speed = "high-speed";
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm/dts/mt8512.dtsi b/arch/arm/dts/mt8512.dtsi
index 01a02a7..b5d833c 100644
--- a/arch/arm/dts/mt8512.dtsi
+++ b/arch/arm/dts/mt8512.dtsi
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 / {
compatible = "mediatek,mt8512";
@@ -100,6 +101,44 @@
status = "disabled";
};
 
+   ssusb: usb@11211000 {
+   compatible = "mediatek,mt8512-mtu3", "mediatek,mtu3";
+   reg = <0x11211000 0x2dff>,
+ <0x11213e00 0x0100>;
+   reg-names = "mac", "ippc";
+   interrupts = ;
+   phys = < PHY_TYPE_USB2>;
+   clocks = < CLK_INFRA_USB_SYS>,
+< CLK_TOP_SSUSB_TOP_CK_EN>,
+< CLK_INFRA_ICUSB>;
+   clock-names = "sys_ck", "ref_ck", "mcu_ck";
+   status = "disabled";
+   };
+
+   u3phy: usb-phy@11cc {
+   compatible = "mediatek,mt8512-tphy",
+"mediatek,generic-tphy-v2";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+   status = "disabled";
+
+   u2port0: usb-phy@11cc {
+   reg = <0x11cc 0x400>;
+   clocks = < CLK_TOP_USB20_48M_EN>;
+   clock-names = "ref";
+   #phy-cells = <1>;
+   mediatek,discth = <15>;
+   status = "okay";
+   };
+
+   u2port1: usb-phy@11c4 {
+   reg = <0x11c4 0x400>;
+   #phy-cells = <1>;
+   status = "okay";
+   };
+   };
+
mmc0: mmc@1123 {
compatible = "mediatek,mt8512-mmc";
reg = <0x1123 0x1000>,
@@ -112,4 +151,4 @@
status = "disabled";
};
 
-};
\ No newline at end of file
+};
-- 
1.9.1

[PATCH v2 1/6] dt-binding: usb: add bindings for some common properties

[Chunfeng Yun]

Add bindings for common properties, include maximum-speed,
dr_mode and phy_type

Signed-off-by: Chunfeng Yun 
---
v2: no changes
---
 doc/device-tree-bindings/usb/generic.txt | 31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 doc/device-tree-bindings/usb/generic.txt

diff --git a/doc/device-tree-bindings/usb/generic.txt 
b/doc/device-tree-bindings/usb/generic.txt
new file mode 100644
index 000..a02a198
--- /dev/null
+++ b/doc/device-tree-bindings/usb/generic.txt
@@ -0,0 +1,31 @@
+Generic USB Properties
+
+Optional properties:
+ - maximum-speed: tells USB controllers we want to work up to a certain
+   speed. Valid arguments are "super-speed-plus",
+   "super-speed", "high-speed", "full-speed" and
+   "low-speed". In case this isn't passed via DT, USB
+   controllers should default to their maximum HW
+   capability.
+ - dr_mode: tells Dual-Role USB controllers that we want to work on a
+   particular mode. Valid arguments are "host",
+   "peripheral" and "otg". In case this attribute isn't
+   passed via DT, USB DRD controllers should default to
+   OTG.
+ - phy_type: tells USB controllers that we want to configure the core to 
support
+   a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
+   selected. Valid arguments are "utmi" and "utmi_wide".
+   In case this isn't passed via DT, USB controllers should
+   default to HW capability.
+
+This is an attribute to a USB controller such as:
+
+dwc3@4a03 {
+   compatible = "synopsys,dwc3";
+   reg = <0x4a03 0xcfff>;
+   interrupts = <0 92 4>
+   usb-phy = <_phy>, <,phy>;
+   maximum-speed = "super-speed";
+   dr_mode = "otg";
+   phy_type = "utmi_wide";
+};
-- 
1.9.1

[PATCH v2 2/6] dt-bindings: usb: mtu3: add bindings for MediaTek USB3 DRD

[Chunfeng Yun]

Add dt-binding for MediaTek USB3 DRD Driver

Signed-off-by: Chunfeng Yun 
---
v2: no changes
---
 doc/device-tree-bindings/usb/mediatek,mtu3.txt | 74 ++
 1 file changed, 74 insertions(+)
 create mode 100644 doc/device-tree-bindings/usb/mediatek,mtu3.txt

diff --git a/doc/device-tree-bindings/usb/mediatek,mtu3.txt 
b/doc/device-tree-bindings/usb/mediatek,mtu3.txt
new file mode 100644
index 000..75a6785
--- /dev/null
+++ b/doc/device-tree-bindings/usb/mediatek,mtu3.txt
@@ -0,0 +1,74 @@
+The device node for Mediatek USB3.0 DRD controller
+
+Required properties:
+ - compatible : should be "mediatek,-mtu3", "mediatek,mtu3",
+   soc-model is the name of SoC, such as mt8512 etc,
+   when using "mediatek,mtu3" compatible string, you need SoC specific
+   ones in addition, one of:
+   - "mediatek,mt8512-mtu3"
+ - reg : specifies physical base address and size of the registers
+ - reg-names: should be
+   - "mac" : device IP,
+   - "ippc" : IP port control
+ - interrupts : interrupt used by the device IP
+ - power-domains : a phandle to USB power domain node to control USB's MTCMOS
+ - vusb33-supply : regulator of USB AVDD3.3v
+ - clocks : a list of phandle + clock-specifier pairs, one for each
+   entry in clock-names
+ - clock-names : must contain "sys_ck" for clock of controller,
+   the following clocks are optional:
+   "ref_ck", "mcu_ck" and "dma_ck";
+ - phys : list of all the USB PHYs on this HCD
+ - dr_mode : should be one of "host", "peripheral" or "otg",
+   see : usb/generic.txt
+
+Optional properties:
+ - #address-cells, #size-cells : should be '2' if the device has sub-nodes
+   with 'reg' property
+ - ranges : allows valid 1:1 translation between child's address space and
+   parent's address space
+ - vbus-supply : reference to the VBUS regulator, needed when supports
+   dual-role mode.
+ - pinctrl-names : a pinctrl state named "default" is optional
+ - pinctrl-0 : pin control group
+   See: pinctrl/pinctrl-bindings.txt
+
+ - maximum-speed : valid arguments are "full-speed", "high-speed",
+   "super-speed" and "super-speed-plus",
+   see: usb/generic.txt.
+
+Sub-nodes:
+The xhci should be added as subnode to mtu3 as shown in the following example
+if host mode is enabled. The DT binding details of xhci can be found in:
+   - usb/mediatek,mtk-xhci.txt
+
+Example:
+ssusb: usb@11271000 {
+   compatible = "mediatek,mt8173-mtu3", "mediatek,mtu3";
+   reg = <0 0x11271000 0 0x3000>,
+ <0 0x11280700 0 0x0100>;
+   reg-names = "mac", "ippc";
+   interrupts = ;
+   phys = <_port0 PHY_TYPE_USB3>,
+  <_port1 PHY_TYPE_USB2>;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>, <>;
+   clock-names = "sys_ck", "ref_ck";
+   vusb33-supply = <_vusb_reg>;
+   vbus-supply = <_p0_vbus>;
+   dr_mode = "otg";
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   usb_host: xhci@1127 {
+   compatible = "mediatek,mt8173-xhci", "mediatek,mtk-xhci";
+   reg = <0 0x1127 0 0x1000>;
+   reg-names = "mac";
+   interrupts = ;
+   power-domains = < MT8173_POWER_DOMAIN_USB>;
+   clocks = < CLK_TOP_USB30_SEL>, <>;
+   clock-names = "sys_ck", "ref_ck";
+   vusb33-supply = <_vusb_reg>;
+   };
+};
-- 
1.9.1

[PATCH 2/2] x86: apl: Re-enable loading of SPL

[Simon Glass]

At present the SPL loader is not included in the TPL image so SPL cannot
be loaded. Fix it by including this file for both SPL and TPL.

Signed-off-by: Simon Glass 
Fixes: c87f9ce2273 ("x86: Don't build some unused objects in TPL")
---

 arch/x86/cpu/apollolake/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/apollolake/Makefile b/arch/x86/cpu/apollolake/Makefile
index 11621256ae..3aa2a55676 100644
--- a/arch/x86/cpu/apollolake/Makefile
+++ b/arch/x86/cpu/apollolake/Makefile
@@ -3,6 +3,7 @@
 # Copyright 2019 Google LLC
 
 obj-$(CONFIG_SPL_BUILD) += cpu_spl.o
+obj-$(CONFIG_SPL_BUILD) += spl.o
 obj-$(CONFIG_SPL_BUILD) += systemagent.o
 obj-y += cpu_common.o
 
@@ -11,7 +12,6 @@ obj-y += cpu.o
 obj-y += punit.o
 obj-y += fsp_bindings.o
 ifdef CONFIG_SPL_BUILD
-obj-y += spl.o
 obj-y += fsp_m.o
 endif
 endif
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH 1/2] spi: Remove unnecessary #ifdefs in header file

[Simon Glass]

These prevent use of compile-time checks such as:

if (CONFIG_IS_ENABLED(DM_SPI))

since, for example, if CONFIG_SPL_DM_SPI is not enabled then the
definitions are not included by spi.h and the C code will not build.

The #ifdefs are unnecessary since there are no conflicts with the pre-DM
code. In any case we have almost switched over to driver model for SPI.

Drop these #ifdefs from spi.h to fix a build warning on chromebook_coral
in the following patch.

Signed-off-by: Simon Glass 
---

 include/spi.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/include/spi.h b/include/spi.h
index 9b4fb8dc0b..7fca646759 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -39,7 +39,6 @@
 
 #define SPI_DEFAULT_WORDLEN8
 
-#if CONFIG_IS_ENABLED(DM_SPI)
 /* TODO(s...@chromium.org): Remove this and use max_hz from struct spi_slave */
 struct dm_spi_bus {
uint max_hz;
@@ -65,8 +64,6 @@ struct dm_spi_slave_platdata {
uint mode;
 };
 
-#endif /* CONFIG_DM_SPI */
-
 /**
  * enum spi_clock_phase - indicates  the clock phase to use for SPI (CPHA)
  *
@@ -317,7 +314,6 @@ void spi_flash_copy_mmap(void *data, void *offset, size_t 
len);
  */
 int spi_cs_is_valid(unsigned int bus, unsigned int cs);
 
-#if !CONFIG_IS_ENABLED(DM_SPI)
 /**
  * Activate a SPI chipselect.
  * This function is provided by the board code when using a driver
@@ -343,7 +339,6 @@ void spi_cs_deactivate(struct spi_slave *slave);
  * @hz:The transfer speed
  */
 void spi_set_speed(struct spi_slave *slave, uint hz);
-#endif
 
 /**
  * Write 8 bits, then read 8 bits.
@@ -367,8 +362,6 @@ static inline int spi_w8r8(struct spi_slave *slave, 
unsigned char byte)
return ret < 0 ? ret : din[1];
 }
 
-#if CONFIG_IS_ENABLED(DM_SPI)
-
 /**
  * struct spi_cs_info - Information about a bus chip select
  *
@@ -717,6 +710,5 @@ int dm_spi_get_mmap(struct udevice *dev, ulong *map_basep, 
uint *map_sizep,
 /* Access the operations for a SPI device */
 #define spi_get_ops(dev)   ((struct dm_spi_ops *)(dev)->driver->ops)
 #define spi_emul_get_ops(dev)  ((struct dm_spi_emul_ops *)(dev)->driver->ops)
-#endif /* CONFIG_DM_SPI */
 
 #endif /* _SPI_H_ */
-- 
2.27.0.212.ge8ba1cc988-goog

Re: [PATCH 1/1] sandbox: make RAM size configurable

[Simon Glass]

On Sun, 7 Jun 2020 at 10:47, Heinrich Schuchardt  wrote:
>
> Up to now the RAM size of the sandbox is hard coded as 128 MiB. This does
> not allow testing the correct handling of addresses outside the 32bit
> range. 128 MiB is also rather small when tracing functions where the trace
> is written to RAM.
>
> Provide configuration variable CONFIG_SANDBOX_RAM_SIZE_MB to set the RAM
> size in MiB. It defaults to 128 MiB with a minimum of 64 MiB.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/sandbox/Kconfig | 10 ++
>  arch/sandbox/include/asm/state.h |  2 +-
>  include/configs/sandbox.h|  5 -
>  3 files changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/1] sandbox: handling out of memory

[Simon Glass]

On 6/7/20 4:02 PM, Heinrich Schuchardt wrote:
> Am June 7, 2020 1:45:53 PM UTC schrieb Simon Glass :
>> Hi Heinrich,
>>
>> On Thu, 4 Jun 2020 at 11:28, Heinrich Schuchardt 
>> wrote:
>>>
>>> assert() only works in debug mode. So checking a successful memory
>>> allocation should not use assert().
>>>
>>
>> Reviewed-by: Simon Glass 
>>
>> What sort of environment are you using that returns NULL in this case?
>
> You will get NULL here if mmap() fails. This should happen if your machine 
> has less then 128 MiB left over or you increase the RAM size of the sandbox.
>
> For testing I suggest you increase the sandbox memory size beyond the RAM and 
> swap size of your computer.
>
> Best regards
>
> Heinrich

An excessive RAM sandbox w/o the patch:

$ ./u-boot
Segmentation fault

with the patch

$ ./u-boot
Out of memory

Best regards

Heinrich

>
>>
>> Regards,
>> Simon
>>
>>
>>> Signed-off-by: Heinrich Schuchardt 
>>> ---
>>>  arch/sandbox/cpu/state.c | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
Applied to u-boot-dm/next, thanks!
Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/1] sandbox: spi: sandbox_sf_state_name() is required

[Simon Glass]

On Sun, 7 Jun 2020 at 01:28, Heinrich Schuchardt  wrote:
>
> Compiling drivers/mtd/spi/sandbox.c fails when compiled with
> CONFIG_LOG=n:
>
> In file included from include/common.h:20,
>  from drivers/mtd/spi/sandbox.c:13:
> drivers/mtd/spi/sandbox.c:295:15: error: format ‘%s’ expects argument of
> type ‘char *’, but argument 7 has type ‘int’ [-Werror=format=]
>   295 |   log_content(" cmd: transition to %s state\n",
>   |   ^~~~
> include/linux/printk.h:37:21: note: in definition of macro ‘pr_fmt’
>37 | #define pr_fmt(fmt) fmt
>   | ^~~
> include/log.h:128:30: note: in expansion of macro ‘log_nop’
>   128 | #define log_content(_fmt...) log_nop(LOG_CATEGORY, \
>   |  ^~~
> drivers/mtd/spi/sandbox.c:295:3: note: in expansion of macro
> ‘log_content’
>   295 |   log_content(" cmd: transition to %s state\n",
>   |   ^~~
> drivers/mtd/spi/sandbox.c:295:37: note: format string is defined here
>   295 |   log_content(" cmd: transition to %s state\n",
>   |~^
>   | |
>   | char *
>   |%d
>
> Supply function sandbox_sf_state_name() independent of CONFIG_LOG.
>
> Fixes: c3aed5db591e ("sandbox: spi: Add more logging")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  drivers/mtd/spi/sandbox.c | 2 --
>  1 file changed, 2 deletions(-)
>

Reviewed-by: Simon Glass 

We probably need to update test/run and CI config with a test that
builds sandbox with this CONFIG_LOG=n

Applied to u-boot-dm/next, thanks!
Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 1/6] patman: Drop unnecessary import in gitutil

[Simon Glass]

The checkpatch module is not used, so drop it.

Signed-off-by: Simon Glass 
Reported-by: Stefan Bosch 
---

(no changes since v3)

Changes in v3:
- Split out the gitutil change into a separate patch

 tools/patman/gitutil.py | 1 -
 1 file changed, 1 deletion(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 3/6] patman: Use a dict in gitutil to avoid importing series

[Simon Glass]

Only a few members of this class are used and only in a test. To avoid
importing the module, convert the test to use a dict.

Signed-off-by: Simon Glass 
Reported-by: Stefan Bosch 
---

(no changes since v1)

 tools/patman/gitutil.py | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 4/6] patman: Pass in maintainer dirs to avoid and import

[Simon Glass]

Adjust the get_maintainer module to accept a list of directories to search
for the script. This avoids needing to import gitutil.

Signed-off-by: Simon Glass 
Reported-by: Stefan Bosch 
---

(no changes since v1)

 tools/patman/get_maintainer.py | 14 +++---
 tools/patman/series.py |  3 ++-
 2 files changed, 9 insertions(+), 8 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 2/6] patman: Avoid circular dependency between command and tools

[Simon Glass]

This seems to cause problems in some cases. Split the dependency by
copying the code to command.

Reported-by: Stefan Bosch 
Signed-off-by: Simon Glass 
---

(no changes since v1)

 tools/patman/command.py | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 5/6] patman: Avoid importing gitutil in settings

[Simon Glass]

Pass this module in so that settings does not need to import it.

Signed-off-by: Simon Glass 
Reported-by: Stefan Bosch 
---

(no changes since v3)

Changes in v3:
- Add more patches based on testing on a dusty Ubuntu 14.04

Changes in v2:
- Update gitutil as well

 tools/patman/main.py | 2 +-
 tools/patman/settings.py | 7 +++
 2 files changed, 4 insertions(+), 5 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 6/6] patman: Drop import of test_util in test_util

[Simon Glass]

This module doesn't need to import itself. It causes problems on
very old Python 3 (e.g. 3.4.0). Drop it.

Signed-off-by: Simon Glass 
---

Changes in v4:
- Add a new patch to drop import of test_util in test_util
- Add a cover letter

 tools/patman/test_util.py | 1 -
 1 file changed, 1 deletion(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/1] log: uclass_get_name() depends on CONFIG_SPL_DM

[Simon Glass]

Hi Heinrich,

On Mon, 8 Jun 2020 at 10:04, Heinrich Schuchardt  wrote:
>
> If CONFIG_SPL_DM=n and CONFIG_SPL_LOG=y a build error occurs:
>
> ld.bfd: common/built-in.o: in function `log_get_cat_name':
> common/log.c:48: undefined reference to `uclass_get_name'
> make[1]: *** [scripts/Makefile.spl:422: spl/u-boot-spl] Error 1
>
> Call uclass_get_name() only if DM is enabled.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  common/log.c | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/1] sandbox: handling out of memory

[Simon Glass]

On 6/7/20 4:02 PM, Heinrich Schuchardt wrote:
> Am June 7, 2020 1:45:53 PM UTC schrieb Simon Glass :
>> Hi Heinrich,
>>
>> On Thu, 4 Jun 2020 at 11:28, Heinrich Schuchardt 
>> wrote:
>>>
>>> assert() only works in debug mode. So checking a successful memory
>>> allocation should not use assert().
>>>
>>
>> Reviewed-by: Simon Glass 
>>
>> What sort of environment are you using that returns NULL in this case?
>
> You will get NULL here if mmap() fails. This should happen if your machine 
> has less then 128 MiB left over or you increase the RAM size of the sandbox.
>
> For testing I suggest you increase the sandbox memory size beyond the RAM and 
> swap size of your computer.
>
> Best regards
>
> Heinrich

An excessive RAM sandbox w/o the patch:

$ ./u-boot
Segmentation fault

with the patch

$ ./u-boot
Out of memory

Best regards

Heinrich

>
>>
>> Regards,
>> Simon
>>
>>
>>> Signed-off-by: Heinrich Schuchardt 
>>> ---
>>>  arch/sandbox/cpu/state.c | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/1] sandbox: spi: sandbox_sf_state_name() is required

[Simon Glass]

On Sun, 7 Jun 2020 at 01:28, Heinrich Schuchardt  wrote:
>
> Compiling drivers/mtd/spi/sandbox.c fails when compiled with
> CONFIG_LOG=n:
>
> In file included from include/common.h:20,
>  from drivers/mtd/spi/sandbox.c:13:
> drivers/mtd/spi/sandbox.c:295:15: error: format ‘%s’ expects argument of
> type ‘char *’, but argument 7 has type ‘int’ [-Werror=format=]
>   295 |   log_content(" cmd: transition to %s state\n",
>   |   ^~~~
> include/linux/printk.h:37:21: note: in definition of macro ‘pr_fmt’
>37 | #define pr_fmt(fmt) fmt
>   | ^~~
> include/log.h:128:30: note: in expansion of macro ‘log_nop’
>   128 | #define log_content(_fmt...) log_nop(LOG_CATEGORY, \
>   |  ^~~
> drivers/mtd/spi/sandbox.c:295:3: note: in expansion of macro
> ‘log_content’
>   295 |   log_content(" cmd: transition to %s state\n",
>   |   ^~~
> drivers/mtd/spi/sandbox.c:295:37: note: format string is defined here
>   295 |   log_content(" cmd: transition to %s state\n",
>   |~^
>   | |
>   | char *
>   |%d
>
> Supply function sandbox_sf_state_name() independent of CONFIG_LOG.
>
> Fixes: c3aed5db591e ("sandbox: spi: Add more logging")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  drivers/mtd/spi/sandbox.c | 2 --
>  1 file changed, 2 deletions(-)
>

Reviewed-by: Simon Glass 

We probably need to update test/run and CI config with a test that
builds sandbox with this CONFIG_LOG=n

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 11/14] dm: doc: update of-plat with new phandle support

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> Update documentation to reflect the new phandle support when OF_PLATDATA
> is used. Now phandles are implemented as pointers to U_BOOT_DEVICE,
> which makes it possible to get a pointer to the actual device.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  doc/driver-model/of-plat.rst | 24 
>  1 file changed, 16 insertions(+), 8 deletions(-)
>

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH 3/5] patman: Decode output from the '--show-types' option

[Simon Glass]

Collect the 'checkpatch type' from each error, warning and check. Provide
this to patman and update the uclass test to use it.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py  | 24 +++-
 tools/patman/test_checkpatch.py |  4 ++--
 2 files changed, 17 insertions(+), 11 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 05/14] dtoc: add option to disable warnings

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> As dtoc now performs checks for valid driver names, when running dtoc
> tests several warnings arise as these tests don't use valid driver
> names.
>
> This patch adds an option to disable those warning, which is only
> intended for running tests.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  tools/dtoc/dtb_platdata.py  | 13 ++--
>  tools/dtoc/dtoc_test_invalid_driver.dts | 15 
>  tools/dtoc/test_dtoc.py | 91 +
>  3 files changed, 85 insertions(+), 34 deletions(-)
>  create mode 100644 tools/dtoc/dtoc_test_invalid_driver.dts

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH 2/5] patman: Add a test for the 'possible new uclass' check

[Simon Glass]

It is quite likely that the number of U-Boot-specific tests in
checkpatch.pl will increase over time. We should have tests for these to
avoid undefined behaviour and bugs being introduced, which might cause
people to ignore the warnings.

Add a simple new class that can generate a patch with a single-line
addition in it. Use that to add a test for one of the checkpatch checks.

Signed-off-by: Simon Glass 
---

 tools/patman/test_checkpatch.py | 77 +
 1 file changed, 77 insertions(+)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/5] patman: Rename test.py to test_checkpatch.py

[Simon Glass]

These tests check checkpatch.pl operation and can server as our tests for
the U-Boot-specific updates to that script. Rename the file and update
comments to indicate this.

Signed-off-by: Simon Glass 
---

 tools/patman/main.py | 4 ++--
 tools/patman/{test.py => test_checkpatch.py} | 7 +++
 2 files changed, 5 insertions(+), 6 deletions(-)
 rename tools/patman/{test.py => test_checkpatch.py} (98%)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 1/2] log: don't show function by default

[Simon Glass]

On Wed, 17 Jun 2020 at 13:52, Heinrich Schuchardt  wrote:
>
> The name of the function emitting a log message may be of interest for a
> developer but is distracting for normal users. See the example below:
>
> try_load_entry() Booting: Debian
>
> Make the default format for log messages customizable. By default show
> only the message text.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v4:
> leave an empty line in enum log_fmt before composite values
> rebase on origin/master
> v3:
> replace #ifdef by IS_ENABLED()
> ---
>  cmd/log.c |  4 ++--
>  common/Kconfig| 18 ++
>  common/log.c  |  2 +-
>  include/log.h | 18 +-
>  test/log/syslog_test.c| 20 ++--
>  test/py/tests/test_log.py |  2 ++
>  6 files changed, 54 insertions(+), 10 deletions(-)
>

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 2/2] log: use BIT() instead of 1 <

[Simon Glass]

Use the BIT() macro when creating a bitmask for the logging fields.

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Simon Glass 
---
v4:
no change
v3:
new patch
---
 common/log_console.c | 14 +++---
 common/log_syslog.c  | 14 +++---
 2 files changed, 14 insertions(+), 14 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH] dm: core: Correct comment on uclass_id_foreach_dev()

[Simon Glass]

On Sun, Jun 14, 2020 at 4:49 PM Simon Glass  wrote:
>
> This parameter should be a struct uclass, not struct udevice. Correct it.
>
> Signed-off-by: Simon Glass 
> ---
>
>  include/dm/uclass.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied to u-boot-dm/next, thanks!

Re: [PATCH 4/5] patman: Add tests for the rest of the checkpatch checks

[Simon Glass]

Finish off the tests for our small collection of checkpatch checks.

Signed-off-by: Simon Glass 
---

 tools/patman/test_checkpatch.py | 47 ++---
 1 file changed, 43 insertions(+), 4 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 1/3] test/dm: fdtdec: Add the missing gd declaration

[Simon Glass]

From: Bin Meng 

Add DECLARE_GLOBAL_DATA_PTR since it is referenced in the test codes.

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 
---

Changes in v4:
- drop the first 2 patches that are already applied
- rebase against u-boot/next branch

 test/dm/fdtdec.c | 2 ++
 1 file changed, 2 insertions(+)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 2/3] test/dm: fdtdec: Corect a typo in dm_test_fdtdec_set_carveout()

[Simon Glass]

From: Bin Meng 

It should be "writable".

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 
---

(no changes since v1)

 test/dm/fdtdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 02/14] dtoc: add missing code comments

[Simon Glass]

Hi Walter,

On Fri, 26 Jun 2020 at 07:18, Walter Lozano  wrote:
>
> Hi Simon,
>
> On 25/6/20 22:42, Simon Glass wrote:
> > On Wed, 24 Jun 2020 at 22:10, Walter Lozano  
> > wrote:
> >> Add missing information about internal class members in order to make
> >> the code easier to follow.
> >>
> >> Signed-off-by: Walter Lozano 
> >> ---
> >>
> >>   tools/dtoc/dtb_platdata.py | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> > Reviewed-by: Simon Glass 
> >
> > BTW if there was a review tag from last version, please add it to your 
> > commit.
>
>
> Thanks for pointing that, I'll take you advice for those commit that
> doesn't change at all. Also thank you for all the time you spent
> reviewing this series and sharing suggestions.

You're welcome. It's a great improvement to of-platdata! You can add
it the tag unless you make a major change to a commit.

Regards,
Simon

Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/1] cmd: fdt: remove CMD_FDT_MAX_DUMP

[Simon Glass]

On Fri, 19 Jun 2020 at 11:46, Heinrich Schuchardt  wrote:
>
> When printing the device tree we want to get an output that can be used as
> input for the device tree compiler. This requires that we do not write
> bogus lines like
>
> pcie@1000 {
> interrupt-map = * 0x4000127c [0x0280];
>
> For instance the QEMU virt device has a property interrupt-map with 640
> bytes which exceeds CMD_FDT_MAX_DUMP=64.
>
> So lets do away with the artificial limitation to 64 bytes.
>
> As indicated in commit f0a29d43313c ("fdt: Limit printed hex in fdt print
> and list commands") if a device tree contains binary blobs, it may still
> be desirable to limit the output length. Provide environment variable
> fdt_max_dump for this purpose.
>
> Fixes: 5d927b428622 ("Kconfig: Drop CONFIG_CMD_FDT_MAX_DUMP")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  cmd/fdt.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 3/3] test/dm: fdtdec: Add tests for fdtdec_add_reserved_memory()

[Simon Glass]

From: Bin Meng 

This adds a test case to test the functionality of the fdtdec API
fdtdec_add_reserved_memory().

Signed-off-by: Bin Meng 
Reviewed-by: Simon Glass 
---

(no changes since v3)

Changes in v3:
- correct typo in the comments, and some minor rewording

 test/dm/fdtdec.c | 69 
 1 file changed, 69 insertions(+)

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 08/14] core: extend struct driver_info to point to device

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> Currently when creating an U_BOOT_DEVICE entry a struct driver_info
> is declared, which contains the data needed to instantiate the device.
> However, the actual device is created at runtime and there is no proper
> way to get the device based on its struct driver_info.
>
> This patch extends struct driver_info adding a pointer to udevice which
> is populated during the bind process, allowing to generate a set of
> functions to get the device based on its struct driver_info.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  drivers/core/device.c | 26 +++---
>  drivers/core/root.c   |  4 
>  include/dm/device.h   | 15 +++
>  include/dm/platdata.h | 14 ++
>  4 files changed, 56 insertions(+), 3 deletions(-)


Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 09/14] sandbox: Move section u_boot_list to make it RW

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> In order to be able to update data in u_boot_list, move this section to
> make it RW.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  arch/sandbox/cpu/u-boot-spl.lds | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 07/14] core: drop const for struct driver_info

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> In order to prepare for a new support of phandle when OF_PLATDATA is used
> drop the const for struct driver_info as this struct will need to be
> updated on runtime.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  drivers/core/device.c| 2 +-
>  drivers/core/root.c  | 2 +-
>  include/dm/device-internal.h | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 06/14] dm: doc: update of-plat with the support for driver aliases

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> Update the documentation with the support for driver aliases using
> U_BOOT_DRIVER_ALIAS.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  doc/driver-model/of-plat.rst | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 

Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 12/14] arm: dts: include gpio nodes for card detect

[Simon Glass]

Hi Adam,

On 26/6/20 09:26, Adam Ford wrote:
> On Thu, Jun 25, 2020 at 11:37 PM Adam Ford  wrote:
>> On Wed, Jun 24, 2020 at 11:11 PM Walter Lozano
>>  wrote:
>>> Several MMC drivers use GPIO for card detection with cd-gpios property in
>>> the MMC node pointing to a GPIO node. However, as U-Boot tries to save
>>> space by keeping only required nodes using u-boot* properties, several
>>> devices tree result in having only in the MMC node but not the GPIO node
>>> associated to cd-gpios.
>>>
>>> This patch, fixes several ocurrence of this issue.
>>>
>>> Signed-off-by: Walter Lozano 
>>> ---
>>>
>>>   arch/arm/dts/da850-evm-u-boot.dtsi|  4 
>>>   arch/arm/dts/da850-lcdk-u-boot.dtsi   |  4 
>>>   arch/arm/dts/rk3288-u-boot.dtsi   |  4 
>>>   arch/arm/dts/rk3288-veyron-speedy-u-boot.dtsi |  2 +-
>>>   arch/arm/dts/rk3288-veyron-u-boot.dtsi| 11 +++
>>>   5 files changed, 24 insertions(+), 1 deletion(-)
>>>   create mode 100644 arch/arm/dts/rk3288-veyron-u-boot.dtsi
>>>
Applied to u-boot-dm/next, thanks!

Re: [PATCH v4 14/14] dtoc: add test for cd-gpios

[sjg]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> Add a test for dtoc taking into account the cd-gpios property.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  tools/dtoc/dtoc_test_phandle_cd_gpios.dts | 42 ++
>  tools/dtoc/test_dtoc.py   | 67 +++
>  2 files changed, 109 insertions(+)
>  create mode 100644 tools/dtoc/dtoc_test_phandle_cd_gpios.dts

Reviewed-by: Simon Glass 

Applied to u-boot-dm, thanks!

Re: [PATCH v4 13/14] dtoc: update dtb_platdata to support cd-gpios

[Simon Glass]

On Wed, 24 Jun 2020 at 22:11, Walter Lozano  wrote:
>
> Currently dtoc does not support the property cd-gpios used to declare
> the gpios for card detect in mmc.
>
> This patch adds support to cd-gpios property.
>
> Signed-off-by: Walter Lozano 
> ---
>
>  tools/dtoc/dtb_platdata.py | 16 ++--
>  tools/dtoc/test_dtoc.py|  2 +-
>  2 files changed, 11 insertions(+), 7 deletions(-)
>

Reviewed-by: Simon Glass 

Applied to u-boot-dm, thanks!

[PATCH 15/15] sunxi: Pine-H64: Explicitly enable PHY regulator

[Andre Przywara]

According to the devicetree and the schematic, the 3.3V power rail for
the PHY is enabled by GPIO PC16. It's wired as active-high, with a
pull-up resistor, so actually works already when the GPIO is in
High-Z state.

However we should not take any chances and explicitly set the GPIO pin
to high, to avoid accidentally losing the PHY power.
The existing MACPWR Kconfig allows to do this easily.

Signed-off-by: Andre Przywara 
---
 configs/pine_h64_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/pine_h64_defconfig b/configs/pine_h64_defconfig
index 87871fd19f..fb18bb7df7 100644
--- a/configs/pine_h64_defconfig
+++ b/configs/pine_h64_defconfig
@@ -11,5 +11,6 @@ CONFIG_SPL_SPI_SUNXI=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-pine-h64"
 CONFIG_SUN8I_EMAC=y
+CONFIG_MACPWR="PC16"
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_OHCI_HCD=y
-- 
2.17.5

[PATCH 13/15] net: sun8i-emac: Make internal PHY handling more robust

[Andre Przywara]

The current implementation of sun8i_get_ephy_nodes() makes quite some
assumptions, in general relying on DT path names is a bad idea.
I think the idea of the code was to determine if we are using the
internal PHY, for which there are simpler and more robust methods:

Rewrite (and rename) the existing function to simply lookup the DT node
that "phy-handle" points to, using the device's DT node.
Then check whether the parent of that PHY node is using an "H3 internal
MDIO" compatible string. If we ever get another internal MDIO bus
implementation, we will probably need code adjustments anyway, so this
is good enough for now.

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 48 ++--
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 48ba244f21..dc716f94f3 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -809,47 +809,31 @@ static const struct eth_ops sun8i_emac_eth_ops = {
.stop   = sun8i_emac_eth_stop,
 };
 
-static int sun8i_get_ephy_nodes(struct emac_eth_dev *priv)
+static int sun8i_handle_internal_phy(struct udevice *dev)
 {
-   int emac_node, ephy_node, ret, ephy_handle;
-
-   emac_node = fdt_path_offset(gd->fdt_blob,
-   "/soc/ethernet@1c3");
-   if (emac_node < 0) {
-   debug("failed to get emac node\n");
-   return emac_node;
-   }
-   ephy_handle = fdtdec_lookup_phandle(gd->fdt_blob,
-   emac_node, "phy-handle");
+   struct emac_eth_dev *priv = dev_get_priv(dev);
+   struct ofnode_phandle_args phandle;
+   int ret;
 
-   /* look for mdio-mux node for internal PHY node */
-   ephy_node = fdt_path_offset(gd->fdt_blob,
-   
"/soc/ethernet@1c3/mdio-mux/mdio@1/ethernet-phy@1");
-   if (ephy_node < 0) {
-   debug("failed to get mdio-mux with internal PHY\n");
-   return ephy_node;
-   }
+   ret = ofnode_parse_phandle_with_args(dev_ofnode(dev), "phy-handle",
+NULL, 0, 0, );
+   if (ret)
+   return ret;
 
-   /* This is not the phy we are looking for */
-   if (ephy_node != ephy_handle)
+   /* If the PHY node is not a child of the internal MDIO bus, we are
+* using some external PHY.
+*/
+   if (!ofnode_device_is_compatible(ofnode_get_parent(phandle.node),
+"allwinner,sun8i-h3-mdio-internal"))
return 0;
 
-   ret = fdt_node_check_compatible(gd->fdt_blob, ephy_node,
-   "allwinner,sun8i-h3-mdio-internal");
-   if (ret < 0) {
-   debug("failed to find mdio-internal node\n");
-   return ret;
-   }
-
-   ret = clk_get_by_index_nodev(offset_to_ofnode(ephy_node), 0,
->ephy_clk);
+   ret = clk_get_by_index_nodev(phandle.node, 0, >ephy_clk);
if (ret) {
dev_err(dev, "failed to get EPHY TX clock\n");
return ret;
}
 
-   ret = reset_get_by_index_nodev(offset_to_ofnode(ephy_node), 0,
-  >ephy_rst);
+   ret = reset_get_by_index_nodev(phandle.node, 0, >ephy_rst);
if (ret) {
dev_err(dev, "failed to get EPHY TX reset\n");
return ret;
@@ -941,7 +925,7 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice 
*dev)
}
 
if (priv->variant == H3_EMAC) {
-   ret = sun8i_get_ephy_nodes(priv);
+   ret = sun8i_handle_internal_phy(dev);
if (ret)
return ret;
}
-- 
2.17.5

[PATCH 11/15] net: sun8i_emac: Fix MAC soft reset

[Andre Przywara]

The EMAC soft reset routine was subtly broken, using an open coded
timeout routine without any actual delay.
Remove the unneeded initial reset bit read, and call wait_for_bit_le32()
to handle the timeout correctly.

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 21 +++--
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 93b746161c..e2694d3619 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -447,22 +447,15 @@ static void tx_descs_init(struct emac_eth_dev *priv)
 static int sun8i_emac_eth_start(struct udevice *dev)
 {
struct emac_eth_dev *priv = dev_get_priv(dev);
-   u32 reg;
-   int timeout = 100;
int ret;
 
-   reg = readl((priv->mac_reg + EMAC_CTL1));
-
-   if (!(reg & 0x1)) {
-   /* Soft reset MAC */
-   setbits_le32((priv->mac_reg + EMAC_CTL1), 0x1);
-   do {
-   reg = readl(priv->mac_reg + EMAC_CTL1);
-   } while ((reg & 0x01) != 0 &&  (--timeout));
-   if (!timeout) {
-   printf("%s: Timeout\n", __func__);
-   return -1;
-   }
+   /* Soft reset MAC */
+   writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1);
+   ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
+   EMAC_CTL1_SOFT_RST, false, 10, true);
+   if (ret) {
+   printf("%s: Timeout\n", __func__);
+   return ret;
}
 
/* Rewrite mac address after reset */
-- 
2.17.5

[PATCH 14/15] net: sun8i-emac: Lower MDIO frequency

[Andre Przywara]

When sending a command via the MDIO bus, the Designware MAC expects some
bits in the CMD register to describe the clock divider value between
the main clock and the MDIO clock.
So far we were omitting these bits, resulting in setting "00", which
means "/ 16", so ending up with an MDIO frequency of either 18.75 or
12.5 MHz.
All the internal PHYs in the H3/H5/H6 SoCs as well as the Gbit Realtek
PHYs seem to be fine with that - although it looks like to be severly
overclocked (the MDIO spec limits the frequency to 2.5 MHz).
However the external 100Mbit PHY on the Pine64 (non-plus) board is
not happy with that, Ethernet was actually never working there, as the
PHY didn't probe.

As we set the EMAC clock (via AHB2) to 300 MHz in ATF (on the 64-bit
SoCs), and use 200 MHz on the H3, we need the highest divider of 128
to let the MDIO clock end up below the required 2.5 MHz.

This enables Ethernet on the Pine64(non-plus).

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index dc716f94f3..b2007b4c1d 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -41,6 +41,11 @@
 #define MDIO_CMD_MII_PHY_REG_ADDR_SHIFT4
 #define MDIO_CMD_MII_PHY_ADDR_MASK 0x0001f000
 #define MDIO_CMD_MII_PHY_ADDR_SHIFT12
+#define MDIO_CMD_MII_CLK_CSR_DIV_160x0
+#define MDIO_CMD_MII_CLK_CSR_DIV_320x1
+#define MDIO_CMD_MII_CLK_CSR_DIV_640x2
+#define MDIO_CMD_MII_CLK_CSR_DIV_128   0x3
+#define MDIO_CMD_MII_CLK_CSR_SHIFT 20
 
 #define CONFIG_TX_DESCR_NUM32
 #define CONFIG_RX_DESCR_NUM32
@@ -199,6 +204,12 @@ static int sun8i_mdio_read(struct mii_dev *bus, int addr, 
int devad, int reg)
mii_cmd |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
MDIO_CMD_MII_PHY_ADDR_MASK;
 
+   /*
+* The EMAC clock is either 200 or 300 MHz, so we need a divider
+* of 128 to get the MDIO frequency below the required 2.5 MHz.
+*/
+   mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 << MDIO_CMD_MII_CLK_CSR_SHIFT;
+
mii_cmd |= MDIO_CMD_MII_BUSY;
 
writel(mii_cmd, priv->mac_reg + EMAC_MII_CMD);
@@ -224,6 +235,12 @@ static int sun8i_mdio_write(struct mii_dev *bus, int addr, 
int devad, int reg,
mii_cmd |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
MDIO_CMD_MII_PHY_ADDR_MASK;
 
+   /*
+* The EMAC clock is either 200 or 300 MHz, so we need a divider
+* of 128 to get the MDIO frequency below the required 2.5 MHz.
+*/
+   mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 << MDIO_CMD_MII_CLK_CSR_SHIFT;
+
mii_cmd |= MDIO_CMD_MII_WRITE;
mii_cmd |= MDIO_CMD_MII_BUSY;
 
-- 
2.17.5

[PATCH 10/15] net: sun8i_emac: Fix overlong lines

[Andre Przywara]

When iterating over all RX/TX buffers, we were using a rather long "idx"
control variable, which lead to a nasty overlong line.

Replace "idx" with "i" to avoid this.

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 939dfbade1..93b746161c 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -389,7 +389,7 @@ static void rx_descs_init(struct emac_eth_dev *priv)
struct emac_dma_desc *desc_table_p = >rx_chain[0];
char *rxbuffs = >rxbuffer[0];
struct emac_dma_desc *desc_p;
-   u32 idx;
+   int i;
 
/*
 * Make sure we don't have dirty cache lines around, which could
@@ -400,11 +400,10 @@ static void rx_descs_init(struct emac_eth_dev *priv)
invalidate_dcache_range((uintptr_t)rxbuffs,
(uintptr_t)rxbuffs + sizeof(priv->rxbuffer));
 
-   for (idx = 0; idx < CONFIG_RX_DESCR_NUM; idx++) {
-   desc_p = _table_p[idx];
-   desc_p->buf_addr = (uintptr_t)[idx * CONFIG_ETH_BUFSIZE]
-   ;
-   desc_p->next = (uintptr_t)_table_p[idx + 1];
+   for (i = 0; i < CONFIG_RX_DESCR_NUM; i++) {
+   desc_p = _table_p[i];
+   desc_p->buf_addr = (uintptr_t)[i * CONFIG_ETH_BUFSIZE];
+   desc_p->next = (uintptr_t)_table_p[i + 1];
desc_p->ctl_size = CONFIG_ETH_RXSIZE;
desc_p->status = EMAC_DESC_OWN_DMA;
}
@@ -425,13 +424,12 @@ static void tx_descs_init(struct emac_eth_dev *priv)
struct emac_dma_desc *desc_table_p = >tx_chain[0];
char *txbuffs = >txbuffer[0];
struct emac_dma_desc *desc_p;
-   u32 idx;
+   int i;
 
-   for (idx = 0; idx < CONFIG_TX_DESCR_NUM; idx++) {
-   desc_p = _table_p[idx];
-   desc_p->buf_addr = (uintptr_t)[idx * CONFIG_ETH_BUFSIZE]
-   ;
-   desc_p->next = (uintptr_t)_table_p[idx + 1];
+   for (i = 0; i < CONFIG_TX_DESCR_NUM; i++) {
+   desc_p = _table_p[i];
+   desc_p->buf_addr = (uintptr_t)[i * CONFIG_ETH_BUFSIZE];
+   desc_p->next = (uintptr_t)_table_p[i + 1];
desc_p->ctl_size = 0;
desc_p->status = 0;
}
-- 
2.17.5

[PATCH 12/15] net: sun8i_emac: Simplify and fix error handling for RX

[Andre Przywara]

The error handling in recv() is somewhat broken, for instance
good_packet isn't really used, and it's hardly readable. Also we try
to check for short or too big packets, but those are actually filtered
out by the hardware.

Simplify the whole routine and improve the error handling:
- Bail out early if the current RX descriptor is not ready.
- Enable propagation of runt, huge and broken packets.
- Check for runt and huge packets, and return 0 to indicate this.
  This will force the framework to call free_pkt for cleanup.
- Avoid aligning the packet buffer for invalidation again.

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 56 +---
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index e2694d3619..48ba244f21 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -107,6 +107,8 @@
 #defineEMAC_RX_CTL0_RX_EN  BIT(31)
 #define EMAC_RX_CTL1   0x28
 #defineEMAC_RX_CTL1_RX_MD  BIT(1)
+#defineEMAC_RX_CTL1_RX_RUNT_FRMBIT(2)
+#defineEMAC_RX_CTL1_RX_ERR_FRM BIT(3)
 #defineEMAC_RX_CTL1_RX_DMA_EN  BIT(30)
 #defineEMAC_RX_CTL1_RX_DMA_START   BIT(31)
 #define EMAC_RX_DMA_DESC   0x34
@@ -125,6 +127,8 @@
 #define EMAC_DESC_FIRST_DESC   BIT(29)
 #define EMAC_DESC_CHAIN_SECOND BIT(24)
 
+#define EMAC_DESC_RX_ERROR_MASK0x400068db
+
 DECLARE_GLOBAL_DATA_PTR;
 
 enum emac_variant {
@@ -485,7 +489,8 @@ static int sun8i_emac_eth_start(struct udevice *dev)
sun8i_adjust_link(priv, priv->phydev);
 
/* Start RX/TX DMA */
-   setbits_le32(priv->mac_reg + EMAC_RX_CTL1, EMAC_RX_CTL1_RX_DMA_EN);
+   setbits_le32(priv->mac_reg + EMAC_RX_CTL1, EMAC_RX_CTL1_RX_DMA_EN |
+EMAC_RX_CTL1_RX_ERR_FRM | EMAC_RX_CTL1_RX_RUNT_FRM);
setbits_le32(priv->mac_reg + EMAC_TX_CTL1, EMAC_TX_CTL1_TX_DMA_EN);
 
/* Enable RX/TX */
@@ -565,10 +570,8 @@ static int sun8i_emac_eth_recv(struct udevice *dev, int 
flags, uchar **packetp)
struct emac_eth_dev *priv = dev_get_priv(dev);
u32 status, desc_num = priv->rx_currdescnum;
struct emac_dma_desc *desc_p = >rx_chain[desc_num];
-   int length = -EAGAIN;
-   int good_packet = 1;
-   ulong data_start = (uintptr_t)desc_p->buf_addr;
-   ulong data_end;
+   uintptr_t data_start = (uintptr_t)desc_p->buf_addr;
+   int length;
 
/* Invalidate entire buffer descriptor */
cache_inv_descriptor(desc_p);
@@ -576,30 +579,31 @@ static int sun8i_emac_eth_recv(struct udevice *dev, int 
flags, uchar **packetp)
status = desc_p->status;
 
/* Check for DMA own bit */
-   if (!(status & EMAC_DESC_OWN_DMA)) {
-   length = (desc_p->status >> 16) & 0x3FFF;
+   if (status & EMAC_DESC_OWN_DMA)
+   return -EAGAIN;
 
-   if (length < 0x40) {
-   good_packet = 0;
-   debug("RX: Bad Packet (runt)\n");
-   }
+   length = (status >> 16) & 0x3fff;
 
-   data_end = data_start + length;
-   /* Invalidate received data */
-   invalidate_dcache_range(rounddown(data_start,
- ARCH_DMA_MINALIGN),
-   roundup(data_end,
-   ARCH_DMA_MINALIGN));
-   if (good_packet) {
-   if (length > CONFIG_ETH_RXSIZE) {
-   printf("Received packet is too big (len=%d)\n",
-  length);
-   return -EMSGSIZE;
-   }
-   *packetp = (uchar *)(ulong)desc_p->buf_addr;
-   return length;
-   }
+   /* make sure we read from DRAM, not our cache */
+   invalidate_dcache_range(data_start,
+   data_start + roundup(length, 
ARCH_DMA_MINALIGN));
+
+   if (status & EMAC_DESC_RX_ERROR_MASK) {
+   debug("RX: packet error: 0x%x\n",
+ status & EMAC_DESC_RX_ERROR_MASK);
+   return 0;
}
+   if (length < 0x40) {
+   debug("RX: Bad Packet (runt)\n");
+   return 0;
+   }
+
+   if (length > CONFIG_ETH_RXSIZE) {
+   debug("RX: Too large packet (%d bytes)\n", length);
+   return 0;
+   }
+
+   *packetp = (uchar *)(ulong)desc_p->buf_addr;
 
return length;
 }
-- 
2.17.5

[PATCH 09/15] net: sun8i_emac: Wrap and simplify cache maintenance operations

[Andre Przywara]

To meet the current alignment requirements for our cache maintenance
functions, we were explicitly aligning the *arguments* to those calls.
This is not only ugly to read, but also wrong, as we need to make sure
we are not accidentally stepping on other data.

Provide wrapper functions for the common case of cleaning or
invalidating a descriptor, to make the cache maintenance calls more
readable. This fixes a good deal of the problematic calls.

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 33 ++---
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index b704ec47a3..939dfbade1 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -376,6 +376,14 @@ static int sun8i_phy_init(struct emac_eth_dev *priv, void 
*dev)
return 0;
 }
 
+#define cache_clean_descriptor(desc)   \
+   flush_dcache_range((uintptr_t)(desc),   \
+  (uintptr_t)(desc) + sizeof(struct emac_dma_desc))
+
+#define cache_inv_descriptor(desc) \
+   invalidate_dcache_range((uintptr_t)(desc),  \
+  (uintptr_t)(desc) + sizeof(struct emac_dma_desc))
+
 static void rx_descs_init(struct emac_eth_dev *priv)
 {
struct emac_dma_desc *desc_table_p = >rx_chain[0];
@@ -432,9 +440,7 @@ static void tx_descs_init(struct emac_eth_dev *priv)
desc_p->next =  (uintptr_t)_table_p[0];
 
/* Flush the first TX buffer descriptor we will tell the MAC about. */
-   flush_dcache_range((uintptr_t)priv->tx_chain,
-  (uintptr_t)priv->tx_chain +
-  sizeof(priv->tx_chain[0]));
+   cache_clean_descriptor(desc_table_p);
 
writel((uintptr_t)_table_p[0], priv->mac_reg + EMAC_TX_DMA_DESC);
priv->tx_currdescnum = 0;
@@ -570,15 +576,11 @@ static int sun8i_emac_eth_recv(struct udevice *dev, int 
flags, uchar **packetp)
struct emac_dma_desc *desc_p = >rx_chain[desc_num];
int length = -EAGAIN;
int good_packet = 1;
-   uintptr_t desc_start = (uintptr_t)desc_p;
-   uintptr_t desc_end = desc_start +
-   roundup(sizeof(*desc_p), ARCH_DMA_MINALIGN);
-
ulong data_start = (uintptr_t)desc_p->buf_addr;
ulong data_end;
 
/* Invalidate entire buffer descriptor */
-   invalidate_dcache_range(desc_start, desc_end);
+   cache_inv_descriptor(desc_p);
 
status = desc_p->status;
 
@@ -616,10 +618,6 @@ static int sun8i_emac_eth_send(struct udevice *dev, void 
*packet, int length)
struct emac_eth_dev *priv = dev_get_priv(dev);
u32 desc_num = priv->tx_currdescnum;
struct emac_dma_desc *desc_p = >tx_chain[desc_num];
-   uintptr_t desc_start = (uintptr_t)desc_p;
-   uintptr_t desc_end = desc_start +
-   roundup(sizeof(*desc_p), ARCH_DMA_MINALIGN);
-
uintptr_t data_start = (uintptr_t)desc_p->buf_addr;
uintptr_t data_end = data_start +
roundup(length, ARCH_DMA_MINALIGN);
@@ -635,8 +633,8 @@ static int sun8i_emac_eth_send(struct udevice *dev, void 
*packet, int length)
desc_p->ctl_size |= EMAC_DESC_LAST_DESC | EMAC_DESC_FIRST_DESC;
desc_p->status = EMAC_DESC_OWN_DMA;
 
-   /*Descriptors st and status field has changed, so FLUSH it */
-   flush_dcache_range(desc_start, desc_end);
+   /* make sure the MAC reads the actual data from DRAM */
+   cache_clean_descriptor(desc_p);
 
/* Move to next Descriptor and wrap around */
if (++desc_num >= CONFIG_TX_DESCR_NUM)
@@ -756,15 +754,12 @@ static int sun8i_eth_free_pkt(struct udevice *dev, uchar 
*packet,
struct emac_eth_dev *priv = dev_get_priv(dev);
u32 desc_num = priv->rx_currdescnum;
struct emac_dma_desc *desc_p = >rx_chain[desc_num];
-   uintptr_t desc_start = (uintptr_t)desc_p;
-   uintptr_t desc_end = desc_start +
-   roundup(sizeof(u32), ARCH_DMA_MINALIGN);
 
-   /* Make the current descriptor valid again */
+   /* give the current descriptor back to the MAC */
desc_p->status |= EMAC_DESC_OWN_DMA;
 
/* Flush Status field of descriptor */
-   flush_dcache_range(desc_start, desc_end);
+   cache_clean_descriptor(desc_p);
 
/* Move to next desc and wrap-around condition. */
if (++desc_num >= CONFIG_RX_DESCR_NUM)
-- 
2.17.5

[PATCH 08/15] net: sun8i_emac: Drop unneeded cache invalidation before sending

[Andre Przywara]

There is no reason to invalidate a TX descriptor before we are setting
it up, as we will only write to a field.

Remove the not needed invalidate_dcache_range() call.

Signed-off-by: Andre Przywara 
---
 drivers/net/sun8i_emac.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 38c56bde70..b704ec47a3 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -624,9 +624,6 @@ static int sun8i_emac_eth_send(struct udevice *dev, void 
*packet, int length)
uintptr_t data_end = data_start +
roundup(length, ARCH_DMA_MINALIGN);
 
-   /* Invalidate entire buffer descriptor */
-   invalidate_dcache_range(desc_start, desc_end);
-
desc_p->ctl_size = length | EMAC_DESC_CHAIN_SECOND;
 
memcpy((void *)data_start, packet, length);
-- 
2.17.5