[U-Boot] Uboot send pull request

Auer, Lukas lukas.auer at aisec.fraunhofer.de
Thu Nov 22 10:16:41 UTC 2018


Hi Rick,

On Thu, 2018-11-22 at 17:42 +0800, Rick Chen wrote:
> Auer, Lukas <lukas.auer at aisec.fraunhofer.de> 於 2018年11月22日 週四
> 下午5:18寫道:
> > 
> > Hi Rick,
> > 
> > On Thu, 2018-11-22 at 16:38 +0800, Rick Chen wrote:
> > > Auer, Lukas <lukas.auer at aisec.fraunhofer.de> 於 2018年11月21日 週三
> > > 下午9:09寫道:
> > > > 
> > > > Hi Rick,
> > > > 
> > > > On Wed, 2018-11-21 at 17:37 +0800, Rick Chen wrote:
> > > > > Hi Lukas
> > > > > 
> > > > > > > 
> > > > > > > Hi Rick,
> > > > > > > 
> > > > > > > Thanks for pulling my changes! I have some notes and
> > > > > > > questions on
> > > > > > > it.
> > > > > > > 
> > > > > > > I can't find all of your patches on the mailing list, for
> > > > > > > example
> > > > > > > the patch "configs:
> > > > > > > ax25-ae350: Enable DISPLAY_CPUINFO & DISPLAY_BOARDINFO".
> > > > > > > Others
> > > > > > > include
> > > > > > > changes, which have not been submitted to the mailing
> > > > > > > list.
> > > > > > > Please send all
> > > > > > > changes to the mailing list before including them in a
> > > > > > > pull
> > > > > > > request. This is really
> > > > > > > helpful for me and others, to be able to see if there are
> > > > > > > any
> > > > > > > conflicts with other
> > > > > > > patches currently under development or with other boards.
> > > > > > > 
> > > > > 
> > > > > That is my mistake. Forget to send it to mailing list.
> > > > > I will drop this patch.
> > > > > 
> > > > > > > Something seems to have gone wrong while applying the
> > > > > > > patch
> > > > > > > "riscv:
> > > > > > > enable -fdata-sections". This is one of my patches and
> > > > > > > part
> > > > > > > of
> > > > > > > the patch series.
> > > > > > > 
> > > > > 
> > > > > That is because your patch
> > > > > [PATCH v3 00/28] General fixes / cleanup for RISC-V and
> > > > > improvements
> > > > > to qemu-riscv
> > > > > still have some conflict with master.
> > > > > 
> > > > > It conflict with this commit
> > > > > Kbuild: add LDFLAGS_STANDALONE
> > > > > 
> > > > > I am hesitate to ask you to send v4 which shall rebase on
> > > > > master
> > > > > yesterday.
> > > > > Finally I decide to merge it by myself.
> > > > > 
> > > > > I am not sure it is inappropriate.
> > > > > Maybe I shall ask for you and wait for your v4 patchsets
> > > > > which
> > > > > are
> > > > > rebase on master, right ?
> > > > > 
> > > > 
> > > > Ok, I wasn't aware of that. In general, I think it is always
> > > > easier
> > > > if
> > > > you ask me to resend the patch series (or just the relevant
> > > > patch),
> > > > but
> > > > I don't know, how this is usually handled. I just saw that
> > > > there
> > > > are a
> > > > few reviewed-bys missing in some of the patches. So if you
> > > > want, I
> > > > can
> > > > add those, rebase on u-boot/master, and then send v4 to you.
> > > > 
> > > > > 
> > > > > > > Can you please consider removing your patch "riscv:
> > > > > > > cache:
> > > > > > > Implement i/dcache
> > > > > > > [status, enable, disable]" from this pull request and re-
> > > > > > > sending
> > > > > > > it with the next?
> > > > > > > There are still some points you did not reply to in my
> > > > > > > comments
> > > > > > > on v2 of your
> > > > > > > patch. For example, I think it makes sense to split this
> > > > > > > patch
> > > > > > > into multiple
> > > > > > > patches to make it clearer what it changes. If you want,
> > > > > > > I
> > > > > > > can
> > > > > > > re-send my
> > > > > > > comments in reply to v3 of your patch :)
> > > > > > > 
> > > > > 
> > > > > In my memory :
> > > > > 
> > > > > In v2
> > > > > you have some suggestions and I reply as below
> > > > > 
> > > > 
> > > > 
> > 
> > 
http://u-boot.10912.n7.nabble.com/PATCH-v2-riscv-cache-Implement-i-dcache-status-enable-disable-td346350.html
> > > > > 
> > > > > But in v3
> > > > > I do not remember you have any comments about v3
> > > > > 
> > > > 
> > > > 
> > 
> > 
http://u-boot.10912.n7.nabble.com/PATCH-v3-riscv-cache-Implement-i-dcache-status-enable-disable-td346902.html
> > > > > 
> > > > > Is it right ?
> > > > > 
> > > > > Rick
> > > > > 
> > > > 
> > > > In my last email to v2 I still expressed some concerns (for
> > > > example
> > > > the
> > > > order, in which you disable the caches), to which you did not
> > > > reply. Of
> > > 
> > > I think I do have responsed about the order issue clearly in
> > > previous
> > > mail at that time.
> > > 
> > > > course it is perfectly fine if you don't agree, but I would
> > > > really
> > > > appreciate a quick reply. Could be that I'm just wrong as well
> > > > :)
> > > > 
> > > > That is right, I did not restate my comments on your v3, which
> > > > I
> > > > should
> > > > have done. If you don't mind, I will send them now.
> > > 
> > > I have send v3 in 11/7. I have waited for about 2 weeks and then
> > > send
> > > PR.
> > > I still hope this patch can be accepted at this time merge work.
> > > After that you can send another patch to refine the flow you care
> > > about.
> > > How do you think about it ?
> > > 
> > 
> > Of course, my intention was not to delay this. Sure, that is ok for
> > me.
> > 
> 
> Very appreciate for your agreement :)
> 

If you find the time, I would appreciate your feedback on my notes to
your patch. Not to change anything in the patch, but for future
changes. :)

In particular, I think it would be good to be able to enable cache
support by default and I am not sure if we can flush the dcache with
the fence instruction.

> > Do you want me to send v4 of my patch series (rebased on u-
> > boot/master
> > and with the reviewed-bys)?
> 
> Actually I merge very carefully, I still do not know what is missing
> about reviewed-bys.
> I only know the Author name become Rick Chen from  Lukas Auer indeed.
> I am both ok if you want to resend v4 or not.
> Up to you !
> 

There are some reviewed-bys on the mailing list to v3, so there is
nothing missing from the actual patches.
I like patchwork to get a quick overview of the current state and tags
of the series (for example [1] for my patch series). It also gives the
option to download the complete series at once and including all the
tags sent over the mailing list, which is really useful especially for
large patch series.

I will send v4 since I already have it ready.

[1]: https://patchwork.ozlabs.org/project/uboot/list/?series=74999

> Next time if this kind of conflict happen again, I will inform you at
> the first time to avoid misunderstanding.
> 

Thanks! :)

Lukas

> Rick
> 
> > 
> > Thanks,
> > Lukas
> > 
> > > B.R
> > > Rick
> > > 
> > > 
> > > > 
> > > > Thanks,
> > > > Lukas
> > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > > > Thank you!
> > > > > > > Lukas
> > > > > > > 
> > > > > > > 
> > > > > > > >  arch/nds32/cpu/n1213/start.S                          
> > > > > > > >    |
> > > > > > > >   51
> > > > > > > > ---
> > > > > > > > -----------
> > > > > > > >  arch/riscv/Kconfig                                    
> > > > > > > >    |
> > > > > > > >   34
> > > > > > > > ++++++---
> > > > > > > >  arch/riscv/Makefile                                   
> > > > > > > >    |
> > > > > > > >   20
> > > > > > > > ++++++
> > > > > > > >  arch/riscv/config.mk                                  
> > > > > > > >    |
> > > > > > > >    7
> > > > > > > > +-
> > > > > > > >  arch/riscv/cpu/ax25/Kconfig                           
> > > > > > > >    |
> > > > > > > >    7
> > > > > > > > ++
> > > > > > > >  arch/riscv/cpu/ax25/Makefile                          
> > > > > > > >    |
> > > > > > > >    1
> > > > > > > > +
> > > > > > > >  arch/riscv/cpu/ax25/cache.c                           
> > > > > > > >    |
> > > > > > > >   95
> > > > > > > > ++++++++++++++++++++++++++
> > > > > > > >  arch/riscv/cpu/ax25/cpu.c                             
> > > > > > > >    |
> > > > > > > >    4
> > > > > > > > ++
> > > > > > > >  arch/riscv/cpu/cpu.c                                  
> > > > > > > >    |
> > > > > > > >    6
> > > > > > > > ++
> > > > > > > >  arch/riscv/cpu/qemu/cpu.c                             
> > > > > > > >    |
> > > > > > > >    2
> > > > > > > > +-
> > > > > > > >  arch/riscv/cpu/start.S                                
> > > > > > > >    |
> > > > > > > > 344
> > > > > > > > +++++++++++++++++++++++++++++++++++++++++++++--------
> > > > > > > > ----
> > > > > > > > ----
> > > > > > > > ------
> > > > > > > > -------------------------
> > > > > > > >  arch/riscv/dts/Makefile                               
> > > > > > > >    |
> > > > > > > >    1
> > > > > > > > -
> > > > > > > >  arch/riscv/dts/ae350.dts                              
> > > > > > > >    |
> > > > > > > > 107
> > > > > > > > +++++++++++++++++++++++++----
> > > > > > > >  arch/riscv/dts/ae350_32.dts                           
> > > > > > > >    |
> > > > > > > > 229
> > > > > > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > ++++
> > > > > > > > ++
> > > > > > > >  arch/riscv/dts/ae350_64.dts                           
> > > > > > > >    |
> > > > > > > > 229
> > > > > > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > ++++
> > > > > > > > ++
> > > > > > > >  arch/riscv/include/asm/barrier.h                      
> > > > > > > >    |
> > > > > > > >   67
> > > > > > > > ++++++++++++++++++
> > > > > > > >  arch/riscv/include/asm/cache.h                        
> > > > > > > >    |
> > > > > > > >    3
> > > > > > > > +
> > > > > > > >  arch/riscv/include/asm/io.h                           
> > > > > > > >    |
> > > > > > > >   48
> > > > > > > > +++
> > > > > > > > ----------
> > > > > > > >  arch/riscv/include/asm/posix_types.h                  
> > > > > > > >    |
> > > > > > > >    6
> > > > > > > > +-
> > > > > > > >  arch/riscv/include/asm/types.h                        
> > > > > > > >    |
> > > > > > > >    4
> > > > > > > > ++
> > > > > > > >  arch/riscv/lib/bootm.c                                
> > > > > > > >    |
> > > > > > > >   97
> > > > > > > > ++++++++++++++++++--------
> > > > > > > >  arch/riscv/lib/cache.c                                
> > > > > > > >    |
> > > > > > > >   36
> > > > > > > > ++++++++--
> > > > > > > >  arch/riscv/lib/interrupts.c                           
> > > > > > > >    |
> > > > > > > >   31
> > > > > > > > +++++++--
> > > > > > > >  arch/riscv/lib/setjmp.S                               
> > > > > > > >    |
> > > > > > > >    2
> > > > > > > > +-
> > > > > > > >  board/AndesTech/ax25-ae350/ax25-
> > > > > > > > ae350.c                  |   3
> > > > > > > > +-
> > > > > > > >  board/armltd/integrator/README                        
> > > > > > > >    |
> > > > > > > >    4
> > > > > > > > +-
> > > > > > > >  board/emulation/qemu-
> > > > > > > > riscv/Kconfig                       |   2
> > > > > > > > +
> > > > > > > >  board/emulation/qemu-riscv/qemu-
> > > > > > > > riscv.c                  |  73
> > > > > > > > +++++++++++++++++---
> > > > > > > >  configs/{ax25-ae350_defconfig => a25-
> > > > > > > > ae350_32_defconfig}
> > > > > > > > >   5
> > > > > > > > 
> > > > > > > > +-
> > > > > > > >  configs/ax25-
> > > > > > > > ae350_64_defconfig                          |  39
> > > > > > > > +++++++++++
> > > > > > > >  configs/qemu-
> > > > > > > > riscv32_defconfig                           |   5
> > > > > > > > +-
> > > > > > > >  configs/qemu-
> > > > > > > > riscv64_defconfig                           |   7
> > > > > > > > +-
> > > > > > > >  doc/README.distro                                     
> > > > > > > >    |
> > > > > > > >    3
> > > > > > > 
> > > > > > > +-
> > > > > > > >  dts/Makefile                                          
> > > > > > > >    |
> > > > > > > >    2
> > > > > > > > +-
> > > > > > > >  include/common.h                                      
> > > > > > > >    |
> > > > > > > >    5
> > > > > > > > --
> > > > > > > >  include/config_distro_bootcmd.h                       
> > > > > > > >    |
> > > > > > > >   21
> > > > > > > > ++++-
> > > > > > > > -
> > > > > > > >  include/configs/qemu-
> > > > > > > > riscv.h                             |  28
> > > > > > > > ++++++++
> > > > > > > >  include/dm/ofnode.h                                   
> > > > > > > >    |
> > > > > > > >   10
> > > > > > > 
> > > > > > > +++
> > > > > > > >  scripts/config_whitelist.txt                          
> > > > > > > >    |
> > > > > > > >    1
> > > > > > > > -
> > > > > > > >  tools/.gitignore                                      
> > > > > > > >    |
> > > > > > > >    1
> > > > > > > > +
> > > > > > > >  40 files changed, 1271 insertions(+), 369 deletions(-
> > > > > > > > )  create
> > > > > > > > mode
> > > > > > > > 100644 arch/riscv/cpu/ax25/Kconfig  create mode 100644
> > > > > > > > arch/riscv/cpu/ax25/cache.c  create mode 100644
> > > > > > > > arch/riscv/dts/ae350_32.dts  create mode 100644
> > > > > > > > arch/riscv/dts/ae350_64.dts  create mode 100644
> > > > > > > > arch/riscv/include/asm/barrier.h  rename configs/{ax25-
> > > > > > > > ae350_defconfig
> > > > > > > > => a25-ae350_32_defconfig}
> > > > > > > > (89%)
> > > > > > > >  create mode 100644 configs/ax25-ae350_64_defconfig
> > > > > > > > _______________________________________________
> > > > > > > > U-Boot mailing list
> > > > > > > > U-Boot at lists.denx.de
> > > > > > > > https://lists.denx.de/listinfo/u-boot
> > > > > > 
> > > > > > CONFIDENTIALITY NOTICE:
> > > > > > 
> > > > > > This e-mail (and its attachments) may contain confidential
> > > > > > and
> > > > > > legally privileged information or information protected
> > > > > > from
> > > > > > disclosure. If you are not the intended recipient, you are
> > > > > > hereby
> > > > > > notified that any disclosure, copying, distribution, or use
> > > > > > of
> > > > > > the
> > > > > > information contained herein is strictly prohibited. In
> > > > > > this
> > > > > > case,
> > > > > > please immediately notify the sender by return e-mail,
> > > > > > delete
> > > > > > the
> > > > > > message (and any accompanying documents) and destroy all
> > > > > > printed
> > > > > > hard copies. Thank you for your cooperation.
> > > > > > 
> > > > > > Copyright ANDES TECHNOLOGY CORPORATION - All Rights
> > > > > > Reserved.


More information about the U-Boot mailing list