dt: device tree support for TI EMIF driver
cover dt: device tree support for TI EMIF driver
This series adds device tree support for TI EMIF SDRAM controller driver. For this, a binding has been added for representing AC timing parameters and other details of LPDDR2 memories.
This series depends my series for adding EMIF driver [1]
[1] http://marc.info/?l=linux-omap&m=133122243430942&w=2
Aneesh V (4):
dt: device tree bindings for LPDDR2 memories
dt: emif: device tree bindings for TI's EMIF sdram controller
arm: dts: EMIF and LPDDR2 device tree data for OMAP4 boards
misc: emif: add device tree support to emif driver
.../devicetree/bindings/lpddr2/lpddr2-timings.txt | 52 ++++ .../devicetree/bindings/lpddr2/lpddr2.txt | 102 +++++++ .../bindings/memory-controllers/ti/emif.txt | 55 ++++ arch/arm/boot/dts/elpida_ecb240abacn.dtsi | 67 +++++ arch/arm/boot/dts/omap4-panda.dts | 13 + arch/arm/boot/dts/omap4-sdp.dts | 13 + arch/arm/boot/dts/omap4.dtsi | 18 ++ drivers/misc/emif.c | 289 +++++++++++++++++++- 8 files changed, 608 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti/emif.txt create mode 100644 arch/arm/boot/dts/elpida_ecb240abacn.dtsi
PATCH 1/4 dt: device tree bindings for LPDDR2 memories
device tree bindings for LPDDR2 SDRAM memories compliant to JESD209-2 standard.
The 'lpddr2' binding in-turn uses another binding 'lpddr2-timings' for specifying the AC timing parameters of the memory device at different speed-bins.
---
Changes since RFC v4:
- Removed two DDR3 only timing parameters that were inadvertently added in the binding
--- .../devicetree/bindings/lpddr2/lpddr2-timings.txt | 52 ++++++++++ .../devicetree/bindings/lpddr2/lpddr2.txt | 102 ++++++++++++++++++++ 2 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt create mode 100644 Documentation/devicetree/bindings/lpddr2/lpddr2.txt
Documentation/devicetree/bindings/lpddr2/lpddr2-timings.txt new file +52 -0
@@ -0,0 +1,52 @@ +* AC timing parameters of LPDDR2(JESD209-2) memories for a given speed-bin + +Required properties: +- compatible : Should be "jedec,lpddr2-timings" +- min-freq : minimum DDR clock frequency for the speed-bin. Type is <u32> +- max-freq : maximum DDR clock frequency for the speed-bin. Type is <u32> + +Optional properties: + +The following properties represent AC timing parameters from the memory +data-sheet of the device for a given speed-bin. All these properties are +of type <u32> and the default unit is ps (pico seconds). Parameters with +a different unit have a suffix indicating the unit such as 'tRAS-max-ns' +- tRCD +- tWR +- tRAS-min +- tRRD +- tWTR +- tXP +- tRTP +- tDQSCK-max +- tFAW +- tZQCS +- tZQinit +- tRPab +- tZQCL +- tCKESR +- tRAS-max-ns +- tDQSCK-max-derated + +Example: + +timings_elpida_ECB240ABACN_400mhz: lpddr2-timings at 0 { + compatible = "jedec,lpddr2-timings"; + min-freq = <10000000>; + max-freq = <400000000>; + tRPab = <21000>; + tRCD = <18000>; + tWR = <15000>; + tRAS-min = <42000>; + tRRD = <10000>; + tWTR = <7500>; + tXP = <7500>; + tRTP = <7500>; + tCKESR = <15000>; + tDQSCK-max = <5500>; + tFAW = <50000>; + tZQCS = <90000>; + tZQCL = <360000>; + tZQinit = <1000000>; + tRAS-max-ns = <70000>; +};
Documentation/devicetree/bindings/lpddr2/lpddr2.txt new file +102 -0
102 lines not on this page · raw message
Signature · 1 line
1.7.1
PATCH 2/4 dt: emif: device tree bindings for TI's EMIF sdram controller
In reply to Aneesh VEMIF - External Memory Interface - is an SDRAM controller used in TI SoCs. EMIF supports, based on the IP revision, one or more of DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance of the EMIF IP and memory parts attached to it.
---
Changes sice RFC v4:
- None
--- .../bindings/memory-controllers/ti/emif.txt | 55 ++++++++++++++++++++ 1 files changed, 55 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
Documentation/devicetree/bindings/memory-controllers/ti/emif.txt new file +55 -0
@@ -0,0 +1,55 @@ +* EMIF family of TI SDRAM controllers + +EMIF - External Memory Interface - is an SDRAM controller used in +TI SoCs. EMIF supports, based on the IP revision, one or more of +DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance +of the EMIF IP and memory parts attached to it. + +Required properties: +- compatible : Should be of the form "ti,emif-<ip-rev>" where <ip-rev> + is the IP revision of the specific EMIF instance. + +- phy-type : <u32> indicating the DDR phy type. Following are the + allowed values + <1> : Attila PHY + <2> : Intelli PHY + +- device-handle : phandle to a "lpddr2" node representing the memory part + +- ti,hwmods : For TI hwmods processing and omap device creation + the value shall be "emif<n>" where <n> is the number of the EMIF + instance with base 1. + +Optional properties: +- cs1-used : Have this property if CS1 of this EMIF + instance has a memory part attached to it. If there is a memory + part attached to CS1, it should be the same type as the one on CS0, + so there is no need to give the details of this memory part. + +- cal-resistor-per-cs : Have this property if the board has one + calibration resistor per chip-select. + +- hw-caps-read-idle-ctrl: Have this property if the controller + supports read idle window programming + +- hw-caps-dll-calib-ctrl: Have this property if the controller + supports dll calibration control + +- hw-caps-ll-interface : Have this property if the controller + has a low latency interface and corresponding interrupt events + +- hw-caps-temp-alert : Have this property if the controller + has capability for generating SDRAM temperature alerts + +Example: + +emif1: emif at 0x4c000000 { + compatible = "ti,emif-4d"; + ti,hwmods = "emif2"; + phy-type = <1>; + device-handle = <&elpida_ECB240ABACN>; + cs1-used; + hw-caps-read-idle-ctrl; + hw-caps-ll-interface; + hw-caps-temp-alert; +};
Signature · 1 line
1.7.1
PATCH 3/4 arm: dts: EMIF and LPDDR2 device tree data for OMAP4 boards
In reply to Aneesh VDevice tree data for the EMIF sdram controllers in OMAP4 and LPDDR2 memory devices attached to OMAP4 boards.
---
Changes since RFC v4:
- Removed DDR3 only parameters from
elpida_ecb240abacn.dtsi
--- arch/arm/boot/dts/elpida_ecb240abacn.dtsi | 67 +++++++++++++++++++++++++++++ arch/arm/boot/dts/omap4-panda.dts | 13 ++++++ arch/arm/boot/dts/omap4-sdp.dts | 13 ++++++ arch/arm/boot/dts/omap4.dtsi | 18 ++++++++ 4 files changed, 111 insertions(+), 0 deletions(-) create mode 100644 arch/arm/boot/dts/elpida_ecb240abacn.dtsi
arch/arm/boot/dts/elpida_ecb240abacn.dtsi new file +67 -0
@@ -0,0 +1,67 @@ +/* + * Common devices used in different OMAP boards + */ + +/ { + elpida_ECB240ABACN: lpddr2 { + compatible = "Elpida,ECB240ABACN","jedec,lpddr2-s4"; + density = <2048>; + io-width = <32>; + + tRPab-min-tck = <3>; + tRCD-min-tck = <3>; + tWR-min-tck = <3>; + tRASmin-min-tck = <3>; + tRRD-min-tck = <2>; + tWTR-min-tck = <2>; + tXP-min-tck = <2>; + tRTP-min-tck = <2>; + tCKE-min-tck = <3>; + tCKESR-min-tck = <3>; + tFAW-min-tck = <8>; + + timings_elpida_ECB240ABACN_400mhz: lpddr2-timings at 0 { + compatible = "jedec,lpddr2-timings"; + min-freq = <10000000>; + max-freq = <400000000>; + tRPab = <21000>; + tRCD = <18000>; + tWR = <15000>; + tRAS-min = <42000>; + tRRD = <10000>; + tWTR = <7500>; + tXP = <7500>; + tRTP = <7500>; + tCKESR = <15000>; + tDQSCK-max = <5500>; + tFAW = <50000>; + tZQCS = <90000>; + tZQCL = <360000>; + tZQinit = <1000000>; + tRAS-max-ns = <70000>; + tDQSCK-max-derated = <6000>; + }; + + timings_elpida_ECB240ABACN_200mhz: lpddr2-timings at 1 { + compatible = "jedec,lpddr2-timings"; + min-freq = <10000000>; + max-freq = <200000000>; + tRPab = <21000>; + tRCD = <18000>; + tWR = <15000>; + tRAS-min = <42000>; + tRRD = <10000>; + tWTR = <10000>; + tXP = <7500>; + tRTP = <7500>; + tCKESR = <15000>; + tDQSCK-max = <5500>; + tFAW = <50000>; + tZQCS = <90000>; + tZQCL = <360000>; + tZQinit = <1000000>; + tRAS-max-ns = <70000>; + tDQSCK-max-derated = <6000>; + }; + }; +};
arch/arm/boot/dts/omap4-panda.dts +13 -0
@@ -8,6 +8,7 @@ /dts-v1/; /include/ "omap4.dtsi" +/include/ "elpida_ecb240abacn.dtsi" / { model = "TI OMAP4 PandaBoard";
@@ -17,4 +18,16 @@ device_type = "memory"; reg = <0x80000000 0x40000000>; /* 1 GB */ }; + + ocp { + emif1: emif at 0x4c000000 { + cs1-used; + device-handle = <&elpida_ECB240ABACN>; + }; + + emif2: emif at 0x4d000000 { + cs1-used; + device-handle = <&elpida_ECB240ABACN>; + }; + }; };
arch/arm/boot/dts/omap4-sdp.dts +13 -0
@@ -8,6 +8,7 @@ /dts-v1/; /include/ "omap4.dtsi" +/include/ "elpida_ecb240abacn.dtsi" / { model = "TI OMAP4 SDP board";
@@ -17,4 +18,16 @@ device_type = "memory"; reg = <0x80000000 0x40000000>; /* 1 GB */ }; + + ocp { + emif1: emif at 0x4c000000 { + cs1-used; + device-handle = <&elpida_ECB240ABACN>; + }; + + emif2: emif at 0x4d000000 { + cs1-used; + device-handle = <&elpida_ECB240ABACN>; + }; + }; };
arch/arm/boot/dts/omap4.dtsi +18 -0
@@ -127,5 +127,23 @@ ti,hwmods = "uart4"; clock-frequency = <48000000>; }; + + emif1: emif at 0x4c000000 { + compatible = "ti,emif-4d"; + ti,hwmods = "emif1"; + phy-type = <1>; + hw-caps-read-idle-ctrl; + hw-caps-ll-interface; + hw-caps-temp-alert; + }; + + emif2: emif at 0x4d000000 { + compatible = "ti,emif-4d"; + ti,hwmods = "emif2"; + phy-type = <1>; + hw-caps-read-idle-ctrl; + hw-caps-ll-interface; + hw-caps-temp-alert; + }; }; };
Signature · 1 line
1.7.1
PATCH 4/4 misc: emif: add device tree support to emif driver
In reply to Aneesh V---
Changes since RFC v4:
- Rebased to the latest version of EMIF series
- Replace kzalloc()/kfree() with devm_* variants
--- drivers/misc/emif.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 288 insertions(+), 1 deletions(-)
drivers/misc/emif.c +288 -1
@@ -18,6 +18,7 @@#include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/slab.h> +#include <linux/of.h> #include <linux/debugfs.h> #include <linux/seq_file.h> #include <linux/module.h>
@@ -49,6 +50,7 @@* frequency in effect at the moment) * @plat_data: Pointer to saved platform data. * @debugfs_root: dentry to the root folder for EMIF in debugfs +* @np_ddr: Pointer to ddr device tree node */ struct emif_data {u8 duplicate;
@@ -63,6 +65,9 @@ struct emif_data {struct emif_regs *curr_regs;struct emif_platform_data *plat_data;struct dentry *debugfs_root; +#if defined(CONFIG_OF ) +struct device_node *np_ddr; +#endif };static struct emif_data *emif1;
@@ -1147,6 +1152,270 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,return valid; } +#if defined(CONFIG_OF ) +static void __init of_get_custom_configs (struct device_node *np_emif, +struct emif_data *emif) +{ +struct emif_custom_configs *cust_cfgs =NULL ; +int len; +const int *lpmode, *poll_intvl; + + lpmode =of_get_property (np_emif,"low-power-mode" , &len); + poll_intvl =of_get_property (np_emif,"temp-alert-poll-interval" , &len); + +if (lpmode || poll_intvl) + cust_cfgs =devm_kzalloc (emif->dev,sizeof (*cust_cfgs), +GFP_KERNEL ); + +if (!cust_cfgs) +return ; + +if (lpmode) { + cust_cfgs->mask |=EMIF_CUSTOM_CONFIG_LPMODE ; + cust_cfgs->lpmode = *lpmode; +of_property_read_u32 (np_emif, +"low-power-mode-timeout-performance" , + &cust_cfgs->lpmode_timeout_performance); +of_property_read_u32 (np_emif, +"low-power-mode-timeout-power" , + &cust_cfgs->lpmode_timeout_power); +of_property_read_u32 (np_emif, +"low-power-mode-freq-threshold" , + &cust_cfgs->lpmode_freq_threshold); + } + +if (poll_intvl) { + cust_cfgs->mask |= +EMIF_CUSTOM_CONFIG_TEMP_ALERT_POLL_INTERVAL ; + cust_cfgs->temp_alert_poll_interval_ms = *poll_intvl; + } + +if (!is_custom_config_valid (cust_cfgs, emif->dev)) { +devm_kfree (emif->dev, cust_cfgs); +return ; + } + + emif->plat_data->custom_configs = cust_cfgs; +} + +static void __init of_get_min_tck (struct device_node *np, +struct emif_data *emif) +{ +int ret =0 ; +struct lpddr2_min_tck *min; + + min =devm_kzalloc (emif->dev,sizeof (*min),GFP_KERNEL ); +if (!min) + goto default_min_tck; + + ret |=of_property_read_u32 (np,"tRPab-min-tck" , &min->tRPab); + ret |=of_property_read_u32 (np,"tRCD-min-tck" , &min->tRCD); + ret |=of_property_read_u32 (np,"tWR-min-tck" , &min->tWR); + ret |=of_property_read_u32 (np,"tRASmin-min-tck" , &min->tRASmin); + ret |=of_property_read_u32 (np,"tRRD-min-tck" , &min->tRRD); + ret |=of_property_read_u32 (np,"tWTR-min-tck" , &min->tWTR); + ret |=of_property_read_u32 (np,"tXP-min-tck" , &min->tXP); + ret |=of_property_read_u32 (np,"tRTP-min-tck" , &min->tRTP); + ret |=of_property_read_u32 (np,"tCKE-min-tck" , &min->tCKE); + ret |=of_property_read_u32 (np,"tCKESR-min-tck" , &min->tCKESR); + ret |=of_property_read_u32 (np,"tFAW-min-tck" , &min->tFAW); + +if (ret) { +devm_kfree (emif->dev, min); + goto default_min_tck; + } + + emif->plat_data->min_tck = min; +return ; + +default_min_tck: +dev_warn (emif->dev,"%s: using default min-tck values\n" , __func__); + emif->plat_data->min_tck = &lpddr2_min_tck; +} + +static int __init of_do_get_timings (struct device_node *np, +struct lpddr2_timings *tim) +{ +int ret; + + ret =of_property_read_u32 (np,"max-freq" , &tim->max_freq); + ret |=of_property_read_u32 (np,"min-freq" , &tim->min_freq); + ret |=of_property_read_u32 (np,"tRPab" , &tim->tRPab); + ret |=of_property_read_u32 (np,"tRCD" , &tim->tRCD); + ret |=of_property_read_u32 (np,"tWR" , &tim->tWR); + ret |=of_property_read_u32 (np,"tRAS-min" , &tim->tRAS_min); + ret |=of_property_read_u32 (np,"tRRD" , &tim->tRRD); + ret |=of_property_read_u32 (np,"tWTR" , &tim->tWTR); + ret |=of_property_read_u32 (np,"tXP" , &tim->tXP); + ret |=of_property_read_u32 (np,"tRTP" , &tim->tRTP); + ret |=of_property_read_u32 (np,"tCKESR" , &tim->tCKESR); + ret |=of_property_read_u32 (np,"tDQSCK-max" , &tim->tDQSCK_max); + ret |=of_property_read_u32 (np,"tFAW" , &tim->tFAW); + ret |=of_property_read_u32 (np,"tZQCS" , &tim->tZQCS); + ret |=of_property_read_u32 (np,"tZQCL" , &tim->tZQCL); + ret |=of_property_read_u32 (np,"tZQinit" , &tim->tZQinit); + ret |=of_property_read_u32 (np,"tRAS-max-ns" , &tim->tRAS_max_ns); + ret |=of_property_read_u32 (np,"tDQSCK-max-derated" , + &tim->tDQSCK_max_derated); + +return ret; +} + +static void __init of_get_ddr_timings (struct device_node *np_ddr, +struct emif_data *emif) +{ +struct lpddr2_timings *timings =NULL ; +u32 arr_sz =0 , i =0 ; +struct device_node *np_tim; +char *tim_compat; + +switch (emif->plat_data->device_info->type) { +case DDR_TYPE_LPDDR2_S2 : +case DDR_TYPE_LPDDR2_S4 : + tim_compat ="jedec,lpddr2-timings" ; +break ; +default : +dev_warn (emif->dev,"%s: un-supported memory type\n" , __func__); + } + +for_each_child_of_node (np_ddr, np_tim) +if (of_device_is_compatible (np_tim, tim_compat)) + arr_sz++; + +if (arr_sz) + timings =devm_kzalloc (emif->dev,sizeof (*timings) * arr_sz, +GFP_KERNEL ); + +if (!timings) + goto default_timings; + +for_each_child_of_node (np_ddr, np_tim) { +if (of_device_is_compatible (np_tim, tim_compat)) { +if (of_do_get_timings (np_tim, &timings[i])) { +devm_kfree (emif->dev, timings); + goto default_timings; + } + i++; + } + } + + emif->plat_data->timings = timings; + emif->plat_data->timings_arr_size = arr_sz; + +return ; + +default_timings: +get_default_timings (emif); + +return ; +} + +static void __init of_get_ddr_info (struct device_node *np_emif, +struct device_node *np_ddr, +struct ddr_device_info *dev_info) +{ +u32 density =0 , io_width =0 ; +int len; + +if (of_find_property (np_emif,"cs1-used" , &len)) + dev_info->cs1_used = true; + +if (of_find_property (np_emif,"cal-resistor-per-cs" , &len)) + dev_info->cal_resistors_per_cs = true; + +if (of_device_is_compatible (np_ddr ,"jedec,lpddr2-s4" )) + dev_info->type =DDR_TYPE_LPDDR2_S4 ; +else if (of_device_is_compatible (np_ddr ,"jedec,lpddr2-s2" )) + dev_info->type =DDR_TYPE_LPDDR2_S2 ; + +of_property_read_u32 (np_ddr,"density" , &density); +of_property_read_u32 (np_ddr,"io-width" , &io_width); + +/* Convert from density in Mb to the density encoding in jedc_ddr.h */ +if (density & (density -1 )) + dev_info->density =0 ; +else + dev_info->density =__fls (density) -5 ; + +/* Convert from io_width in bits to io_width encoding in jedc_ddr.h */ +if (io_width & (io_width -1 )) + dev_info->io_width =0 ; +else + dev_info->io_width =__fls (io_width) -1 ; +} + +static struct emif_data *__init of_get_device_details ( +struct device_node *np_emif,struct device *dev) +{ +struct emif_data *emif =NULL ; +struct ddr_device_info *dev_info =NULL ; +struct emif_platform_data *pd =NULL ; +struct device_node *np_ddr; +int len; + + np_ddr =of_parse_phandle (np_emif,"device-handle" ,0 ); +if (!np_ddr) + goto error; + emif =devm_kzalloc (dev,sizeof (struct emif_data ),GFP_KERNEL ); + pd =devm_kzalloc (dev,sizeof (*pd),GFP_KERNEL ); + dev_info =devm_kzalloc (dev,sizeof (*dev_info),GFP_KERNEL ); + +if (!emif || !pd || !dev_info) { +dev_err (dev,"%s: of_get_device_details() failure!!\n" , + __func__); + goto error; + } + + emif->plat_data = pd; + pd->device_info = dev_info; + emif->dev = dev; + emif->np_ddr = np_ddr; + emif->temperature_level =SDRAM_TEMP_NOMINAL ; + +if (of_device_is_compatible (np_emif,"ti,emif-4d" )) + emif->plat_data->ip_rev =EMIF_4D ; +else if (of_device_is_compatible (np_emif,"ti,emif-4d5" )) + emif->plat_data->ip_rev =EMIF_4D5 ; + +of_property_read_u32 (np_emif,"phy-type" , &pd->phy_type); + +if (of_find_property (np_emif,"hw-caps-ll-interface" , &len)) + pd->hw_caps |=EMIF_HW_CAPS_LL_INTERFACE ; + +of_get_ddr_info (np_emif, np_ddr, dev_info); +if (!is_dev_data_valid (pd->device_info->type, pd->device_info->density, + pd->device_info->io_width, pd->phy_type, pd->ip_rev, + emif->dev)) { +dev_err (dev,"%s: invalid device data!!\n" , __func__); + goto error; + } +/* +* For EMIF instances other than EMIF1 see if the devices connected +* are exactly same as on EMIF1(which is typically the case). If so, +* mark it as a duplicate of EMIF1. This will save some memory and +* computation. +*/ +if (emif1 && emif1->np_ddr == np_ddr) { + emif->duplicate = true; + goto out; + }else if (emif1) { +dev_warn (emif->dev,"%s: Non-symmetric DDR geometry\n" , + __func__); + } + + +of_get_custom_configs (np_emif, emif); +of_get_ddr_timings (np_ddr, emif); +of_get_min_tck (np_ddr, emif); + goto out; + +error: +return NULL ; +out: +return emif; +} +#endif +static struct emif_data *__init get_device_details (struct platform_device *pdev) {
@@ -1266,7 +1535,13 @@ static int __init emif_probe(struct platform_device *pdev)struct resource *res;int irq; - emif =get_device_details (pdev); +#if defined(CONFIG_OF ) +if (pdev->dev.of_node) + emif =of_get_device_details (pdev->dev.of_node, &pdev->dev); + else +#endif + emif =get_device_details (pdev); +if (!emif) {pr_err ("%s: error getting device data\n" , __func__); goto error;
@@ -1643,11 +1918,23 @@ static void __attribute__((unused)) freq_post_notify_handling(void)spin_unlock_irqrestore (&emif_lock, irq_state); } +#if defined(CONFIG_OF ) +static const struct of_device_id emif_of_match[] = { + { .compatible ="ti,emif-4d" }, + { .compatible ="ti,emif-4d5" }, + {}, +}; +MODULE_DEVICE_TABLE (of, emif_of_match); +#endif +static struct platform_driver emif_driver = { .remove =__exit_p (emif_remove), .shutdown = emif_shutdown, .driver = { .name ="emif" , +#if defined(CONFIG_OF) + .of_match_table =of_match_ptr (emif_of_match), +#endif }, };
Signature · 1 line
1.7.1
PATCH 4/4 misc: emif: add device tree support to emif driver
Quoted 32 lines · Aneesh V
Cc Rajendra Nayak <[email protected]>Cc Benoit Cousson <[email protected]>Signed-off-by Aneesh V <[email protected]>---
Changes since RFC v4:
- Rebased to the latest version of EMIF series
- Replace kzalloc()/kfree() with devm_* variants--- drivers/misc/emif.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 288 insertions(+), 1 deletions(-)drivers/misc/emif.c +2
-0@@ -18,6 +18,7 @@#include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/slab.h> +#include <linux/of.h> #include <linux/debugfs.h> #include <linux/seq_file.h> #include <linux/module.h> @@ -49,6 +50,7 @@* frequency in effect at the moment) * @plat_data: Pointer to saved platform data. * @debugfs_root: dentry to the root folder for EMIF in debugfs +* @np_ddr: Pointer to ddr device tree node */ struct emif_data {u8 duplicate;
drivers/misc/emif.c struct emif_data { · line 70
Quoted 6 lines
drivers/misc/emif.c +3
-0@@ -63,6 +65,9 @@ struct emif_data {struct emif_regs *curr_regs;struct emif_platform_data *plat_data;struct dentry *debugfs_root; +#if defined(CONFIG_OF ) +struct device_node *np_ddr; +#endif
Don't bother with the #if/#endif wrapper here.
drivers/misc/emif.c static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, · line 1156
Quoted 9 lines
drivers/misc/emif.c +2
-0};static struct emif_data *emif1;@@ -1147,6 +1152,270 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,return valid; } +#if defined(CONFIG_OF ) +static void __init of_get_custom_configs(struct device_node *np_emif,
__devinit. Same through the rest of the file.
[...]
drivers/misc/emif.c static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, · line 1419
Quoted 7 lines
drivers/misc/emif.c +6
-0+return NULL ; +out: +return emif; +} +#endif + static struct emif_data * __init get_device_details(
This function also must be __devinit
Quoted 22 lines
drivers/misc/emif.c +8
-1struct platform_device *pdev) {@@ -1266,7 +1535,13 @@ static int __init emif_probe(struct platform_device *pdev)struct resource *res;int irq; - emif =get_device_details (pdev); +#if defined(CONFIG_OF ) +if (pdev->dev.of_node) + emif =of_get_device_details (pdev->dev.of_node, &pdev->dev); + else +#endif + emif =get_device_details (pdev); +if (!emif) {pr_err ("%s: error getting device data\n" , __func__); goto error;@@ -1643,11 +1918,23 @@ static void __attribute__((unused)) freq_post_notify_handling(void)spin_unlock_irqrestore (&emif_lock, irq_state); } +#if defined(CONFIG_OF )
drivers/misc/emif.c static void __attribute__((unused)) freq_post_notify_handling(void) · line 1931
Quoted 10 lines
drivers/misc/emif.c +8
-0+static const struct of_device_id emif_of_match[] = { + { .compatible ="ti,emif-4d" }, + { .compatible ="ti,emif-4d5" }, + {}, +}; +MODULE_DEVICE_TABLE (of, emif_of_match); +#endif +static struct platform_driver emif_driver = { .remove =__exit_p (emif_remove),
Not part of this patch I realize, but this is a bug. .remove must be in the __devexit section and the __devexit_p() wrapper must be used. Similarly, emif_probe must be in the __devinit section.
drivers/misc/emif.c static void __attribute__((unused)) freq_post_notify_handling(void) · line 1937
Quoted 6 lines
drivers/misc/emif.c +3
-0.shutdown = emif_shutdown, .driver = { .name = "emif", +#if defined(CONFIG_OF ) + .of_match_table = of_match_ptr(emif_of_match), +#endif
The of_match_ptr() macro makes the #if/#endif wrapper redundant.
Otherwise, on brief review this patch looks right.
g.
Quoted 11 lines
drivers/misc/emif.c +0
-0}, };Signature · 7 lines
1.7.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd.
PATCH 4/4 misc: emif: add device tree support to emif driver
Hi Grant,
Quoted 33 lines · Grant Likely
Quoted 32 lines · Aneesh V
Cc Rajendra Nayak<[email protected]>Cc Benoit Cousson<[email protected]>Signed-off-by Aneesh V<[email protected]>---
Changes since RFC v4:
- Rebased to the latest version of EMIF series
- Replace kzalloc()/kfree() with devm_* variants--- drivers/misc/emif.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 288 insertions(+), 1 deletions(-)drivers/misc/emif.c +2
-0@@ -18,6 +18,7 @@#include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/slab.h> +#include <linux/of.h> #include <linux/debugfs.h> #include <linux/seq_file.h> #include <linux/module.h> @@ -49,6 +50,7 @@* frequency in effect at the moment) * @plat_data: Pointer to saved platform data. * @debugfs_root: dentry to the root folder for EMIF in debugfs +* @np_ddr: Pointer to ddr device tree node */ struct emif_data {u8 duplicate;@@ -63,6 +65,9 @@ struct emif_data {
Quoted 8 lines
drivers/misc/emif.c struct emif_data { · line 70
Quoted 6 lines
drivers/misc/emif.c +3
-0struct emif_regs *curr_regs;struct emif_platform_data *plat_data;struct dentry *debugfs_root; +#if defined(CONFIG_OF ) +struct device_node *np_ddr; +#endif Don't bother with the #if/#endif wrapper here.
Ok.
Quoted 8 lines
Quoted 8 lines
drivers/misc/emif.c +0
-0};static struct emif_data *emif1;@@ -1147,6 +1152,270 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,return valid; }
Quoted 4 lines
drivers/misc/emif.c static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, · line 1156
Quoted 2 lines
drivers/misc/emif.c +2
-0+#if defined(CONFIG_OF ) +static void __init of_get_custom_configs(struct device_node *np_emif,
__devinit. Same through the rest of the file.
I am not sure if we need __devinit. I see that __devinit will not have any effect if CONFIG_HOTPLUG is enabled and CONFIG_HOTPLUG is always enabled in our configuration. EMIF devices are not hot-pluggable devices and are always added at boot-time from the board file. They are on-chip IP modules and dynamic discovery and addition doesn't make sense for them. So, can't we save some memory by making them __init.
AFAIU, __init doesn't have any effect in a module. However, I can make that more explicit by using '__init_or_module'. Is that ok?
Quoted 33 lines
[...]
drivers/misc/emif.c static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, · line 1419
Quoted 7 lines
drivers/misc/emif.c +6
-0+return NULL ; +out: +return emif; +} +#endif + static struct emif_data * __init get_device_details(This function also must be
__devinitQuoted 21 lines
struct platform_device *pdev) {drivers/misc/emif.c +7
-1@@ -1266,7 +1535,13 @@ static int __init emif_probe(struct platform_device *pdev)struct resource *res;int irq; - emif =get_device_details (pdev); +#if defined(CONFIG_OF ) +if (pdev->dev.of_node) + emif =of_get_device_details (pdev->dev.of_node,&pdev->dev); + else +#endif + emif =get_device_details (pdev); +if (!emif) {pr_err ("%s: error getting device data\n" , __func__); goto error;@@ -1643,11 +1918,23 @@ static void __attribute__((unused)) freq_post_notify_handling(void)spin_unlock_irqrestore (&emif_lock, irq_state); }
Quoted 15 lines
drivers/misc/emif.c static void __attribute__((unused)) freq_post_notify_handling(void) · line 1931
Quoted 11 lines
drivers/misc/emif.c +9
-0+#if defined(CONFIG_OF ) +static const struct of_device_id emif_of_match[] = { + { .compatible ="ti,emif-4d" }, + { .compatible ="ti,emif-4d5" }, + {}, +}; +MODULE_DEVICE_TABLE (of, emif_of_match); +#endif +static struct platform_driver emif_driver = { .remove =__exit_p (emif_remove),Not part of this patch I realize, but this is a bug. .remove must be in the
__devexitsection and the__devexit_p()wrapper must be used. Similarly,emif_probemust be in the__devinitsection.
Again, in our case remove() is not needed unless the driver is built as a module because the devices will never be un-registered. When it is built as a module the effect is same for both:
#if defined(MODULE) || defined(CONFIG_HOTPLUG)
#define __devexit_p(x) x
#else
#define __devexit_p(x) NULL
#endif
#ifdef MODULE
#define __exit_p(x) x
#else
#define __exit_p(x) NULL
#endif
Quoted 9 lines
drivers/misc/emif.c static void __attribute__((unused)) freq_post_notify_handling(void) · line 1937
Quoted 7 lines
drivers/misc/emif.c +3
-0.shutdown = emif_shutdown, .driver = { .name = "emif", +#if defined(CONFIG_OF ) + .of_match_table = of_match_ptr(emif_of_match), +#endif The
of_match_ptr()macro makes the #if/#endif wrapper redundant.
Ok. I will remove it.
Quoted 2 lines
Otherwise, on brief review this patch looks right.
Thanks for the review. Are you ok with the bindings too?
br,
Aneesh
PATCH 4/4 misc: emif: add device tree support to emif driver
Quoted 119 lines · Aneesh V
Hi Grant,
Quoted 33 lines · Grant Likely
Quoted 32 lines · Aneesh V
Cc Rajendra Nayak<[email protected]>Cc Benoit Cousson<[email protected]>Signed-off-by Aneesh V<[email protected]>---
Changes since RFC v4:
- Rebased to the latest version of EMIF series
- Replace kzalloc()/kfree() with devm_* variants--- drivers/misc/emif.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 288 insertions(+), 1 deletions(-)drivers/misc/emif.c +2
-0@@ -18,6 +18,7 @@#include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/slab.h> +#include <linux/of.h> #include <linux/debugfs.h> #include <linux/seq_file.h> #include <linux/module.h> @@ -49,6 +50,7 @@* frequency in effect at the moment) * @plat_data: Pointer to saved platform data. * @debugfs_root: dentry to the root folder for EMIF in debugfs +* @np_ddr: Pointer to ddr device tree node */ struct emif_data {u8 duplicate;@@ -63,6 +65,9 @@ struct emif_data {Quoted 8 lines
drivers/misc/emif.c struct emif_data { · line 70
Quoted 6 lines
drivers/misc/emif.c +3
-0struct emif_regs *curr_regs;struct emif_platform_data *plat_data;struct dentry *debugfs_root; +#if defined(CONFIG_OF ) +struct device_node *np_ddr; +#endif Don't bother with the #if/#endif wrapper here.
Ok.
Quoted 8 lines
Quoted 8 lines
drivers/misc/emif.c +0
-0};static struct emif_data *emif1;@@ -1147,6 +1152,270 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,return valid; }Quoted 4 lines
drivers/misc/emif.c static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, · line 1156
Quoted 2 lines
drivers/misc/emif.c +2
-0+#if defined(CONFIG_OF ) +static void __init of_get_custom_configs(struct device_node *np_emif,
__devinit. Same through the rest of the file.I am not sure if we need
__devinit. I see that__devinitwill not have any effect ifCONFIG_HOTPLUGis enabled andCONFIG_HOTPLUGis always enabled in our configuration. EMIF devices are not hot-pluggable devices and are always added at boot-time from the board file. They are on-chip IP modules and dynamic discovery and addition doesn't make sense for them. So, can't we save some memory by making them__init.AFAIU,
__initdoesn't have any effect in a module. However, I can make that more explicit by using '__init_or_module'. Is that ok?Quoted 33 lines
[...]
drivers/misc/emif.c static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs, · line 1419
Quoted 7 lines
drivers/misc/emif.c +6
-0+return NULL ; +out: +return emif; +} +#endif + static struct emif_data * __init get_device_details(This function also must be
__devinitQuoted 21 lines
struct platform_device *pdev) {drivers/misc/emif.c +7
-1@@ -1266,7 +1535,13 @@ static int __init emif_probe(struct platform_device *pdev)struct resource *res;int irq; - emif =get_device_details (pdev); +#if defined(CONFIG_OF ) +if (pdev->dev.of_node) + emif =of_get_device_details (pdev->dev.of_node,&pdev->dev); + else +#endif + emif =get_device_details (pdev); +if (!emif) {pr_err ("%s: error getting device data\n" , __func__); goto error;@@ -1643,11 +1918,23 @@ static void __attribute__((unused)) freq_post_notify_handling(void)spin_unlock_irqrestore (&emif_lock, irq_state); }Quoted 15 lines
drivers/misc/emif.c static void __attribute__((unused)) freq_post_notify_handling(void) · line 1931
Quoted 11 lines
drivers/misc/emif.c +9
-0+#if defined(CONFIG_OF ) +static const struct of_device_id emif_of_match[] = { + { .compatible ="ti,emif-4d" }, + { .compatible ="ti,emif-4d5" }, + {}, +}; +MODULE_DEVICE_TABLE (of, emif_of_match); +#endif +static struct platform_driver emif_driver = { .remove =__exit_p (emif_remove),Not part of this patch I realize, but this is a bug. .remove must be in the
__devexitsection and the__devexit_p()wrapper must be used. Similarly,emif_probemust be in the__devinitsection.
Quoted 3 lines
Again, in our case
remove()is not needed unless the driver is built as a module because the devices will never be un-registered. When it is built as a module the effect is same for both:
It's just outside the pattern expected for device drivers. I like to be careful about things like this. That's why I recommend __devinit also, but I see now that the probe hook isn't added to the platform_device so it isn't required.
Quoted 7 lines
#if defined(MODULE) ||
defined(CONFIG_HOTPLUG)
#define__devexit_p(x)x
#else
#define__devexit_p(x)NULL
#endif
Quoted 5 lines
#ifdef MODULE
#define__exit_p(x)x
#else
#define__exit_p(x)NULL
#endif
Don't go with what is written in the code here; go with what the meaning of the sections are. The code can be changed, but the drivers are expected to honor what the sections mean.
__init can be freed after module init__devinit in reality never gets freed because devices can show up later __devexit can be freed if hotplug is not enabled__exit can be freed after module unload
g.