Hi Artur,
some notes below.
On Fri, Jan 31, 2025 at 07:13:55PM +0100, Artur Weber wrote:
> The BCM59056 and BCM59054 are very similar in terms of regulators. Add
> the register definitions for the BCM59054 and enable support for it in
> the driver.
>
> Signed-off-by: Artur Weber <aweber.kernel@xxxxxxxxx>
> ---
[snip]
> + {
> + .type = BCM590XX_REG_TYPE_LDO,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_LDO_DESC(MICLDO, micldo, ldo_1_table),
> + },
> + },
Downstream has MICLDO fixed at 1.8V.
Not sure if that isn't just some vendor hack though :/
https://github.com/Samsung-KYLEPROXX/android_kernel_samsung_kyleproxx/blob/cm-13.0/drivers/regulator/bcmpmu-rgltr-bcm59xxx.c#L83
> +
> + {
> + .type = BCM590XX_REG_TYPE_LDO,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_LDO_DESC(USBLDO, usbldo, ldo_1_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_LDO,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_LDO_DESC(VIBLDO, vibldo, ldo_2_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(CSR, csr, dcdc_csr_ranges),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(IOSR1, iosr1, dcdc_sr_ranges),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(IOSR2, iosr2, dcdc_sr_ranges),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(MMSR, mmsr, dcdc_sr_ranges),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(SDSR1, sdsr1, dcdc_sr_ranges),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(SDSR2, sdsr2, dcdc_sr_ranges),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_SR,
> + .regmap = BCM590XX_REG_REGMAP_PRI,
> + .desc = {
> + BCM59054_SR_DESC(VSR, vsr, dcdc_vsr_ranges),
> + },
> + },
VSR seems to be different between BCM59054(A0?) and BCM59054A1.
These are the A1 ranges.
Should we differentiate between the two versions, same as downstream?
https://github.com/Samsung-KYLEPROXX/android_kernel_samsung_kyleproxx/blob/cm-13.0/drivers/regulator/bcmpmu-rgltr-bcm59xxx.c#L736
> +
> + {
> + .type = BCM590XX_REG_TYPE_GPLDO,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_LDO_DESC(GPLDO1, gpldo1, ldo_1_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_GPLDO,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_LDO_DESC(GPLDO2, gpldo2, ldo_1_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_GPLDO,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_LDO_DESC(GPLDO3, gpldo3, ldo_1_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_GPLDO,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_LDO_DESC(TCXLDO, tcxldo, ldo_1_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_GPLDO,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_LDO_DESC(LVLDO1, lvldo1, ldo_1_table),
According to downstream, this should be ldo_3_table.
https://github.com/Samsung-KYLEPROXX/android_kernel_samsung_kyleproxx/blob/cm-13.0/drivers/regulator/bcmpmu-rgltr-bcm59xxx.c#L683
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_GPLDO,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_LDO_DESC(LVLDO2, lvldo2, ldo_3_table),
> + },
> + },
> +
> + {
> + .type = BCM590XX_REG_TYPE_VBUS,
> + .regmap = BCM590XX_REG_REGMAP_SEC,
> + .desc = {
> + BCM59054_REG_DESC(VBUS, vbus),
> + .ops = &bcm590xx_ops_vbus,
> + .n_voltages = 1,
> + .fixed_uV = 5000000,
> + .enable_reg = BCM59054_OTG_CTRL,
> + .enable_mask = BCM590XX_VBUS_ENABLE,
> + },
> + },
> +};
> +
> static int bcm590xx_probe(struct platform_device *pdev)
> {
> struct bcm590xx *bcm590xx = dev_get_drvdata(pdev->dev.parent);
> @@ -487,8 +864,21 @@ static int bcm590xx_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> pmu->mfd = bcm590xx;
> - pmu->n_regulators = BCM59056_NUM_REGS;
> - pmu->regs = bcm59056_regs;
> +
> + switch (pmu->mfd->device_type) {
> + case BCM59054_TYPE:
> + pmu->n_regulators = BCM59054_NUM_REGS;
> + pmu->regs = bcm59054_regs;
> + break;
> + case BCM59056_TYPE:
> + pmu->n_regulators = BCM59056_NUM_REGS;
> + pmu->regs = bcm59056_regs;
> + break;
> + default:
> + dev_err(bcm590xx->dev,
> + "unknown device type, could not initialize");
> + return -EINVAL;
Missing a '\n' in the error message.
Could use dev_err_probe as well. Maybe in other places in the driver too.
Regards,
Stanislav
> + }
>
> platform_set_drvdata(pdev, pmu);
>
>
> --
> 2.48.1
>
