| Message ID | 1414407950-3029-5-git-send-email-m.szyprowski@samsung.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 27, 2014 at 12:05:47PM +0100, Marek Szyprowski wrote: > From: Tomasz Figa <t.figa@samsung.com> > > Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch > settings configured in registers leading to crashes if L2C is enabled > without overriding them. This patch introduces bindings to enable > prefetch settings to be specified from DT and necessary support in the > driver. > > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > [mszyprow: rebased onto v3.18-rc1, added error messages when property value > is missing] Why? What if the boot loader has already set these up appropriately? Why should we force people to list these in the DT?
Hello, On 2014-10-27 12:14, Russell King - ARM Linux wrote: > On Mon, Oct 27, 2014 at 12:05:47PM +0100, Marek Szyprowski wrote: >> From: Tomasz Figa <t.figa@samsung.com> >> >> Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch >> settings configured in registers leading to crashes if L2C is enabled >> without overriding them. This patch introduces bindings to enable >> prefetch settings to be specified from DT and necessary support in the >> driver. >> >> Signed-off-by: Tomasz Figa <t.figa@samsung.com> >> [mszyprow: rebased onto v3.18-rc1, added error messages when property value >> is missing] > Why? What if the boot loader has already set these up appropriately? Why > should we force people to list these in the DT? The error message is displayed only when user provided prefetch related properties without any value (empty properties). Something that Mark Rutland requested here: https://lkml.org/lkml/2014/9/24/426 I'm sorry if I didn't describe it clearly enough. Best regards
On Mon, Oct 27, 2014 at 12:19:34PM +0100, Marek Szyprowski wrote: > Hello, > > On 2014-10-27 12:14, Russell King - ARM Linux wrote: >> On Mon, Oct 27, 2014 at 12:05:47PM +0100, Marek Szyprowski wrote: >>> From: Tomasz Figa <t.figa@samsung.com> >>> >>> Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch >>> settings configured in registers leading to crashes if L2C is enabled >>> without overriding them. This patch introduces bindings to enable >>> prefetch settings to be specified from DT and necessary support in the >>> driver. >>> >>> Signed-off-by: Tomasz Figa <t.figa@samsung.com> >>> [mszyprow: rebased onto v3.18-rc1, added error messages when property value >>> is missing] >> Why? What if the boot loader has already set these up appropriately? Why >> should we force people to list these in the DT? > > The error message is displayed only when user provided prefetch related > properties without any value (empty properties). Something that Mark Rutland > requested here: https://lkml.org/lkml/2014/9/24/426 I'm sorry if I didn't > describe it clearly enough. Ok. I'd ask for one change. Please make all these messages start with "L2C-310 OF" not "PL310 OF:". The device is described in ARM documentation as a L2C-310 not PL310. (Also note the : is dropped too - most of the other messages don't have the : either.) The: "PL310 OF: cache setting yield illegal associativity PL310 OF: -1073346556 calculated, only 8 and 16 legal" message could also be changed to something like: "L2C-310 OF cache associativity %d invalid, only 8 or 16 permitted\n" Thanks.
On Tue, Oct 28, 2014 at 9:35 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > Ok. > > I'd ask for one change. Please make all these messages start with > "L2C-310 OF" not "PL310 OF:". The device is described in ARM > documentation as a L2C-310 not PL310. (Also note the : is dropped > too - most of the other messages don't have the : either.) > > The: > > "PL310 OF: cache setting yield illegal associativity > PL310 OF: -1073346556 calculated, only 8 and 16 legal" I have sent a patch to address this error message that happens when "cache-size" and "cache-sets" properties are not passed in DT: http://www.spinics.net/lists/arm-kernel/msg372094.html
diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index 292ef7ca3058..0dbabe9a6b0a 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -57,6 +57,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,double-linefill : Override double linefill enable setting. Enable if + non-zero, disable if zero. +- arm,double-linefill-incr : Override double linefill on INCR read. Enable + if non-zero, disable if zero. +- arm,double-linefill-wrap : Override double linefill on WRAP read. Enable + if non-zero, disable if zero. +- arm,prefetch-drop : Override prefetch drop enable setting. Enable if non-zero, + disable if zero. +- arm,prefetch-offset : Override prefetch offset value. Valid values are + 0-7, 15, 23, and 31. Example: diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index ad981894de73..69cfa8359ed3 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -1163,6 +1163,9 @@ static void __init l2c310_of_parse(const struct device_node *np, u32 tag[3] = { 0, 0, 0 }; u32 filter[2] = { 0, 0 }; u32 assoc; + u32 prefetch; + u32 val; + int ret; of_property_read_u32_array(np, "arm,tag-latency", tag, ARRAY_SIZE(tag)); if (tag[0] && tag[1] && tag[2]) @@ -1204,6 +1207,58 @@ static void __init l2c310_of_parse(const struct device_node *np, pr_err("PL310 OF: %d calculated, only 8 and 16 legal\n", assoc); break; } + + prefetch = l2x0_saved_regs.prefetch_ctrl; + + ret = of_property_read_u32(np, "arm,double-linefill", &val); + if (ret == 0) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL; + else + prefetch &= ~L310_PREFETCH_CTRL_DBL_LINEFILL; + } else if (ret != -EINVAL) { + pr_err("PL310 OF: missing value for arm,double-linefill property\n"); + } + + ret = of_property_read_u32(np, "arm,double-linefill-incr", &val); + if (ret == 0) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + else + prefetch &= ~L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + } else if (ret != -EINVAL) { + pr_err("PL310 OF: missing value for arm,double-linefill-incr property\n"); + } + + ret = of_property_read_u32(np, "arm,double-linefill-wrap", &val); + if (ret == 0) { + if (!val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + else + prefetch &= ~L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + } else if (ret != -EINVAL) { + pr_err("PL310 OF: missing value for arm,double-linefill-wrap property\n"); + } + + ret = of_property_read_u32(np, "arm,prefetch-drop", &val); + if (ret == 0) { + if (val) + prefetch |= L310_PREFETCH_CTRL_PREFETCH_DROP; + else + prefetch &= ~L310_PREFETCH_CTRL_PREFETCH_DROP; + } else if (ret != -EINVAL) { + pr_err("PL310 OF: missing value for arm,prefetch-drop property\n"); + } + + ret = of_property_read_u32(np, "arm,prefetch-offset", &val); + if (ret == 0) { + prefetch &= ~L310_PREFETCH_CTRL_OFFSET_MASK; + prefetch |= val & L310_PREFETCH_CTRL_OFFSET_MASK; + } else if (ret != -EINVAL) { + pr_err("PL310 OF: missing value for arm,prefetch-offset property\n"); + } + + l2x0_saved_regs.prefetch_ctrl = prefetch; } static const struct l2c_init_data of_l2c310_data __initconst = {