[alsa-devel] [PATCH 000/102] Convert drivers to explicit reset API
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- 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 +- 5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/img/img-i2s-in.c b/sound/soc/img/img-i2s-in.c index 0389203f85608..567f9767fb73e 100644 --- a/sound/soc/img/img-i2s-in.c +++ b/sound/soc/img/img-i2s-in.c @@ -443,7 +443,7 @@ static int img_i2s_in_probe(struct platform_device *pdev) SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE; i2s->dai_driver.ops = &img_i2s_in_dai_ops;
- rst = devm_reset_control_get(dev, "rst"); + rst = devm_reset_control_get_exclusive(dev, "rst"); if (IS_ERR(rst)) { if (PTR_ERR(rst) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; diff --git a/sound/soc/img/img-i2s-out.c b/sound/soc/img/img-i2s-out.c index 5f997135a8aec..78b7f6cd675b7 100644 --- a/sound/soc/img/img-i2s-out.c +++ b/sound/soc/img/img-i2s-out.c @@ -446,7 +446,7 @@ static int img_i2s_out_probe(struct platform_device *pdev)
i2s->channel_base = base + (max_i2s_chan_pow_2 * 0x20);
- i2s->rst = devm_reset_control_get(&pdev->dev, "rst"); + i2s->rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(i2s->rst)) { if (PTR_ERR(i2s->rst) != -EPROBE_DEFER) dev_err(&pdev->dev, "No top level reset found\n"); diff --git a/sound/soc/img/img-parallel-out.c b/sound/soc/img/img-parallel-out.c index 33ceb207ee704..23b0f0f6ec9cb 100644 --- a/sound/soc/img/img-parallel-out.c +++ b/sound/soc/img/img-parallel-out.c @@ -224,7 +224,7 @@ static int img_prl_out_probe(struct platform_device *pdev)
prl->base = base;
- prl->rst = devm_reset_control_get(&pdev->dev, "rst"); + prl->rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(prl->rst)) { if (PTR_ERR(prl->rst) != -EPROBE_DEFER) dev_err(&pdev->dev, "No top level reset found\n"); diff --git a/sound/soc/img/img-spdif-in.c b/sound/soc/img/img-spdif-in.c index 4d9953d318af4..8adfd65d43902 100644 --- a/sound/soc/img/img-spdif-in.c +++ b/sound/soc/img/img-spdif-in.c @@ -727,7 +727,7 @@ static int img_spdif_in_probe(struct platform_device *pdev) if (ret) return ret;
- rst = devm_reset_control_get(&pdev->dev, "rst"); + rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(rst)) { if (PTR_ERR(rst) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; diff --git a/sound/soc/img/img-spdif-out.c b/sound/soc/img/img-spdif-out.c index 08f93a5dadfe9..383655da2e60d 100644 --- a/sound/soc/img/img-spdif-out.c +++ b/sound/soc/img/img-spdif-out.c @@ -334,7 +334,7 @@ static int img_spdif_out_probe(struct platform_device *pdev)
spdif->base = base;
- spdif->rst = devm_reset_control_get(&pdev->dev, "rst"); + spdif->rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(spdif->rst)) { if (PTR_ERR(spdif->rst) != -EPROBE_DEFER) dev_err(&pdev->dev, "No top level reset found\n");
The patch
ASoC: img: explicitly request exclusive reset control
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 3a4e1b93110362f0ca6f81c564b04b2aa4bab853 Mon Sep 17 00:00:00 2001
From: Philipp Zabel p.zabel@pengutronix.de Date: Wed, 19 Jul 2017 17:26:41 +0200 Subject: [PATCH] ASoC: img: explicitly request exclusive reset control
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Mark Brown broonie@kernel.org --- 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 +- 5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/img/img-i2s-in.c b/sound/soc/img/img-i2s-in.c index 0389203f8560..567f9767fb73 100644 --- a/sound/soc/img/img-i2s-in.c +++ b/sound/soc/img/img-i2s-in.c @@ -443,7 +443,7 @@ static int img_i2s_in_probe(struct platform_device *pdev) SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE; i2s->dai_driver.ops = &img_i2s_in_dai_ops;
- rst = devm_reset_control_get(dev, "rst"); + rst = devm_reset_control_get_exclusive(dev, "rst"); if (IS_ERR(rst)) { if (PTR_ERR(rst) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; diff --git a/sound/soc/img/img-i2s-out.c b/sound/soc/img/img-i2s-out.c index 5f997135a8ae..78b7f6cd675b 100644 --- a/sound/soc/img/img-i2s-out.c +++ b/sound/soc/img/img-i2s-out.c @@ -446,7 +446,7 @@ static int img_i2s_out_probe(struct platform_device *pdev)
i2s->channel_base = base + (max_i2s_chan_pow_2 * 0x20);
- i2s->rst = devm_reset_control_get(&pdev->dev, "rst"); + i2s->rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(i2s->rst)) { if (PTR_ERR(i2s->rst) != -EPROBE_DEFER) dev_err(&pdev->dev, "No top level reset found\n"); diff --git a/sound/soc/img/img-parallel-out.c b/sound/soc/img/img-parallel-out.c index 33ceb207ee70..23b0f0f6ec9c 100644 --- a/sound/soc/img/img-parallel-out.c +++ b/sound/soc/img/img-parallel-out.c @@ -224,7 +224,7 @@ static int img_prl_out_probe(struct platform_device *pdev)
prl->base = base;
- prl->rst = devm_reset_control_get(&pdev->dev, "rst"); + prl->rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(prl->rst)) { if (PTR_ERR(prl->rst) != -EPROBE_DEFER) dev_err(&pdev->dev, "No top level reset found\n"); diff --git a/sound/soc/img/img-spdif-in.c b/sound/soc/img/img-spdif-in.c index 4d9953d318af..8adfd65d4390 100644 --- a/sound/soc/img/img-spdif-in.c +++ b/sound/soc/img/img-spdif-in.c @@ -727,7 +727,7 @@ static int img_spdif_in_probe(struct platform_device *pdev) if (ret) return ret;
- rst = devm_reset_control_get(&pdev->dev, "rst"); + rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(rst)) { if (PTR_ERR(rst) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; diff --git a/sound/soc/img/img-spdif-out.c b/sound/soc/img/img-spdif-out.c index 08f93a5dadfe..383655da2e60 100644 --- a/sound/soc/img/img-spdif-out.c +++ b/sound/soc/img/img-spdif-out.c @@ -334,7 +334,7 @@ static int img_spdif_out_probe(struct platform_device *pdev)
spdif->base = base;
- spdif->rst = devm_reset_control_get(&pdev->dev, "rst"); + spdif->rst = devm_reset_control_get_exclusive(&pdev->dev, "rst"); if (IS_ERR(spdif->rst)) { if (PTR_ERR(spdif->rst) != -EPROBE_DEFER) dev_err(&pdev->dev, "No top level reset found\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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com Cc: alsa-devel@alsa-project.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- sound/soc/stm/stm32_i2s.c | 2 +- sound/soc/stm/stm32_sai.c | 2 +- sound/soc/stm/stm32_spdifrx.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/stm/stm32_i2s.c b/sound/soc/stm/stm32_i2s.c index 8052629a89dfd..6d0bf78d114d0 100644 --- a/sound/soc/stm/stm32_i2s.c +++ b/sound/soc/stm/stm32_i2s.c @@ -840,7 +840,7 @@ static int stm32_i2s_parse_dt(struct platform_device *pdev, }
/* Reset */ - rst = devm_reset_control_get(&pdev->dev, NULL); + rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (!IS_ERR(rst)) { reset_control_assert(rst); udelay(2); diff --git a/sound/soc/stm/stm32_sai.c b/sound/soc/stm/stm32_sai.c index f7713314913b9..1258bef4dcb37 100644 --- a/sound/soc/stm/stm32_sai.c +++ b/sound/soc/stm/stm32_sai.c @@ -85,7 +85,7 @@ static int stm32_sai_probe(struct platform_device *pdev) }
/* reset */ - rst = reset_control_get(&pdev->dev, NULL); + rst = reset_control_get_exclusive(&pdev->dev, NULL); if (!IS_ERR(rst)) { reset_control_assert(rst); udelay(2); diff --git a/sound/soc/stm/stm32_spdifrx.c b/sound/soc/stm/stm32_spdifrx.c index 4e4250bdb75a8..84cc5678beba5 100644 --- a/sound/soc/stm/stm32_spdifrx.c +++ b/sound/soc/stm/stm32_spdifrx.c @@ -930,7 +930,7 @@ static int stm32_spdifrx_probe(struct platform_device *pdev) return ret; }
- rst = devm_reset_control_get(&pdev->dev, NULL); + rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (!IS_ERR(rst)) { reset_control_assert(rst); udelay(2);
The patch
ASoC: stm32: explicitly request exclusive reset control
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 635eac1e54d82c59f621a0f38a9bffae50f150c7 Mon Sep 17 00:00:00 2001
From: Philipp Zabel p.zabel@pengutronix.de Date: Wed, 19 Jul 2017 17:26:42 +0200 Subject: [PATCH] ASoC: stm32: explicitly request exclusive reset control
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: Maxime Coquelin mcoquelin.stm32@gmail.com Cc: Alexandre Torgue alexandre.torgue@st.com Cc: alsa-devel@alsa-project.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/stm/stm32_i2s.c | 2 +- sound/soc/stm/stm32_sai.c | 2 +- sound/soc/stm/stm32_spdifrx.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/stm/stm32_i2s.c b/sound/soc/stm/stm32_i2s.c index 8052629a89df..6d0bf78d114d 100644 --- a/sound/soc/stm/stm32_i2s.c +++ b/sound/soc/stm/stm32_i2s.c @@ -840,7 +840,7 @@ static int stm32_i2s_parse_dt(struct platform_device *pdev, }
/* Reset */ - rst = devm_reset_control_get(&pdev->dev, NULL); + rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (!IS_ERR(rst)) { reset_control_assert(rst); udelay(2); diff --git a/sound/soc/stm/stm32_sai.c b/sound/soc/stm/stm32_sai.c index f7713314913b..1258bef4dcb3 100644 --- a/sound/soc/stm/stm32_sai.c +++ b/sound/soc/stm/stm32_sai.c @@ -85,7 +85,7 @@ static int stm32_sai_probe(struct platform_device *pdev) }
/* reset */ - rst = reset_control_get(&pdev->dev, NULL); + rst = reset_control_get_exclusive(&pdev->dev, NULL); if (!IS_ERR(rst)) { reset_control_assert(rst); udelay(2); diff --git a/sound/soc/stm/stm32_spdifrx.c b/sound/soc/stm/stm32_spdifrx.c index 4e4250bdb75a..84cc5678beba 100644 --- a/sound/soc/stm/stm32_spdifrx.c +++ b/sound/soc/stm/stm32_spdifrx.c @@ -930,7 +930,7 @@ static int stm32_spdifrx_probe(struct platform_device *pdev) return ret; }
- rst = devm_reset_control_get(&pdev->dev, NULL); + rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (!IS_ERR(rst)) { reset_control_assert(rst); udelay(2);
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: Maxime Ripard maxime.ripard@free-electrons.com Cc: Chen-Yu Tsai wens@csie.org Cc: alsa-devel@alsa-project.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- sound/soc/sunxi/sun4i-codec.c | 3 ++- sound/soc/sunxi/sun4i-i2s.c | 2 +- sound/soc/sunxi/sun4i-spdif.c | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c index 150069987c0c3..9eec303605224 100644 --- a/sound/soc/sunxi/sun4i-codec.c +++ b/sound/soc/sunxi/sun4i-codec.c @@ -1574,7 +1574,8 @@ static int sun4i_codec_probe(struct platform_device *pdev) }
if (quirks->has_reset) { - scodec->rst = devm_reset_control_get(&pdev->dev, NULL); + scodec->rst = devm_reset_control_get_exclusive(&pdev->dev, + NULL); if (IS_ERR(scodec->rst)) { dev_err(&pdev->dev, "Failed to get reset control\n"); return PTR_ERR(scodec->rst); diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 3635bbc72cbcd..62b307b0c846c 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -716,7 +716,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev) }
if (quirks->has_reset) { - i2s->rst = devm_reset_control_get(&pdev->dev, NULL); + i2s->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (IS_ERR(i2s->rst)) { dev_err(&pdev->dev, "Failed to get reset control\n"); return PTR_ERR(i2s->rst); diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c index eaefd07a5ed08..c49f3757b686e 100644 --- a/sound/soc/sunxi/sun4i-spdif.c +++ b/sound/soc/sunxi/sun4i-spdif.c @@ -520,7 +520,8 @@ static int sun4i_spdif_probe(struct platform_device *pdev) platform_set_drvdata(pdev, host);
if (quirks->has_reset) { - host->rst = devm_reset_control_get_optional(&pdev->dev, NULL); + host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, + NULL); if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
The patch
ASoC: sun4i: explicitly request exclusive reset control
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 72bfa2117bdb4bb5b07dd5ed833ff3c318fc70b6 Mon Sep 17 00:00:00 2001
From: Philipp Zabel p.zabel@pengutronix.de Date: Wed, 19 Jul 2017 17:26:43 +0200 Subject: [PATCH] ASoC: sun4i: explicitly request exclusive reset control
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: Maxime Ripard maxime.ripard@free-electrons.com Cc: Chen-Yu Tsai wens@csie.org Cc: alsa-devel@alsa-project.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sunxi/sun4i-codec.c | 3 ++- sound/soc/sunxi/sun4i-i2s.c | 2 +- sound/soc/sunxi/sun4i-spdif.c | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c index 4c37231f254b..73d054f466c2 100644 --- a/sound/soc/sunxi/sun4i-codec.c +++ b/sound/soc/sunxi/sun4i-codec.c @@ -1573,7 +1573,8 @@ static int sun4i_codec_probe(struct platform_device *pdev) }
if (quirks->has_reset) { - scodec->rst = devm_reset_control_get(&pdev->dev, NULL); + scodec->rst = devm_reset_control_get_exclusive(&pdev->dev, + NULL); if (IS_ERR(scodec->rst)) { dev_err(&pdev->dev, "Failed to get reset control\n"); return PTR_ERR(scodec->rst); diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 3635bbc72cbc..62b307b0c846 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -716,7 +716,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev) }
if (quirks->has_reset) { - i2s->rst = devm_reset_control_get(&pdev->dev, NULL); + i2s->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (IS_ERR(i2s->rst)) { dev_err(&pdev->dev, "Failed to get reset control\n"); return PTR_ERR(i2s->rst); diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c index eaefd07a5ed0..c49f3757b686 100644 --- a/sound/soc/sunxi/sun4i-spdif.c +++ b/sound/soc/sunxi/sun4i-spdif.c @@ -520,7 +520,8 @@ static int sun4i_spdif_probe(struct platform_device *pdev) platform_set_drvdata(pdev, host);
if (quirks->has_reset) { - host->rst = devm_reset_control_get_optional(&pdev->dev, NULL); + host->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, + NULL); if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) { ret = -EPROBE_DEFER; dev_err(&pdev->dev, "Failed to get reset: %d\n", ret);
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: Thierry Reding thierry.reding@gmail.com Cc: Jonathan Hunter jonathanh@nvidia.com Cc: alsa-devel@alsa-project.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- sound/soc/tegra/tegra30_ahub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c index 8c10ae7982bad..43679aeeb12be 100644 --- a/sound/soc/tegra/tegra30_ahub.c +++ b/sound/soc/tegra/tegra30_ahub.c @@ -544,8 +544,8 @@ static int tegra30_ahub_probe(struct platform_device *pdev) soc_data->mod_list_mask)) continue;
- rst = reset_control_get(&pdev->dev, - configlink_mods[i].rst_name); + rst = reset_control_get_exclusive(&pdev->dev, + configlink_mods[i].rst_name); if (IS_ERR(rst)) { dev_err(&pdev->dev, "Can't get reset %s\n", configlink_mods[i].rst_name);
The patch
ASoC: tegra: explicitly request exclusive reset control
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 181e8ce6a6c9a25ffbf10b09d8481f134c5a79ba Mon Sep 17 00:00:00 2001
From: Philipp Zabel p.zabel@pengutronix.de Date: Wed, 19 Jul 2017 17:26:44 +0200 Subject: [PATCH] ASoC: tegra: explicitly request exclusive reset control
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: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: Thierry Reding thierry.reding@gmail.com Cc: Jonathan Hunter jonathanh@nvidia.com Cc: alsa-devel@alsa-project.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/tegra/tegra30_ahub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c index 8c10ae7982ba..43679aeeb12b 100644 --- a/sound/soc/tegra/tegra30_ahub.c +++ b/sound/soc/tegra/tegra30_ahub.c @@ -544,8 +544,8 @@ static int tegra30_ahub_probe(struct platform_device *pdev) soc_data->mod_list_mask)) continue;
- rst = reset_control_get(&pdev->dev, - configlink_mods[i].rst_name); + rst = reset_control_get_exclusive(&pdev->dev, + configlink_mods[i].rst_name); if (IS_ERR(rst)) { dev_err(&pdev->dev, "Can't get reset %s\n", configlink_mods[i].rst_name);
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
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.
I didn't follow the discussion closely. Shall I still apply the i2c patches?
On Sat, 2017-08-12 at 13:43 +0200, Wolfram Sang wrote:
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.
I didn't follow the discussion closely. Shall I still apply the i2c patches?
Yes, please.
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
Hi,
crypto: rockchip: explicitly request exclusive reset control iio: adc: rockchip_saradc: explicitly request exclusive reset control PCI: rockchip: 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 thermal: rockchip: explicitly request exclusive reset control
for the driver-related Rockchip changes
Acked-by: Heiko Stuebner heiko@sntech.de
participants (9)
-
Dmitry Torokhov -
Greg Kroah-Hartman -
Heiko Stuebner -
Linus Walleij -
Mark Brown -
Maxime Ripard -
Philipp Zabel -
Thomas Petazzoni -
Wolfram Sang