| Message ID | 20230327-tegra-pmic-reboot-v7-5-18699d5dcd76@skidata.com |
|---|---|
| State | New |
| Headers | show |
| Series | mfd: tps6586x: register restart handler | expand |
15.07.2023 10:53, Benjamin Bara пишет: > From: Benjamin Bara <benjamin.bara@skidata.com> > > There are a couple of boards which use a tps6586x as > "ti,system-power-controller", e.g. the tegra20-tamonten.dtsi. > For these, the only registered restart handler is the warm reboot via > tegra's PMC. As the bootloader of the tegra20 requires the VDE, it must > be ensured that VDE is enabled (which is the case after a cold reboot). > For the "normal reboot", this is basically the case since 8f0c714ad9be. > However, this workaround is not executed in case of an emergency restart. > In case of an emergency restart, the system now simply hangs in the > bootloader, as VDE is not enabled (because it is not used). > > The TPS658629-Q1 provides a SOFT RST bit in the SUPPLYENE reg to request > a (cold) reboot, which takes at least 20ms (as the data sheet states). > This avoids the hang-up. > > Tested on a TPS658640. > > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > Acked-for-MFD-by: Lee Jones <lee@kernel.org> Acked-for-MFD-by isn't a valid tag, scripts/checkpatch.pl should tell you about it. In general you may add a comment to a tag, like this: Acked-by: Lee Jones <lee@kernel.org> # for MFD In this particular case, the comment is unnecessary because Lee is the MFD maintainer, hence his ack itself implies the MFD subsys.
Hi Dmitry, thanks for the feedback! On Tue, 18 Jul 2023 at 06:46, Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > 15.07.2023 10:53, Benjamin Bara пишет: > > > > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > > Acked-for-MFD-by: Lee Jones <lee@kernel.org> > > Acked-for-MFD-by isn't a valid tag, scripts/checkpatch.pl should tell > you about it. > > In general you may add a comment to a tag, like this: > > Acked-by: Lee Jones <lee@kernel.org> # for MFD > > In this particular case, the comment is unnecessary because Lee is the > MFD maintainer, hence his ack itself implies the MFD subsys. I saw the warning, but Lee requested to add it like this [1]. @Konstantin: Do you think it makes sense to print a warning when adding "non-standard trailers" during running "b4 trailers -u", maybe around the find_trailers() checks? I could provide a RFC, if considered useful. Best regards, Benjamin [1] https://lore.kernel.org/all/20230518094434.GD404509@google.com/
July 19, 2023 at 4:22 AM, "Benjamin Bara" <bbara93@gmail.com> wrote: > @Konstantin: > Do you think it makes sense to print a warning when adding "non-standard > trailers" during running "b4 trailers -u", maybe around the > find_trailers() checks? I could provide a RFC, if considered useful. With b4 being used for other projects than just the Linux kernel, I don't think it makes sense for us to track what is a valid and what is an invalid "person-trailer". I know that we could make it configurable, but I don't think this will actually improve the situation. One goal for b4 is to allow defining validation tests and requiring them prior to "b4 send", so I think this is a better mechanism for dealing with such situations. -K
diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c index b12c9e18970a..1777d8d3a990 100644 --- a/drivers/mfd/tps6586x.c +++ b/drivers/mfd/tps6586x.c @@ -30,6 +30,7 @@ #include <linux/mfd/tps6586x.h> #define TPS6586X_SUPPLYENE 0x14 +#define SOFT_RST_BIT BIT(0) #define EXITSLREQ_BIT BIT(1) #define SLEEP_MODE_BIT BIT(3) @@ -475,6 +476,19 @@ static int tps6586x_power_off_handler(struct sys_off_data *data) return notifier_from_errno(-ETIME); } +static int tps6586x_restart_handler(struct sys_off_data *data) +{ + int ret; + + /* Put the PMIC into hard reboot state. This takes at least 20ms. */ + ret = tps6586x_set_bits(data->dev, TPS6586X_SUPPLYENE, SOFT_RST_BIT); + if (ret) + return notifier_from_errno(ret); + + mdelay(50); + return notifier_from_errno(-ETIME); +} + static void tps6586x_print_version(struct i2c_client *client, int version) { const char *name; @@ -575,6 +589,13 @@ static int tps6586x_i2c_probe(struct i2c_client *client) dev_err(&client->dev, "register power off handler failed: %d\n", ret); goto err_add_devs; } + + ret = devm_register_restart_handler(&client->dev, &tps6586x_restart_handler, + NULL); + if (ret) { + dev_err(&client->dev, "register restart handler failed: %d\n", ret); + goto err_add_devs; + } } return 0;