Hi Dan, On Wed, 8 Jan 2025 at 10:18, Dan Carpenter <[email protected]> wrote: > > 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 have evolved on that one. > > 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. There's no substitute for tests and naming is hard. We could have a streq() function which just returns 0 or 1, but it would presumably have to return 1/true when equal, thus creating confusion with strcmp(). Perhaps strcheckeq() would be a better name for a function that returns '0' on success? Regards, Simon
