| Message ID | 20210301014329.30104-2-shawn.guo@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add ACPI support for SC8180X pinctrl driver | expand |
On Mon, Mar 01, 2021 at 09:43:28AM +0800, Shawn Guo wrote: > It's not always the case that DT and ACPI describe hardware resource in > the same schema, even for a single platform. For example, on SC8180X, > DT uses the tiles schema while ACPI describe memory resource as a single > region. It patches msm_pinctrl_probe() function to map tiles regions > only for DT. While for ACPI, it maps the single memory resource and > calculate tile bases with offsets passed from SoC data. ... > +#include <linux/acpi.h> No use of this header. See below. (Perhaps you meant mod_devicetable.h) ... > - if (soc_data->tiles) { > + if (soc_data->tiles && !has_acpi_companion(&pdev->dev)) { Any documentation to understand this change? ... > + if (soc_data->tiles) { > + for (i = 0; i < soc_data->ntiles; i++) > + pctrl->regs[i] = base + > + soc_data->tile_offsets[i]; > + } else { > + pctrl->regs[0] = base; > + } And so this?
On Mon, Mar 01, 2021 at 04:34:30PM +0200, Andy Shevchenko wrote: > On Mon, Mar 01, 2021 at 09:43:28AM +0800, Shawn Guo wrote: > > It's not always the case that DT and ACPI describe hardware resource in > > the same schema, even for a single platform. For example, on SC8180X, > > DT uses the tiles schema while ACPI describe memory resource as a single > > region. It patches msm_pinctrl_probe() function to map tiles regions > > only for DT. While for ACPI, it maps the single memory resource and > > calculate tile bases with offsets passed from SoC data. > > ... > > > +#include <linux/acpi.h> > > No use of this header. See below. > (Perhaps you meant mod_devicetable.h) has_acpi_companion() call needs the header. > > ... > > > - if (soc_data->tiles) { > > + if (soc_data->tiles && !has_acpi_companion(&pdev->dev)) { > > Any documentation to understand this change? Well, !has_acpi_companion() is just to rule out ACPI boot and ensure this is a DT boot with tiles. > > ... > > > + if (soc_data->tiles) { > > + for (i = 0; i < soc_data->ntiles; i++) > > + pctrl->regs[i] = base + > > + soc_data->tile_offsets[i]; > > + } else { > > + pctrl->regs[0] = base; > > + } > > And so this? For ACPI boot or DT without tiles, there is only one single memory resource to map. But for SoC driver like pinctrl-sc8180x that defines pins with tiles, even with ACPI boot, we need to have multiple regs[] to hold bases for tiles. I will add comment to make it easier for understanding. Shawn
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 40256663264f..2526f299bdce 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -4,6 +4,7 @@ * Copyright (c) 2013, The Linux Foundation. All rights reserved. */ +#include <linux/acpi.h> #include <linux/delay.h> #include <linux/err.h> #include <linux/io.h> @@ -1399,6 +1400,7 @@ int msm_pinctrl_probe(struct platform_device *pdev, { struct msm_pinctrl *pctrl; struct resource *res; + void __iomem *base; int ret; int i; @@ -1415,7 +1417,7 @@ int msm_pinctrl_probe(struct platform_device *pdev, raw_spin_lock_init(&pctrl->lock); - if (soc_data->tiles) { + if (soc_data->tiles && !has_acpi_companion(&pdev->dev)) { for (i = 0; i < soc_data->ntiles; i++) { res = platform_get_resource_byname(pdev, IORESOURCE_MEM, soc_data->tiles[i]); @@ -1425,9 +1427,17 @@ int msm_pinctrl_probe(struct platform_device *pdev, } } else { res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - pctrl->regs[0] = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(pctrl->regs[0])) - return PTR_ERR(pctrl->regs[0]); + base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + if (soc_data->tiles) { + for (i = 0; i < soc_data->ntiles; i++) + pctrl->regs[i] = base + + soc_data->tile_offsets[i]; + } else { + pctrl->regs[0] = base; + } pctrl->phys_base[0] = res->start; } diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h index e31a5167c91e..91333942d53c 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.h +++ b/drivers/pinctrl/qcom/pinctrl-msm.h @@ -131,6 +131,7 @@ struct msm_pinctrl_soc_data { bool pull_no_keeper; const char *const *tiles; unsigned int ntiles; + const u32 *tile_offsets; const int *reserved_gpios; const struct msm_gpio_wakeirq_map *wakeirq_map; unsigned int nwakeirq_map;
It's not always the case that DT and ACPI describe hardware resource in the same schema, even for a single platform. For example, on SC8180X, DT uses the tiles schema while ACPI describe memory resource as a single region. It patches msm_pinctrl_probe() function to map tiles regions only for DT. While for ACPI, it maps the single memory resource and calculate tile bases with offsets passed from SoC data. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/pinctrl/qcom/pinctrl-msm.c | 18 ++++++++++++++---- drivers/pinctrl/qcom/pinctrl-msm.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)