bus: imx-weim
cover bus: imx-weim
From: Sven Van Asbroeck <[email protected]>
Support multiple address ranges per child node on imx-weim. While we're at it, insert some code which guards against common config conflicts.
Sven Van Asbroeck (2):
bus: imx-weim: support multiple address ranges per child node
bus: imx-weim: guard against timing configuration conflicts
drivers/bus/imx-weim.c | 69 +++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 14 deletions(-)
Signature · 1 line
2.17.1
PATCH 1/2 bus: imx-weim: support multiple address ranges per child node
From: Sven Van Asbroeck <[email protected]>
Ensure that timing values for the child node are applied to all chip selects in the child's address ranges.
Note that this does not support multiple timing settings per child; this can be added in the future if required.
Example:
&weim {
acme@0 {
compatible = "acme,whatever";
reg = <0 0 0x100>, <0 0x400000 0x800>,
<1 0x400000 0x800>;
fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
0x00000000 0xa0000240 0x00000000>;
};
};--- drivers/bus/imx-weim.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)
drivers/bus/imx-weim.c +25 -11
@@ -46,6 +46,7 @@ static const struct imx_weim_devtype imx51_weim_devtype = { };#define MAX_CS_REGS_COUNT 6 +#define OF_REG_SIZE 3static const struct of_device_id weim_id_table[] = {/* i.MX1/21 */
@@ -115,27 +116,40 @@ static int __init weim_timing_setup(struct device_node *np, void __iomem *base, const struct imx_weim_devtype *devtype) {u32 cs_idx, value[MAX_CS_REGS_COUNT ]; -int i, ret; +int i, ret, reg_idx, num_regs;if (WARN_ON (devtype->cs_regs_count >MAX_CS_REGS_COUNT ))return -EINVAL ; -/* get the CS index from this child node's "reg" property. */ - ret =of_property_read_u32 (np,"reg" , &cs_idx); + ret =of_property_read_u32_array (np,"fsl,weim-cs-timing" , + value, devtype->cs_regs_count);if (ret)return ret; -if (cs_idx >= devtype->cs_count) +/* +* the child node's "reg" property may contain multiple address ranges, +* extract the chip select for each. +*/ + num_regs =of_property_count_elems_of_size (np,"reg" ,OF_REG_SIZE ); +if (num_regs <0 ) +return num_regs; +if (!num_regs)return -EINVAL ; +for (reg_idx =0 ; reg_idx < num_regs; reg_idx++) { +/* get the CS index from this child node's "reg" property. */ + ret =of_property_read_u32_index (np,"reg" , + reg_idx*OF_REG_SIZE , &cs_idx); +if (ret) +break ; - ret =of_property_read_u32_array (np,"fsl,weim-cs-timing" , - value, devtype->cs_regs_count); -if (ret) -return ret; +if (cs_idx >= devtype->cs_count) +return -EINVAL ; -/* set the timing for WEIM */ -for (i =0 ; i < devtype->cs_regs_count; i++) -writel (value[i], base + cs_idx * devtype->cs_stride + i *4 ); +/* set the timing for WEIM */ +for (i =0 ; i < devtype->cs_regs_count; i++) +writel (value[i], + base + cs_idx * devtype->cs_stride + i *4 ); + }return 0 ; }
Signature · 1 line
2.17.1
PATCH 2/2 bus: imx-weim: guard against timing configuration conflicts
In reply to [email protected]From: Sven Van Asbroeck <[email protected]>
When adding weim child devices, there is a risk that the developer will (by mistake) specify more than one timing setting for the same chip select.
The driver cannot support such a configuration.
In case of conflict, this patch will print a warning to the log, and will ignore the child node in question.
In this example, node acme@1 will be ignored, as it tries to modify timing settings for CS0:
&weim {
acme@0 {
compatible = "acme,whatever";
reg = <0 0 0x100>;
fsl,weim-cs-timing = <something>;
};
acme@1 {
compatible = "acme,whatnot";
reg = <0 0x500 0x100>;
fsl,weim-cs-timing = <something else>;
};
};However in this example, the driver will be happy:
&weim {
acme@0 {
compatible = "acme,whatever";
reg = <0 0 0x100>;
fsl,weim-cs-timing = <something>;
};
acme@1 {
compatible = "acme,whatnot";
reg = <0 0x500 0x100>;
fsl,weim-cs-timing = <something>;
};
};--- drivers/bus/imx-weim.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)
drivers/bus/imx-weim.c +30 -3
@@ -46,8 +46,18 @@ static const struct imx_weim_devtype imx51_weim_devtype = { };#define MAX_CS_REGS_COUNT 6 +#define MAX_CS_REGS 6#define OF_REG_SIZE 3 +struct cs_timing { +bool is_applied; +u32 regs[MAX_CS_REGS_COUNT ]; +}; + +struct cs_timing_state { +struct cs_timing cs[MAX_CS_REGS ]; +}; +static const struct of_device_id weim_id_table[] = {/* i.MX1/21 */ { .compatible ="fsl,imx1-weim" , .data = &imx1_weim_devtype, },
@@ -112,11 +122,14 @@ static int __init imx_weim_gpr_setup(struct platform_device *pdev) }/* Parse and set the timing for this device. */ -static int __init weim_timing_setup (struct device_node *np,void __iomem *base, -const struct imx_weim_devtype *devtype) +static int __init weim_timing_setup (struct device *dev, +struct device_node *np,void __iomem *base, +const struct imx_weim_devtype *devtype, +struct cs_timing_state *ts) {u32 cs_idx, value[MAX_CS_REGS_COUNT ];int i, ret, reg_idx, num_regs; +struct cs_timing *cst;if (WARN_ON (devtype->cs_regs_count >MAX_CS_REGS_COUNT ))return -EINVAL ;
@@ -145,10 +158,23 @@ static int __init weim_timing_setup(struct device_node *np, void __iomem *base,if (cs_idx >= devtype->cs_count)return -EINVAL ; +/* prevent re-configuring a CS that's already been configured */ + cst = &ts->cs[cs_idx]; +if (cst->is_applied &&memcmp (value, cst->regs, + devtype->cs_regs_count*sizeof (u32))) { +dev_err (dev,"fsl,weim-cs-timing conflict on %pOF" , np); +return -EINVAL ; + } +/* set the timing for WEIM */ for (i =0 ; i < devtype->cs_regs_count; i++)writel (value[i], base + cs_idx * devtype->cs_stride + i *4 ); +if (!cst->is_applied) { + cst->is_applied = true; +memcpy (cst->regs, value, + devtype->cs_regs_count*sizeof (u32)); + } }return 0 ;
@@ -165,6 +191,7 @@ static int __init weim_parse_dt(struct platform_device *pdev,const struct imx_weim_devtype *devtype = of_id->data;struct device_node *child;int ret, have_child =0 ; +struct cs_timing_state ts = {};if (devtype == &imx50_weim_devtype) { ret =imx_weim_gpr_setup (pdev);
@@ -179,7 +206,7 @@ static int __init weim_parse_dt(struct platform_device *pdev, }for_each_available_child_of_node (pdev->dev.of_node, child) { - ret =weim_timing_setup (child, base, devtype); + ret =weim_timing_setup (&pdev->dev, child, base, devtype, &ts);if (ret)dev_warn (&pdev->dev,"%pOF set timing failed.\n" , child);
Signature · 1 line
2.17.1
Re: [PATCH 2/2] bus: imx-weim: guard against timing configuration conflicts
Quoted 8 lines · On Fri, Nov 30, 2018 at 11:08 AM <[email protected]> wrote:
From: Sven Van Asbroeck <[email protected]>
When adding weim child devices, there is a risk that the developer will (by mistake) specify more than one timing setting for the same chip select.
The driver cannot support such a configuration.
Quoted 8 lines
In case of conflict, this patch will print a warning to the log, and will ignore the child node in question.
In this example, node acme@1 will be ignored, as it tries to modify timing settings for CS0:
&weim { acme@0 {
This should be '@0,0'
Quoted 5 lines
compatible = "acme,whatever";
reg = <0 0 0x100>;
fsl,weim-cs-timing = <something>;
};
acme@1 {And '@0,500'
Quoted 110 lines
compatible = "acme,whatnot"; reg = <0 0x500 0x100>; fsl,weim-cs-timing = <something else>; }; };However in this example, the driver will be happy:
&weim { acme@0 { compatible = "acme,whatever"; reg = <0 0 0x100>; fsl,weim-cs-timing = <something>; }; acme@1 { compatible = "acme,whatnot"; reg = <0 0x500 0x100>; fsl,weim-cs-timing = <something>; }; };Signed-off-by Sven Van Asbroeck <[email protected]>--- drivers/bus/imx-weim.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)drivers/bus/imx-weim.c +30
-4@@ -46,8 +46,18 @@ static const struct imx_weim_devtype imx51_weim_devtype = { };#define MAX_CS_REGS_COUNT 6 +#define MAX_CS_REGS 6#define OF_REG_SIZE 3 +struct cs_timing { +bool is_applied; +u32 regs[MAX_CS_REGS_COUNT ]; +}; + +struct cs_timing_state { +struct cs_timing cs[MAX_CS_REGS ]; +}; +static const struct of_device_id weim_id_table[] = {/* i.MX1/21 */ { .compatible ="fsl,imx1-weim" , .data = &imx1_weim_devtype, },@@ -112,11 +122,14 @@ static int __init imx_weim_gpr_setup(struct platform_device *pdev) }/* Parse and set the timing for this device. */ -static int __init weim_timing_setup (struct device_node *np,void __iomem *base, -const struct imx_weim_devtype *devtype) +static int __init weim_timing_setup (struct device *dev, +struct device_node *np,void __iomem *base, +const struct imx_weim_devtype *devtype, +struct cs_timing_state *ts) {u32 cs_idx, value[MAX_CS_REGS_COUNT ];int i, ret, reg_idx, num_regs; +struct cs_timing *cst;if (WARN_ON (devtype->cs_regs_count >MAX_CS_REGS_COUNT ))return -EINVAL ;@@ -145,10 +158,23 @@ static int __init weim_timing_setup(struct device_node *np, void __iomem *base,if (cs_idx >= devtype->cs_count)return -EINVAL ; +/* prevent re-configuring a CS that's already been configured */ + cst = &ts->cs[cs_idx]; +if (cst->is_applied &&memcmp (value, cst->regs, + devtype->cs_regs_count*sizeof (u32))) { +dev_err (dev,"fsl,weim-cs-timing conflict on %pOF" , np); +return -EINVAL ; + } +/* set the timing for WEIM */ for (i =0 ; i < devtype->cs_regs_count; i++)writel (value[i], base + cs_idx * devtype->cs_stride + i *4 ); +if (!cst->is_applied) { + cst->is_applied = true; +memcpy (cst->regs, value, + devtype->cs_regs_count*sizeof (u32)); + } }return 0 ;@@ -165,6 +191,7 @@ static int __init weim_parse_dt(struct platform_device *pdev,const struct imx_weim_devtype *devtype = of_id->data;struct device_node *child;int ret, have_child =0 ; +struct cs_timing_state ts = {};if (devtype == &imx50_weim_devtype) { ret =imx_weim_gpr_setup (pdev);@@ -179,7 +206,7 @@ static int __init weim_parse_dt(struct platform_device *pdev, }for_each_available_child_of_node (pdev->dev.of_node, child) { - ret =weim_timing_setup (child, base, devtype); + ret =weim_timing_setup (&pdev->dev, child, base, devtype, &ts);if (ret)dev_warn (&pdev->dev,"%pOF set timing failed.\n" , child); --2.17.1