Messages in this thread | | | Date | Tue, 27 Dec 2011 17:05:38 +0900 | Subject | Re: [PATCH v2 1/2] power: Charger-Manager: add initial Charger-Manager driver | From | MyungJoo Ham <> |
| |
2011/12/27 Rafael J. Wysocki <rjw@sisk.pl>: > 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 anyway.
>> +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
-- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |