v2 -> v3:
- Rename: "DDR BWMON" -> "CPU BWMON" [2, 6/7]
- Set F_IRQ_STATUS = F_NUM_GLOBAL_FIELDS in enum bwmon_fields to save
one space in the enum
- Correct the struct icc_bwmon->global_regs array size
- Reorder the "Remove unused struct member" patch to come before the big
one
v2: https://lore.kernel.org/r/[email protected]
v1 -> v2:
- Un-mess-up the example in bindings
- Correctly limit reg/-names in bindings
- Introduce "qcom,sdm845-cpu-bwmon"
- Fix incorrect register assignment in msm8998_bwmon_reg_noread_ranges
- Clean up the code around setting global registers on <=8998
- Don't add a separate enum for global registers
- Don't use _GLOBAL vs _GLB
- Add of match entries for targets that abused qcom,msm8998-bwmon before
to keep old DTs working
- Add comments near of match entries to make things clearer
- Instead of if (...) { write to x } else { write to y } make the global
register variable to keep the code more readable
- Add dts patches to stop improperly using the 8998 compatible
- (grumpily) drop Fixes from [2/7]
- Pick up rb on [3/7]
- Re-test on MSM8998 and SM6375 (OOT, uses 845-style BWMONv4)
v1: https://lore.kernel.org/r/[email protected]
BWMONv4 (the one used for DDR scaling on all SoCs from msm8998 to sm8550)
features two register regions: "monitor" and "global" with the first one
containing registers specific to the throughput monitor itself and the
second one containing some sort of a head switch.
The register layout on all BWMON versions an implementations up to that
looked like this:
|..........[GLOBAL].........[MONITOR]........|
however with SDM845 somebody thought it would be a good idea to turn it
into this:
|................[GLOBAL]....................|
|....................[MONITOR]...............|
Sadly, the existing upstream driver was architected with SDM845 in mind,
which means it doesn't support the global registers being somewhere else
than near the beginning of the monitor space. This series tries to address
that in the hopefully least painful way. Tested on msm8998 (the count unit
seems to be wrong, should probably be 1MiB and not 64 KiB but the point is
that this series makes it work at all, as without it the headswitch is
never turned on) and SM6375 (with the "combined" layout introduced in
SDM845). Equally sadly, everybody uses the qcom,msm8998-bwmon compatible
(which frankly should have been just qcom,bwmon-v4) that never actually
worked on MSM8998 , which prevents us from handling it in a simpler way..
While at it, an unused struct member is removed.
One suboptimal feature of this patchset is that it introduces an "invalid
resource" print from within devres. This could be solved with an
introduction of devm_ioremap_resource_optional or by dropping devres
functions in place of manual handling, which also doesn't sound great..
I'll leave it up to the reviewers to decide.
Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (7):
dt-bindings: interconnect: qcom,msm8998-bwmon: Resolve MSM8998 support
soc: qcom: icc-bwmon: Remove unused struct member
soc: qcom: icc-bwmon: Handle global registers correctly
arm64: dts: qcom: sc7280: Use the correct BWMON fallback compatible
arm64: dts: qcom: sc8280xp: Use the correct BWMON fallback compatible
arm64: dts: qcom: sdm845: Use the correct BWMON compatible
arm64: dts: qcom: sm8550: Use the correct BWMON fallback compatible
.../bindings/interconnect/qcom,msm8998-bwmon.yaml | 41 +++-
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 +-
drivers/soc/qcom/icc-bwmon.c | 231 +++++++++++++++++++--
6 files changed, 247 insertions(+), 33 deletions(-)
---
base-commit: 24469a0e5052ba01a35a15f104717a82b7a4798b
change-id: 20230304-topic-ddr_bwmon-609022cd5e35
Best regards,
--
Konrad Dybcio <[email protected]>
BWMONv4 has two sets of registers: one for handling the monitor itself
and one called "global" which hosts some sort of a headswitch and an
interrupt control register. We did not handle that one before, as on
SoCs starting with SDM845 they have been merged into a single contiguous
range.
To make the qcom,msm8998-bwmon less confusing and in preparation for
actual MSM8998 support, describe the global register space and introduce
new "qcom,sdm845-cpu-bwmon" compatible while keeping the
"qcom,sdm845-bwmon" as a fallback for SoCs with this merged register space
scheme.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
.../bindings/interconnect/qcom,msm8998-bwmon.yaml | 41 ++++++++++++++++++----
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
index 12a0d3ecbabb..5d17bdcfdf70 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
@@ -22,14 +22,14 @@ description: |
properties:
compatible:
oneOf:
+ - const: qcom,msm8998-bwmon # BWMON v4
- items:
- enum:
- qcom,sc7280-cpu-bwmon
- qcom,sc8280xp-cpu-bwmon
- - qcom,sdm845-bwmon
+ - qcom,sdm845-cpu-bwmon
- qcom,sm8550-cpu-bwmon
- - const: qcom,msm8998-bwmon
- - const: qcom,msm8998-bwmon # BWMON v4
+ - const: qcom,sdm845-bwmon # BWMON v4, unified register space
- items:
- enum:
- qcom,sc8280xp-llcc-bwmon
@@ -49,9 +49,13 @@ properties:
type: object
reg:
- # BWMON v4 (currently described) and BWMON v5 use one register address
- # space. BWMON v2 uses two register spaces - not yet described.
- maxItems: 1
+ # BWMON v5 uses one register address space, v1-v4 use one or two.
+ minItems: 1
+ maxItems: 2
+
+ reg-names:
+ minItems: 1
+ maxItems: 2
required:
- compatible
@@ -63,13 +67,36 @@ required:
additionalProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ const: qcom,msm8998-bwmon
+ then:
+ properties:
+ reg:
+ minItems: 2
+
+ reg-names:
+ items:
+ - const: monitor
+ - const: global
+
+ else:
+ properties:
+ reg:
+ maxItems: 1
+
+ reg-names:
+ maxItems: 1
+
examples:
- |
#include <dt-bindings/interconnect/qcom,sdm845.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
pmu@1436400 {
- compatible = "qcom,sdm845-bwmon", "qcom,msm8998-bwmon";
+ compatible = "qcom,sdm845-cpu-bwmon", "qcom,sdm845-bwmon";
reg = <0x01436400 0x600>;
interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
interconnects = <&gladiator_noc MASTER_APPSS_PROC 3 &mem_noc SLAVE_LLCC 3>;
--
2.39.2
bwmon->regmap was never used, as the regmap for bwmon is registered
through devres and accessed through bwmon's regmap_field members.
Remove it
Fixes: ec63dcd3c863 ("soc: qcom: icc-bwmon: use regmap and prepare for BWMON v5")
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/soc/qcom/icc-bwmon.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
index d07be3700db6..d45caf512e2d 100644
--- a/drivers/soc/qcom/icc-bwmon.c
+++ b/drivers/soc/qcom/icc-bwmon.c
@@ -164,7 +164,6 @@ struct icc_bwmon {
const struct icc_bwmon_data *data;
int irq;
- struct regmap *regmap;
struct regmap_field *regs[F_NUM_FIELDS];
unsigned int max_bw_kbps;
--
2.39.2
The BWMON hardware has two sets of registers: one for the monitor itself
and one called "global". It has what seems to be some kind of a head
switch and an interrupt control register. It's usually 0x200 in size.
On fairly recent SoCs (with the starting point seemingly being moving
the OSM programming to the firmware) these two register sets are
contiguous and overlapping, like this (on sm8450):
/* notice how base.start == global_base.start+0x100 */
reg = <0x90b6400 0x300>, <0x90b6300 0x200>;
reg-names = "base", "global_base";
Which led to some confusion and the assumption that since the
"interesting" global registers begin right after global_base+0x100,
there's no need to map two separate regions and one can simply subtract
0x100 from the offsets.
This is however not the case for anything older than SDM845, as the
global region can appear in seemingly random spots on the register map.
Handle the case where the global registers are mapped separately to allow
proper functioning of BWMONv4 on MSM8998 and older. Add specific
compatibles for 845, 8280xp, 7280 and 8550 (all of which use the single
reg space scheme) to keep backwards compatibility with old DTs.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/soc/qcom/icc-bwmon.c | 230 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 209 insertions(+), 21 deletions(-)
diff --git a/drivers/soc/qcom/icc-bwmon.c b/drivers/soc/qcom/icc-bwmon.c
index d45caf512e2d..fd58c5b69897 100644
--- a/drivers/soc/qcom/icc-bwmon.c
+++ b/drivers/soc/qcom/icc-bwmon.c
@@ -34,14 +34,27 @@
/* Internal sampling clock frequency */
#define HW_TIMER_HZ 19200000
-#define BWMON_V4_GLOBAL_IRQ_CLEAR 0x008
-#define BWMON_V4_GLOBAL_IRQ_ENABLE 0x00c
+#define BWMON_V4_GLOBAL_IRQ_CLEAR 0x108
+#define BWMON_V4_GLOBAL_IRQ_ENABLE 0x10c
/*
* All values here and further are matching regmap fields, so without absolute
* register offsets.
*/
#define BWMON_V4_GLOBAL_IRQ_ENABLE_ENABLE BIT(0)
+/*
+ * Starting with SDM845, the BWMON4 register space has changed a bit:
+ * the global registers were jammed into the beginning of the monitor region.
+ * To keep the proper offsets, one would have to map <GLOBAL_BASE 0x200> and
+ * <GLOBAL_BASE+0x100 0x300>, which is straight up wrong.
+ * To facilitate for that, while allowing the older, arguably more proper
+ * implementations to work, offset the global registers by -0x100 to avoid
+ * having to map half of the global registers twice.
+ */
+#define BWMON_V4_845_OFFSET 0x100
+#define BWMON_V4_GLOBAL_IRQ_CLEAR_845 (BWMON_V4_GLOBAL_IRQ_CLEAR - BWMON_V4_845_OFFSET)
+#define BWMON_V4_GLOBAL_IRQ_ENABLE_845 (BWMON_V4_GLOBAL_IRQ_ENABLE - BWMON_V4_845_OFFSET)
+
#define BWMON_V4_IRQ_STATUS 0x100
#define BWMON_V4_IRQ_CLEAR 0x108
@@ -118,9 +131,13 @@
#define BWMON_NEEDS_FORCE_CLEAR BIT(1)
enum bwmon_fields {
+ /* Global region fields, keep them at the top */
F_GLOBAL_IRQ_CLEAR,
F_GLOBAL_IRQ_ENABLE,
- F_IRQ_STATUS,
+ F_NUM_GLOBAL_FIELDS,
+
+ /* Monitor region fields */
+ F_IRQ_STATUS = F_NUM_GLOBAL_FIELDS,
F_IRQ_CLEAR,
F_IRQ_ENABLE,
F_ENABLE,
@@ -157,6 +174,9 @@ struct icc_bwmon_data {
const struct regmap_config *regmap_cfg;
const struct reg_field *regmap_fields;
+
+ const struct regmap_config *global_regmap_cfg;
+ const struct reg_field *global_regmap_fields;
};
struct icc_bwmon {
@@ -165,6 +185,7 @@ struct icc_bwmon {
int irq;
struct regmap_field *regs[F_NUM_FIELDS];
+ struct regmap_field *global_regs[F_NUM_GLOBAL_FIELDS];
unsigned int max_bw_kbps;
unsigned int min_bw_kbps;
@@ -174,8 +195,8 @@ struct icc_bwmon {
/* BWMON v4 */
static const struct reg_field msm8998_bwmon_reg_fields[] = {
- [F_GLOBAL_IRQ_CLEAR] = REG_FIELD(BWMON_V4_GLOBAL_IRQ_CLEAR, 0, 0),
- [F_GLOBAL_IRQ_ENABLE] = REG_FIELD(BWMON_V4_GLOBAL_IRQ_ENABLE, 0, 0),
+ [F_GLOBAL_IRQ_CLEAR] = {},
+ [F_GLOBAL_IRQ_ENABLE] = {},
[F_IRQ_STATUS] = REG_FIELD(BWMON_V4_IRQ_STATUS, 4, 7),
[F_IRQ_CLEAR] = REG_FIELD(BWMON_V4_IRQ_CLEAR, 4, 7),
[F_IRQ_ENABLE] = REG_FIELD(BWMON_V4_IRQ_ENABLE, 4, 7),
@@ -201,7 +222,6 @@ static const struct reg_field msm8998_bwmon_reg_fields[] = {
};
static const struct regmap_range msm8998_bwmon_reg_noread_ranges[] = {
- regmap_reg_range(BWMON_V4_GLOBAL_IRQ_CLEAR, BWMON_V4_GLOBAL_IRQ_CLEAR),
regmap_reg_range(BWMON_V4_IRQ_CLEAR, BWMON_V4_IRQ_CLEAR),
regmap_reg_range(BWMON_V4_CLEAR, BWMON_V4_CLEAR),
};
@@ -221,16 +241,33 @@ static const struct regmap_access_table msm8998_bwmon_reg_volatile_table = {
.n_yes_ranges = ARRAY_SIZE(msm8998_bwmon_reg_volatile_ranges),
};
+static const struct reg_field msm8998_bwmon_global_reg_fields[] = {
+ [F_GLOBAL_IRQ_CLEAR] = REG_FIELD(BWMON_V4_GLOBAL_IRQ_CLEAR, 0, 0),
+ [F_GLOBAL_IRQ_ENABLE] = REG_FIELD(BWMON_V4_GLOBAL_IRQ_ENABLE, 0, 0),
+};
+
+static const struct regmap_range msm8998_bwmon_global_reg_noread_ranges[] = {
+ regmap_reg_range(BWMON_V4_GLOBAL_IRQ_CLEAR, BWMON_V4_GLOBAL_IRQ_CLEAR),
+};
+
+static const struct regmap_access_table msm8998_bwmon_global_reg_read_table = {
+ .no_ranges = msm8998_bwmon_global_reg_noread_ranges,
+ .n_no_ranges = ARRAY_SIZE(msm8998_bwmon_global_reg_noread_ranges),
+};
+
/*
* Fill the cache for non-readable registers only as rest does not really
* matter and can be read from the device.
*/
static const struct reg_default msm8998_bwmon_reg_defaults[] = {
- { BWMON_V4_GLOBAL_IRQ_CLEAR, 0x0 },
{ BWMON_V4_IRQ_CLEAR, 0x0 },
{ BWMON_V4_CLEAR, 0x0 },
};
+static const struct reg_default msm8998_bwmon_global_reg_defaults[] = {
+ { BWMON_V4_GLOBAL_IRQ_CLEAR, 0x0 },
+};
+
static const struct regmap_config msm8998_bwmon_regmap_cfg = {
.reg_bits = 32,
.reg_stride = 4,
@@ -251,6 +288,93 @@ static const struct regmap_config msm8998_bwmon_regmap_cfg = {
.cache_type = REGCACHE_RBTREE,
};
+static const struct regmap_config msm8998_bwmon_global_regmap_cfg = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ /*
+ * No concurrent access expected - driver has one interrupt handler,
+ * regmap is not shared, no driver or user-space API.
+ */
+ .disable_locking = true,
+ .rd_table = &msm8998_bwmon_global_reg_read_table,
+ .reg_defaults = msm8998_bwmon_global_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(msm8998_bwmon_global_reg_defaults),
+ /*
+ * Cache is necessary for using regmap fields with non-readable
+ * registers.
+ */
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static const struct reg_field sdm845_cpu_bwmon_reg_fields[] = {
+ [F_GLOBAL_IRQ_CLEAR] = REG_FIELD(BWMON_V4_GLOBAL_IRQ_CLEAR_845, 0, 0),
+ [F_GLOBAL_IRQ_ENABLE] = REG_FIELD(BWMON_V4_GLOBAL_IRQ_ENABLE_845, 0, 0),
+ [F_IRQ_STATUS] = REG_FIELD(BWMON_V4_IRQ_STATUS, 4, 7),
+ [F_IRQ_CLEAR] = REG_FIELD(BWMON_V4_IRQ_CLEAR, 4, 7),
+ [F_IRQ_ENABLE] = REG_FIELD(BWMON_V4_IRQ_ENABLE, 4, 7),
+ /* F_ENABLE covers entire register to disable other features */
+ [F_ENABLE] = REG_FIELD(BWMON_V4_ENABLE, 0, 31),
+ [F_CLEAR] = REG_FIELD(BWMON_V4_CLEAR, 0, 1),
+ [F_SAMPLE_WINDOW] = REG_FIELD(BWMON_V4_SAMPLE_WINDOW, 0, 23),
+ [F_THRESHOLD_HIGH] = REG_FIELD(BWMON_V4_THRESHOLD_HIGH, 0, 11),
+ [F_THRESHOLD_MED] = REG_FIELD(BWMON_V4_THRESHOLD_MED, 0, 11),
+ [F_THRESHOLD_LOW] = REG_FIELD(BWMON_V4_THRESHOLD_LOW, 0, 11),
+ [F_ZONE_ACTIONS_ZONE0] = REG_FIELD(BWMON_V4_ZONE_ACTIONS, 0, 7),
+ [F_ZONE_ACTIONS_ZONE1] = REG_FIELD(BWMON_V4_ZONE_ACTIONS, 8, 15),
+ [F_ZONE_ACTIONS_ZONE2] = REG_FIELD(BWMON_V4_ZONE_ACTIONS, 16, 23),
+ [F_ZONE_ACTIONS_ZONE3] = REG_FIELD(BWMON_V4_ZONE_ACTIONS, 24, 31),
+ [F_THRESHOLD_COUNT_ZONE0] = REG_FIELD(BWMON_V4_THRESHOLD_COUNT, 0, 7),
+ [F_THRESHOLD_COUNT_ZONE1] = REG_FIELD(BWMON_V4_THRESHOLD_COUNT, 8, 15),
+ [F_THRESHOLD_COUNT_ZONE2] = REG_FIELD(BWMON_V4_THRESHOLD_COUNT, 16, 23),
+ [F_THRESHOLD_COUNT_ZONE3] = REG_FIELD(BWMON_V4_THRESHOLD_COUNT, 24, 31),
+ [F_ZONE0_MAX] = REG_FIELD(BWMON_V4_ZONE_MAX(0), 0, 11),
+ [F_ZONE1_MAX] = REG_FIELD(BWMON_V4_ZONE_MAX(1), 0, 11),
+ [F_ZONE2_MAX] = REG_FIELD(BWMON_V4_ZONE_MAX(2), 0, 11),
+ [F_ZONE3_MAX] = REG_FIELD(BWMON_V4_ZONE_MAX(3), 0, 11),
+};
+
+static const struct regmap_range sdm845_cpu_bwmon_reg_noread_ranges[] = {
+ regmap_reg_range(BWMON_V4_GLOBAL_IRQ_CLEAR_845, BWMON_V4_GLOBAL_IRQ_CLEAR_845),
+ regmap_reg_range(BWMON_V4_IRQ_CLEAR, BWMON_V4_IRQ_CLEAR),
+ regmap_reg_range(BWMON_V4_CLEAR, BWMON_V4_CLEAR),
+};
+
+static const struct regmap_access_table sdm845_cpu_bwmon_reg_read_table = {
+ .no_ranges = sdm845_cpu_bwmon_reg_noread_ranges,
+ .n_no_ranges = ARRAY_SIZE(sdm845_cpu_bwmon_reg_noread_ranges),
+};
+
+/*
+ * Fill the cache for non-readable registers only as rest does not really
+ * matter and can be read from the device.
+ */
+static const struct reg_default sdm845_cpu_bwmon_reg_defaults[] = {
+ { BWMON_V4_GLOBAL_IRQ_CLEAR_845, 0x0 },
+ { BWMON_V4_IRQ_CLEAR, 0x0 },
+ { BWMON_V4_CLEAR, 0x0 },
+};
+
+static const struct regmap_config sdm845_cpu_bwmon_regmap_cfg = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ /*
+ * No concurrent access expected - driver has one interrupt handler,
+ * regmap is not shared, no driver or user-space API.
+ */
+ .disable_locking = true,
+ .rd_table = &sdm845_cpu_bwmon_reg_read_table,
+ .volatile_table = &msm8998_bwmon_reg_volatile_table,
+ .reg_defaults = sdm845_cpu_bwmon_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(sdm845_cpu_bwmon_reg_defaults),
+ /*
+ * Cache is necessary for using regmap fields with non-readable
+ * registers.
+ */
+ .cache_type = REGCACHE_RBTREE,
+};
+
/* BWMON v5 */
static const struct reg_field sdm845_llcc_bwmon_reg_fields[] = {
[F_GLOBAL_IRQ_CLEAR] = {},
@@ -349,6 +473,13 @@ static void bwmon_clear_counters(struct icc_bwmon *bwmon, bool clear_all)
static void bwmon_clear_irq(struct icc_bwmon *bwmon)
{
+ struct regmap_field *global_irq_clr;
+
+ if (bwmon->data->global_regmap_fields)
+ global_irq_clr = bwmon->global_regs[F_GLOBAL_IRQ_CLEAR];
+ else
+ global_irq_clr = bwmon->regs[F_GLOBAL_IRQ_CLEAR];
+
/*
* Clear zone and global interrupts. The order and barriers are
* important. Quoting downstream Qualcomm msm-4.9 tree:
@@ -369,15 +500,22 @@ static void bwmon_clear_irq(struct icc_bwmon *bwmon)
if (bwmon->data->quirks & BWMON_NEEDS_FORCE_CLEAR)
regmap_field_force_write(bwmon->regs[F_IRQ_CLEAR], 0);
if (bwmon->data->quirks & BWMON_HAS_GLOBAL_IRQ)
- regmap_field_force_write(bwmon->regs[F_GLOBAL_IRQ_CLEAR],
+ regmap_field_force_write(global_irq_clr,
BWMON_V4_GLOBAL_IRQ_ENABLE_ENABLE);
}
static void bwmon_disable(struct icc_bwmon *bwmon)
{
+ struct regmap_field *global_irq_en;
+
+ if (bwmon->data->global_regmap_fields)
+ global_irq_en = bwmon->global_regs[F_GLOBAL_IRQ_ENABLE];
+ else
+ global_irq_en = bwmon->regs[F_GLOBAL_IRQ_ENABLE];
+
/* Disable interrupts. Strict ordering, see bwmon_clear_irq(). */
if (bwmon->data->quirks & BWMON_HAS_GLOBAL_IRQ)
- regmap_field_write(bwmon->regs[F_GLOBAL_IRQ_ENABLE], 0x0);
+ regmap_field_write(global_irq_en, 0x0);
regmap_field_write(bwmon->regs[F_IRQ_ENABLE], 0x0);
/*
@@ -389,10 +527,18 @@ static void bwmon_disable(struct icc_bwmon *bwmon)
static void bwmon_enable(struct icc_bwmon *bwmon, unsigned int irq_enable)
{
+ struct regmap_field *global_irq_en;
+
+ if (bwmon->data->global_regmap_fields)
+ global_irq_en = bwmon->global_regs[F_GLOBAL_IRQ_ENABLE];
+ else
+ global_irq_en = bwmon->regs[F_GLOBAL_IRQ_ENABLE];
+
/* Enable interrupts */
if (bwmon->data->quirks & BWMON_HAS_GLOBAL_IRQ)
- regmap_field_write(bwmon->regs[F_GLOBAL_IRQ_ENABLE],
+ regmap_field_write(global_irq_en,
BWMON_V4_GLOBAL_IRQ_ENABLE_ENABLE);
+
regmap_field_write(bwmon->regs[F_IRQ_ENABLE], irq_enable);
/* Enable bwmon */
@@ -555,7 +701,9 @@ static int bwmon_init_regmap(struct platform_device *pdev,
struct device *dev = &pdev->dev;
void __iomem *base;
struct regmap *map;
+ int ret;
+ /* Map the monitor base */
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return dev_err_probe(dev, PTR_ERR(base),
@@ -566,12 +714,35 @@ static int bwmon_init_regmap(struct platform_device *pdev,
return dev_err_probe(dev, PTR_ERR(map),
"failed to initialize regmap\n");
+ BUILD_BUG_ON(ARRAY_SIZE(msm8998_bwmon_global_reg_fields) != F_NUM_GLOBAL_FIELDS);
BUILD_BUG_ON(ARRAY_SIZE(msm8998_bwmon_reg_fields) != F_NUM_FIELDS);
+ BUILD_BUG_ON(ARRAY_SIZE(sdm845_cpu_bwmon_reg_fields) != F_NUM_FIELDS);
BUILD_BUG_ON(ARRAY_SIZE(sdm845_llcc_bwmon_reg_fields) != F_NUM_FIELDS);
- return devm_regmap_field_bulk_alloc(dev, map, bwmon->regs,
+ ret = devm_regmap_field_bulk_alloc(dev, map, bwmon->regs,
bwmon->data->regmap_fields,
F_NUM_FIELDS);
+ if (ret)
+ return ret;
+
+ if (bwmon->data->global_regmap_cfg) {
+ /* Map the global base, if separate */
+ base = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(base))
+ return dev_err_probe(dev, PTR_ERR(base),
+ "failed to map bwmon global registers\n");
+
+ map = devm_regmap_init_mmio(dev, base, bwmon->data->global_regmap_cfg);
+ if (IS_ERR(map))
+ return dev_err_probe(dev, PTR_ERR(map),
+ "failed to initialize global regmap\n");
+
+ ret = devm_regmap_field_bulk_alloc(dev, map, bwmon->global_regs,
+ bwmon->data->global_regmap_fields,
+ F_NUM_GLOBAL_FIELDS);
+ }
+
+ return ret;
}
static int bwmon_probe(struct platform_device *pdev)
@@ -644,6 +815,21 @@ static const struct icc_bwmon_data msm8998_bwmon_data = {
.quirks = BWMON_HAS_GLOBAL_IRQ,
.regmap_fields = msm8998_bwmon_reg_fields,
.regmap_cfg = &msm8998_bwmon_regmap_cfg,
+ .global_regmap_fields = msm8998_bwmon_global_reg_fields,
+ .global_regmap_cfg = &msm8998_bwmon_global_regmap_cfg,
+};
+
+static const struct icc_bwmon_data sdm845_cpu_bwmon_data = {
+ .sample_ms = 4,
+ .count_unit_kb = 64,
+ .default_highbw_kbps = 4800 * 1024, /* 4.8 GBps */
+ .default_medbw_kbps = 512 * 1024, /* 512 MBps */
+ .default_lowbw_kbps = 0,
+ .zone1_thres_count = 16,
+ .zone3_thres_count = 1,
+ .quirks = BWMON_HAS_GLOBAL_IRQ,
+ .regmap_fields = sdm845_cpu_bwmon_reg_fields,
+ .regmap_cfg = &sdm845_cpu_bwmon_regmap_cfg,
};
static const struct icc_bwmon_data sdm845_llcc_bwmon_data = {
@@ -672,16 +858,18 @@ static const struct icc_bwmon_data sc7280_llcc_bwmon_data = {
};
static const struct of_device_id bwmon_of_match[] = {
- {
- .compatible = "qcom,msm8998-bwmon",
- .data = &msm8998_bwmon_data
- }, {
- .compatible = "qcom,sdm845-llcc-bwmon",
- .data = &sdm845_llcc_bwmon_data
- }, {
- .compatible = "qcom,sc7280-llcc-bwmon",
- .data = &sc7280_llcc_bwmon_data
- },
+ /* BWMONv4, separate monitor and global register spaces */
+ { .compatible = "qcom,msm8998-bwmon", .data = &msm8998_bwmon_data },
+ /* BWMONv4, unified register space */
+ { .compatible = "qcom,sdm845-bwmon", .data = &sdm845_cpu_bwmon_data },
+ /* BWMONv5 */
+ { .compatible = "qcom,sdm845-llcc-bwmon", .data = &sdm845_llcc_bwmon_data },
+ { .compatible = "qcom,sc7280-llcc-bwmon", .data = &sc7280_llcc_bwmon_data },
+
+ /* Compatibles kept for legacy reasons */
+ { .compatible = "qcom,sc7280-cpu-bwmon", .data = &sdm845_cpu_bwmon_data },
+ { .compatible = "qcom,sc8280xp-cpu-bwmon", .data = &sdm845_cpu_bwmon_data },
+ { .compatible = "qcom,sm8550-cpu-bwmon", .data = &sdm845_cpu_bwmon_data },
{}
};
MODULE_DEVICE_TABLE(of, bwmon_of_match);
--
2.39.2
Use the correct fallback compatible for the BWMONv4 with merged global and
monitor register spaces.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 8f4ab6bd2886..f15fea6cc316 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3533,7 +3533,7 @@ opp-7 {
};
pmu@90b6400 {
- compatible = "qcom,sc7280-cpu-bwmon", "qcom,msm8998-bwmon";
+ compatible = "qcom,sc7280-cpu-bwmon", "qcom,sdm845-bwmon";
reg = <0 0x090b6400 0 0x600>;
interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
--
2.39.2
Use the correct fallback compatible for the BWMONv4 with merged global and
monitor register spaces.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 0d02599d8867..131b99bfe771 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -2946,7 +2946,7 @@ opp-12 {
};
pmu@90b6400 {
- compatible = "qcom,sc8280xp-cpu-bwmon", "qcom,msm8998-bwmon";
+ compatible = "qcom,sc8280xp-cpu-bwmon", "qcom,sdm845-bwmon";
reg = <0 0x090b6400 0 0x600>;
interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
--
2.39.2
Drop the incorrect msm8998 fallback and use the new qcom,sdm845-cpu-bwmon
compatible to distinguish the CPU BWMON found on this platform.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 479859bd8ab3..1f2a97a20ef3 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2241,7 +2241,7 @@ opp-4 {
};
pmu@1436400 {
- compatible = "qcom,sdm845-bwmon", "qcom,msm8998-bwmon";
+ compatible = "qcom,sdm845-cpu-bwmon", "qcom,sdm845-bwmon";
reg = <0 0x01436400 0 0x600>;
interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
interconnects = <&gladiator_noc MASTER_APPSS_PROC 3 &mem_noc SLAVE_LLCC 3>;
--
2.39.2
Use the correct fallback compatible for the BWMONv4 with merged global and
monitor register spaces.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 25f51245fe9b..b5488c6822bd 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3391,7 +3391,7 @@ opp-8 {
};
pmu@240b6400 {
- compatible = "qcom,sm8550-cpu-bwmon", "qcom,msm8998-bwmon";
+ compatible = "qcom,sm8550-cpu-bwmon", "qcom,sdm845-bwmon";
reg = <0 0x240b6400 0 0x600>;
interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
interconnects = <&gem_noc MASTER_APPSS_PROC 3 &gem_noc SLAVE_LLCC 3>;
--
2.39.2
On 15/03/2023 15:11, Konrad Dybcio wrote:
> The BWMON hardware has two sets of registers: one for the monitor itself
> and one called "global". It has what seems to be some kind of a head
> switch and an interrupt control register. It's usually 0x200 in size.
>
> On fairly recent SoCs (with the starting point seemingly being moving
> the OSM programming to the firmware) these two register sets are
> contiguous and overlapping, like this (on sm8450):
>
> /* notice how base.start == global_base.start+0x100 */
> reg = <0x90b6400 0x300>, <0x90b6300 0x200>;
> reg-names = "base", "global_base";
>
> Which led to some confusion and the assumption that since the
> "interesting" global registers begin right after global_base+0x100,
> there's no need to map two separate regions and one can simply subtract
> 0x100 from the offsets.
>
> This is however not the case for anything older than SDM845, as the
> global region can appear in seemingly random spots on the register map.
>
> Handle the case where the global registers are mapped separately to allow
> proper functioning of BWMONv4 on MSM8998 and older. Add specific
> compatibles for 845, 8280xp, 7280 and 8550 (all of which use the single
> reg space scheme) to keep backwards compatibility with old DTs.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/soc/qcom/icc-bwmon.c | 230 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 209 insertions(+), 21 deletions(-)
>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 15/03/2023 15:11, Konrad Dybcio wrote:
> Use the correct fallback compatible for the BWMONv4 with merged global and
> monitor register spaces.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sm8550.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index 25f51245fe9b..b5488c6822bd 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -3391,7 +3391,7 @@ opp-8 {
> };
>
> pmu@240b6400 {
> - compatible = "qcom,sm8550-cpu-bwmon", "qcom,msm8998-bwmon";
> + compatible = "qcom,sm8550-cpu-bwmon", "qcom,sdm845-bwmon";
> reg = <0 0x240b6400 0 0x600>;
> interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
> interconnects = <&gem_noc MASTER_APPSS_PROC 3 &gem_noc SLAVE_LLCC 3>;
>
I can't test right now, but:
Reviewed-by: Neil Armstrong <[email protected]>
On Wed, Mar 15, 2023 at 03:11:19PM +0100, Konrad Dybcio wrote:
> BWMONv4 has two sets of registers: one for handling the monitor itself
> and one called "global" which hosts some sort of a headswitch and an
> interrupt control register. We did not handle that one before, as on
> SoCs starting with SDM845 they have been merged into a single contiguous
> range.
>
> To make the qcom,msm8998-bwmon less confusing and in preparation for
> actual MSM8998 support, describe the global register space and introduce
> new "qcom,sdm845-cpu-bwmon" compatible while keeping the
> "qcom,sdm845-bwmon" as a fallback for SoCs with this merged register space
> scheme.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>
Georgi, can you please pick this patch through your tree?
Regards,
Bjorn
> ---
> .../bindings/interconnect/qcom,msm8998-bwmon.yaml | 41 ++++++++++++++++++----
> 1 file changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
> index 12a0d3ecbabb..5d17bdcfdf70 100644
> --- a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
> @@ -22,14 +22,14 @@ description: |
> properties:
> compatible:
> oneOf:
> + - const: qcom,msm8998-bwmon # BWMON v4
> - items:
> - enum:
> - qcom,sc7280-cpu-bwmon
> - qcom,sc8280xp-cpu-bwmon
> - - qcom,sdm845-bwmon
> + - qcom,sdm845-cpu-bwmon
> - qcom,sm8550-cpu-bwmon
> - - const: qcom,msm8998-bwmon
> - - const: qcom,msm8998-bwmon # BWMON v4
> + - const: qcom,sdm845-bwmon # BWMON v4, unified register space
> - items:
> - enum:
> - qcom,sc8280xp-llcc-bwmon
> @@ -49,9 +49,13 @@ properties:
> type: object
>
> reg:
> - # BWMON v4 (currently described) and BWMON v5 use one register address
> - # space. BWMON v2 uses two register spaces - not yet described.
> - maxItems: 1
> + # BWMON v5 uses one register address space, v1-v4 use one or two.
> + minItems: 1
> + maxItems: 2
> +
> + reg-names:
> + minItems: 1
> + maxItems: 2
>
> required:
> - compatible
> @@ -63,13 +67,36 @@ required:
>
> additionalProperties: false
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + const: qcom,msm8998-bwmon
> + then:
> + properties:
> + reg:
> + minItems: 2
> +
> + reg-names:
> + items:
> + - const: monitor
> + - const: global
> +
> + else:
> + properties:
> + reg:
> + maxItems: 1
> +
> + reg-names:
> + maxItems: 1
> +
> examples:
> - |
> #include <dt-bindings/interconnect/qcom,sdm845.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
>
> pmu@1436400 {
> - compatible = "qcom,sdm845-bwmon", "qcom,msm8998-bwmon";
> + compatible = "qcom,sdm845-cpu-bwmon", "qcom,sdm845-bwmon";
> reg = <0x01436400 0x600>;
> interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
> interconnects = <&gladiator_noc MASTER_APPSS_PROC 3 &mem_noc SLAVE_LLCC 3>;
>
> --
> 2.39.2
>
On 4.04.23 22:35, Bjorn Andersson wrote:
> On Wed, Mar 15, 2023 at 03:11:19PM +0100, Konrad Dybcio wrote:
>> BWMONv4 has two sets of registers: one for handling the monitor itself
>> and one called "global" which hosts some sort of a headswitch and an
>> interrupt control register. We did not handle that one before, as on
>> SoCs starting with SDM845 they have been merged into a single contiguous
>> range.
>>
>> To make the qcom,msm8998-bwmon less confusing and in preparation for
>> actual MSM8998 support, describe the global register space and introduce
>> new "qcom,sdm845-cpu-bwmon" compatible while keeping the
>> "qcom,sdm845-bwmon" as a fallback for SoCs with this merged register space
>> scheme.
>>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>
> Georgi, can you please pick this patch through your tree?
Yes, sure!
BR,
Georgi
> Regards,
> Bjorn
>
>> ---
>> .../bindings/interconnect/qcom,msm8998-bwmon.yaml | 41 ++++++++++++++++++----
>> 1 file changed, 34 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
>> index 12a0d3ecbabb..5d17bdcfdf70 100644
>> --- a/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8998-bwmon.yaml
>> @@ -22,14 +22,14 @@ description: |
>> properties:
>> compatible:
>> oneOf:
>> + - const: qcom,msm8998-bwmon # BWMON v4
>> - items:
>> - enum:
>> - qcom,sc7280-cpu-bwmon
>> - qcom,sc8280xp-cpu-bwmon
>> - - qcom,sdm845-bwmon
>> + - qcom,sdm845-cpu-bwmon
>> - qcom,sm8550-cpu-bwmon
>> - - const: qcom,msm8998-bwmon
>> - - const: qcom,msm8998-bwmon # BWMON v4
>> + - const: qcom,sdm845-bwmon # BWMON v4, unified register space
>> - items:
>> - enum:
>> - qcom,sc8280xp-llcc-bwmon
>> @@ -49,9 +49,13 @@ properties:
>> type: object
>>
>> reg:
>> - # BWMON v4 (currently described) and BWMON v5 use one register address
>> - # space. BWMON v2 uses two register spaces - not yet described.
>> - maxItems: 1
>> + # BWMON v5 uses one register address space, v1-v4 use one or two.
>> + minItems: 1
>> + maxItems: 2
>> +
>> + reg-names:
>> + minItems: 1
>> + maxItems: 2
>>
>> required:
>> - compatible
>> @@ -63,13 +67,36 @@ required:
>>
>> additionalProperties: false
>>
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + const: qcom,msm8998-bwmon
>> + then:
>> + properties:
>> + reg:
>> + minItems: 2
>> +
>> + reg-names:
>> + items:
>> + - const: monitor
>> + - const: global
>> +
>> + else:
>> + properties:
>> + reg:
>> + maxItems: 1
>> +
>> + reg-names:
>> + maxItems: 1
>> +
>> examples:
>> - |
>> #include <dt-bindings/interconnect/qcom,sdm845.h>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>>
>> pmu@1436400 {
>> - compatible = "qcom,sdm845-bwmon", "qcom,msm8998-bwmon";
>> + compatible = "qcom,sdm845-cpu-bwmon", "qcom,sdm845-bwmon";
>> reg = <0x01436400 0x600>;
>> interrupts = <GIC_SPI 581 IRQ_TYPE_LEVEL_HIGH>;
>> interconnects = <&gladiator_noc MASTER_APPSS_PROC 3 &mem_noc SLAVE_LLCC 3>;
>>
>> --
>> 2.39.2
>>
On Wed, 15 Mar 2023 15:11:18 +0100, Konrad Dybcio wrote:
> v2 -> v3:
> - Rename: "DDR BWMON" -> "CPU BWMON" [2, 6/7]
> - Set F_IRQ_STATUS = F_NUM_GLOBAL_FIELDS in enum bwmon_fields to save
> one space in the enum
> - Correct the struct icc_bwmon->global_regs array size
> - Reorder the "Remove unused struct member" patch to come before the big
> one
>
> [...]
Applied, thanks!
[4/7] arm64: dts: qcom: sc7280: Use the correct BWMON fallback compatible
commit: bad26511c4cb5469545834886e8df1d2b4c117d8
[5/7] arm64: dts: qcom: sc8280xp: Use the correct BWMON fallback compatible
commit: 5e1b11c00ffc0a2d105af5a92053fa10ca15fcd0
[6/7] arm64: dts: qcom: sdm845: Use the correct BWMON compatible
commit: e95b60f1207dee16e0d3d66254ece655fd4e73c0
[7/7] arm64: dts: qcom: sm8550: Use the correct BWMON fallback compatible
commit: feffd767971e25f43ba47bf2ab23a660f3a3758c
Best regards,
--
Bjorn Andersson <[email protected]>