| Message ID | 20240324201210.232301-3-andreas@kemnade.info (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mfd: rohm-bd71828: Add power off | expand |
On Mon, 25 Mar 2024, Lee Jones wrote: > On Mon, 25 Mar 2024, Matti Vaittinen wrote: > > > Hi Andreas, > > > > On 3/25/24 14:16, Andreas Kemnade wrote: > > > On Mon, 25 Mar 2024 13:31:15 +0200 > > > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > > > > > > On 3/24/24 22:12, Andreas Kemnade wrote: > > > > > Since the chip can power off the system, add the corresponding > > > > > functionality. > > > > > Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2 > > > > > No information source about the magic numbers found. > > > > > > > > Oh, interesting repository :) Thanks for linking to it! I didn't know > > > > someone had reworked this driver... > > > > > > > which btw: contains this interesting snippet (output from fdtdump) > > > bd71828-i2c@4b { > > > reg = <0x0000004b>; > > > compatible = "rohm,bd71828"; > > > gpio_int = <0x00000008 0x00000013 0x00000001>; > > > gpio_wdogb = <0x00000039 0x00000018 0x00000001>; > > > #address-cells = <0x00000001>; > > > #size-cells = <0x00000000>; > > > pmic@4b { > > > compatible = "rohm,bd71828"; > > > regulators { > > > BUCK1 { > > > regulator-name = "buck1"; > > > > > > > > > and to make it work since basically no regulators are registered > > > instead just some regmap_write()s are done to configure something > > > in probe(). It is a pitfall to think that the information below pmic@4b > > > is used, especially since it is not that obvious in the source. > > Odd! Not only did I not receive the original patch, I also did not > receive your response Andreas. Spam is empty too. > > LORE too: https://lore.kernel.org/all/?q=%22mfd:%20rohm-bd71828:%20Add%20power%20off%20functionality%22 Super weird! They just all came through. The LORE link above is now working too. I suspect an issue with kernel.org.
On Mon, 25 Mar 2024 16:51:34 +0000 Lee Jones <lee@kernel.org> wrote: > On Mon, 25 Mar 2024, Lee Jones wrote: > > > On Mon, 25 Mar 2024, Matti Vaittinen wrote: > > > > > Hi Andreas, > > > > > > On 3/25/24 14:16, Andreas Kemnade wrote: > > > > On Mon, 25 Mar 2024 13:31:15 +0200 > > > > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > > > > > > > > On 3/24/24 22:12, Andreas Kemnade wrote: > > > > > > Since the chip can power off the system, add the corresponding > > > > > > functionality. > > > > > > Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2 > > > > > > No information source about the magic numbers found. > > > > > > > > > > Oh, interesting repository :) Thanks for linking to it! I didn't know > > > > > someone had reworked this driver... > > > > > > > > > which btw: contains this interesting snippet (output from fdtdump) > > > > bd71828-i2c@4b { > > > > reg = <0x0000004b>; > > > > compatible = "rohm,bd71828"; > > > > gpio_int = <0x00000008 0x00000013 0x00000001>; > > > > gpio_wdogb = <0x00000039 0x00000018 0x00000001>; > > > > #address-cells = <0x00000001>; > > > > #size-cells = <0x00000000>; > > > > pmic@4b { > > > > compatible = "rohm,bd71828"; > > > > regulators { > > > > BUCK1 { > > > > regulator-name = "buck1"; > > > > > > > > > > > > and to make it work since basically no regulators are registered > > > > instead just some regmap_write()s are done to configure something > > > > in probe(). It is a pitfall to think that the information below pmic@4b > > > > is used, especially since it is not that obvious in the source. > > > > Odd! Not only did I not receive the original patch, I also did not > > receive your response Andreas. Spam is empty too. > > > > LORE too: https://lore.kernel.org/all/?q=%22mfd:%20rohm-bd71828:%20Add%20power%20off%20functionality%22 > > Super weird! They just all came through. > > The LORE link above is now working too. > > I suspect an issue with kernel.org. > Well, no, emails from me to everyone sitting behind a mail server without IPv6 address were affected. TCP ACK (3rd part of the handshake) was eaten up by something. Whatever... So nothing to blame kernel.org besides not having IPv6. Regards, Andreas
On Mon, 25 Mar 2024 13:13:13 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 24/03/2024 21:12, Andreas Kemnade wrote: > > struct regmap_irq_chip_data *irq_data; > > @@ -542,7 +560,18 @@ static int bd71828_i2c_probe(struct i2c_client *i2c) > > ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells, > > NULL, 0, regmap_irq_get_domain(irq_data)); > > if (ret) > > - dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n"); > > + return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n"); > > + > > + if (of_device_is_system_power_controller(i2c->dev.of_node)) { > > + if (!pm_power_off) { > > + bd71828_dev = i2c; > > + pm_power_off = bd71828_power_off; > > + ret = devm_add_action_or_reset(&i2c->dev, > > + bd71828_remove_poweroff, > > + NULL); > > + } else > > + dev_warn(&i2c->dev, "Poweroff callback already assigned\n"); > > Missing {} > > Please run scripts/checkpatch.pl and fix reported warnings. Some > warnings can be ignored, but the code here looks like it needs a fix. > Feel free to get in touch if the warning is not clear. > No, it does not complain about the {}. I was a bit unsure whether it is required or not, but I was sure that checkpatch.pl does catch such things. Yes, documentation clearly says that braces are required in those cases. Regards, Andreas
On 25/03/2024 21:21, Andreas Kemnade wrote: > On Mon, 25 Mar 2024 13:13:13 +0100 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 24/03/2024 21:12, Andreas Kemnade wrote: >>> struct regmap_irq_chip_data *irq_data; >>> @@ -542,7 +560,18 @@ static int bd71828_i2c_probe(struct i2c_client *i2c) >>> ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells, >>> NULL, 0, regmap_irq_get_domain(irq_data)); >>> if (ret) >>> - dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n"); >>> + return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n"); >>> + >>> + if (of_device_is_system_power_controller(i2c->dev.of_node)) { >>> + if (!pm_power_off) { >>> + bd71828_dev = i2c; >>> + pm_power_off = bd71828_power_off; >>> + ret = devm_add_action_or_reset(&i2c->dev, >>> + bd71828_remove_poweroff, >>> + NULL); >>> + } else >>> + dev_warn(&i2c->dev, "Poweroff callback already assigned\n"); >> >> Missing {} >> >> Please run scripts/checkpatch.pl and fix reported warnings. Some >> warnings can be ignored, but the code here looks like it needs a fix. >> Feel free to get in touch if the warning is not clear. >> > No, it does not complain about the {}. I was a bit unsure whether it is > required or not, but I was sure that checkpatch.pl does catch such things. > Yes, documentation clearly says that braces are required in those cases. "CHECK: braces {} should be used on all arms of this statement" I will update my template-response to use --strict. Best regards, Krzysztof
diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c index 594718f7e8e1..5a55aa3620d0 100644 --- a/drivers/mfd/rohm-bd71828.c +++ b/drivers/mfd/rohm-bd71828.c @@ -464,6 +464,24 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap, OUT32K_MODE_CMOS); } +static struct i2c_client *bd71828_dev; +static void bd71828_power_off(void) +{ + i2c_smbus_write_byte_data(bd71828_dev, 0x03, 0xff); + mdelay(500); + i2c_smbus_write_byte_data(bd71828_dev, BD71828_REG_INT_DCIN2, 0x02); + mdelay(500); + while (true) { + i2c_smbus_write_byte_data(bd71828_dev, BD71828_REG_PS_CTRL_1, 0x02); + mdelay(500); + } +} + +static void bd71828_remove_poweroff(void *data) +{ + bd71828_dev = NULL; +} + static int bd71828_i2c_probe(struct i2c_client *i2c) { struct regmap_irq_chip_data *irq_data; @@ -542,7 +560,18 @@ static int bd71828_i2c_probe(struct i2c_client *i2c) ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells, NULL, 0, regmap_irq_get_domain(irq_data)); if (ret) - dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n"); + return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n"); + + if (of_device_is_system_power_controller(i2c->dev.of_node)) { + if (!pm_power_off) { + bd71828_dev = i2c; + pm_power_off = bd71828_power_off; + ret = devm_add_action_or_reset(&i2c->dev, + bd71828_remove_poweroff, + NULL); + } else + dev_warn(&i2c->dev, "Poweroff callback already assigned\n"); + } return ret; }
Since the chip can power off the system, add the corresponding functionality. Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2 No information source about the magic numbers found. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> --- drivers/mfd/rohm-bd71828.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)