Message ID | 1364044070-10486-3-git-send-email-jogo@openwrt.org |
---|---|
State | New, archived |
Headers | show |
Hello Jonas, A few minor comments below: Le samedi 23 mars 2013 14:07:48, Jonas Gorski a écrit : > Signed-off-by: Jonas Gorski <jogo@openwrt.org> > --- > arch/mips/bcm63xx/nvram.c | 11 +++++++++++ > arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h | 2 ++ > 2 files changed, 13 insertions(+) > > diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c > index 6206116..44af608 100644 > --- a/arch/mips/bcm63xx/nvram.c > +++ b/arch/mips/bcm63xx/nvram.c > @@ -35,6 +35,8 @@ struct bcm963xx_nvram { > u32 checksum_high; > }; > > +#define BCM63XX_DEFAULT_PSI_SIZE 64 > + > static struct bcm963xx_nvram nvram; > static int mac_addr_used; > > @@ -105,3 +107,12 @@ int bcm63xx_nvram_get_mac_address(u8 *mac) > return 0; > } > EXPORT_SYMBOL(bcm63xx_nvram_get_mac_address); > + > +int bcm63xx_nvram_get_psi_size(void) > +{ > + if (nvram.psi_size > 0) > + return nvram.psi_size; > + > + return BCM63XX_DEFAULT_PSI_SIZE; I wonder if it would not be better to return a size in KBytes here instead of a multiple of KBytes? > +} > +EXPORT_SYMBOL(bcm63xx_nvram_get_psi_size); > diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h > b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h index > 62d6a3b..d76a486 100644 > --- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h > +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h > @@ -32,4 +32,6 @@ u8 *bcm63xx_nvram_get_name(void); > */ > int bcm63xx_nvram_get_mac_address(u8 *mac); > > +int bcm63xx_nvram_get_psi_size(void); You could use an unsigned int here.
On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote: > Signed-off-by: Jonas Gorski <jogo@openwrt.org> > --- > arch/mips/bcm63xx/nvram.c | 11 +++++++++++ > arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h | 2 ++ > 2 files changed, 13 insertions(+) Acks from MIPS folks would be nice to have, but I pushed this patch to the l2-mtd.git tree, thanks!
On Fri, May 10, 2013 at 2:36 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote: >> Signed-off-by: Jonas Gorski <jogo@openwrt.org> >> --- >> arch/mips/bcm63xx/nvram.c | 11 +++++++++++ >> arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h | 2 ++ >> 2 files changed, 13 insertions(+) > > Acks from MIPS folks would be nice to have, but I pushed this patch to > the l2-mtd.git tree, thanks! I had expected Florian's valid comment from preventing this series from going in, but if you pushed it already then I will fix the return type problem that Florian pointed out in a separate patch (or rather add add some range check for nvram.psi_size). Luckily it is a theoretical issue only, as I haven't seen a device yet with an invalid value. Regards Jonas
Le 12/05/2013 12:48, Jonas Gorski a écrit : > On Fri, May 10, 2013 at 2:36 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote: >> On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote: >>> Signed-off-by: Jonas Gorski <jogo@openwrt.org> >>> --- >>> arch/mips/bcm63xx/nvram.c | 11 +++++++++++ >>> arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h | 2 ++ >>> 2 files changed, 13 insertions(+) >> >> Acks from MIPS folks would be nice to have, but I pushed this patch to >> the l2-mtd.git tree, thanks! > > I had expected Florian's valid comment from preventing this series > from going in, but if you pushed it already then I will fix the return > type problem that Florian pointed out in a separate patch (or rather > add add some range check for nvram.psi_size). Luckily it is a > theoretical issue only, as I haven't seen a device yet with an invalid > value. Right, but this is no blocker from my perspective. As about the MIPS folks, Maxime, Kevin, Jonas and myself have been the "historical" contributors to the MIPS BCM63XX port, so I would consider Jonas to be authoritave here for these paches. John and Ralf usually do not comment unless the see something bad. -- Florian
diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c index 6206116..44af608 100644 --- a/arch/mips/bcm63xx/nvram.c +++ b/arch/mips/bcm63xx/nvram.c @@ -35,6 +35,8 @@ struct bcm963xx_nvram { u32 checksum_high; }; +#define BCM63XX_DEFAULT_PSI_SIZE 64 + static struct bcm963xx_nvram nvram; static int mac_addr_used; @@ -105,3 +107,12 @@ int bcm63xx_nvram_get_mac_address(u8 *mac) return 0; } EXPORT_SYMBOL(bcm63xx_nvram_get_mac_address); + +int bcm63xx_nvram_get_psi_size(void) +{ + if (nvram.psi_size > 0) + return nvram.psi_size; + + return BCM63XX_DEFAULT_PSI_SIZE; +} +EXPORT_SYMBOL(bcm63xx_nvram_get_psi_size); diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h index 62d6a3b..d76a486 100644 --- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h @@ -32,4 +32,6 @@ u8 *bcm63xx_nvram_get_name(void); */ int bcm63xx_nvram_get_mac_address(u8 *mac); +int bcm63xx_nvram_get_psi_size(void); + #endif /* BCM63XX_NVRAM_H */
Signed-off-by: Jonas Gorski <jogo@openwrt.org> --- arch/mips/bcm63xx/nvram.c | 11 +++++++++++ arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h | 2 ++ 2 files changed, 13 insertions(+)