| Message ID | 20241104203555.61104-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] platform/x86/intel: bytcrc_pwrsrc: Optionally register a power_supply dev | expand |
On Mon, Nov 4, 2024 at 10:36 PM Hans de Goede <hdegoede@redhat.com> wrote: > > On some Android tablets with Crystal Cove PMIC the DSDT lacks an ACPI AC > device to indicate whether a charger is plugged in or not. > > Add support for registering a "crystal_cove_pwrsrc" power_supply class > device to indicate charger online status. This is made conditional on > a "linux,register-pwrsrc-power_supply" boolean device-property to avoid > registering a duplicate power_supply class device on devices where this > is already handled by an ACPI AC device. > > Note the "linux,register-pwrsrc-power_supply" property is only used on > x86/ACPI (non devicetree) devs and the devicetree-bindings maintainers > have requested properties like these to not be added to the devicetree > bindings, so the new property is deliberately not added to any bindings. Reviewed-by: Andy Shevchenko <andy@kernel.org> ... + array_size.h > +#include <linux/bits.h> > #include <linux/debugfs.h> > +#include <linux/interrupt.h> > #include <linux/mfd/intel_soc_pmic.h> > #include <linux/module.h> > #include <linux/platform_device.h> > +#include <linux/power_supply.h> > +#include <linux/property.h> > #include <linux/regmap.h> ... > + if (device_property_read_bool(pdev->dev.parent, "linux,register-pwrsrc-power_supply")) { Btw, is that property type of boolean? If not, device_property_present() has to be used. ... > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return dev_err_probe(&pdev->dev, irq, "getting IRQ\n"); This dups the embedded error message. ... > + data->psy = devm_power_supply_register(&pdev->dev, &crc_pwrsrc_psy_desc, &psy_cfg); > + if (IS_ERR(data->psy)) > + return dev_err_probe(&pdev->dev, PTR_ERR(data->psy), > + "registering power-supply\n"); > + > + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, > + crc_pwrsrc_irq_handler, > + IRQF_ONESHOT, KBUILD_MODNAME, data); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "requesting IRQ\n"); With struct device *dev = &pdev->dev; at the top of the function you may make lines shorten and neater. > + }
Hi, On 5-Nov-24 9:51 AM, Andy Shevchenko wrote: > On Mon, Nov 4, 2024 at 10:36 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> On some Android tablets with Crystal Cove PMIC the DSDT lacks an ACPI AC >> device to indicate whether a charger is plugged in or not. >> >> Add support for registering a "crystal_cove_pwrsrc" power_supply class >> device to indicate charger online status. This is made conditional on >> a "linux,register-pwrsrc-power_supply" boolean device-property to avoid >> registering a duplicate power_supply class device on devices where this >> is already handled by an ACPI AC device. >> >> Note the "linux,register-pwrsrc-power_supply" property is only used on >> x86/ACPI (non devicetree) devs and the devicetree-bindings maintainers >> have requested properties like these to not be added to the devicetree >> bindings, so the new property is deliberately not added to any bindings. > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > > ... > > + array_size.h > >> +#include <linux/bits.h> >> #include <linux/debugfs.h> >> +#include <linux/interrupt.h> >> #include <linux/mfd/intel_soc_pmic.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> +#include <linux/power_supply.h> >> +#include <linux/property.h> >> #include <linux/regmap.h> > > ... > >> + if (device_property_read_bool(pdev->dev.parent, "linux,register-pwrsrc-power_supply")) { > > Btw, is that property type of boolean? If not, > device_property_present() has to be used. Yes this is a boolean property, as for your other remarks I agree and I'll address them for the upcoming v2. Regards, Hans
diff --git a/drivers/platform/x86/intel/bytcrc_pwrsrc.c b/drivers/platform/x86/intel/bytcrc_pwrsrc.c index 418b71af27ff..98b5057d4c68 100644 --- a/drivers/platform/x86/intel/bytcrc_pwrsrc.c +++ b/drivers/platform/x86/intel/bytcrc_pwrsrc.c @@ -8,13 +8,21 @@ * Copyright (C) 2013 Intel Corporation */ +#include <linux/bits.h> #include <linux/debugfs.h> +#include <linux/interrupt.h> #include <linux/mfd/intel_soc_pmic.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/power_supply.h> +#include <linux/property.h> #include <linux/regmap.h> +#define CRYSTALCOVE_PWRSRC_IRQ 0x03 #define CRYSTALCOVE_SPWRSRC_REG 0x1E +#define CRYSTALCOVE_SPWRSRC_USB BIT(0) +#define CRYSTALCOVE_SPWRSRC_DC BIT(1) +#define CRYSTALCOVE_SPWRSRC_BATTERY BIT(2) #define CRYSTALCOVE_RESETSRC0_REG 0x20 #define CRYSTALCOVE_RESETSRC1_REG 0x21 #define CRYSTALCOVE_WAKESRC_REG 0x22 @@ -22,6 +30,7 @@ struct crc_pwrsrc_data { struct regmap *regmap; struct dentry *debug_dentry; + struct power_supply *psy; unsigned int resetsrc0; unsigned int resetsrc1; unsigned int wakesrc; @@ -118,11 +127,57 @@ static int crc_pwrsrc_read_and_clear(struct crc_pwrsrc_data *data, return regmap_write(data->regmap, reg, *val); } +static irqreturn_t crc_pwrsrc_irq_handler(int irq, void *_data) +{ + struct crc_pwrsrc_data *data = _data; + unsigned int irq_mask; + + if (regmap_read(data->regmap, CRYSTALCOVE_PWRSRC_IRQ, &irq_mask)) + return IRQ_NONE; + + regmap_write(data->regmap, CRYSTALCOVE_PWRSRC_IRQ, irq_mask); + + power_supply_changed(data->psy); + return IRQ_HANDLED; +} + +static int crc_pwrsrc_psy_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct crc_pwrsrc_data *data = power_supply_get_drvdata(psy); + unsigned int pwrsrc; + int ret; + + if (psp != POWER_SUPPLY_PROP_ONLINE) + return -EINVAL; + + ret = regmap_read(data->regmap, CRYSTALCOVE_SPWRSRC_REG, &pwrsrc); + if (ret) + return ret; + + val->intval = !!(pwrsrc & (CRYSTALCOVE_SPWRSRC_USB | + CRYSTALCOVE_SPWRSRC_DC)); + return 0; +} + +static const enum power_supply_property crc_pwrsrc_psy_props[] = { + POWER_SUPPLY_PROP_ONLINE, +}; + +static const struct power_supply_desc crc_pwrsrc_psy_desc = { + .name = "crystal_cove_pwrsrc", + .type = POWER_SUPPLY_TYPE_MAINS, + .properties = crc_pwrsrc_psy_props, + .num_properties = ARRAY_SIZE(crc_pwrsrc_psy_props), + .get_property = crc_pwrsrc_psy_get_property, +}; + static int crc_pwrsrc_probe(struct platform_device *pdev) { struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); struct crc_pwrsrc_data *data; - int ret; + int irq, ret; data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); if (!data) @@ -149,6 +204,25 @@ static int crc_pwrsrc_probe(struct platform_device *pdev) if (ret) return ret; + if (device_property_read_bool(pdev->dev.parent, "linux,register-pwrsrc-power_supply")) { + struct power_supply_config psy_cfg = { .drv_data = data }; + + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return dev_err_probe(&pdev->dev, irq, "getting IRQ\n"); + + data->psy = devm_power_supply_register(&pdev->dev, &crc_pwrsrc_psy_desc, &psy_cfg); + if (IS_ERR(data->psy)) + return dev_err_probe(&pdev->dev, PTR_ERR(data->psy), + "registering power-supply\n"); + + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, + crc_pwrsrc_irq_handler, + IRQF_ONESHOT, KBUILD_MODNAME, data); + if (ret) + return dev_err_probe(&pdev->dev, ret, "requesting IRQ\n"); + } + data->debug_dentry = debugfs_create_dir(KBUILD_MODNAME, NULL); debugfs_create_file("pwrsrc", 0444, data->debug_dentry, data, &pwrsrc_fops); debugfs_create_file("resetsrc", 0444, data->debug_dentry, data, &resetsrc_fops);
On some Android tablets with Crystal Cove PMIC the DSDT lacks an ACPI AC device to indicate whether a charger is plugged in or not. Add support for registering a "crystal_cove_pwrsrc" power_supply class device to indicate charger online status. This is made conditional on a "linux,register-pwrsrc-power_supply" boolean device-property to avoid registering a duplicate power_supply class device on devices where this is already handled by an ACPI AC device. Note the "linux,register-pwrsrc-power_supply" property is only used on x86/ACPI (non devicetree) devs and the devicetree-bindings maintainers have requested properties like these to not be added to the devicetree bindings, so the new property is deliberately not added to any bindings. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/intel/bytcrc_pwrsrc.c | 76 +++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-)