Re: [PATCH v2 1/2] power: Charger-Manager: add initialCharger-Manager driver

From: MyungJoo Ham
Date: Tue Dec 27 2011 - 03:05:44 EST

2011/12/27 Rafael J. Wysocki <rjw@xxxxxxx>:
> That should be called rtc_name and I'm not sure if using the name here is
> very convenient.  Perhaps it's better if the caller is responsible for
> opening the RTC device.

Hi Rafael,

Because struct rtc_device is created at rtc_device_register() call of
rtc device driver, the caller (rtc drivers are not supposed to create
charger manager) needs to call rtc_class_open(rtc_name) in order to
feed struct rtc_device to charger manager. Thus, I think that
enforcing the caller to provide struct rtc_device has no significant
benefit. Besides, it would prohibit statically defining struct
charger_global_desc: cannot define struct charger_global_desc example
= { .rtc_device = rtc_class_open("blahblah") };

>> +#define      CM_JIFFIES_SMALL        (2)
> Why do you want to use jiffies instead of ktime?

It is based on work schedule, which is based on jiffies. If we define
the "small time" with ktime, it should be translated to jiffies

>> +static int charger_manager_probe(struct platform_device *pdev)
>> +{
>> +     struct charger_desc *desc = dev_get_platdata(&pdev->dev);
>> +     if (!desc) {
>> +             dev_err(&pdev->dev, "No platform data (desc) found.\n");
>> +             ret = -ENODEV;
>> +             goto err_alloc;
>> +     }
> Is there any way to detect whether dev_get_platdata(&pdev->dev) really points
> to an instance of struct charger_desc ?

We are verifying the member values (whether they are in the proper
ranges) later in this probe function.
Other than that, I don't know a method to detect whether this pointer
is really pointing to struct charger_desc except for simply checking
whether it is NULL or not or enforcing users to add some magic values
in the struct to check (like mutex debug mode).

>> +     /* Basic Values. Unspecified are Null or 0 */
>> +     cm->dev = &pdev->dev;
>> +     cm->desc = desc;

We will let cm->desc to alloc some space and copy contents of desc
into cm->desc because desc may be __initdata.

Donggeun will update and submit patchset v3 mostly based on your
valueable comments.

Thank you so much, Rafael.

Cheers! And happy new year!

MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at