arm: ep93xx: use soc bus
PATCH 1/1 arm: ep93xx: use soc bus
Use the soc bus to report the silicon revision and Maverick Key. Both are not currently exposed to the user. In addition, fill in the SoC family and machine for completeness.
--- arch/arm/mach-ep93xx/Kconfig | 1 + arch/arm/mach-ep93xx/core.c | 106 ++++++++++++++++++++++++++- arch/arm/mach-ep93xx/include/mach/platform.h | 3 +- 3 files changed, 108 insertions(+), 2 deletions(-)
arch/arm/mach-ep93xx/Kconfig +1 -0
@@ -5,6 +5,7 @@ menu "Cirrus EP93xx Implementation Options" config EP93XX_SOC_COMMON bool default y + select SOC_BUS select LEDS_GPIO_REGISTER config CRUNCH
arch/arm/mach-ep93xx/core.c +105 -1
137 lines not on this page · raw message
arch/arm/mach-ep93xx/include/mach/platform.h +2 -1
@@ -4,6 +4,7 @@#ifndef __ASSEMBLY__ +struct device ;struct i2c_gpio_platform_data ;struct i2c_board_info ;struct spi_board_info ;
@@ -52,7 +53,7 @@ void ep93xx_register_ide(void);int ep93xx_ide_acquire_gpio (struct platform_device *pdev);void ep93xx_ide_release_gpio (struct platform_device *pdev); -void ep93xx_init_devices (void ); +struct device *ep93xx_init_devices (void );extern void ep93xx_timer_init (void );void ep93xx_restart (char ,const char *);
Signature · 1 line
1.8.1.4
PATCH 1/1 arm: ep93xx: use soc bus
Quoted 6 lines · H Hartley Sweeten
Use the soc bus to report the silicon revision and Maverick Key. Both are not currently exposed to the user. In addition, fill in the SoC family and machine for completeness.
Signed-off-by H Hartley Sweeten <[email protected]>Cc Ryan Mallon <[email protected]>
Hey Hartley,
Looks mostly good. Couple of comments below.
~Ryan
Quoted 57 lines
--- arch/arm/mach-ep93xx/Kconfig | 1 + arch/arm/mach-ep93xx/core.c | 106 ++++++++++++++++++++++++++- arch/arm/mach-ep93xx/include/mach/platform.h | 3 +- 3 files changed, 108 insertions(+), 2 deletions(-)arch/arm/mach-ep93xx/Kconfig +1
-0@@ -5,6 +5,7 @@ menu "Cirrus EP93xx Implementation Options" config EP93XX_SOC_COMMON bool default y + select SOC_BUS select LEDS_GPIO_REGISTER config CRUNCHarch/arm/mach-ep93xx/core.c +16
-1@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/dma-mapping.h> +#include <linux/sys_soc.h> #include <linux/timex.h> #include <linux/irq.h> #include <linux/io.h> @@ -42,6 +43,7 @@#include <linux/platform_data/spi-ep93xx.h> #include <mach/gpio-ep93xx.h> +#include <asm/mach/arch.h> #include <asm/mach/map.h> #include <asm/mach/time.h> @@ -895,8 +897,106 @@ void ep93xx_ide_release_gpio(struct platform_device *pdev) }EXPORT_SYMBOL (ep93xx_ide_release_gpio); -void __init ep93xx_init_devices (void ) +/************************************************************************* +* EP93xx Security peripheral +*************************************************************************/ + +/* +* The Maverick Key is 256 bits of micro fuses blown at the factory during +* manufacturing to uniquely identify a part. +* +* See: http://arm.cirrus.com/forum/viewtopic.php?t=486&highlight=maverick+key +*/ +#define EP93XX_SECURITY_REG (x) (EP93XX_SECURITY_BASE + (x)) +#define EP93XX_SECURITY_UNIQID EP93XX_SECURITY_REG(0x2440) +#define EP93XX_SECURITY_UNIQCHK EP93XX_SECURITY_REG(0x2450) +#define EP93XX_SECURITY_SECID1 EP93XX_SECURITY_REG(0x2500)
arch/arm/mach-ep93xx/core.c void ep93xx_ide_release_gpio(struct platform_device *pdev) · line 923
Quoted 10 lines
arch/arm/mach-ep93xx/core.c +10
-0+#define EP93XX_SECURITY_SECCHK1 EP93XX_SECURITY_REG(0x2520) +#define EP93XX_SECURITY_SECID2 EP93XX_SECURITY_REG(0x2504) +#define EP93XX_SECURITY_SECCHK2 EP93XX_SECURITY_REG(0x2524) +#define EP93XX_SECURITY_FUSEFLG EP93XX_SECURITY_REG(0x2410) +#define EP93XX_SECURITY_UNIQID2 EP93XX_SECURITY_REG(0x2700) +#define EP93XX_SECURITY_UNIQID3 EP93XX_SECURITY_REG(0x2704) +#define EP93XX_SECURITY_UNIQID4 EP93XX_SECURITY_REG(0x2708) +#define EP93XX_SECURITY_UNIQID5 EP93XX_SECURITY_REG(0x270c) +#define EP93XX_SECURITY_SECFLG EP93XX_SECURITY_REG(0x2400) +#define EP93XX_SECURITY_UNIQVAL EP93XX_SECURITY_REG(0x2460)
You are only using about half of these defines. Are you expecting to use some more of them in the future, or could we just remove the unused ones?
Quoted 10 lines
arch/arm/mach-ep93xx/core.c +10
-0+ +static const char __init *ep93xx_get_soc_id (void ) +{ +unsigned int id, id2, id3, id4, id5; + +if (__raw_readl (EP93XX_SECURITY_UNIQVAL ) !=1 ) +return kasprintf (GFP_KERNEL ,"bad Hamming code" ); + + id =__raw_readl (EP93XX_SECURITY_UNIQID ); + id2 =__raw_readl (EP93XX_SECURITY_UNIQID2 );
arch/arm/mach-ep93xx/core.c void ep93xx_ide_release_gpio(struct platform_device *pdev) · line 943
Quoted 10 lines
arch/arm/mach-ep93xx/core.c +10
-0+ id3 =__raw_readl (EP93XX_SECURITY_UNIQID3 ); + id4 =__raw_readl (EP93XX_SECURITY_UNIQID4 ); + id5 =__raw_readl (EP93XX_SECURITY_UNIQID5 ); + +if (id != id2) +return kasprintf (GFP_KERNEL ,"invalid" ); + +return kasprintf (GFP_KERNEL ,"%08x%08x%08x%08x" , + id2, id3, id4, id5); +}
You don't need to use the allocator functions here:
static char soc_id[33];
static const char __init *ep93xx_get_soc_id(void)
{
if (...)
return "bad Hamming code";
...
snprintf(soc_id, sizeof(soc_id), ...);
return soc_id;
}Quoted 34 lines
arch/arm/mach-ep93xx/core.c +34
-0+ +static const char __init *ep93xx_get_soc_rev (void ) +{ +int rev =ep93xx_chip_revision (); + +switch (rev) { +case EP93XX_CHIP_REV_D0 : +return "D0" ; +case EP93XX_CHIP_REV_D1 : +return "D1" ; +case EP93XX_CHIP_REV_E0 : +return "E0" ; +case EP93XX_CHIP_REV_E1 : +return "E1" ; +case EP93XX_CHIP_REV_E2 : +return "E2" ; +default : +return "unknown" ; + } +} + +static const char __init *ep93xx_get_machine_name (void ) +{ +return kasprintf (GFP_KERNEL ,"%s" , machine_desc->name); +} + +static struct device __init *ep93xx_init_soc (void ) +{ +struct soc_device_attribute *soc_dev_attr; +struct soc_device *soc_dev; + + soc_dev_attr =kzalloc (sizeof (*soc_dev_attr),GFP_KERNEL ); +if (!soc_dev_attr) +return NULL ;
arch/arm/mach-ep93xx/core.c void ep93xx_ide_release_gpio(struct platform_device *pdev) · line 987
Quoted 10 lines
arch/arm/mach-ep93xx/core.c +10
-0+ + soc_dev_attr->machine =ep93xx_get_machine_name (); + soc_dev_attr->family ="Cirrus EP93xx" ; + soc_dev_attr->revision =ep93xx_get_soc_rev (); + soc_dev_attr->soc_id =ep93xx_get_soc_id (); + + soc_dev =soc_device_register (soc_dev_attr); +if (IS_ERR (soc_dev)) { +kfree (soc_dev_attr->soc_id); +kfree (soc_dev_attr->revision);
Revision wasn't allocated with kmalloc.
Quoted 25 lines
arch/arm/mach-ep93xx/core.c +13
-0+kfree (soc_dev_attr->machine); +kfree (soc_dev_attr); +return NULL ; + } + +return soc_device_to_device (soc_dev); +} + +struct device __init *ep93xx_init_devices (void ) { +struct device *parent; +/* Disallow access to MaverickCrunch initially */ ep93xx_devcfg_clear_bits (EP93XX_SYSCON_DEVCFG_CPENA );@@ -907,6 +1007,8 @@ void __init ep93xx_init_devices(void)EP93XX_SYSCON_DEVCFG_GONIDE | EP93XX_SYSCON_DEVCFG_HONIDE); + parent =ep93xx_init_soc (); +/* Get the GPIO working early, other devices need it */ platform_device_register (&ep93xx_gpio_device);
arch/arm/mach-ep93xx/core.c void __init ep93xx_init_devices(void) · line 1025
Quoted 5 lines
arch/arm/mach-ep93xx/core.c +2
-0@@ -919,6 +1021,8 @@ void __init ep93xx_init_devices(void)platform_device_register (&ep93xx_wdt_device);gpio_led_register_device (-1 , &ep93xx_led_data); + +return parent;
Nothing appears to use or check this return value?
Quoted 25 lines
arch/arm/mach-ep93xx/core.c +0
-0}void ep93xx_restart (char mode,const char *cmd)arch/arm/mach-ep93xx/include/mach/platform.h +2
-1@@ -4,6 +4,7 @@#ifndef __ASSEMBLY__ +struct device ;struct i2c_gpio_platform_data ;struct i2c_board_info ;struct spi_board_info ;@@ -52,7 +53,7 @@ void ep93xx_register_ide(void);int ep93xx_ide_acquire_gpio (struct platform_device *pdev);void ep93xx_ide_release_gpio (struct platform_device *pdev); -void ep93xx_init_devices (void ); +struct device *ep93xx_init_devices (void );extern void ep93xx_timer_init (void );void ep93xx_restart (char ,const char *);
PATCH 1/1 arm: ep93xx: use soc bus
Quoted 76 lines · Ryan Mallon
Quoted 6 lines · H Hartley Sweeten
Use the soc bus to report the silicon revision and Maverick Key. Both are not currently exposed to the user. In addition, fill in the SoC family and machine for completeness.
Signed-off-by H Hartley Sweeten <[email protected]>Cc Ryan Mallon <[email protected]>Hey Hartley,
Looks mostly good. Couple of comments below.
~Ryan
Quoted 62 lines
--- arch/arm/mach-ep93xx/Kconfig | 1 + arch/arm/mach-ep93xx/core.c | 106 ++++++++++++++++++++++++++- arch/arm/mach-ep93xx/include/mach/platform.h | 3 +- 3 files changed, 108 insertions(+), 2 deletions(-)arch/arm/mach-ep93xx/Kconfig +1
-0@@ -5,6 +5,7 @@ menu "Cirrus EP93xx Implementation Options" config EP93XX_SOC_COMMON bool default y + select SOC_BUS select LEDS_GPIO_REGISTER config CRUNCHarch/arm/mach-ep93xx/core.c +21
-1@@ -21,6 +21,7 @@#include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/dma-mapping.h> +#include <linux/sys_soc.h> #include <linux/timex.h> #include <linux/irq.h> #include <linux/io.h> @@ -42,6 +43,7 @@#include <linux/platform_data/spi-ep93xx.h> #include <mach/gpio-ep93xx.h> +#include <asm/mach/arch.h> #include <asm/mach/map.h> #include <asm/mach/time.h> @@ -895,8 +897,106 @@ void ep93xx_ide_release_gpio(struct platform_device *pdev) }EXPORT_SYMBOL (ep93xx_ide_release_gpio); -void __init ep93xx_init_devices (void ) +/************************************************************************* +* EP93xx Security peripheral +*************************************************************************/ + +/* +* The Maverick Key is 256 bits of micro fuses blown at the factory during +* manufacturing to uniquely identify a part. +* +* See: http://arm.cirrus.com/forum/viewtopic.php?t=486&highlight=maverick+key +*/ +#define EP93XX_SECURITY_REG (x) (EP93XX_SECURITY_BASE + (x)) +#define EP93XX_SECURITY_UNIQID EP93XX_SECURITY_REG(0x2440) +#define EP93XX_SECURITY_UNIQCHK EP93XX_SECURITY_REG(0x2450) +#define EP93XX_SECURITY_SECID1 EP93XX_SECURITY_REG(0x2500) +#define EP93XX_SECURITY_SECCHK1 EP93XX_SECURITY_REG(0x2520) +#define EP93XX_SECURITY_SECID2 EP93XX_SECURITY_REG(0x2504) +#define EP93XX_SECURITY_SECCHK2 EP93XX_SECURITY_REG(0x2524) +#define EP93XX_SECURITY_FUSEFLG EP93XX_SECURITY_REG(0x2410) +#define EP93XX_SECURITY_UNIQID2 EP93XX_SECURITY_REG(0x2700)
Quoted 8 lines
arch/arm/mach-ep93xx/core.c void ep93xx_ide_release_gpio(struct platform_device *pdev) · line 923
Quoted 5 lines
arch/arm/mach-ep93xx/core.c +5
-0+#define EP93XX_SECURITY_UNIQID3 EP93XX_SECURITY_REG(0x2704) +#define EP93XX_SECURITY_UNIQID4 EP93XX_SECURITY_REG(0x2708) +#define EP93XX_SECURITY_UNIQID5 EP93XX_SECURITY_REG(0x270c) +#define EP93XX_SECURITY_SECFLG EP93XX_SECURITY_REG(0x2400) +#define EP93XX_SECURITY_UNIQVAL EP93XX_SECURITY_REG(0x2460)You are only using about half of these defines. Are you expecting to use some more of them in the future, or could we just remove the unused ones?
The Maverick Key is not documented in the EP93xx Users Guide.
Chapter 29 - Security, just says to contact Cirrus Logic for additional information.
I would like to keep the defines here for documentation.
Quoted 26 lines
Quoted 10 lines
arch/arm/mach-ep93xx/core.c +10
-0+ +static const char __init *ep93xx_get_soc_id (void ) +{ +unsigned int id, id2, id3, id4, id5; + +if (__raw_readl (EP93XX_SECURITY_UNIQVAL ) !=1 ) +return kasprintf (GFP_KERNEL ,"bad Hamming code" ); + + id =__raw_readl (EP93XX_SECURITY_UNIQID ); + id2 =__raw_readl (EP93XX_SECURITY_UNIQID2 );arch/arm/mach-ep93xx/core.c void ep93xx_ide_release_gpio(struct platform_device *pdev) · line 943
Quoted 10 lines
arch/arm/mach-ep93xx/core.c +10
-0+ id3 =__raw_readl (EP93XX_SECURITY_UNIQID3 ); + id4 =__raw_readl (EP93XX_SECURITY_UNIQID4 ); + id5 =__raw_readl (EP93XX_SECURITY_UNIQID5 ); + +if (id != id2) +return kasprintf (GFP_KERNEL ,"invalid" ); + +return kasprintf (GFP_KERNEL ,"%08x%08x%08x%08x" , + id2, id3, id4, id5); +}You don't need to use the allocator functions here:
static char soc_id[33];
static const char
__init*ep93xx_get_soc_id(void)
Quoted 8 lines
{
if (...)
return "bad Hamming code";
...
snprintf(soc_id, sizeof(soc_id), ...);
return soc_id;
}I'm not sure which is cleaner. Only one of the allocator functions actually gets called.
Quoted 38 lines
Quoted 38 lines
arch/arm/mach-ep93xx/core.c +38
-0+ +static const char __init *ep93xx_get_soc_rev (void ) +{ +int rev =ep93xx_chip_revision (); + +switch (rev) { +case EP93XX_CHIP_REV_D0 : +return "D0" ; +case EP93XX_CHIP_REV_D1 : +return "D1" ; +case EP93XX_CHIP_REV_E0 : +return "E0" ; +case EP93XX_CHIP_REV_E1 : +return "E1" ; +case EP93XX_CHIP_REV_E2 : +return "E2" ; +default : +return "unknown" ; + } +} + +static const char __init *ep93xx_get_machine_name (void ) +{ +return kasprintf (GFP_KERNEL ,"%s" , machine_desc->name); +} + +static struct device __init *ep93xx_init_soc (void ) +{ +struct soc_device_attribute *soc_dev_attr; +struct soc_device *soc_dev; + + soc_dev_attr =kzalloc (sizeof (*soc_dev_attr),GFP_KERNEL ); +if (!soc_dev_attr) +return NULL ; + + soc_dev_attr->machine =ep93xx_get_machine_name (); + soc_dev_attr->family ="Cirrus EP93xx" ; + soc_dev_attr->revision =ep93xx_get_soc_rev ();
Quoted 8 lines
arch/arm/mach-ep93xx/core.c void ep93xx_ide_release_gpio(struct platform_device *pdev) · line 987
Quoted 6 lines
arch/arm/mach-ep93xx/core.c +6
-0+ soc_dev_attr->soc_id =ep93xx_get_soc_id (); + + soc_dev =soc_device_register (soc_dev_attr); +if (IS_ERR (soc_dev)) { +kfree (soc_dev_attr->soc_id); +kfree (soc_dev_attr->revision);Revision wasn't allocated with kmalloc.
Doh...
Quoted 24 lines
Quoted 24 lines
arch/arm/mach-ep93xx/core.c +13
-0+kfree (soc_dev_attr->machine); +kfree (soc_dev_attr); +return NULL ; + } + +return soc_device_to_device (soc_dev); +} + +struct device __init *ep93xx_init_devices (void ) { +struct device *parent; +/* Disallow access to MaverickCrunch initially */ ep93xx_devcfg_clear_bits (EP93XX_SYSCON_DEVCFG_CPENA );@@ -907,6 +1007,8 @@ void __init ep93xx_init_devices(void)EP93XX_SYSCON_DEVCFG_GONIDE | EP93XX_SYSCON_DEVCFG_HONIDE); + parent =ep93xx_init_soc (); +/* Get the GPIO working early, other devices need it */ platform_device_register (&ep93xx_gpio_device);
Quoted 8 lines
arch/arm/mach-ep93xx/core.c void __init ep93xx_init_devices(void) · line 1025
Quoted 6 lines
arch/arm/mach-ep93xx/core.c +2
-0@@ -919,6 +1021,8 @@ void __init ep93xx_init_devices(void)platform_device_register (&ep93xx_wdt_device);gpio_led_register_device (-1 , &ep93xx_led_data); + +return parent;Nothing appears to use or check this return value?
Not yet. :-)
The callers can use the return value to add additional board specific information to the soc bus. For instance the ts72xx has the FPGA revision and some additional information that might be useful.
I'll wait to see if there are any additional comments before posting an updated patch.
Regards,
Hartley