Quoting Rajendra Nayak (2018-12-11 01:49:34) > The RPMh power domain driver aggregates the corner votes from various > consumers for the ARC resources and communicates it to RPMh. > > With RPMh we use 2 different numbering space for corners, one used > by the clients to express their performance needs, and another used > to communicate to RPMh hardware. > > The clients express their performance requirements using a sparse > numbering space which are mapped to meaningful levels like RET, SVS, > NOMINAL, TURBO etc which then get mapped to another number space > between 0 and 15 which is communicated to RPMh. The sparse number space, > also referred to as vlvl is mapped to the continuous number space of 0 > to 15, also referred to as hlvl, using command DB. > > Some power domain clients could request a performance state only while > the CPU is active, while some others could request for a certain > performance state all the time regardless of the state of the CPU. > We handle this by internally aggregating the votes from both type of > clients and then send the aggregated votes to RPMh. > > There are also 3 different types of Votes that are comunicated to RPMh Why capitalize vote? > for every resource. > 1. ACTIVE_ONLY: This specifies the requirement for the resource when the > CPU is active > 2. SLEEP: This specifies the requirement for the resource when the CPU > is going to sleep > 3. WAKE_ONLY: This specifies the requirement for the resource when the > CPU is coming out of sleep to active state Can you tab these in? > > We add data for all power domains on sdm845 SoC as part of the patch. > The driver can be extended to support other SoCs which support RPMh > > Signed-off-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> > Reviewed-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> Just minor nitpicks ahead and a warning. > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile > index f1b25fdcf2ad..dd6ca92985ee 100644 > --- a/drivers/soc/qcom/Makefile > +++ b/drivers/soc/qcom/Makefile > @@ -22,3 +22,4 @@ obj-$(CONFIG_QCOM_APR) += apr.o > obj-$(CONFIG_QCOM_LLCC) += llcc-slice.o > obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o > obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o > +obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o Put this before RPMPD? At least it would be semi-sorted then. > diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c > new file mode 100644 > index 000000000000..f993a86be48c > --- /dev/null > +++ b/drivers/soc/qcom/rpmhpd.c > @@ -0,0 +1,417 @@ [..] > + > +static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd) > +{ > + u8 buf[RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE]; > + int i, j, len, ret; > + > + len = cmd_db_read_aux_data_len(rpmhpd->res_name); > + if (len <= 0) > + return len; > + else if (len > RPMH_ARC_MAX_LEVELS * RPMH_ARC_LEVEL_SIZE) > + return -EINVAL; > + > + ret = cmd_db_read_aux_data(rpmhpd->res_name, buf, len); > + if (ret < 0) > + return ret; I've changed cmd_db_read_aux_data() and that change is winding through the arm-soc tree. This will break in the compilation phase in linux-next. > + > + rpmhpd->level_count = len / RPMH_ARC_LEVEL_SIZE; > + > + for (i = 0; i < rpmhpd->level_count; i++) { > + rpmhpd->level[i] = 0; > + for (j = 0; j < RPMH_ARC_LEVEL_SIZE; j++) > + rpmhpd->level[i] |= > + buf[i * RPMH_ARC_LEVEL_SIZE + j] << (8 * j); > + > + /* > + * The AUX data may be zero padded. These 0 valued entries at > + * the end of the map must be ignored. > + */ > + if (i > 0 && rpmhpd->level[i] == 0) { > + rpmhpd->level_count = i; > + break; > + } > + pr_debug("%s: ARC hlvl=%2d --> vlvl=%4u\n", rpmhpd->res_name, i, > + rpmhpd->level[i]); > + } > + > + return 0; > +} > + > +static int rpmhpd_probe(struct platform_device *pdev) > +{ > + int i, ret; > + size_t num_pds; > + struct device *dev = &pdev->dev; > + struct genpd_onecell_data *data; > + struct rpmhpd **rpmhpds; > + const struct rpmhpd_desc *desc; > + > + desc = of_device_get_match_data(dev); > + if (!desc) > + return -EINVAL; > + > + rpmhpds = desc->rpmhpds; > + num_pds = desc->num_pds; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->domains = devm_kcalloc(dev, num_pds, sizeof(*data->domains), > + GFP_KERNEL); > + if (!data->domains) > + return -ENOMEM; > + > + data->num_domains = num_pds; > + > + ret = cmd_db_ready(); Does this matter? I thought we forced cmd_db_ready() in the rpmh driver probe, and that was the parent of all rpmh devices so we should be fine to not need to check it again here? > + if (ret) { > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Command DB unavailable, ret=%d\n", ret); > + return ret; > + } > +