The reset control API has two modes: exclusive access, where the driver expects to have full and immediate control over the state of the reset line, and shared (clock-like) access, where drivers only request reset deassertion while active, but don't care about the state of the reset line while inactive.
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior.
This series converts all drivers that currently implicitly request exclusive reset controls to the corresponding explicit API call. It is, for the most part, generated from the following semantic patch:
@@ expression rstc, dev, id; @@ -rstc = reset_control_get(dev, id); +rstc = reset_control_get_exclusive(dev, id); @@ expression rstc, dev, id; @@ -rstc = reset_control_get_optional(dev, id); +rstc = reset_control_get_optional_exclusive(dev, id); @@ expression rstc, node, id; @@ -rstc = of_reset_control_get(node, id); +rstc = of_reset_control_get_exclusive(node, id); @@ expression rstc, node, index; @@ -rstc = of_reset_control_get_by_index(node, index); +rstc = of_reset_control_get_exclusive_by_index(node, index); @@ expression rstc, dev, id; @@ -rstc = devm_reset_control_get(dev, id); +rstc = devm_reset_control_get_exclusive(dev, id); @@ expression rstc, dev, id; @@ -rstc = devm_reset_control_get_optional(dev, id); +rstc = devm_reset_control_get_optional_exclusive(dev, id); @@ expression rstc, dev, index; @@ -rstc = devm_reset_control_get_by_index(dev, index); +rstc = devm_reset_control_get_exclusive_by_index(dev, index);
After all driver patches are applied, the temporary transition helpers can be removed.
regards Philipp
Philipp Zabel (102): ARM: rockchip: explicitly request exclusive reset control ARM: socfpga: explicitly request exclusive reset control MIPS: pci-mt7620: explicitly request exclusive reset control ahci: st: explicitly request exclusive reset control ata: sata_gemini: explicitly request exclusive reset control ata: ahci_tegra: explicitly request exclusive reset control bus: sunxi-rsb: explicitly request exclusive reset control bus: tegra-gmi: explicitly request exclusive reset control clk: sunxi: explicitly request exclusive reset control clk: tegra: explicitly request exclusive reset control clocksource/drivers/timer-stm32: explicitly request exclusive reset control clocksource/drivers/sun5i: explicitly request exclusive reset control crypto: rockchip: explicitly request exclusive reset control crypto: sun4i-ss - request exclusive reset control PM / devfreq: tegra: explicitly request exclusive reset control dmaengine: stm32-dma: explicitly request exclusive reset control dmaengine: sun6i: explicitly request exclusive reset control dmaengine: tegra-apb: explicitly request exclusive reset control drm: kirin: explicitly request exclusive reset control drm/nouveau/tegra: explicitly request exclusive reset control drm/rockchip: explicitly request exclusive reset control drm/sti: explicitly request exclusive reset control drm/stm: explicitly request exclusive reset control drm/sun4i: explicitly request exclusive reset control drm/tegra: explicitly request exclusive reset control gpu: host1x: explicitly request exclusive reset control i2c: mv64xxx: explicitly request exclusive reset control i2c: stm32f4: explicitly request exclusive reset control i2c: sun6i-pw2i: explicitly request exclusive reset control i2c: tegra: explicitly request exclusive reset control iio: adc: rockchip_saradc: explicitly request exclusive reset control iio: dac: stm32-dac-core: explicitly request exclusive reset control Input: tegra-kbc - request exclusive reset control coda: explicitly request exclusive reset control st-rc: explicitly request exclusive reset control stm32-dcmi: explicitly request exclusive reset control rc: sunxi-cir: explicitly request exclusive reset control mmc: dw_mmc: explicitly request exclusive reset control mmc: sdhci-st: explicitly request exclusive reset control mmc: sunxi: explicitly request exclusive reset control mmc: tegra: explicitly request exclusive reset control mtd: nand: sunxi: explicitly request exclusive reset control mtd: spi-nor: stm32-quadspi: explicitly request exclusive reset control net: dsa: mt7530: explicitly request exclusive reset control net: ethernet: hisi_femac: explicitly request exclusive reset control net: ethernet: hix5hd2_gmac: explicitly request exclusive reset control net: stmmac: explicitly request exclusive reset control net: stmmac: dwc-qos: explicitly request exclusive reset control ath10k: explicitly request exclusive reset control nvmem: lpc18xx-eeprom: explicitly request exclusive reset control PCI: dwc: pcie-qcom: explicitly request exclusive reset control PCI: imx6: explicitly request exclusive reset control PCI: tegra: explicitly request exclusive reset control PCI: rockchip: explicitly request exclusive reset control phy: berlin-usb: explicitly request exclusive reset control PCI: mediatek: explicitly request exclusive reset control phy: qcom-usb-hs: explicitly request exclusive reset control phy: rockchip-pcie: explicitly request exclusive reset control phy: rockchip-typec: explicitly request exclusive reset control phy: rockchip-usb: explicitly request exclusive reset control phy: sun4i-usb: explicitly request exclusive reset control phy: sun9i-usb: explicitly request exclusive reset control phy: tegra: explicitly request exclusive reset control phy: qcom-qmp: explicitly request exclusive reset control phy: qcom-qusb2: explicitly request exclusive reset control pinctrl: stm32: explicitly request exclusive reset control pinctrl: sunxi: explicitly request exclusive reset control pinctrl: tegra: explicitly request exclusive reset control pwm: hibvt: explicitly request exclusive reset control pwm: tegra: explicitly request exclusive reset control remoteproc/keystone: explicitly request exclusive reset control remoteproc: qcom: explicitly request exclusive reset control remoteproc: st: explicitly request exclusive reset control soc: mediatek: PMIC wrap: explicitly request exclusive reset control soc/tegra: pmc: explicitly request exclusive reset control spi: stm32: explicitly request exclusive reset control spi: sun6i: explicitly request exclusive reset control spi: tegra20-slink: explicitly request exclusive reset control spi: tegra114: explicitly request exclusive reset control spi: tegra20-sflash: explicitly request exclusive reset control staging: nvec: explicitly request exclusive reset control thermal: rockchip: explicitly request exclusive reset control thermal: tegra: explicitly request exclusive reset control serial: 8250_dw: explicitly request exclusive reset control serial: tegra: explicitly request exclusive reset control usb: chipidea: msm: explicitly request exclusive reset control usb: dwc2: explicitly request exclusive reset control usb: host: ehci-tegra: explicitly request exclusive reset control usb: host: xhci-tegra: explicitly request exclusive reset control usb: musb: sunxi: explicitly request exclusive reset control usb: phy: msm: explicitly request exclusive reset control usb: phy: qcom-8x16-usb: explicitly request exclusive reset control watchdog: asm9260: explicitly request exclusive reset control watchdog: mt7621: explicitly request exclusive reset control watchdog: rt2880: explicitly request exclusive reset control watchdog: zx2967: explicitly request exclusive reset control ASoC: img: explicitly request exclusive reset control ASoC: stm32: explicitly request exclusive reset control ASoC: sun4i: explicitly request exclusive reset control ASoC: tegra: explicitly request exclusive reset control Documentation: devres: add explicit exclusive/shared reset control request calls reset: finish transition to explicit exclusive reset control requests
Documentation/driver-model/devres.txt | 7 ++- arch/arm/mach-rockchip/platsmp.c | 2 +- arch/mips/pci/pci-mt7620.c | 2 +- drivers/ata/ahci_st.c | 6 +-- drivers/ata/ahci_tegra.c | 8 ++-- drivers/ata/sata_gemini.c | 4 +- drivers/bus/sunxi-rsb.c | 2 +- drivers/bus/tegra-gmi.c | 2 +- drivers/clk/sunxi/clk-sun9i-mmc.c | 2 +- drivers/clk/tegra/clk-dfll.c | 2 +- drivers/clocksource/timer-stm32.c | 2 +- drivers/clocksource/timer-sun5i.c | 2 +- drivers/crypto/rockchip/rk3288_crypto.c | 2 +- drivers/crypto/sunxi-ss/sun4i-ss-core.c | 3 +- drivers/devfreq/tegra-devfreq.c | 2 +- drivers/dma/stm32-dma.c | 2 +- drivers/dma/sun6i-dma.c | 2 +- drivers/dma/tegra20-apb-dma.c | 2 +- drivers/fpga/altera-hps2fpga.c | 3 +- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c | 2 +- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +- drivers/gpu/drm/rockchip/cdn-dp-core.c | 8 ++-- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +- drivers/gpu/drm/sti/sti_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/sti/sti_tvout.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 2 +- drivers/gpu/drm/sun4i/sun4i_backend.c | 4 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +- drivers/gpu/drm/sun4i/sun4i_tv.c | 2 +- drivers/gpu/drm/sun4i/sun6i_drc.c | 2 +- drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 +- drivers/gpu/drm/tegra/dc.c | 2 +- drivers/gpu/drm/tegra/dpaux.c | 3 +- drivers/gpu/drm/tegra/dsi.c | 2 +- drivers/gpu/drm/tegra/gr3d.c | 6 +-- drivers/gpu/drm/tegra/hdmi.c | 2 +- drivers/gpu/drm/tegra/sor.c | 2 +- drivers/gpu/host1x/dev.c | 2 +- drivers/i2c/busses/i2c-mv64xxx.c | 2 +- drivers/i2c/busses/i2c-stm32f4.c | 2 +- drivers/i2c/busses/i2c-sun6i-p2wi.c | 2 +- drivers/i2c/busses/i2c-tegra.c | 2 +- drivers/iio/adc/rockchip_saradc.c | 3 +- drivers/iio/dac/stm32-dac-core.c | 2 +- drivers/input/keyboard/tegra-kbc.c | 2 +- drivers/media/platform/coda/coda-common.c | 3 +- drivers/media/platform/stm32/stm32-dcmi.c | 2 +- drivers/media/rc/st_rc.c | 2 +- drivers/media/rc/sunxi-cir.c | 2 +- drivers/mmc/host/dw_mmc.c | 2 +- drivers/mmc/host/sdhci-st.c | 2 +- drivers/mmc/host/sdhci-tegra.c | 3 +- drivers/mmc/host/sunxi-mmc.c | 3 +- drivers/mtd/nand/sunxi_nand.c | 2 +- drivers/mtd/spi-nor/stm32-quadspi.c | 2 +- drivers/net/dsa/mt7530.c | 3 +- drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 6 +-- .../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 +- .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 +- drivers/net/wireless/ath/ath10k/ahb.c | 15 ++++--- drivers/nvmem/lpc18xx_eeprom.c | 2 +- drivers/pci/dwc/pci-imx6.c | 7 +-- drivers/pci/dwc/pcie-qcom.c | 40 +++++++++-------- drivers/pci/host/pci-tegra.c | 6 +-- drivers/pci/host/pcie-mediatek.c | 2 +- drivers/pci/host/pcie-rockchip.c | 15 ++++--- drivers/phy/allwinner/phy-sun4i-usb.c | 2 +- drivers/phy/allwinner/phy-sun9i-usb.c | 4 +- drivers/phy/marvell/phy-berlin-usb.c | 2 +- drivers/phy/qualcomm/phy-qcom-qmp.c | 4 +- drivers/phy/qualcomm/phy-qcom-qusb2.c | 3 +- drivers/phy/qualcomm/phy-qcom-usb-hs.c | 3 +- drivers/phy/rockchip/phy-rockchip-pcie.c | 2 +- drivers/phy/rockchip/phy-rockchip-typec.c | 6 +-- drivers/phy/rockchip/phy-rockchip-usb.c | 2 +- drivers/phy/tegra/xusb-tegra210.c | 4 +- drivers/phy/tegra/xusb.c | 2 +- drivers/pinctrl/stm32/pinctrl-stm32.c | 2 +- drivers/pinctrl/sunxi/pinctrl-sun6i-a31-r.c | 2 +- drivers/pinctrl/sunxi/pinctrl-sun8i-a23-r.c | 2 +- drivers/pinctrl/tegra/pinctrl-tegra-xusb.c | 2 +- drivers/pwm/pwm-hibvt.c | 2 +- drivers/pwm/pwm-tegra.c | 2 +- drivers/remoteproc/keystone_remoteproc.c | 2 +- drivers/remoteproc/qcom_q6v5_pil.c | 3 +- drivers/remoteproc/st_remoteproc.c | 6 ++- drivers/reset/core.c | 2 +- drivers/soc/mediatek/mtk-pmic-wrap.c | 5 ++- drivers/soc/tegra/pmc.c | 2 +- drivers/spi/spi-stm32.c | 2 +- drivers/spi/spi-sun6i.c | 2 +- drivers/spi/spi-tegra114.c | 2 +- drivers/spi/spi-tegra20-sflash.c | 2 +- drivers/spi/spi-tegra20-slink.c | 2 +- drivers/staging/nvec/nvec.c | 2 +- drivers/thermal/rockchip_thermal.c | 3 +- drivers/thermal/tegra/soctherm.c | 3 +- drivers/tty/serial/8250/8250_dw.c | 2 +- drivers/tty/serial/serial-tegra.c | 2 +- drivers/usb/chipidea/ci_hdrc_msm.c | 2 +- drivers/usb/dwc2/platform.c | 3 +- drivers/usb/host/ehci-tegra.c | 5 ++- drivers/usb/host/xhci-tegra.c | 6 ++- drivers/usb/musb/sunxi.c | 2 +- drivers/usb/phy/phy-msm-usb.c | 4 +- drivers/usb/phy/phy-qcom-8x16-usb.c | 2 +- drivers/watchdog/asm9260_wdt.c | 2 +- drivers/watchdog/mt7621_wdt.c | 2 +- drivers/watchdog/rt2880_wdt.c | 2 +- drivers/watchdog/zx2967_wdt.c | 2 +- include/linux/reset.h | 50 ---------------------- sound/soc/img/img-i2s-in.c | 2 +- sound/soc/img/img-i2s-out.c | 2 +- sound/soc/img/img-parallel-out.c | 2 +- sound/soc/img/img-spdif-in.c | 2 +- sound/soc/img/img-spdif-out.c | 2 +- sound/soc/stm/stm32_i2s.c | 2 +- sound/soc/stm/stm32_sai.c | 2 +- sound/soc/stm/stm32_spdifrx.c | 2 +- sound/soc/sunxi/sun4i-codec.c | 3 +- sound/soc/sunxi/sun4i-i2s.c | 2 +- sound/soc/sunxi/sun4i-spdif.c | 3 +- sound/soc/tegra/tegra30_ahub.c | 4 +- 128 files changed, 226 insertions(+), 235 deletions(-)
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Xinliang Liu z.liuxinliang@hisilicon.com Cc: Rongrong Zou zourongrong@gmail.com Cc: Xinwei Kong kong.kongxinwei@hisilicon.com Cc: Chen Feng puck.chen@hisilicon.com Cc: dri-devel@lists.freedesktop.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c index c96c228a98980..5cef7ee83a168 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c @@ -933,7 +933,7 @@ static int ade_dts_parse(struct platform_device *pdev, struct ade_hw_ctx *ctx) return PTR_ERR(ctx->base); }
- ctx->reset = devm_reset_control_get(dev, NULL); + ctx->reset = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(ctx->reset)) return PTR_ERR(ctx->reset);
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Ben Skeggs bskeggs@redhat.com Cc: David Airlie airlied@linux.ie Cc: Thierry Reding thierry.reding@gmail.com Cc: Jonathan Hunter jonathanh@nvidia.com Cc: dri-devel@lists.freedesktop.org Cc: nouveau@lists.freedesktop.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c index 189ed80e21ffb..ac5d4cf058c25 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c @@ -282,7 +282,7 @@ nvkm_device_tegra_new(const struct nvkm_device_tegra_func *func, } }
- tdev->rst = devm_reset_control_get(&pdev->dev, "gpu"); + tdev->rst = devm_reset_control_get_exclusive(&pdev->dev, "gpu"); if (IS_ERR(tdev->rst)) { ret = PTR_ERR(tdev->rst); goto free;
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Mark Yao mark.yao@rock-chips.com Cc: Heiko Stuebner heiko@sntech.de Cc: dri-devel@lists.freedesktop.org Cc: linux-rockchip@lists.infradead.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +- drivers/gpu/drm/rockchip/cdn-dp-core.c | 8 ++++---- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 9606121fa185a..172930e7645e7 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -297,7 +297,7 @@ static int rockchip_dp_init(struct rockchip_dp_device *dp) return PTR_ERR(dp->pclk); }
- dp->rst = devm_reset_control_get(dev, "dp"); + dp->rst = devm_reset_control_get_exclusive(dev, "dp"); if (IS_ERR(dp->rst)) { dev_err(dev, "failed to get dp reset control\n"); return PTR_ERR(dp->rst); diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c index 9b0b0588bbedb..b7f5c5d9f245d 100644 --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c @@ -765,25 +765,25 @@ static int cdn_dp_parse_dt(struct cdn_dp_device *dp) return PTR_ERR(dp->grf_clk); }
- dp->spdif_rst = devm_reset_control_get(dev, "spdif"); + dp->spdif_rst = devm_reset_control_get_exclusive(dev, "spdif"); if (IS_ERR(dp->spdif_rst)) { DRM_DEV_ERROR(dev, "no spdif reset control found\n"); return PTR_ERR(dp->spdif_rst); }
- dp->dptx_rst = devm_reset_control_get(dev, "dptx"); + dp->dptx_rst = devm_reset_control_get_exclusive(dev, "dptx"); if (IS_ERR(dp->dptx_rst)) { DRM_DEV_ERROR(dev, "no uphy reset control found\n"); return PTR_ERR(dp->dptx_rst); }
- dp->core_rst = devm_reset_control_get(dev, "core"); + dp->core_rst = devm_reset_control_get_exclusive(dev, "core"); if (IS_ERR(dp->core_rst)) { DRM_DEV_ERROR(dev, "no core reset control found\n"); return PTR_ERR(dp->core_rst); }
- dp->apb_rst = devm_reset_control_get(dev, "apb"); + dp->apb_rst = devm_reset_control_get_exclusive(dev, "apb"); if (IS_ERR(dp->apb_rst)) { DRM_DEV_ERROR(dev, "no apb reset control found\n"); return PTR_ERR(dp->apb_rst); diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 21b9737662ae9..c3501ae59db35 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -1221,7 +1221,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, * Note that the reset was not defined in the initial device tree, so * we have to be prepared for it not being found. */ - apb_rst = devm_reset_control_get(dev, "apb"); + apb_rst = devm_reset_control_get_exclusive(dev, "apb"); if (IS_ERR(apb_rst)) { ret = PTR_ERR(apb_rst); if (ret == -ENOENT) { diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 5d450332c2fd7..18b582cd81e50 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -1443,7 +1443,7 @@ static int vop_initial(struct vop *vop) /* * do hclk_reset, reset all vop registers. */ - ahb_rst = devm_reset_control_get(vop->dev, "ahb"); + ahb_rst = devm_reset_control_get_exclusive(vop->dev, "ahb"); if (IS_ERR(ahb_rst)) { dev_err(vop->dev, "failed to get ahb reset\n"); ret = PTR_ERR(ahb_rst); @@ -1469,7 +1469,7 @@ static int vop_initial(struct vop *vop) /* * do dclk_reset, let all config take affect. */ - vop->dclk_rst = devm_reset_control_get(vop->dev, "dclk"); + vop->dclk_rst = devm_reset_control_get_exclusive(vop->dev, "dclk"); if (IS_ERR(vop->dclk_rst)) { dev_err(vop->dev, "failed to get dclk reset\n"); ret = PTR_ERR(vop->dclk_rst);
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: dri-devel@lists.freedesktop.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/sti/sti_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/sti/sti_tvout.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index a59c95a8081b7..ea6e5b5a3725b 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -1428,7 +1428,7 @@ static int sti_hdmi_probe(struct platform_device *pdev) if (!hdmi->notifier) goto release_adapter;
- hdmi->reset = devm_reset_control_get(dev, "hdmi"); + hdmi->reset = devm_reset_control_get_exclusive(dev, "hdmi"); /* Take hdmi out of reset */ if (!IS_ERR(hdmi->reset)) reset_control_deassert(hdmi->reset); diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index a1c161f778044..2809db8c03216 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1375,7 +1375,7 @@ static int sti_hqvdp_probe(struct platform_device *pdev) }
/* Get reset resources */ - hqvdp->reset = devm_reset_control_get(dev, "hqvdp"); + hqvdp->reset = devm_reset_control_get_exclusive(dev, "hqvdp"); if (!IS_ERR(hqvdp->reset)) reset_control_deassert(hqvdp->reset);
diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index 8959fcc743a8e..cbe0f5c162348 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -857,7 +857,7 @@ static int sti_tvout_probe(struct platform_device *pdev) return -ENOMEM;
/* get reset resources */ - tvout->reset = devm_reset_control_get(dev, "tvout"); + tvout->reset = devm_reset_control_get_exclusive(dev, "tvout"); /* take tvout out of reset */ if (!IS_ERR(tvout->reset)) reset_control_deassert(tvout->reset);
2017-07-19 17:25 GMT+02:00 Philipp Zabel p.zabel@pengutronix.de:
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: dri-devel@lists.freedesktop.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de
Acked-by: Benjamin Gaignard benjamin.gaignard@linaro.org
drivers/gpu/drm/sti/sti_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/sti/sti_tvout.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index a59c95a8081b7..ea6e5b5a3725b 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -1428,7 +1428,7 @@ static int sti_hdmi_probe(struct platform_device *pdev) if (!hdmi->notifier) goto release_adapter;
hdmi->reset = devm_reset_control_get(dev, "hdmi");
hdmi->reset = devm_reset_control_get_exclusive(dev, "hdmi"); /* Take hdmi out of reset */ if (!IS_ERR(hdmi->reset)) reset_control_deassert(hdmi->reset);diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index a1c161f778044..2809db8c03216 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1375,7 +1375,7 @@ static int sti_hqvdp_probe(struct platform_device *pdev) }
/* Get reset resources */
hqvdp->reset = devm_reset_control_get(dev, "hqvdp");
hqvdp->reset = devm_reset_control_get_exclusive(dev, "hqvdp"); if (!IS_ERR(hqvdp->reset)) reset_control_deassert(hqvdp->reset);diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index 8959fcc743a8e..cbe0f5c162348 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -857,7 +857,7 @@ static int sti_tvout_probe(struct platform_device *pdev) return -ENOMEM;
/* get reset resources */
tvout->reset = devm_reset_control_get(dev, "tvout");
tvout->reset = devm_reset_control_get_exclusive(dev, "tvout"); /* take tvout out of reset */ if (!IS_ERR(tvout->reset)) reset_control_deassert(tvout->reset);-- 2.11.0
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Yannick Fertre yannick.fertre@st.com Cc: Philippe Cornu philippe.cornu@st.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Vincent Abriou vincent.abriou@st.com Cc: dri-devel@lists.freedesktop.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/stm/ltdc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 1b9483d4f2a4e..59d344a519097 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -1026,7 +1026,7 @@ int ltdc_load(struct drm_device *ddev) if (!ldev->panel) return -EPROBE_DEFER;
- rstc = of_reset_control_get(np, NULL); + rstc = of_reset_control_get_exclusive(np, NULL);
mutex_init(&ldev->err_lock);
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Maxime Ripard maxime.ripard@free-electrons.com Cc: Chen-Yu Tsai wens@csie.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/sun4i/sun4i_backend.c | 4 ++-- drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 +- drivers/gpu/drm/sun4i/sun4i_tv.c | 2 +- drivers/gpu/drm/sun4i/sun6i_drc.c | 2 +- drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c index cf480218daa50..b7eb908798f6e 100644 --- a/drivers/gpu/drm/sun4i/sun4i_backend.c +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c @@ -248,7 +248,7 @@ static int sun4i_backend_init_sat(struct device *dev) { struct sun4i_backend *backend = dev_get_drvdata(dev); int ret;
- backend->sat_reset = devm_reset_control_get(dev, "sat"); + backend->sat_reset = devm_reset_control_get_exclusive(dev, "sat"); if (IS_ERR(backend->sat_reset)) { dev_err(dev, "Couldn't get the SAT reset line\n"); return PTR_ERR(backend->sat_reset); @@ -376,7 +376,7 @@ static int sun4i_backend_bind(struct device *dev, struct device *master, return PTR_ERR(backend->engine.regs); }
- backend->reset = devm_reset_control_get(dev, NULL); + backend->reset = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(backend->reset)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(backend->reset); diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index d9791292553ef..2135ae51356a1 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -523,7 +523,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, tcon->id = engine->id; tcon->quirks = of_device_get_match_data(dev);
- tcon->lcd_rst = devm_reset_control_get(dev, "lcd"); + tcon->lcd_rst = devm_reset_control_get_exclusive(dev, "lcd"); if (IS_ERR(tcon->lcd_rst)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(tcon->lcd_rst); diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c index 338b9e5bb2a3e..a8b93b2658c44 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tv.c +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c @@ -593,7 +593,7 @@ static int sun4i_tv_bind(struct device *dev, struct device *master, return PTR_ERR(tv->regs); }
- tv->reset = devm_reset_control_get(dev, NULL); + tv->reset = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(tv->reset)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(tv->reset); diff --git a/drivers/gpu/drm/sun4i/sun6i_drc.c b/drivers/gpu/drm/sun4i/sun6i_drc.c index 09bba853e2a42..8b018e539a9e3 100644 --- a/drivers/gpu/drm/sun4i/sun6i_drc.c +++ b/drivers/gpu/drm/sun4i/sun6i_drc.c @@ -33,7 +33,7 @@ static int sun6i_drc_bind(struct device *dev, struct device *master, return -ENOMEM; dev_set_drvdata(dev, drc);
- drc->reset = devm_reset_control_get(dev, NULL); + drc->reset = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(drc->reset)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(drc->reset); diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index cb193c5f16862..13adac261d1f8 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -287,7 +287,7 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, return PTR_ERR(mixer->engine.regs); }
- mixer->reset = devm_reset_control_get(dev, NULL); + mixer->reset = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(mixer->reset)) { dev_err(dev, "Couldn't get our reset line\n"); return PTR_ERR(mixer->reset);
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Thierry Reding thierry.reding@gmail.com Cc: Jonathan Hunter jonathanh@nvidia.com Cc: dri-devel@lists.freedesktop.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/tegra/dc.c | 2 +- drivers/gpu/drm/tegra/dpaux.c | 3 ++- drivers/gpu/drm/tegra/dsi.c | 2 +- drivers/gpu/drm/tegra/gr3d.c | 6 +++--- drivers/gpu/drm/tegra/hdmi.c | 2 +- drivers/gpu/drm/tegra/sor.c | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index c875f11786b93..61d476a3006af 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -2011,7 +2011,7 @@ static int tegra_dc_probe(struct platform_device *pdev) return PTR_ERR(dc->clk); }
- dc->rst = devm_reset_control_get(&pdev->dev, "dc"); + dc->rst = devm_reset_control_get_exclusive(&pdev->dev, "dc"); if (IS_ERR(dc->rst)) { dev_err(&pdev->dev, "failed to get reset\n"); return PTR_ERR(dc->rst); diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index 2fde44c3a1b30..1cf18f4f98f06 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -445,7 +445,8 @@ static int tegra_dpaux_probe(struct platform_device *pdev) }
if (!pdev->dev.pm_domain) { - dpaux->rst = devm_reset_control_get(&pdev->dev, "dpaux"); + dpaux->rst = devm_reset_control_get_exclusive(&pdev->dev, + "dpaux"); if (IS_ERR(dpaux->rst)) { dev_err(&pdev->dev, "failed to get reset control: %ld\n", diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index 3dea1216bafdc..af8850c74abe9 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -1518,7 +1518,7 @@ static int tegra_dsi_probe(struct platform_device *pdev) dsi->lanes = 4;
if (!pdev->dev.pm_domain) { - dsi->rst = devm_reset_control_get(&pdev->dev, "dsi"); + dsi->rst = devm_reset_control_get_exclusive(&pdev->dev, "dsi"); if (IS_ERR(dsi->rst)) return PTR_ERR(dsi->rst); } diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c index cee2ab645cde9..e8dd13e02483d 100644 --- a/drivers/gpu/drm/tegra/gr3d.c +++ b/drivers/gpu/drm/tegra/gr3d.c @@ -260,7 +260,7 @@ static int gr3d_probe(struct platform_device *pdev) return PTR_ERR(gr3d->clk); }
- gr3d->rst = devm_reset_control_get(&pdev->dev, "3d"); + gr3d->rst = devm_reset_control_get_exclusive(&pdev->dev, "3d"); if (IS_ERR(gr3d->rst)) { dev_err(&pdev->dev, "cannot get reset\n"); return PTR_ERR(gr3d->rst); @@ -273,8 +273,8 @@ static int gr3d_probe(struct platform_device *pdev) return PTR_ERR(gr3d->clk_secondary); }
- gr3d->rst_secondary = devm_reset_control_get(&pdev->dev, - "3d2"); + gr3d->rst_secondary = devm_reset_control_get_exclusive(&pdev->dev, + "3d2"); if (IS_ERR(gr3d->rst_secondary)) { dev_err(&pdev->dev, "cannot get secondary reset\n"); return PTR_ERR(gr3d->rst_secondary); diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index cda0491ed6bf8..49d1cade94742 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -1686,7 +1686,7 @@ static int tegra_hdmi_probe(struct platform_device *pdev) return PTR_ERR(hdmi->clk); }
- hdmi->rst = devm_reset_control_get(&pdev->dev, "hdmi"); + hdmi->rst = devm_reset_control_get_exclusive(&pdev->dev, "hdmi"); if (IS_ERR(hdmi->rst)) { dev_err(&pdev->dev, "failed to get reset\n"); return PTR_ERR(hdmi->rst); diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index a8f528925009e..4a4796ceeb541 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -2609,7 +2609,7 @@ static int tegra_sor_probe(struct platform_device *pdev) }
if (!pdev->dev.pm_domain) { - sor->rst = devm_reset_control_get(&pdev->dev, "sor"); + sor->rst = devm_reset_control_get_exclusive(&pdev->dev, "sor"); if (IS_ERR(sor->rst)) { err = PTR_ERR(sor->rst); dev_err(&pdev->dev, "failed to get reset control: %d\n",
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior. Convert all drivers requesting exclusive resets to the explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Thierry Reding thierry.reding@gmail.com Cc: dri-devel@lists.freedesktop.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/host1x/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 2c58a390123a1..5af7055280c6c 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -170,7 +170,7 @@ static int host1x_probe(struct platform_device *pdev) return err; }
- host->rst = devm_reset_control_get(&pdev->dev, "host1x"); + host->rst = devm_reset_control_get_exclusive(&pdev->dev, "host1x"); if (IS_ERR(host->rst)) { err = PTR_ERR(host->rst); dev_err(&pdev->dev, "failed to get reset: %d\n", err);
Hello,
On Wed, 19 Jul 2017 17:25:04 +0200, Philipp Zabel wrote:
The reset control API has two modes: exclusive access, where the driver expects to have full and immediate control over the state of the reset line, and shared (clock-like) access, where drivers only request reset deassertion while active, but don't care about the state of the reset line while inactive.
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior.
This series converts all drivers that currently implicitly request exclusive reset controls to the corresponding explicit API call. It is, for the most part, generated from the following semantic patch:
@@ expression rstc, dev, id; @@ -rstc = reset_control_get(dev, id); +rstc = reset_control_get_exclusive(dev, id);
I don't know if it has been discussed in the past, so forgive me if it has been. Have you considered adding a "int flags" argument to the existing reset_control_get_*() functions, rather than introducing separate exclusive variants ?
Indeed, with a "int flags" argument you could in the future add more variants/behaviors without actually multiplying the number of functions. Something like the "flags" argument for request_irq() for example.
Best regards,
Thomas
Hi Thomas,
On Wed, 2017-07-19 at 21:15 +0200, Thomas Petazzoni wrote:
Hello,
On Wed, 19 Jul 2017 17:25:04 +0200, Philipp Zabel wrote:
The reset control API has two modes: exclusive access, where the driver expects to have full and immediate control over the state of the reset line, and shared (clock-like) access, where drivers only request reset deassertion while active, but don't care about the state of the reset line while inactive.
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior.
This series converts all drivers that currently implicitly request exclusive reset controls to the corresponding explicit API call. It is, for the most part, generated from the following semantic patch:
@@ expression rstc, dev, id; @@ -rstc = reset_control_get(dev, id); +rstc = reset_control_get_exclusive(dev, id);
I don't know if it has been discussed in the past, so forgive me if it has been. Have you considered adding a "int flags" argument to the existing reset_control_get_*() functions, rather than introducing separate exclusive variants ?
Indeed, with a "int flags" argument you could in the future add more variants/behaviors without actually multiplying the number of functions. Something like the "flags" argument for request_irq() for example.
I can't find the discussion right now, but I remember we had talked about this in the past. Behind the scenes, all the inline API functions already call common entry points with flags (well, currently separate bool parameters for shared and optional). One reason against exposing those as an int flags in the user facing API is the possibility to accidentally provide a wrong value.
regards Philipp
Hello,
On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote:
I don't know if it has been discussed in the past, so forgive me if it has been. Have you considered adding a "int flags" argument to the existing reset_control_get_*() functions, rather than introducing separate exclusive variants ?
Indeed, with a "int flags" argument you could in the future add more variants/behaviors without actually multiplying the number of functions. Something like the "flags" argument for request_irq() for example.
I can't find the discussion right now, but I remember we had talked about this in the past. Behind the scenes, all the inline API functions already call common entry points with flags (well, currently separate bool parameters for shared and optional). One reason against exposing those as an int flags in the user facing API is the possibility to accidentally provide a wrong value.
This is a quite strange argument. You could also accidentally use the wrong variant of the function, just like you could use the wrong flag.
Once again, the next time you have another parameter for those reset functions, beyond the exclusive/shared variant, you will multiply again by two the number of functions ? You already have the exclusive/shared and optional/mandatory variants, so 4 variants. When you'll add a new parameter, you'll have 8 variants. Doesn't seem really good.
What about reset_control_get(struct device *, const char *, int flags) to replace all those variants ?
Best regards,
Thomas
Hi Thomas,
On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote:
Hello,
On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote:
I don't know if it has been discussed in the past, so forgive me if it has been. Have you considered adding a "int flags" argument to the existing reset_control_get_*() functions, rather than introducing separate exclusive variants ?
Indeed, with a "int flags" argument you could in the future add more variants/behaviors without actually multiplying the number of functions. Something like the "flags" argument for request_irq() for example.
I can't find the discussion right now, but I remember we had talked about this in the past. Behind the scenes, all the inline API functions already call common entry points with flags (well, currently separate bool parameters for shared and optional). One reason against exposing those as an int flags in the user facing API is the possibility to accidentally provide a wrong value.
This is a quite strange argument. You could also accidentally use the wrong variant of the function, just like you could use the wrong flag.
You can't accidentally use no flag at all or a completely bogus value with the "plethora of inline functions" variant.
Once again, the next time you have another parameter for those reset functions, beyond the exclusive/shared variant, you will multiply again by two the number of functions ? You already have the exclusive/shared and optional/mandatory variants, so 4 variants. When you'll add a new parameter, you'll have 8 variants. Doesn't seem really good.
I'd rather avoid adding more variants, if possible. The complexity increases regardless of whether the API is expressed as a bunch of functions or as a single function with a bunch of flags.
What about reset_control_get(struct device *, const char *, int flags) to replace all those variants ?
While I like how this looks, unfortunately (devm_)reset_control_get already exists without the flags, so we can't change to that with a gentle transition.
regards Philipp
On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel p.zabel@pengutronix.de wrote:
Hi Thomas,
On Thu, 2017-07-20 at 12:36 +0200, Thomas Petazzoni wrote:
Hello,
On Thu, 20 Jul 2017 11:36:55 +0200, Philipp Zabel wrote:
I don't know if it has been discussed in the past, so forgive me if it has been. Have you considered adding a "int flags" argument to the existing reset_control_get_*() functions, rather than introducing separate exclusive variants ?
Indeed, with a "int flags" argument you could in the future add more variants/behaviors without actually multiplying the number of functions. Something like the "flags" argument for request_irq() for example.
I can't find the discussion right now, but I remember we had talked about this in the past. Behind the scenes, all the inline API functions already call common entry points with flags (well, currently separate bool parameters for shared and optional). One reason against exposing those as an int flags in the user facing API is the possibility to accidentally provide a wrong value.
This is a quite strange argument. You could also accidentally use the wrong variant of the function, just like you could use the wrong flag.
You can't accidentally use no flag at all or a completely bogus value with the "plethora of inline functions" variant.
Once again, the next time you have another parameter for those reset functions, beyond the exclusive/shared variant, you will multiply again by two the number of functions ? You already have the exclusive/shared and optional/mandatory variants, so 4 variants. When you'll add a new parameter, you'll have 8 variants. Doesn't seem really good.
I'd rather avoid adding more variants, if possible. The complexity increases regardless of whether the API is expressed as a bunch of functions or as a single function with a bunch of flags.
What about reset_control_get(struct device *, const char *, int flags) to replace all those variants ?
While I like how this looks, unfortunately (devm_)reset_control_get already exists without the flags, so we can't change to that with a gentle transition.
This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users.
On Thu, Jul 20, 2017 at 10:46 PM, Dmitry Torokhov dmitry.torokhov@gmail.com wrote:
On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel p.zabel@pengutronix.de wrote:
What about reset_control_get(struct device *, const char *, int flags) to replace all those variants ?
While I like how this looks, unfortunately (devm_)reset_control_get already exists without the flags, so we can't change to that with a gentle transition.
This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users.
For your reference:
commit bae48da237fcedd7ad09569025483b988635efb7 "gpiolib: add gpiod_get() and gpiod_put() functions"
commit 39b2bbe3d715cf5013b5c48695ccdd25bd3bf120 "gpio: add flags argument to gpiod_get*() functions"
commit 0dbc8b7afef6e4fddcfebcbacbeb269a0a3b06d5 "gpio: move varargs hack outside #ifdef GPIOLIB"
commit b17d1bf16cc72a374a48d748940f700009d40ff4 "gpio: make flags mandatory for gpiod_get functions"
Retrospectively ... was that really a good idea... it was a LOT of trouble to add a flag, maybe it had been better to try and just slam all users in a single go.
But it worked.
Yours, Linus Walleij
On Sun, 2017-07-23 at 20:41 +0200, Linus Walleij wrote:
On Thu, Jul 20, 2017 at 10:46 PM, Dmitry Torokhov dmitry.torokhov@gmail.com wrote:
On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel p.zabel@pengutronix.de wrote:
What about reset_control_get(struct device *, const char *, int flags) to replace all those variants ?
While I like how this looks, unfortunately (devm_)reset_control_get already exists without the flags, so we can't change to that with a gentle transition.
This was done for gpiod_get() and its flags argument with horrifying #define-ry, which thankfully was completely hidden from users.
For your reference:
commit bae48da237fcedd7ad09569025483b988635efb7 "gpiolib: add gpiod_get() and gpiod_put() functions"
commit 39b2bbe3d715cf5013b5c48695ccdd25bd3bf120 "gpio: add flags argument to gpiod_get*() functions"
commit 0dbc8b7afef6e4fddcfebcbacbeb269a0a3b06d5 "gpio: move varargs hack outside #ifdef GPIOLIB"
commit b17d1bf16cc72a374a48d748940f700009d40ff4 "gpio: make flags mandatory for gpiod_get functions"
Retrospectively ... was that really a good idea... it was a LOT of trouble to add a flag, maybe it had been better to try and just slam all users in a single go.
But it worked.
Thanks for the hint and the references. It seems this turned out okay, but I wouldn't dare to introduce such macro horror^Wmagic. I'd rather have all users converted to the _exclusive/_shared function calls and maybe then replace the internal __reset_control_get with Thomas' suggestion.
regards Philipp
On Wed, Jul 19, 2017 at 05:25:04PM +0200, Philipp Zabel wrote:
The reset control API has two modes: exclusive access, where the driver expects to have full and immediate control over the state of the reset line, and shared (clock-like) access, where drivers only request reset deassertion while active, but don't care about the state of the reset line while inactive.
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior.
This series converts all drivers that currently implicitly request exclusive reset controls to the corresponding explicit API call. It is, for the most part, generated from the following semantic patch:
@@ expression rstc, dev, id; @@ -rstc = reset_control_get(dev, id); +rstc = reset_control_get_exclusive(dev, id); @@ expression rstc, dev, id; @@ -rstc = reset_control_get_optional(dev, id); +rstc = reset_control_get_optional_exclusive(dev, id); @@ expression rstc, node, id; @@ -rstc = of_reset_control_get(node, id); +rstc = of_reset_control_get_exclusive(node, id); @@ expression rstc, node, index; @@ -rstc = of_reset_control_get_by_index(node, index); +rstc = of_reset_control_get_exclusive_by_index(node, index); @@ expression rstc, dev, id; @@ -rstc = devm_reset_control_get(dev, id); +rstc = devm_reset_control_get_exclusive(dev, id); @@ expression rstc, dev, id; @@ -rstc = devm_reset_control_get_optional(dev, id); +rstc = devm_reset_control_get_optional_exclusive(dev, id); @@ expression rstc, dev, index; @@ -rstc = devm_reset_control_get_by_index(dev, index); +rstc = devm_reset_control_get_exclusive_by_index(dev, index);
After all driver patches are applied, the temporary transition helpers can be removed.
regards Philipp
Philipp Zabel (102): ARM: rockchip: explicitly request exclusive reset control ARM: socfpga: explicitly request exclusive reset control MIPS: pci-mt7620: explicitly request exclusive reset control ahci: st: explicitly request exclusive reset control ata: sata_gemini: explicitly request exclusive reset control ata: ahci_tegra: explicitly request exclusive reset control bus: sunxi-rsb: explicitly request exclusive reset control bus: tegra-gmi: explicitly request exclusive reset control clk: sunxi: explicitly request exclusive reset control clk: tegra: explicitly request exclusive reset control clocksource/drivers/timer-stm32: explicitly request exclusive reset control clocksource/drivers/sun5i: explicitly request exclusive reset control crypto: rockchip: explicitly request exclusive reset control crypto: sun4i-ss - request exclusive reset control PM / devfreq: tegra: explicitly request exclusive reset control dmaengine: stm32-dma: explicitly request exclusive reset control dmaengine: sun6i: explicitly request exclusive reset control dmaengine: tegra-apb: explicitly request exclusive reset control drm: kirin: explicitly request exclusive reset control drm/nouveau/tegra: explicitly request exclusive reset control drm/rockchip: explicitly request exclusive reset control drm/sti: explicitly request exclusive reset control drm/stm: explicitly request exclusive reset control drm/sun4i: explicitly request exclusive reset control drm/tegra: explicitly request exclusive reset control gpu: host1x: explicitly request exclusive reset control i2c: mv64xxx: explicitly request exclusive reset control i2c: stm32f4: explicitly request exclusive reset control i2c: sun6i-pw2i: explicitly request exclusive reset control i2c: tegra: explicitly request exclusive reset control iio: adc: rockchip_saradc: explicitly request exclusive reset control iio: dac: stm32-dac-core: explicitly request exclusive reset control Input: tegra-kbc - request exclusive reset control coda: explicitly request exclusive reset control st-rc: explicitly request exclusive reset control stm32-dcmi: explicitly request exclusive reset control rc: sunxi-cir: explicitly request exclusive reset control mmc: dw_mmc: explicitly request exclusive reset control mmc: sdhci-st: explicitly request exclusive reset control mmc: sunxi: explicitly request exclusive reset control mmc: tegra: explicitly request exclusive reset control mtd: nand: sunxi: explicitly request exclusive reset control mtd: spi-nor: stm32-quadspi: explicitly request exclusive reset control net: dsa: mt7530: explicitly request exclusive reset control net: ethernet: hisi_femac: explicitly request exclusive reset control net: ethernet: hix5hd2_gmac: explicitly request exclusive reset control net: stmmac: explicitly request exclusive reset control net: stmmac: dwc-qos: explicitly request exclusive reset control ath10k: explicitly request exclusive reset control nvmem: lpc18xx-eeprom: explicitly request exclusive reset control PCI: dwc: pcie-qcom: explicitly request exclusive reset control PCI: imx6: explicitly request exclusive reset control PCI: tegra: explicitly request exclusive reset control PCI: rockchip: explicitly request exclusive reset control phy: berlin-usb: explicitly request exclusive reset control PCI: mediatek: explicitly request exclusive reset control phy: qcom-usb-hs: explicitly request exclusive reset control phy: rockchip-pcie: explicitly request exclusive reset control phy: rockchip-typec: explicitly request exclusive reset control phy: rockchip-usb: explicitly request exclusive reset control phy: sun4i-usb: explicitly request exclusive reset control phy: sun9i-usb: explicitly request exclusive reset control phy: tegra: explicitly request exclusive reset control phy: qcom-qmp: explicitly request exclusive reset control phy: qcom-qusb2: explicitly request exclusive reset control pinctrl: stm32: explicitly request exclusive reset control pinctrl: sunxi: explicitly request exclusive reset control pinctrl: tegra: explicitly request exclusive reset control pwm: hibvt: explicitly request exclusive reset control pwm: tegra: explicitly request exclusive reset control remoteproc/keystone: explicitly request exclusive reset control remoteproc: qcom: explicitly request exclusive reset control remoteproc: st: explicitly request exclusive reset control soc: mediatek: PMIC wrap: explicitly request exclusive reset control soc/tegra: pmc: explicitly request exclusive reset control spi: stm32: explicitly request exclusive reset control spi: sun6i: explicitly request exclusive reset control spi: tegra20-slink: explicitly request exclusive reset control spi: tegra114: explicitly request exclusive reset control spi: tegra20-sflash: explicitly request exclusive reset control staging: nvec: explicitly request exclusive reset control thermal: rockchip: explicitly request exclusive reset control thermal: tegra: explicitly request exclusive reset control serial: 8250_dw: explicitly request exclusive reset control serial: tegra: explicitly request exclusive reset control usb: chipidea: msm: explicitly request exclusive reset control usb: dwc2: explicitly request exclusive reset control usb: host: ehci-tegra: explicitly request exclusive reset control usb: host: xhci-tegra: explicitly request exclusive reset control usb: musb: sunxi: explicitly request exclusive reset control usb: phy: msm: explicitly request exclusive reset control usb: phy: qcom-8x16-usb: explicitly request exclusive reset control watchdog: asm9260: explicitly request exclusive reset control watchdog: mt7621: explicitly request exclusive reset control watchdog: rt2880: explicitly request exclusive reset control watchdog: zx2967: explicitly request exclusive reset control ASoC: img: explicitly request exclusive reset control ASoC: stm32: explicitly request exclusive reset control ASoC: sun4i: explicitly request exclusive reset control ASoC: tegra: explicitly request exclusive reset control Documentation: devres: add explicit exclusive/shared reset control request calls reset: finish transition to explicit exclusive reset control requests
For all sunxi patches: Acked-by: Maxime Ripard maxime.ripard@free-electrons.com
Maxime
On Wed, Jul 19, 2017 at 05:25:04PM +0200, Philipp Zabel wrote:
The reset control API has two modes: exclusive access, where the driver expects to have full and immediate control over the state of the reset line, and shared (clock-like) access, where drivers only request reset deassertion while active, but don't care about the state of the reset line while inactive.
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior.
This series converts all drivers that currently implicitly request exclusive reset controls to the corresponding explicit API call. It is, for the most part, generated from the following semantic patch:
Hey, I'm all for large api changes, but this really seems ackward, isn't there a "better" way to do this?
Why not, as you say the "implicit" request is exclusive, just leave everything alone and state that the "reset_control_get()" call is exclusive and make the shared one the "odd" usage as that seems to not be the normal case.
That should be a much smaller patch right?
That way you don't break everything here, and require 100+ patches to just change the name of a function from one to another and do nothing else.
thanks,
greg k-h
Hi Greg,
The patches in this series are completely independent of each other, and I would like the subsystem maintainers to apply them at their own leisure. Well, except for the last one, which I will apply only after there are no more users of the transition helpers.
On Thu, 2017-07-20 at 10:11 +0200, Greg Kroah-Hartman wrote:
On Wed, Jul 19, 2017 at 05:25:04PM +0200, Philipp Zabel wrote:
The reset control API has two modes: exclusive access, where the driver expects to have full and immediate control over the state of the reset line, and shared (clock-like) access, where drivers only request reset deassertion while active, but don't care about the state of the reset line while inactive.
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines") started to transition the reset control request API calls to explicitly state whether the driver needs exclusive or shared reset control behavior.
This series converts all drivers that currently implicitly request exclusive reset controls to the corresponding explicit API call. It is, for the most part, generated from the following semantic patch:
Hey, I'm all for large api changes, but this really seems ackward, isn't there a "better" way to do this?
It is a bit awkward. I am sorry I haven't done this earlier. Quite a few new drivers started using the old API after the explicit requests were introduced last year.
Why not, as you say the "implicit" request is exclusive, just leave everything alone and state that the "reset_control_get()" call is exclusive
I think it is better to let the drivers explicitly state what they expect from the API, and using reset_control_get_exclusive vs _shared helps driver developers to make a conscious decision.
Further, the implicit API call predates shared reset support, so it is not clear that all of the old users really need exclusive control. A few drivers have been switched to the shared API already.
and make the shared one the "odd" usage as that seems to not be the normal case.
I am not sure, there have been people arguing that the "clock-like" case really is the common one. I suppose some of those drivers touched by the 100 patches in this series could also be changed to shared. But I don't dare to make this decision for each of them.
That should be a much smaller patch right?
That way you don't break everything here, and require 100+ patches to just change the name of a function from one to another and do nothing else.
I don't break anything here, and I'm absolutely fine with squashing patches together per subsystem where that is preferable.
regards Philipp
dri-devel@lists.freedesktop.org
-
Benjamin Gaignard -
Dmitry Torokhov -
Greg Kroah-Hartman -
Linus Walleij -
Maxime Ripard -
Philipp Zabel -
Thomas Petazzoni