| Message ID | 1619508824-14413-5-git-send-email-sibis@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | Enable miscellaneous hardware blocks to boot WPSS | expand |
Hi Sibi, On Tue, 2021-04-27 at 13:03 +0530, Sibi Sankar wrote: > Add PDC Global reset signals for Wireless Processor Subsystem (WPSS) > on SC7280 SoCs. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > > v2: > * place resets and num_resets adjacent to each other [Stephen] [...] > +struct qcom_pdc_reset_desc { > + const struct qcom_pdc_reset_map *resets; > + size_t num_resets; > + unsigned int offset; > +}; [...] For consistency, please do the same here: > +static const struct qcom_pdc_reset_desc sdm845_pdc_reset_desc = { > + .resets = sdm845_pdc_resets, > + .offset = RPMH_SDM845_PDC_SYNC_RESET, > + .num_resets = ARRAY_SIZE(sdm845_pdc_resets), > +}; [...] and here: > +static const struct qcom_pdc_reset_desc sc7280_pdc_reset_desc = { > + .resets = sc7280_pdc_resets, > + .offset = RPMH_SC7280_PDC_SYNC_RESET, > + .num_resets = ARRAY_SIZE(sc7280_pdc_resets), > +}; [...] > @@ -54,19 +89,18 @@ static int qcom_pdc_control_assert(struct reset_controller_dev *rcdev, > unsigned long idx) > { > struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev); > + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx]; > > - return regmap_update_bits(data->regmap, RPMH_PDC_SYNC_RESET, > - BIT(sdm845_pdc_resets[idx].bit), > - BIT(sdm845_pdc_resets[idx].bit)); > + return regmap_update_bits(data->regmap, data->desc->offset, BIT(map->bit), BIT(map->bit)); > } Why not go one step further: u32 mask = BIT(data->desc->resets[idx].bit); return regmap_update_bits(data->regmap, data->desc->offset, mask, mask); That seems to be a common pattern in other qcom drivers. Either way, with the above reset/num_reset changes: Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> Also, Acked-by: Philipp Zabel <p.zabel@pengutronix.de> for the whole series to go through the qcom tree, or let me know if you want me to pick up patches 2-4 next round. regards Philipp
Hey Philipp, Thanks for the review. Will get them fixed in the next re-spin. On 2021-04-27 13:28, Philipp Zabel wrote: > Hi Sibi, > > On Tue, 2021-04-27 at 13:03 +0530, Sibi Sankar wrote: >> Add PDC Global reset signals for Wireless Processor Subsystem (WPSS) >> on SC7280 SoCs. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> >> v2: >> * place resets and num_resets adjacent to each other [Stephen] > [...] >> +struct qcom_pdc_reset_desc { >> + const struct qcom_pdc_reset_map *resets; >> + size_t num_resets; >> + unsigned int offset; >> +}; > [...] > > For consistency, please do the same here: > >> +static const struct qcom_pdc_reset_desc sdm845_pdc_reset_desc = { >> + .resets = sdm845_pdc_resets, >> + .offset = RPMH_SDM845_PDC_SYNC_RESET, >> + .num_resets = ARRAY_SIZE(sdm845_pdc_resets), >> +}; > [...] > > and here: > >> +static const struct qcom_pdc_reset_desc sc7280_pdc_reset_desc = { >> + .resets = sc7280_pdc_resets, >> + .offset = RPMH_SC7280_PDC_SYNC_RESET, >> + .num_resets = ARRAY_SIZE(sc7280_pdc_resets), >> +}; > > [...] >> @@ -54,19 +89,18 @@ static int qcom_pdc_control_assert(struct >> reset_controller_dev *rcdev, >> unsigned long idx) >> { >> struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev); >> + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx]; >> >> - return regmap_update_bits(data->regmap, RPMH_PDC_SYNC_RESET, >> - BIT(sdm845_pdc_resets[idx].bit), >> - BIT(sdm845_pdc_resets[idx].bit)); >> + return regmap_update_bits(data->regmap, data->desc->offset, >> BIT(map->bit), BIT(map->bit)); >> } > > Why not go one step further: > > u32 mask = BIT(data->desc->resets[idx].bit); > > return regmap_update_bits(data->regmap, data->desc->offset, mask, > mask); > > That seems to be a common pattern in other qcom drivers. will send out a separate patch for the other reset driver. > Either way, with the above reset/num_reset changes: > > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> > > Also, > > Acked-by: Philipp Zabel <p.zabel@pengutronix.de> > > for the whole series to go through the qcom tree, or let me know if you > want me to pick up patches 2-4 next round. > > regards > Philipp -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On Tue 27 Apr 02:58 CDT 2021, Philipp Zabel wrote: > Hi Sibi, > > On Tue, 2021-04-27 at 13:03 +0530, Sibi Sankar wrote: [..] > Acked-by: Philipp Zabel <p.zabel@pengutronix.de> > > for the whole series to go through the qcom tree, or let me know if you > want me to pick up patches 2-4 next round. > Philipp, please do take patch 2-4 through your tree, that way we avoid any potential conflicts in the driver - and things will come together nicely for validation in linux-next anyways. Regards, Bjorn
diff --git a/drivers/reset/reset-qcom-pdc.c b/drivers/reset/reset-qcom-pdc.c index ab74bccd4a5b..c10393f3ef8c 100644 --- a/drivers/reset/reset-qcom-pdc.c +++ b/drivers/reset/reset-qcom-pdc.c @@ -11,18 +11,26 @@ #include <dt-bindings/reset/qcom,sdm845-pdc.h> -#define RPMH_PDC_SYNC_RESET 0x100 +#define RPMH_SDM845_PDC_SYNC_RESET 0x100 +#define RPMH_SC7280_PDC_SYNC_RESET 0x1000 struct qcom_pdc_reset_map { u8 bit; }; +struct qcom_pdc_reset_desc { + const struct qcom_pdc_reset_map *resets; + size_t num_resets; + unsigned int offset; +}; + struct qcom_pdc_reset_data { struct reset_controller_dev rcdev; struct regmap *regmap; + const struct qcom_pdc_reset_desc *desc; }; -static const struct regmap_config sdm845_pdc_regmap_config = { +static const struct regmap_config pdc_regmap_config = { .name = "pdc-reset", .reg_bits = 32, .reg_stride = 4, @@ -44,6 +52,33 @@ static const struct qcom_pdc_reset_map sdm845_pdc_resets[] = { [PDC_MODEM_SYNC_RESET] = {9}, }; +static const struct qcom_pdc_reset_desc sdm845_pdc_reset_desc = { + .resets = sdm845_pdc_resets, + .offset = RPMH_SDM845_PDC_SYNC_RESET, + .num_resets = ARRAY_SIZE(sdm845_pdc_resets), +}; + +static const struct qcom_pdc_reset_map sc7280_pdc_resets[] = { + [PDC_APPS_SYNC_RESET] = {0}, + [PDC_SP_SYNC_RESET] = {1}, + [PDC_AUDIO_SYNC_RESET] = {2}, + [PDC_SENSORS_SYNC_RESET] = {3}, + [PDC_AOP_SYNC_RESET] = {4}, + [PDC_DEBUG_SYNC_RESET] = {5}, + [PDC_GPU_SYNC_RESET] = {6}, + [PDC_DISPLAY_SYNC_RESET] = {7}, + [PDC_COMPUTE_SYNC_RESET] = {8}, + [PDC_MODEM_SYNC_RESET] = {9}, + [PDC_WLAN_RF_SYNC_RESET] = {10}, + [PDC_WPSS_SYNC_RESET] = {11}, +}; + +static const struct qcom_pdc_reset_desc sc7280_pdc_reset_desc = { + .resets = sc7280_pdc_resets, + .offset = RPMH_SC7280_PDC_SYNC_RESET, + .num_resets = ARRAY_SIZE(sc7280_pdc_resets), +}; + static inline struct qcom_pdc_reset_data *to_qcom_pdc_reset_data( struct reset_controller_dev *rcdev) { @@ -54,19 +89,18 @@ static int qcom_pdc_control_assert(struct reset_controller_dev *rcdev, unsigned long idx) { struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev); + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx]; - return regmap_update_bits(data->regmap, RPMH_PDC_SYNC_RESET, - BIT(sdm845_pdc_resets[idx].bit), - BIT(sdm845_pdc_resets[idx].bit)); + return regmap_update_bits(data->regmap, data->desc->offset, BIT(map->bit), BIT(map->bit)); } static int qcom_pdc_control_deassert(struct reset_controller_dev *rcdev, unsigned long idx) { struct qcom_pdc_reset_data *data = to_qcom_pdc_reset_data(rcdev); + const struct qcom_pdc_reset_map *map = &data->desc->resets[idx]; - return regmap_update_bits(data->regmap, RPMH_PDC_SYNC_RESET, - BIT(sdm845_pdc_resets[idx].bit), 0); + return regmap_update_bits(data->regmap, data->desc->offset, BIT(map->bit), 0); } static const struct reset_control_ops qcom_pdc_reset_ops = { @@ -76,22 +110,27 @@ static const struct reset_control_ops qcom_pdc_reset_ops = { static int qcom_pdc_reset_probe(struct platform_device *pdev) { + const struct qcom_pdc_reset_desc *desc; struct qcom_pdc_reset_data *data; struct device *dev = &pdev->dev; void __iomem *base; struct resource *res; + desc = device_get_match_data(&pdev->dev); + if (!desc) + return -EINVAL; + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; + data->desc = desc; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); base = devm_ioremap_resource(dev, res); if (IS_ERR(base)) return PTR_ERR(base); - data->regmap = devm_regmap_init_mmio(dev, base, - &sdm845_pdc_regmap_config); + data->regmap = devm_regmap_init_mmio(dev, base, &pdc_regmap_config); if (IS_ERR(data->regmap)) { dev_err(dev, "Unable to initialize regmap\n"); return PTR_ERR(data->regmap); @@ -99,14 +138,15 @@ static int qcom_pdc_reset_probe(struct platform_device *pdev) data->rcdev.owner = THIS_MODULE; data->rcdev.ops = &qcom_pdc_reset_ops; - data->rcdev.nr_resets = ARRAY_SIZE(sdm845_pdc_resets); + data->rcdev.nr_resets = desc->num_resets; data->rcdev.of_node = dev->of_node; return devm_reset_controller_register(dev, &data->rcdev); } static const struct of_device_id qcom_pdc_reset_of_match[] = { - { .compatible = "qcom,sdm845-pdc-global" }, + { .compatible = "qcom,sc7280-pdc-global", .data = &sc7280_pdc_reset_desc }, + { .compatible = "qcom,sdm845-pdc-global", .data = &sdm845_pdc_reset_desc }, {} }; MODULE_DEVICE_TABLE(of, qcom_pdc_reset_of_match);
Add PDC Global reset signals for Wireless Processor Subsystem (WPSS) on SC7280 SoCs. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- v2: * place resets and num_resets adjacent to each other [Stephen] drivers/reset/reset-qcom-pdc.c | 62 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 11 deletions(-)