On Mon, Apr 29, 2019 at 02:31:55PM +0200, Jorge Ramirez wrote:
> On 4/27/19 20:21, Mark Brown wrote:
> > Since the point of this change is AFAICT that this regulator only has a
> > single linear range it seems like it should just be able to use the
> > existing generic functions shouldn't it?
> yes that would have been ideal but it does not seem to be the case for
> this hardware.
> The register that stores the voltage range for all other SPMI regulators
> (SPMI_COMMON_REG_VOLTAGE_RANGE 0x40) is used by something else in the
> HFS430: SPMI_HFS430_REG_VOLTAGE_LB 0x40 stores the voltage level in two
> bytes 0x40 and 0x41;
> This overlap really what is creating the pain: HFS430 cant use 0x40 to
> store the range (even if it is only one)
> so yeah, most of the changes in the patch are working around this fact.
I'm not sure I follow here, sorry - I can see that the driver needs a
custom get/set selector operation but shouldn't it be able to use the
standard list and map operations for linear ranges?
>
> enum spmi_common_regulator_registers {
> SPMI_COMMON_REG_DIG_MAJOR_REV = 0x01,
> SPMI_COMMON_REG_TYPE = 0x04,
> SPMI_COMMON_REG_SUBTYPE = 0x05,
> SPMI_COMMON_REG_VOLTAGE_RANGE = 0x40, ******
> SPMI_COMMON_REG_VOLTAGE_SET = 0x41,
> SPMI_COMMON_REG_MODE = 0x45,
> SPMI_COMMON_REG_ENABLE = 0x46,
> SPMI_COMMON_REG_PULL_DOWN = 0x48,
> SPMI_COMMON_REG_SOFT_START = 0x4c,
> SPMI_COMMON_REG_STEP_CTRL = 0x61,
> };
>
> enum spmi_hfs430_registers {
> SPMI_HFS430_REG_VOLTAGE_LB = 0x40, *******
> SPMI_HFS430_REG_VOLTAGE_VALID_LB = 0x42,
> SPMI_HFS430_REG_MODE = 0x45,
> };
>
> It just needs it's own
> > set/get_voltage_sel() operations. As far as I can see the main thing
> > the driver is doing with the custom stuff is handling the fact that
> > there's multiple ranges but that's not an issue for this regulator.
> > It's possible I'm missing something there but that was the main thing
> > (and we do have some generic support for multiple linear ranges in the
> > helper code already, can't remember why this driver isn't using that -
> > the ranges overlap IIRC?).
> >
> > TBH looking at the uses of find_range() I'm not sure they're 100%
> > sensible as they are - the existing _time_sel() is assuming we only need
> > to work out the ramp time between voltages in the same range which is
> > going to have trouble.
> >
>
Attachment:
signature.asc
Description: PGP signature
