| Message ID | 20231030-bcm59054-v2-0-5fa4011aa5ba@gmail.com |
|---|---|
| Headers | show |
| Series | mfd: bcm590xx: Add support for BCM59054 | expand |
On Mon, Oct 30, 2023 at 08:41:47PM +0100, Artur Weber wrote: > The BCM59054 is fairly similar in terms of regulators to the already > supported BCM59056, as included in the BCM590XX driver. > Add support for the BCM59054's regulators to the BCM590XX driver. > Switch from using defines for common checks to using functions which > return different values depending on the identified MFD model. > While we're at it, fix a bug where the enable/vsel register > offsets for GPLDO and LDO regulators were calculated incorrectly. > Also, change the regulator enable bitmask to cover the entire PMMODE > register. As is very clear from the commit log this is a whole bunch of changes which really should be split out into multiple patches. There's the bug fix, there's the multiple refactorings to support the new device and the new device itself. As covered in submitting-patches.rst you should do one change per patch, this makes things much easier to follow. > + if (pmu->mfd->device_type == BCM59054_TYPE) { > + info = bcm59054_regs; > + n_regulators = BCM59054_NUM_REGS; > + } else if (pmu->mfd->device_type == BCM59056_TYPE) { > + info = bcm59056_regs; > + n_regulators = BCM59056_NUM_REGS; > + } There's no error handling here if there's an unknown type. > - if ((BCM590XX_REG_IS_LDO(i)) || (BCM590XX_REG_IS_GPLDO(i))) { > + if (bcm590xx_reg_is_ldo(pmu, i) || \ > + bcm590xx_reg_is_gpldo(pmu, i)) { > pmu->desc[i].ops = &bcm590xx_ops_ldo; > pmu->desc[i].vsel_mask = BCM590XX_LDO_VSEL_MASK; > - } else if (BCM590XX_REG_IS_VBUS(i)) > - pmu->desc[i].ops = &bcm590xx_ops_vbus; > - else { > + } else if (bcm590xx_reg_is_static(pmu, i)) { > + pmu->desc[i].ops = &bcm590xx_ops_static; > + } else { > pmu->desc[i].ops = &bcm590xx_ops_dcdc; > pmu->desc[i].vsel_mask = BCM590XX_SR_VSEL_MASK; > } It seems like everything would be a lot simpler and clearer to just have tables of regulators per device rather than have all these conditionals.
On Mon, Oct 30, 2023 at 08:41:48PM +0100, Artur Weber wrote: > + for (i = 0; i < pmctrl_count; i++) { > + ret = regmap_write(regmap, pmctrl_addr + i, mode_mask); > + if (ret) > + return ret; > + } Why not a bulk write? What happens when the new values are partially written, both if there's an error and just transiently?
Add support for the BCM59054 MFD to the bcm590xx driver and fix a couple of small bugs in it that also affected the already supported BCM59056. While we're at it - convert the devicetree bindings to YAML format and drop the bcm59056 DTS in favor of describing the PMU in users' DTS files, as is done for most other MFDs. The BCM59054 is fairly similar to the BCM59056, with the primary difference being the different number and layout of regulators. It is primarily used in devices using the BCM21664 and BCM23550 chipsets. I'd appreciate testing on BCM59056-equipped boards to make sure they aren't affected negatively by the changes. So far, I've tested this patch series on a Samsung Galaxy Grand Neo (BAFFINLITE REV02) with a BCM23550 chipset (BCM59054 MFD); this device is not yet supported in the mainline kernel, but I'm working on adding support for it, and other commercially-available devices using Broadcom Kona chips. Hopefully some of that work will make it into mainline in the near future ;) Signed-off-by: Artur Weber <aweber.kernel@gmail.com> --- Changes in v2: - Fixed BCM59054 ID being passed to BCM59056 function in the regulator driver - Dropped linux-rpi-kernel from the CC list - Link to v1: https://lore.kernel.org/r/20231030-bcm59054-v1-0-3517f980c1e3@gmail.com --- Artur Weber (6): dt-bindings: mfd: brcm,bcm59056: Convert to YAML dt-bindings: mfd: brcm,bcm59056: Add compatible for BCM59054 ARM: dts: Drop DTS for BCM59056 PMIC mfd: bcm590xx: Add compatible for BCM59054 regulator: bcm590xx: Add support for BCM59054 regulator: bcm590xx: Add proper handling for PMMODE registers .../devicetree/bindings/mfd/brcm,bcm59056.txt | 39 -- .../devicetree/bindings/mfd/brcm,bcm59056.yaml | 142 +++++ arch/arm/boot/dts/broadcom/bcm28155-ap.dts | 68 +- arch/arm/boot/dts/broadcom/bcm59056.dtsi | 91 --- drivers/mfd/bcm590xx.c | 5 +- drivers/regulator/bcm590xx-regulator.c | 708 ++++++++++++++++----- include/linux/mfd/bcm590xx.h | 7 + 7 files changed, 728 insertions(+), 332 deletions(-) --- base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1 change-id: 20231029-bcm59054-3ed65e649435 Best regards,