Re: [PATCH] drm/tegra: sor: Ensure regulators are disabled on teardown

2020-11-10 Thread Thierry Reding
On Tue, Oct 13, 2020 at 10:51:58AM +0100, Marc Zyngier wrote:
> The Tegra SOR driver uses the devm infrastructure to request regulators,
> but enables them without registering them with the infrastructure.
> 
> This results in the following splat if probing fails for any odd resaon
> (such as dependencies not being available):
> 
> [8.974187] tegra-sor 1558.sor: cannot get HDMI supply: -517
> [9.414403] tegra-sor 1558.sor: failed to probe HDMI: -517
> [9.421240] [ cut here ]
> [9.425879] WARNING: CPU: 1 PID: 164 at drivers/regulator/core.c:2089 
> _regulator_put.part.0+0x16c/0x174
> [9.435259] Modules linked in: tegra_drm(E+) cec(E) ahci_tegra(E) 
> drm_kms_helper(E) drm(E) libahci_platform(E) libahci(E) max77620_regulator(E) 
> xhci_tegra(E+) sdhci_tegra(E) xhci_hcd(E) libata(E) sdhci_pltfm(E) cqhci(E) 
> fixed(E) usbcore(E) scsi_mod(E) sdhci(E) host1x(E)
> [9.459211] CPU: 1 PID: 164 Comm: systemd-udevd Tainted: G SD W   E
>  5.9.0-rc7-00298-gf6337624c4fe #1980
> [9.469285] Hardware name: NVIDIA Jetson TX2 Developer Kit (DT)
> [9.475202] pstate: 8005 (Nzcv daif -PAN -UAO BTYPE=--)
> [9.480784] pc : _regulator_put.part.0+0x16c/0x174
> [9.485581] lr : regulator_put+0x44/0x60
> [9.489501] sp : ffc011d837b0
> [9.492814] x29: ffc011d837b0 x28: ff81dd085900
> [9.498141] x27: ff81de1c8ec0 x26: ff81de1c8c10
> [9.503464] x25: ff81dd085800 x24: ffc008f2c6b0
> [9.508790] x23: ffc0117373f0 x22: 0005
> [9.514101] x21: ff81dd085900 x20: ffc01172b098
> [9.515822] ata1: SATA link down (SStatus 0 SControl 300)
> [9.519426] x19: ff81dd085100 x18: 0030
> [9.530122] x17:  x16: 
> [9.535453] x15:  x14: 038f
> [9.540777] x13: 0003 x12: 0040
> [9.546105] x11: ff81eb80 x10: 0ae0
> [9.551417] x9 : ffc0106fea24 x8 : ff81de83e6c0
> [9.556728] x7 : 0018 x6 : 03c3
> [9.562064] x5 : 5660 x4 : 
> [9.567392] x3 : ffc01172b388 x2 : ff81de83db80
> [9.572702] x1 :  x0 : 0001
> [9.578034] Call trace:
> [9.580494]  _regulator_put.part.0+0x16c/0x174
> [9.584940]  regulator_put+0x44/0x60
> [9.588522]  devm_regulator_release+0x20/0x2c
> [9.592885]  release_nodes+0x1c8/0x2c0
> [9.596636]  devres_release_all+0x44/0x6c
> [9.600649]  really_probe+0x1ec/0x504
> [9.604316]  driver_probe_device+0x100/0x170
> [9.608589]  device_driver_attach+0xcc/0xd4
> [9.612774]  __driver_attach+0xb0/0x17c
> [9.616614]  bus_for_each_dev+0x7c/0xd4
> [9.620450]  driver_attach+0x30/0x3c
> [9.624027]  bus_add_driver+0x154/0x250
> [9.627867]  driver_register+0x84/0x140
> [9.631719]  __platform_register_drivers+0xa0/0x180
> [9.636660]  host1x_drm_init+0x60/0x1000 [tegra_drm]
> [9.641629]  do_one_initcall+0x54/0x2d0
> [9.645490]  do_init_module+0x68/0x29c
> [9.649244]  load_module+0x2178/0x26c0
> [9.652997]  __do_sys_finit_module+0xb0/0x120
> [9.657356]  __arm64_sys_finit_module+0x2c/0x40
> [9.661902]  el0_svc_common.constprop.0+0x80/0x240
> [9.666701]  do_el0_svc+0x30/0xa0
> [9.670022]  el0_svc+0x18/0x50
> [9.673081]  el0_sync_handler+0x90/0x318
> [9.677006]  el0_sync+0x158/0x180
> [9.680324] ---[ end trace 90f6c89d62d85ff6 ]---
> 
> Instead, let's register a callback that will disable the regulators
> on teardown. This allows for the removal of the .remove callbacks,
> which are not needed anymore.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/gpu/drm/tegra/sor.c | 59 +++--
>  1 file changed, 24 insertions(+), 35 deletions(-)

Applied, thanks.

Thierry


signature.asc
Description: PGP signature


[PATCH] drm/tegra: sor: Ensure regulators are disabled on teardown

2020-10-13 Thread Marc Zyngier
The Tegra SOR driver uses the devm infrastructure to request regulators,
but enables them without registering them with the infrastructure.

This results in the following splat if probing fails for any odd resaon
(such as dependencies not being available):

[8.974187] tegra-sor 1558.sor: cannot get HDMI supply: -517
[9.414403] tegra-sor 1558.sor: failed to probe HDMI: -517
[9.421240] [ cut here ]
[9.425879] WARNING: CPU: 1 PID: 164 at drivers/regulator/core.c:2089 
_regulator_put.part.0+0x16c/0x174
[9.435259] Modules linked in: tegra_drm(E+) cec(E) ahci_tegra(E) 
drm_kms_helper(E) drm(E) libahci_platform(E) libahci(E) max77620_regulator(E) 
xhci_tegra(E+) sdhci_tegra(E) xhci_hcd(E) libata(E) sdhci_pltfm(E) cqhci(E) 
fixed(E) usbcore(E) scsi_mod(E) sdhci(E) host1x(E)
[9.459211] CPU: 1 PID: 164 Comm: systemd-udevd Tainted: G SD W   E 
5.9.0-rc7-00298-gf6337624c4fe #1980
[9.469285] Hardware name: NVIDIA Jetson TX2 Developer Kit (DT)
[9.475202] pstate: 8005 (Nzcv daif -PAN -UAO BTYPE=--)
[9.480784] pc : _regulator_put.part.0+0x16c/0x174
[9.485581] lr : regulator_put+0x44/0x60
[9.489501] sp : ffc011d837b0
[9.492814] x29: ffc011d837b0 x28: ff81dd085900
[9.498141] x27: ff81de1c8ec0 x26: ff81de1c8c10
[9.503464] x25: ff81dd085800 x24: ffc008f2c6b0
[9.508790] x23: ffc0117373f0 x22: 0005
[9.514101] x21: ff81dd085900 x20: ffc01172b098
[9.515822] ata1: SATA link down (SStatus 0 SControl 300)
[9.519426] x19: ff81dd085100 x18: 0030
[9.530122] x17:  x16: 
[9.535453] x15:  x14: 038f
[9.540777] x13: 0003 x12: 0040
[9.546105] x11: ff81eb80 x10: 0ae0
[9.551417] x9 : ffc0106fea24 x8 : ff81de83e6c0
[9.556728] x7 : 0018 x6 : 03c3
[9.562064] x5 : 5660 x4 : 
[9.567392] x3 : ffc01172b388 x2 : ff81de83db80
[9.572702] x1 :  x0 : 0001
[9.578034] Call trace:
[9.580494]  _regulator_put.part.0+0x16c/0x174
[9.584940]  regulator_put+0x44/0x60
[9.588522]  devm_regulator_release+0x20/0x2c
[9.592885]  release_nodes+0x1c8/0x2c0
[9.596636]  devres_release_all+0x44/0x6c
[9.600649]  really_probe+0x1ec/0x504
[9.604316]  driver_probe_device+0x100/0x170
[9.608589]  device_driver_attach+0xcc/0xd4
[9.612774]  __driver_attach+0xb0/0x17c
[9.616614]  bus_for_each_dev+0x7c/0xd4
[9.620450]  driver_attach+0x30/0x3c
[9.624027]  bus_add_driver+0x154/0x250
[9.627867]  driver_register+0x84/0x140
[9.631719]  __platform_register_drivers+0xa0/0x180
[9.636660]  host1x_drm_init+0x60/0x1000 [tegra_drm]
[9.641629]  do_one_initcall+0x54/0x2d0
[9.645490]  do_init_module+0x68/0x29c
[9.649244]  load_module+0x2178/0x26c0
[9.652997]  __do_sys_finit_module+0xb0/0x120
[9.657356]  __arm64_sys_finit_module+0x2c/0x40
[9.661902]  el0_svc_common.constprop.0+0x80/0x240
[9.666701]  do_el0_svc+0x30/0xa0
[9.670022]  el0_svc+0x18/0x50
[9.673081]  el0_sync_handler+0x90/0x318
[9.677006]  el0_sync+0x158/0x180
[9.680324] ---[ end trace 90f6c89d62d85ff6 ]---

Instead, let's register a callback that will disable the regulators
on teardown. This allows for the removal of the .remove callbacks,
which are not needed anymore.

Signed-off-by: Marc Zyngier 
---
 drivers/gpu/drm/tegra/sor.c | 59 +++--
 1 file changed, 24 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 45b5258c77a2..39e6b32f6c10 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -397,7 +397,6 @@ struct tegra_sor;
 struct tegra_sor_ops {
const char *name;
int (*probe)(struct tegra_sor *sor);
-   int (*remove)(struct tegra_sor *sor);
void (*audio_enable)(struct tegra_sor *sor);
void (*audio_disable)(struct tegra_sor *sor);
 };
@@ -2942,6 +2941,24 @@ static const struct drm_encoder_helper_funcs 
tegra_sor_dp_helpers = {
.atomic_check = tegra_sor_encoder_atomic_check,
 };
 
+static void tegra_sor_disable_regulator(void *data)
+{
+   struct regulator *reg = data;
+
+   regulator_disable(reg);
+}
+
+static int tegra_sor_enable_regulator(struct tegra_sor *sor, struct regulator 
*reg)
+{
+   int err;
+
+   err = regulator_enable(reg);
+   if (err)
+   return err;
+
+   return devm_add_action_or_reset(sor->dev, tegra_sor_disable_regulator, 
reg);
+}
+
 static int tegra_sor_hdmi_probe(struct tegra_sor *sor)
 {
int err;
@@ -2953,7 +2970,7 @@ static int tegra_sor_hdmi_probe(struct tegra_sor *sor)
return PTR_ERR(sor->avdd_io_supply);
}
 
-   err = regulator_enable(sor->avdd_io_supply);
+