[U-Boot] [PATCH v3 05/17] dm: pmic: add implementation of driver model pmic uclass

Simon Glass
2015-04-05T18:30:44Z

Hi Przemyslaw,

On 3 April 2015 at 10:08, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> Hello Simon,
>
> On 03/29/2015 03:07 PM, Simon Glass wrote:
>>
>> Hi Prazemyslaw,
>>
>> On 24 March 2015 at 14:30, Przemyslaw Marczak <p.marczak@samsung.com>
>> wrote:
>>>
>
>>> + UCLASS_PMIC,
>>> +
>>> UCLASS_COUNT,
>>> UCLASS_INVALID = -1,
>>> };
>>> diff --git a/include/power/pmic.h b/include/power/pmic.h
>>> index afbc5aa..b55304f 100644
>>> --- a/include/power/pmic.h
>>> +++ b/include/power/pmic.h
>>> @@ -1,4 +1,7 @@
>>> /*
>>> + * Copyright (C) 2014-2015 Samsung Electronics
>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>> + *
>>> * Copyright (C) 2011-2012 Samsung Electronics
>>> * Lukasz Majewski <l.majewski@samsung.com>
>>> *
>>> @@ -9,10 +12,13 @@
>>> #define __CORE_PMIC_H_
>>>
>>> #include <linux/list.h>
>>> +#include <spi.h>
>>> #include <i2c.h>
>>> #include <power/power_chrg.h>
>>>
>>> enum { PMIC_I2C, PMIC_SPI, PMIC_NONE};
>>> +
>>> +#ifdef CONFIG_POWER
>>> enum { I2C_PMIC, I2C_NUM, };
>>> enum { PMIC_READ, PMIC_WRITE, };
>>> enum { PMIC_SENSOR_BYTE_ORDER_LITTLE, PMIC_SENSOR_BYTE_ORDER_BIG, };
>>> @@ -77,7 +83,210 @@ struct pmic {
>>> struct pmic *parent;
>>> struct list_head list;
>>> };
>>> +#endif /* CONFIG_POWER */
>>> +
>>> +#ifdef CONFIG_DM_PMIC
>>> +/**
>>> + * Driver model pmic framework.
>>> + * The PMIC_UCLASS uclass is designed to provide a common I/O
>>> + * interface for pmic child devices of various uclass types.
>>
>>
>> I worry about having the docs in multiple places. Should you adjust
>> this to point to the Kconfig? Or change the Kconfig to point here? Or
>> maybe it would be better to drop both and put these in your README? I
>> am concerned that if we later change something, we end up with
>> inconsistent docs.
>>
>
> Right, this could be an issue in the future. I suppose that first thing is
> to look at this header file, before anyone starts work with this framework.
> So maybe it's better keep the description only in this file, and add some
> basic info to Kconfig and doc, with the links to here?

Yes that sounds good to me. It's great that you have documented this so well.

Regards,
Simon