On Wed, Jan 08, 2025 at 10:03:08AM -0700, Simon Glass wrote: > Hi Dan, > > On Wed, 8 Jan 2025 at 05:37, Dan Carpenter <[email protected]> wrote: > > > > On Wed, Jan 08, 2025 at 12:21:18PM +0100, Heinrich Schuchardt wrote: > > > Am 8. Januar 2025 12:11:05 MEZ schrieb Yixun Lan <[email protected]>: > > > >Hi Huan: > > > > > > > >On 16:49 Wed 08 Jan , Huan Zhou wrote: > > > >> > > > >> > > > >.. > > > >> --- > > > >remove above "---"? otherwise following commit message will be > > > >dropped during patch application.. > > > > > > > >> This patch introduce improvement for get dram size on bananapi BPI-F3, > > > >> retrieving the dram size dynamically. > > > >> Have tested on bananapi BPIF3 4G and jupiter 8G. > > > >> > > > >> Signed-off-by: Huan Zhou <[email protected]> > > > >> --- > > > >> arch/riscv/cpu/k1/dram.c | 40 ++++++++++++++++++++++++++++++++++++++-- > > > >> 1 file changed, 38 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/arch/riscv/cpu/k1/dram.c b/arch/riscv/cpu/k1/dram.c > > > >> index > > > >> c477c15cbfb19f0e3a0ee72985b602f5bda352d7..095217f2a4c053f7477d62c0776bcb51e623db47 > > > >> 100644 > > > >> --- a/arch/riscv/cpu/k1/dram.c > > > >> +++ b/arch/riscv/cpu/k1/dram.c > > > >> @@ -4,17 +4,53 @@ > > > >> */ > > > >> > > > >> #include <asm/global_data.h> > > > >> +#include <asm/io.h> > > > >> #include <config.h> > > > >> +#include <bitfield.h> > > > >> #include <fdt_support.h> > > > >> #include <linux/sizes.h> > > > >> > > > >> +#define DDR_BASE 0xC0000000 > > > >> DECLARE_GLOBAL_DATA_PTR; > > > >> > > > >> +static inline u32 map_format_size(u32 val) > > > >> +{ > > > >> + u32 tmp; > > > >> + > > > >> + if (val & 0x1 == 0) > > > >please add brackets explicitly, something like > > > > if ((val & 0x1) == 0) > > > > > > We tend to avoid == 0 in U-Boot > > > > > > if (val & BIT(0)) > > > > > > > That's reversed. > > > > if (!(val & BIT(0)) { > > > > I have really complicated rules about when to use ! vs == 0. > > https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/ > > I mostly agree with that and it provides some motivation for the > conventions which have built up over the years in Linux/U-Boot/etc. > > If you want to send a patch: > > $ git grep 'if (ret != EFI_SUCCESS)' |wc > 864 4841 51380 > > I think the only place I would differ is with strcmp(), where I've got > used to 0 meaning success, like in much other code. >
Huh... I've never thought about it as a success/fail thing. I feel like I've seens quite a few reversed strcmp() functions but it's hard to catch these sorts of bugs with static analysis because it totally depends on the context if you want the strings to be equal or not equal. I can't imagine what a static checker rule would look like. regards, dan carpenter
