Patchwork [2/3] MIPS: BCM63XX: export PSI size from nvram

login
register
mail settings
Submitter Jonas Gorski
Date March 23, 2013, 1:07 p.m.
Message ID <1364044070-10486-3-git-send-email-jogo@openwrt.org>
Download mbox | patch
Permalink /patch/230335/
State New
Headers show

Comments

Jonas Gorski - March 23, 2013, 1:07 p.m.
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(+)
Florian Fainelli - March 23, 2013, 1:39 p.m.
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.
Artem Bityutskiy - May 10, 2013, 12:36 p.m.
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!
Jonas Gorski - May 12, 2013, 10:48 a.m.
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
Florian Fainelli - May 12, 2013, 2:52 p.m.
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

Patch

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 */