Re: [PATCH v2 1/3] mfd: lp87565: Add lp87565 PMIC support

[Lee Jones]

On Tue, 30 May 2017, Keerthy wrote:

> 
> 
> On Tuesday 30 May 2017 02:53 PM, Lee Jones wrote:
> > On Tue, 23 May 2017, Keerthy wrote:
> > 
> >> The LP87565 chip is a power management IC for Portable Navigation Systems
> >> and Tablet Computing devices. It contains the following components:
> >>
> >>         - Configurable Bucks(Single and multi-phase).
> >>         - Configurable General Purpose Output Signals (GPO).
> >>
> >> The LP87565-Q1 variant device uses two 2-phase outputs configuration,
> >> Buck0 is master for Buck0/1 output and Buck2 is master for Buck2/3
> >> output.
> >>
> >> Signed-off-by: Keerthy <j-keerthy@xxxxxx>
> >> ---
> >>
> >> Changes in v2:
> >>
> >>   * Fixed a bunch of whitespace errors.
> >>   * Changed the License to short form.
> >>   * Added the generic compatible lp87565
> >>   * Removed i2c_device_id table.
> >>   * Introduced probe_new function in place of probe.
> >>
> >>  Documentation/devicetree/bindings/mfd/lp87565.txt |  45 ++++
> >>  drivers/mfd/Kconfig                               |  14 ++
> >>  drivers/mfd/Makefile                              |   1 +
> >>  drivers/mfd/lp87565.c                             |  95 ++++++++
> >>  include/linux/mfd/lp87565.h                       | 271 ++++++++++++++++++++++
> >>  5 files changed, 426 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mfd/lp87565.txt
> >>  create mode 100644 drivers/mfd/lp87565.c
> >>  create mode 100644 include/linux/mfd/lp87565.h

[...]

> >> +static int lp87565_probe(struct i2c_client *client)
> >> +{
> >> +	struct lp87565 *lp87565;
> >> +	const struct of_device_id *of_id;
> >> +	int ret;
> >> +	unsigned int otpid;
> >> +
> >> +	lp87565 = devm_kzalloc(&client->dev, sizeof(*lp87565), GFP_KERNEL);
> >> +	if (!lp87565)
> >> +		return -ENOMEM;
> >> +
> >> +	lp87565->dev = &client->dev;
> >> +
> >> +	lp87565->regmap = devm_regmap_init_i2c(client, &lp87565_regmap_config);
> >> +	if (IS_ERR(lp87565->regmap)) {
> >> +		ret = PTR_ERR(lp87565->regmap);
> >> +		dev_err(lp87565->dev,
> >> +			"Failed to initialize register map: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = regmap_read(lp87565->regmap, LP87565_REG_OTP_REV, &otpid);
> >> +	if (ret) {
> >> +		dev_err(lp87565->dev, "Failed to read OTP ID\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	lp87565->rev = otpid & LP87565_OTP_REV_OTP_ID;
> >> +
> >> +	of_id = of_match_device(of_lp87565_match_table, &client->dev);
> >> +	if (of_id)
> >> +		lp87565->dev_type = (enum lp87565_device_type)of_id->data;
> > 
> > So you can interigate the device for a revision number, but not a
> > device type ID, is that correct?
> 
> I hope you meant integrate? Revision of a particular type. Then device
> type can mean something like lp87565-q1 which is modeling 4 individual
> regulators as 2 dual phase regulators. There can be other devices which
> has 4 single phase regulators. I have no document which talks about that
> yet but possible.

No I meant "interrogate".  Odd that my spell checker didn't pick that
one up.

I'll put it another way.  You are able to read the revision number
from the device, right?  Is there another register that you can read
from to obtain the device type?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog