[lkml]   [2011]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH v2 1/2] power: Charger-Manager: add initial Charger-Manager driver
    2011/12/27 Rafael J. Wysocki <>:
    > 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
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2011-12-27 09:07    [W:0.028 / U:10.156 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site