[U-Boot] [PATCH v2 15/40] power: Add AMS AS3722 PMIC support

[Heiko Schocher]

Hello Thierry,

Am 27.08.2014 08:28, schrieb Thierry Reding:
> On Wed, Aug 27, 2014 at 07:26:12AM +0200, Heiko Schocher wrote:
>> Hello Thierry,
>>
>> Am 26.08.2014 17:34, schrieb Thierry Reding:
> [...]
>>> +int as3722_init(struct as3722 **pmicp, const void *fdt)
>>> +{
>>> +	struct as3722 *pmic =&as3722_pmic;
>>> +	int count, nodes[1], i;
>>> +	int err;
>>> +
>>> +	count = fdtdec_find_aliases_for_id(fdt, NULL, COMPAT_AMS_AS3722,
>>> +					   nodes, ARRAY_SIZE(nodes));
>>> +	for (i = 0; i<   count; i++) {
>>> +		int parent = fdt_parent_offset(fdt, nodes[i]), bus;
>>> +		struct i2c_adapter *adapter;
>>> +		fdt_addr_t address;
>>> +		u8 id, revision;
>>> +
>>> +		bus = i2c_get_bus_num_fdt(parent);
>>> +		if (bus<   0) {
>>> +			error("invalid bus %d", bus);
>>> +			continue;
>>> +		}
>>> +
>>> +		address = fdtdec_get_addr(fdt, nodes[i], "reg");
>>> +		if (address == FDT_ADDR_T_NONE) {
>>> +			error("slave address not found");
>>> +			continue;
>>> +		}
>>> +
>>> +		adapter = i2c_adapter_get(bus);
>>> +		if (!adapter) {
>>> +			error("I2C adapter for bus %d not found", bus);
>>> +			continue;
>>> +		}
>>
>> Why is i2c_set_bus_num() not enough for you? Why introducing a new
>> API? If another uses this PMIC behind a i2c mux, this driver maybe
>> not work correctly.
>
> As I explained earlier, I think requiring users to manually micro-manage
> the I2C busses is a bad idea. It's very tedious and error prone. It does

I fully aggree with you.

> also mean that every patch needs to be carefully checked that the I2C
> API is used correctly. The intent of the i2c_client API was to introduce
> a higher-level object that people can use and that does the bulk of the
> work behind the scenes rather than duplicating it all over the place. It

I understand you, but why not fixing the current API, so all can
provide from your work?

> is true that perhaps it currently doesn't work well with I2C muxes. But
> the advantage is that when it does, it does so for everybody that uses
> it.

Please fix the i2c_read/write functions, and it works for free with
i2c muxes!

On my ToDo list:

- Fix all old style i2c driver
- make i2c_set_bus_num() static, and add to i2c_read/write()
   the new function parameter "bus" ...
- get rid of HARD_I2C
- remove all i2c_init() calls, as they no longer needed
- remove all oldstyle defines, functions, structs ...
- introduce a deinit() functions for i2c adapters, so if we switch
   to another i2c bus, we deactivate the current i2c hw adapter. So
   if we boot an OS, we have all i2c hw adapters deactivated.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany