Re: [PATCH v3 4/4] regulator: qcom_rpm-regulator: Add support for pm8018 rpm regulator

[Stephen Boyd]

On 07/19/2016 06:51 AM, Neil Armstrong wrote:
> In order to support eh Qualcomm MDM9615 SoC, add support for the

s/eh/the/

> PM8018 RPM regulator in the qcom_rpm-regulator driver.
>
> Acked-by: Mark Brown <broonie@xxxxxxxxxx>
> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>
> diff --git a/drivers/regulator/qcom_rpm-regulator.c b/drivers/regulator/qcom_rpm-regulator.c
> index e254272..0734a5f 100644
> --- a/drivers/regulator/qcom_rpm-regulator.c
> +++ b/drivers/regulator/qcom_rpm-regulator.c
> @@ -70,6 +70,40 @@ struct qcom_rpm_reg {
>  	bool supports_force_mode_bypass;
>  };
>  
> +static struct rpm_reg_parts rpm8018_ldo_parts = {
> +	.request_len	= 2,
> +	.uV		= { 0, 0x007FFFFF,  0},
> +	.pd		= { 0, 0x00800000, 23},
> +	.pc		= { 0, 0x0F000000, 24},
> +	.pf		= { 0, 0xF0000000, 28},
> +	.ip		= { 1, 0x000003FF,  0},
> +	.ia		= { 1, 0x000FFC00, 10},
> +	.fm		= { 1, 0x00700000, 20},
> +};
> +
> +static struct rpm_reg_parts rpm8018_smps_parts = {
> +	.request_len	= 2,
> +	.uV		= {  0, 0x007FFFFF,  0},
> +	.pd		= {  0, 0x00800000, 23},
> +	.pc		= {  0, 0x0F000000, 24},
> +	.pf		= {  0, 0xF0000000, 28},
> +	.ip		= {  1, 0x000003FF,  0},
> +	.ia		= {  1, 0x000FFC00, 10},
> +	.fm		= {  1, 0x00700000, 20},
> +	.pm		= {  1, 0x00800000, 23},
> +	.freq		= {  1, 0x1F000000, 24},
> +	.freq_clk_src	= {  1, 0x60000000, 29},
> +};
> +
> +static struct rpm_reg_parts rpm8018_switch_parts = {
> +	.request_len	= 1,
> +	.enable_state	= {  0, 0x00000001,  0},
> +	.pd		= {  0, 0x00000002,  1},
> +	.pc		= {  0, 0x0000003C,  2},
> +	.pf		= {  0, 0x000003C0,  6},
> +	.hpm		= {  0, 0x00000C00, 10},
> +};
> +

These are all the same as the rpm8960 ones, so why don't we reuse those
structures?

>  static const struct rpm_reg_parts rpm8660_ldo_parts = {
>  	.request_len    = 2,
>  	.mV             = { 0, 0x00000FFF,  0 },
> @@ -448,6 +482,44 @@ static struct regulator_ops switch_ops = {
>  };
>  
>  /*
> + * PM8018 regulators
> + */
> +static const struct qcom_rpm_reg pm8018_pldo = {
> +	.desc.linear_ranges = pldo_ranges,
> +	.desc.n_linear_ranges = ARRAY_SIZE(pldo_ranges),
> +	.desc.n_voltages = 161,
> +	.desc.ops = &uV_ops,
> +	.parts = &rpm8018_ldo_parts,
> +	.supports_force_mode_auto = false,
> +	.supports_force_mode_bypass = false,
> +};
> +
> +static const struct qcom_rpm_reg pm8018_nldo = {
> +	.desc.linear_ranges = nldo_ranges,
> +	.desc.n_linear_ranges = ARRAY_SIZE(nldo_ranges),
> +	.desc.n_voltages = 64,
> +	.desc.ops = &uV_ops,
> +	.parts = &rpm8018_ldo_parts,
> +	.supports_force_mode_auto = false,
> +	.supports_force_mode_bypass = false,
> +};
> +
> +static const struct qcom_rpm_reg pm8018_smps = {
> +	.desc.linear_ranges = smps_ranges,
> +	.desc.n_linear_ranges = ARRAY_SIZE(smps_ranges),
> +	.desc.n_voltages = 154,
> +	.desc.ops = &uV_ops,
> +	.parts = &rpm8018_smps_parts,
> +	.supports_force_mode_auto = false,
> +	.supports_force_mode_bypass = false,
> +};
> +
> +static const struct qcom_rpm_reg pm8018_switch = {
> +	.desc.ops = &switch_ops,
> +	.parts = &rpm8018_switch_parts,
> +};
> +

These are all the same as the pm8921 ones too? So just use those instead?

We should probably rethink this design and have these structures based
on PMIC derivatives instead of SoCs because things are quite similar
across many PMICs.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project