On 08/08/2022 13:05, Yassine Oudjana wrote: > > On Mon, Aug 8 2022 at 11:55:02 +03:00:00, Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> On 08/08/2022 10:34, Yassine Oudjana wrote: >>> From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx> >>> >>> Add a driver for the switch-mode battery charger found on >>> PMICs such as PMI8994. This block is referred to in the vendor >>> kernel[1] as smbcharger or SMBCHG. It has USB and DC inputs, >>> and can generate VBUS for USB OTG with a boost regulator. >>> It supports Qualcomm Quick Charge 2.0, and can operate along >>> with a parallel charger (SMB1357, or SMB1351 for added Quick >>> Charge 3.0 support) for improved efficiency at higher currents. >>> >>> At the moment, this driver supports charging off of the USB input >>> at 5V with input current limit up to 3A. It also includes support >>> for operating the OTG boost regulator as well as extcon >>> functionality, reporting states of USB and USB_HOST with VBUS and >>> charge port types. >>> >>> Co-developed-by: Alejandro Tafalla <atafalla@xxxxxxxxx> >>> Signed-off-by: Alejandro Tafalla <atafalla@xxxxxxxxx> >>> Signed-off-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx> >>> >>> [1] >>> https://github.com/android-linux-stable/msm-3.18/blob/kernel.lnx.3.18.r34-rel/drivers/power/qpnp-smbcharger.c >>> --- >>> MAINTAINERS | 2 + >>> drivers/power/supply/Kconfig | 11 + >>> drivers/power/supply/Makefile | 1 + >>> drivers/power/supply/qcom-smbchg.c | 1664 >>> ++++++++++++++++++++++++++++ >>> drivers/power/supply/qcom-smbchg.h | 428 +++++++ >>> 5 files changed, 2106 insertions(+) >>> create mode 100644 drivers/power/supply/qcom-smbchg.c >>> create mode 100644 drivers/power/supply/qcom-smbchg.h >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index f6cf3a27d132..9b8693050890 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -16964,6 +16964,8 @@ L: linux-pm@xxxxxxxxxxxxxxx >>> L: linux-arm-msm@xxxxxxxxxxxxxxx >>> S: Maintained >>> F: Documentation/devicetree/bindings/power/supply/qcom,smbchg.yaml >>> +F: drivers/power/supply/qcom-smbchg.c >>> +F: drivers/power/supply/qcom-smbchg.h >>> >>> QUALCOMM TSENS THERMAL DRIVER >>> M: Amit Kucheria <amitk@xxxxxxxxxx> >>> diff --git a/drivers/power/supply/Kconfig >>> b/drivers/power/supply/Kconfig >>> index 1aa8323ad9f6..246bfc118d9f 100644 >>> --- a/drivers/power/supply/Kconfig >>> +++ b/drivers/power/supply/Kconfig >>> @@ -633,6 +633,17 @@ config CHARGER_QCOM_SMBB >>> documentation for more detail. The base name for this driver is >>> 'pm8941_charger'. >>> >>> +config CHARGER_QCOM_SMBCHG >>> + tristate "Qualcomm Switch-Mode Battery Charger" >> >> As I mentioned in cover letter, this should be either squashed into >> Caleb's work, merged into some common part or kept separate but with >> clear explaining why it cannot be merged. >> >> Some incomplete review follows: >> >>> + depends on MFD_SPMI_PMIC || COMPILE_TEST >>> + depends on OF >>> + depends on EXTCON >>> + depends on REGULATOR >>> + select QCOM_PMIC_SEC_WRITE >>> + help >>> + Say Y to include support for the Switch-Mode Battery Charger >>> block >>> + found in Qualcomm PMICs such as PMI8994. >>> + >>> config CHARGER_BQ2415X >>> tristate "TI BQ2415x battery charger driver" >>> depends on I2C >>> diff --git a/drivers/power/supply/Makefile >>> b/drivers/power/supply/Makefile >>> index 7f02f36aea55..7c2c037cd8b1 100644 >>> --- a/drivers/power/supply/Makefile >>> +++ b/drivers/power/supply/Makefile >>> @@ -83,6 +83,7 @@ obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o >>> obj-$(CONFIG_CHARGER_MP2629) += mp2629_charger.o >>> obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o >>> obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o >>> +obj-$(CONFIG_CHARGER_QCOM_SMBCHG) += qcom-smbchg.o >>> obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o >>> obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o >>> obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o >>> diff --git a/drivers/power/supply/qcom-smbchg.c >>> b/drivers/power/supply/qcom-smbchg.c >>> new file mode 100644 >>> index 000000000000..23a9667953c9 >>> --- /dev/null >>> +++ b/drivers/power/supply/qcom-smbchg.c >>> @@ -0,0 +1,1664 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >> >> Several things look like based from original sources, so please retain >> original copyright. > > Do I replace the existing copyright here with the original one, just > add it, or mention that this driver is based on downstream sources > (maybe putting a link as well) then add it? Add original copyright and optionally mention that it is based on downstream source. Links are not needed. >> >>> + >>> +static int smbchg_probe(struct platform_device *pdev) >>> +{ >>> + struct smbchg_chip *chip; >>> + struct regulator_config config = {}; >>> + struct power_supply_config supply_config = {}; >>> + int i, irq, ret; >>> + >>> + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL); >>> + if (!chip) >>> + return -ENOMEM; >>> + >>> + chip->dev = &pdev->dev; >>> + >>> + chip->regmap = dev_get_regmap(chip->dev->parent, NULL); >>> + if (!chip->regmap) { >>> + dev_err(chip->dev, "Failed to get regmap\n"); >>> + return -ENODEV; >>> + } >>> + >>> + ret = of_property_read_u32(chip->dev->of_node, "reg", >>> &chip->base); >> >> First: device_xxx > > Okay. > >> Second: what if address is bigger than u32? Shouldn't this be >> of_read_number or something similar in device_xxx API? > > The address wouldn't exceed sizeof(u16). Actually now I think I > should've used property_read_u16 instead. I couldn't find a device_* > equivalent of of_read_number (or at least not a direct one), are you > sure it exists? I think u16 would be confusing as reg size is minimum u32 (with address-cells==1). Instead of of_read_number(), maybe this should be of_get_address() (see pm8941-pwrkey.c), but there is no device_xxx() equivalent. Still I think it would be the most appropriate to parse actual address. Best regards, Krzysztof
