| Message ID | cover.1747409892.git.samuel.kayode@savoirfairelinux.com |
|---|---|
| Headers | show |
| Series | add support for pf1550 PMIC MFD-based drivers | expand |
On 16/05/2025 20:47, Samuel Kayode wrote: > Add the DT binding document for the battery charger module of pf1550. > > Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com> > --- > .../bindings/power/supply/pf1550_charger.yaml | 44 +++++++++++++++++++ > 1 file changed, 44 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/pf1550_charger.yaml Filename matching compatible. > > diff --git a/Documentation/devicetree/bindings/power/supply/pf1550_charger.yaml b/Documentation/devicetree/bindings/power/supply/pf1550_charger.yaml > new file mode 100644 > index 000000000000..10fc0b35917c > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/pf1550_charger.yaml > @@ -0,0 +1,44 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/power/supply/pf1550_charger.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Battery charger driver for PF1550 PMIC from NXP. Describe hardware, not driver. Also drop full stop. <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time. Please kindly resend and include all necessary To/Cc entries. </form letter> Best regards, Krzysztof
On 16/05/2025 20:52, Samuel Kayode wrote: > Add the DT binding document for the onkey module of pf1550. > > Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com> > --- > .../bindings/input/pf1550_onkey.yaml | 31 +++++++++++++++++++ Same comments as for other patch. Best regards, Krzysztof
On 16/05/2025 20:54, Samuel Kayode wrote: > + > +static int pf1550_i2c_probe(struct i2c_client *i2c) > +{ > + struct pf1550_dev *pf1550; > + unsigned int reg_data = 0; > + int ret = 0; > + > + pf1550 = devm_kzalloc(&i2c->dev, > + sizeof(struct pf1550_dev), GFP_KERNEL); sizeof(*) > + if (!pf1550) > + return -ENOMEM; > + > + i2c_set_clientdata(i2c, pf1550); > + pf1550->dev = &i2c->dev; > + pf1550->i2c = i2c; > + pf1550->irq = i2c->irq; > + > + pf1550->regmap = devm_regmap_init_i2c(i2c, &pf1550_regmap_config); > + if (IS_ERR(pf1550->regmap)) { > + ret = PTR_ERR(pf1550->regmap); > + dev_err(pf1550->dev, "failed to allocate register map: %d\n", > + ret); Syntax is always: return dev_err_probe > + return ret; > + } > + > + ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, ®_data); > + if (ret < 0 || reg_data != PF1550_DEVICE_ID) { > + dev_err(pf1550->dev, "device not found!\n"); > + return ret; Syntax is always: return dev_err_probe > + } > + > + pf1550->type = PF1550; > + dev_info(pf1550->dev, "pf1550 found.\n"); Drop. Drivers should be silent. This is really useless and just pollutes log. See also coding style. > + > + ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, > + pf1550->irq, > + IRQF_ONESHOT | IRQF_SHARED | > + IRQF_TRIGGER_FALLING, 0, > + &pf1550_regulator_irq_chip, > + &pf1550->irq_data_regulator); > + if (ret) { > + dev_err(pf1550->dev, "failed to add irq1 chip: %d\n", ret); > + return ret; Syntax is always: return dev_err_probe > + } > + > + ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, > + pf1550->irq, > + IRQF_ONESHOT | IRQF_SHARED | > + IRQF_TRIGGER_FALLING, 0, > + &pf1550_onkey_irq_chip, > + &pf1550->irq_data_onkey); > + if (ret) { > + dev_err(pf1550->dev, "failed to add irq3 chip: %d\n", ret); > + return ret; Syntax is always: return dev_err_probe > + } > + > + ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, > + pf1550->irq, > + IRQF_ONESHOT | IRQF_SHARED | > + IRQF_TRIGGER_FALLING, 0, > + &pf1550_charger_irq_chip, > + &pf1550->irq_data_charger); > + if (ret) { > + dev_err(pf1550->dev, "failed to add irq4 chip: %d\n", ret); > + return ret; Syntax is always: return dev_err_probe > + } > + > + return devm_mfd_add_devices(pf1550->dev, -1, pf1550_devs, > + ARRAY_SIZE(pf1550_devs), NULL, 0, NULL); > +} > + > +static const struct i2c_device_id pf1550_i2c_id[] = { > + { "pf1550", PF1550 }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(i2c, pf1550_i2c_id); Table IDs are next to each other. > + > +static int pf1550_suspend(struct device *dev) > +{ > + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev); > + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c); > + > + if (device_may_wakeup(dev)) { > + enable_irq_wake(pf1550->irq); > + disable_irq(pf1550->irq); > + } > + > + return 0; > +} > + > +static int pf1550_resume(struct device *dev) > +{ > + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev); > + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c); > + > + if (device_may_wakeup(dev)) { > + disable_irq_wake(pf1550->irq); > + enable_irq(pf1550->irq); > + } > + > + return 0; > +} > + > +static DEFINE_SIMPLE_DEV_PM_OPS(pf1550_pm, pf1550_suspend, pf1550_resume); > + > +static const struct of_device_id pf1550_dt_match[] = { > + { .compatible = "fsl,pf1550" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, pf1550_dt_match); > + > +static struct i2c_driver pf1550_i2c_driver = { > + .driver = { > + .name = "pf1550", > + .pm = pm_sleep_ptr(&pf1550_pm), > + .of_match_table = of_match_ptr(pf1550_dt_match), Drop of_match_ptr, you have warnings here. > + }, > + .probe = pf1550_i2c_probe, > + .id_table = pf1550_i2c_id, > +}; > + > +module_i2c_driver(pf1550_i2c_driver); > + > +MODULE_DESCRIPTION("Freescale PF1550 multi-function core driver"); > +MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>"); > +MODULE_LICENSE("GPL v2"); Best regards, Krzysztof