linux-kernel - Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 23 Sep 2006 02:50:02 -0400
From:	"Mike Frysinger" <vapier.adi@...il.com>
To:	"Arnd Bergmann" <arnd@...db.de>
Cc:	"Luke Yang" <luke.adi@...il.com>, linux-kernel@...r.kernel.org,
	"Andrew Morton" <akpm@...l.org>
Subject: Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18

On 9/22/06, Arnd Bergmann <arnd@...db.de> wrote:
> Use pr_debug here instead of making up your own macros

done

> static inline void

done

> It would be nice if you could use a generic way to pass this partition data
> to the kernel from the mtd medium, instead of hardcoding it here.

i often wish for such things myself :)

unfortunately, the boot loader we utilize (u-boot) isnt exactly
friendly to the idea of managing flash partitions like redboot, and
what we have here is the current standard method for defining flash
partitions with mtd

> Use C99 initializers here, like

done

> There is not much point in trying to use the same numbers as an existing
> architecture if that means that you have to leave holes like setup().
> I don't know if you still have the choice of completely changing the
> syscall numbers, but it would make it nicer in the future.

we do, fortunately, have this luxury ... so we can look forward to a
nice cleaning of our syscall interface

> What's th point of these files, can't you just remove them all?

punted

> > +#define BUG() do { \
> > +     dump_stack(); \
> > +     printk("\nkernel BUG at %s:%d!\n", __FILE__, __LINE__); \
> > +     panic("BUG!"); \
> > +} while (0)
>
> This is probably better done as an external function:

*shrug* i just did it the same way everyone else does ... i'm a sheep like that

> > diff -urN linux-2.6.18/include/asm-blackfin/mach-bf533/anomaly.h
> linux-2.6.18.patch1/include/asm-blackfin/mach-bf533/anomaly.h
> > --- linux-2.6.18/include/asm-blackfin/mach-bf533/anomaly.h    1970-01-01
> 08:00:00.000000000 +0800
> > +++ linux-2.6.18.patch1/include/asm-blackfin/mach-bf533/anomaly.h     2006-09-21
> 09:29:49.000000000 +0800
> > @@ -0,0 +1,172 @@
> > +/*
> > + * File:         include/asm-blackfin/mach-bf533/anomaly.h
>
> You seem to have lots of these machine specfic header files in include/asm.
> Please move them to the respective machine implementation directory
> if they are only used from there

these are arch specific, not machine (aka board)

this is what the blackfin family looks like (first entry is the architecture):
 - BF535
 - BF533 / BF532 / BF531
 - BF537 / BF536 / BF534
 - BF561

which is why you find the anomaly definitions in the architecture
specific header dir

> > +/* Clock and System Control (0xFFC0 0400-0xFFC0 07FF) */
> > +#define bfin_read_PLL_CTL()                  bfin_read16(PLL_CTL)
> > +#define bfin_write_PLL_CTL(val)              bfin_write16(PLL_CTL,val)
> > +#define bfin_read_PLL_STAT()                 bfin_read16(PLL_STAT)
> (and 700 more of these)
>
> What's the point, are you getting paid by lines of code? Just use
> the registers directly!

in our last submission we were doing exactly that ... and we were told
to switch to a function style method of reading/writing memory mapped
registers

> One more thing about the headers: please add a Kbuild file in there so
> 'make headers_install' works

will do

thanks for reading through that monster :)
-mike
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/