| Message ID | 1409062680-15906-5-git-send-email-t.figa@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26/08/2014 at 16:17:57 +0200, Tomasz Figa wrote : > 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> Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> It is working and useful on Atmel's sama5d4 were the bootloader is not configuring the L2C prefetch. However, I'm wondering whether we should add support for setting L310_PREFETCH_CTRL_DATA_PREFETCH and L310_PREFETCH_CTRL_INSTR_PREFETCH. I'm currently doing it by using ".l2c_aux_val = L310_AUX_CTRL_DATA_PREFETCH | L310_AUX_CTRL_INSTR_PREFETCH" (those are the same bits) but this has the disadvantage of displaying the "L2C: platform modifies aux control register:" twice. > + if (!of_property_read_u32(np, "arm,prefetch-offset", &val)) { > + prefetch &= ~L310_PREFETCH_CTRL_OFFSET_MASK; > + prefetch |= val & L310_PREFETCH_CTRL_OFFSET_MASK; > + } > + While you use val directly here, later, while printing the offset, val + 1 is used. Maybe it would be better to have the same number in both places, else you end up with having "arm,prefetch-offset = <1>" in your DT and the kernel printing "L2C-310 ID prefetch enabled, offset 2 lines".
On Fri, Sep 19, 2014 at 11:50:01AM +0200, Alexandre Belloni wrote: > On 26/08/2014 at 16:17:57 +0200, Tomasz Figa wrote : > > 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> > > Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > > It is working and useful on Atmel's sama5d4 were the bootloader is not > configuring the L2C prefetch. However, I'm wondering whether we should > add support for setting L310_PREFETCH_CTRL_DATA_PREFETCH and > L310_PREFETCH_CTRL_INSTR_PREFETCH. I'm currently doing it by using > ".l2c_aux_val = L310_AUX_CTRL_DATA_PREFETCH | > L310_AUX_CTRL_INSTR_PREFETCH" (those are the same bits) but this has the > disadvantage of displaying the "L2C: platform modifies aux control > register:" twice. The L2C documentation, freely available from the ARM infocentre website, has the answer to this for you. The two bits in the prefetch control register which control the data and instruction prefetching are aliases of the aux control register. If you set them to a value in one register, they are reflected in the other. The reason for that is that once the L2 cache is enabled, writes to the aux control register are no longer permitted, but it's safe to enable and disable the prefetching with the cache already enabled. This reason is even stated in the documentation.
On 19/09/2014 at 17:39:32 +0100, Russell King - ARM Linux wrote : > On Fri, Sep 19, 2014 at 11:50:01AM +0200, Alexandre Belloni wrote: > > On 26/08/2014 at 16:17:57 +0200, Tomasz Figa wrote : > > > 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> > > > > Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > > > > It is working and useful on Atmel's sama5d4 were the bootloader is not > > configuring the L2C prefetch. However, I'm wondering whether we should > > add support for setting L310_PREFETCH_CTRL_DATA_PREFETCH and > > L310_PREFETCH_CTRL_INSTR_PREFETCH. I'm currently doing it by using > > ".l2c_aux_val = L310_AUX_CTRL_DATA_PREFETCH | > > L310_AUX_CTRL_INSTR_PREFETCH" (those are the same bits) but this has the > > disadvantage of displaying the "L2C: platform modifies aux control > > register:" twice. > > The L2C documentation, freely available from the ARM infocentre website, > has the answer to this for you. > > The two bits in the prefetch control register which control the data > and instruction prefetching are aliases of the aux control register. > If you set them to a value in one register, they are reflected in the > other. > > The reason for that is that once the L2 cache is enabled, writes to > the aux control register are no longer permitted, but it's safe to > enable and disable the prefetching with the cache already enabled. > This reason is even stated in the documentation. > Yeah, so my question still holds, should we have an other way to enable/disable I/D prefetch by adding two other DT bindings ?
On Fri, Sep 19, 2014 at 08:30:07PM +0200, Alexandre Belloni wrote: > On 19/09/2014 at 17:39:32 +0100, Russell King - ARM Linux wrote : > > On Fri, Sep 19, 2014 at 11:50:01AM +0200, Alexandre Belloni wrote: > > > On 26/08/2014 at 16:17:57 +0200, Tomasz Figa wrote : > > > > 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> > > > > > > Tested-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > > > > > > It is working and useful on Atmel's sama5d4 were the bootloader is not > > > configuring the L2C prefetch. However, I'm wondering whether we should > > > add support for setting L310_PREFETCH_CTRL_DATA_PREFETCH and > > > L310_PREFETCH_CTRL_INSTR_PREFETCH. I'm currently doing it by using > > > ".l2c_aux_val = L310_AUX_CTRL_DATA_PREFETCH | > > > L310_AUX_CTRL_INSTR_PREFETCH" (those are the same bits) but this has the > > > disadvantage of displaying the "L2C: platform modifies aux control > > > register:" twice. > > > > The L2C documentation, freely available from the ARM infocentre website, > > has the answer to this for you. > > > > The two bits in the prefetch control register which control the data > > and instruction prefetching are aliases of the aux control register. > > If you set them to a value in one register, they are reflected in the > > other. > > > > The reason for that is that once the L2 cache is enabled, writes to > > the aux control register are no longer permitted, but it's safe to > > enable and disable the prefetching with the cache already enabled. > > This reason is even stated in the documentation. > > > > Yeah, so my question still holds, should we have an other way to > enable/disable I/D prefetch by adding two other DT bindings ? Your question doesn't hold, because the above answers it conclusively. No.
diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index af527ee..3443d2d 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -47,6 +47,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 84c6c55..af90a6f 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -1059,6 +1059,8 @@ static void __init l2c310_of_parse(const struct device_node *np, u32 data[3] = { 0, 0, 0 }; u32 tag[3] = { 0, 0, 0 }; u32 filter[2] = { 0, 0 }; + u32 prefetch; + u32 val; of_property_read_u32_array(np, "arm,tag-latency", tag, ARRAY_SIZE(tag)); if (tag[0] && tag[1] && tag[2]) @@ -1083,6 +1085,43 @@ static void __init l2c310_of_parse(const struct device_node *np, l2x0_saved_regs.filter_start = (filter[0] & ~(SZ_1M - 1)) | L310_ADDR_FILTER_EN; } + + prefetch = l2x0_saved_regs.prefetch_ctrl; + + if (!of_property_read_u32(np, "arm,double-linefill", &val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL; + else + prefetch &= ~L310_PREFETCH_CTRL_DBL_LINEFILL; + } + + if (!of_property_read_u32(np, "arm,double-linefill-incr", &val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + else + prefetch &= ~L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + } + + if (!of_property_read_u32(np, "arm,double-linefill-wrap", &val)) { + if (!val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + else + prefetch &= ~L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + } + + if (!of_property_read_u32(np, "arm,prefetch-drop", &val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_PREFETCH_DROP; + else + prefetch &= ~L310_PREFETCH_CTRL_PREFETCH_DROP; + } + + if (!of_property_read_u32(np, "arm,prefetch-offset", &val)) { + prefetch &= ~L310_PREFETCH_CTRL_OFFSET_MASK; + prefetch |= val & L310_PREFETCH_CTRL_OFFSET_MASK; + } + + l2x0_saved_regs.prefetch_ctrl = prefetch; } static const struct l2c_init_data of_l2c310_data __initconst = {
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> --- Documentation/devicetree/bindings/arm/l2cc.txt | 10 +++++++ arch/arm/mm/cache-l2x0.c | 39 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+)