lkml.org 
[lkml]   [2011]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2 1/2] power: Charger-Manager: add initial Charger-Manager driver
    From
    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/

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