Re: [rtc-linux] [RESEND PATCH] RTC: MAX77686: Add Maxim 77686 driver

2012-12-19 Thread jonghwa3 . lee
Hi, Andrew,
On 2012년 12월 19일 09:30, Andrew Morton wrote:
> On Wed, 28 Nov 2012 15:50:57 +0900
> Jonghwa Lee  wrote:
> 
>> Add driver for support max77686 rtc.
>> MAX77686 rtc support smpl and wtsr mode. It has two alarm register
>> which can be used for alarming to wake system up. This drvier uses regmap
>> to access its register.
>>
>> ...
>>
>> +static inline int max77686_rtc_calculate_wday(u8 shifted)
>> +{
>> +int counter = -1;
>> +while (shifted) {
>> +shifted >>= 1;
>> +counter++;
>> +}
>> +return counter;
>> +}
> 
> We should be able to use log2() in here?
> 
Okay, and I'd like to use Joe's recommendation. fls(shitfted) - 1

>>
>> ...
>>
>> +static inline int max77686_rtc_update(struct max77686_rtc_info *info,
>> +enum MAX77686_RTC_OP op)
>> +{
>> +int ret;
>> +unsigned int data;
>> +
>> +switch (op) {
>> +case MAX77686_RTC_WRITE:
>> +data = 1 << RTC_UDR_SHIFT;
>> +break;
>> +case MAX77686_RTC_READ:
>> +data = 1 << RTC_RBUDR_SHIFT;
>> +break;
>> +}
>> +
>> +ret = regmap_update_bits(info->max77686->rtc_regmap,
>> + MAX77686_RTC_UPDATE0, data, data);
>> +if (ret < 0)
>> +dev_err(info->dev, "%s: fail to write update reg(ret=%d, 
>> data=0x%x)\n",
>> +__func__, ret, data);
>> +else {
>> +/* Minimum 16ms delay required before RTC update. */
>> +msleep(MAX77686_RTC_UPDATE_DELAY);
>> +}
>> +
>> +return ret;
>> +}
> 
> This function has about ten callers.  No way in the world do we want it
> to be inlined!  Probably the compiler will just ignore your `inline',
> but we should remove it to be sure.
> 
Okay, I'll remove it.
>>
>> ...
>>
>> +static const struct rtc_class_ops max77686_rtc_ops = {
>> +.read_time = max77686_rtc_read_time,
>> +.set_time = max77686_rtc_set_time,
>> +.read_alarm = max77686_rtc_read_alarm,
>> +.set_alarm = max77686_rtc_set_alarm,
>> +.alarm_irq_enable = max77686_rtc_alarm_irq_enable,
>> +};
>> +
>> +#ifdef MAX77686_RTC_WTSR_SMPL
> 
> hm, this is always undefined.  Why is this code here?
>
Yes, you're right, It isn't needed. I'll fix it right.

>> +static void max77686_rtc_enable_wtsr(struct max77686_rtc_info *info, bool 
>> enable)
>> +{
>> +int ret;
>> +unsigned int val, mask;
>> +
>> +if (enable)
>> +val = (1 << WTSR_EN_SHIFT) | (3 << WTSRT_SHIFT);
>> +else
>> +val = 0;
>> +
>> +mask = WTSR_EN_MASK | WTSRT_MASK;
>> +
>> +dev_info(info->dev, "%s: %s WTSR\n", __func__,
>> +enable ? "enable" : "disable");
>> +
>> +ret = regmap_update_bits(info->max77686->rtc_regmap,
>> + MAX77686_WTSR_SMPL_CNTL, mask, val);
>> +if (ret < 0) {
>> +dev_err(info->dev, "%s: fail to update WTSR reg(%d)\n",
>> +__func__, ret);
>> +return;
>> +}
>> +
>> +max77686_rtc_update(info, MAX77686_RTC_WRITE);
>> +}
>> +
>> +static void max77686_rtc_enable_smpl(struct max77686_rtc_info *info, bool 
>> enable)
>> +{
>> +int ret;
>> +unsigned int val, mask;
>> +
>> +if (enable)
>> +val = (1 << SMPL_EN_SHIFT) | (0 << SMPLT_SHIFT);
>> +else
>> +val = 0;
>> +
>> +mask = SMPL_EN_MASK | SMPLT_MASK;
>> +
>> +dev_info(info->dev, "%s: %s SMPL\n", __func__,
>> +enable ? "enable" : "disable");
>> +
>> +ret = regmap_update_bits(info->max77686->rtc_regmap,
>> + MAX77686_WTSR_SMPL_CNTL, mask, val);
>> +if (ret < 0) {
>> +dev_err(info->dev, "%s: fail to update SMPL reg(%d)\n",
>> +__func__, ret);
>> +return;
>> +}
>> +
>> +max77686_rtc_update(info, MAX77686_RTC_WRITE);
>> +
>> +val = 0;
>> +regmap_read(info->max77686->rtc_regmap, MAX77686_WTSR_SMPL_CNTL, );
>> +pr_info("%s: WTSR_SMPL(0x%02x)\n", __func__, val);
>> +}
>> +#endif /* MAX77686_RTC_WTSR_SMPL */
>>
>> ...
>>
>> +static int __devinit max77686_rtc_probe(struct platform_device *pdev)
> 
> CONFIG_HOTPLUG is removed, so we should stop using devinit, devexit,
> devinit_p, etc.
> 
>>
>> ...
>>
> 
> --- a/drivers/rtc/rtc-max77686.c~rtc-max77686-add-maxim-77686-driver-fix
> +++ a/drivers/rtc/rtc-max77686.c
> @@ -129,7 +129,7 @@ static int max77686_rtc_tm_to_data(struc
>   return 0;
>  }
>  
> -static inline int max77686_rtc_update(struct max77686_rtc_info *info,
> +static int max77686_rtc_update(struct max77686_rtc_info *info,
>   enum MAX77686_RTC_OP op)
>  {
>   int ret;
> @@ -501,7 +501,7 @@ static struct regmap_config max77686_rtc
>   .val_bits = 8,
>  };
>  
> -static int __devinit max77686_rtc_probe(struct platform_device *pdev)
> +static int max77686_rtc_probe(struct platform_device *pdev)
>  {
>   struct max77686_dev *max77686 = dev_get_drvdata(pdev->dev.parent);
>   

Re: [rtc-linux] [RESEND PATCH] RTC: MAX77686: Add Maxim 77686 driver

2012-12-19 Thread jonghwa3 . lee
Hi, Andrew,
On 2012년 12월 19일 09:30, Andrew Morton wrote:
 On Wed, 28 Nov 2012 15:50:57 +0900
 Jonghwa Lee jonghwa3@samsung.com wrote:
 
 Add driver for support max77686 rtc.
 MAX77686 rtc support smpl and wtsr mode. It has two alarm register
 which can be used for alarming to wake system up. This drvier uses regmap
 to access its register.

 ...

 +static inline int max77686_rtc_calculate_wday(u8 shifted)
 +{
 +int counter = -1;
 +while (shifted) {
 +shifted = 1;
 +counter++;
 +}
 +return counter;
 +}
 
 We should be able to use log2() in here?
 
Okay, and I'd like to use Joe's recommendation. fls(shitfted) - 1


 ...

 +static inline int max77686_rtc_update(struct max77686_rtc_info *info,
 +enum MAX77686_RTC_OP op)
 +{
 +int ret;
 +unsigned int data;
 +
 +switch (op) {
 +case MAX77686_RTC_WRITE:
 +data = 1  RTC_UDR_SHIFT;
 +break;
 +case MAX77686_RTC_READ:
 +data = 1  RTC_RBUDR_SHIFT;
 +break;
 +}
 +
 +ret = regmap_update_bits(info-max77686-rtc_regmap,
 + MAX77686_RTC_UPDATE0, data, data);
 +if (ret  0)
 +dev_err(info-dev, %s: fail to write update reg(ret=%d, 
 data=0x%x)\n,
 +__func__, ret, data);
 +else {
 +/* Minimum 16ms delay required before RTC update. */
 +msleep(MAX77686_RTC_UPDATE_DELAY);
 +}
 +
 +return ret;
 +}
 
 This function has about ten callers.  No way in the world do we want it
 to be inlined!  Probably the compiler will just ignore your `inline',
 but we should remove it to be sure.
 
Okay, I'll remove it.

 ...

 +static const struct rtc_class_ops max77686_rtc_ops = {
 +.read_time = max77686_rtc_read_time,
 +.set_time = max77686_rtc_set_time,
 +.read_alarm = max77686_rtc_read_alarm,
 +.set_alarm = max77686_rtc_set_alarm,
 +.alarm_irq_enable = max77686_rtc_alarm_irq_enable,
 +};
 +
 +#ifdef MAX77686_RTC_WTSR_SMPL
 
 hm, this is always undefined.  Why is this code here?

Yes, you're right, It isn't needed. I'll fix it right.

 +static void max77686_rtc_enable_wtsr(struct max77686_rtc_info *info, bool 
 enable)
 +{
 +int ret;
 +unsigned int val, mask;
 +
 +if (enable)
 +val = (1  WTSR_EN_SHIFT) | (3  WTSRT_SHIFT);
 +else
 +val = 0;
 +
 +mask = WTSR_EN_MASK | WTSRT_MASK;
 +
 +dev_info(info-dev, %s: %s WTSR\n, __func__,
 +enable ? enable : disable);
 +
 +ret = regmap_update_bits(info-max77686-rtc_regmap,
 + MAX77686_WTSR_SMPL_CNTL, mask, val);
 +if (ret  0) {
 +dev_err(info-dev, %s: fail to update WTSR reg(%d)\n,
 +__func__, ret);
 +return;
 +}
 +
 +max77686_rtc_update(info, MAX77686_RTC_WRITE);
 +}
 +
 +static void max77686_rtc_enable_smpl(struct max77686_rtc_info *info, bool 
 enable)
 +{
 +int ret;
 +unsigned int val, mask;
 +
 +if (enable)
 +val = (1  SMPL_EN_SHIFT) | (0  SMPLT_SHIFT);
 +else
 +val = 0;
 +
 +mask = SMPL_EN_MASK | SMPLT_MASK;
 +
 +dev_info(info-dev, %s: %s SMPL\n, __func__,
 +enable ? enable : disable);
 +
 +ret = regmap_update_bits(info-max77686-rtc_regmap,
 + MAX77686_WTSR_SMPL_CNTL, mask, val);
 +if (ret  0) {
 +dev_err(info-dev, %s: fail to update SMPL reg(%d)\n,
 +__func__, ret);
 +return;
 +}
 +
 +max77686_rtc_update(info, MAX77686_RTC_WRITE);
 +
 +val = 0;
 +regmap_read(info-max77686-rtc_regmap, MAX77686_WTSR_SMPL_CNTL, val);
 +pr_info(%s: WTSR_SMPL(0x%02x)\n, __func__, val);
 +}
 +#endif /* MAX77686_RTC_WTSR_SMPL */

 ...

 +static int __devinit max77686_rtc_probe(struct platform_device *pdev)
 
 CONFIG_HOTPLUG is removed, so we should stop using devinit, devexit,
 devinit_p, etc.
 

 ...

 
 --- a/drivers/rtc/rtc-max77686.c~rtc-max77686-add-maxim-77686-driver-fix
 +++ a/drivers/rtc/rtc-max77686.c
 @@ -129,7 +129,7 @@ static int max77686_rtc_tm_to_data(struc
   return 0;
  }
  
 -static inline int max77686_rtc_update(struct max77686_rtc_info *info,
 +static int max77686_rtc_update(struct max77686_rtc_info *info,
   enum MAX77686_RTC_OP op)
  {
   int ret;
 @@ -501,7 +501,7 @@ static struct regmap_config max77686_rtc
   .val_bits = 8,
  };
  
 -static int __devinit max77686_rtc_probe(struct platform_device *pdev)
 +static int max77686_rtc_probe(struct platform_device *pdev)
  {
   struct max77686_dev *max77686 = dev_get_drvdata(pdev-dev.parent);
   struct max77686_rtc_info *info;
 @@ -575,7 +575,7 @@ out:
   return ret;
  }
  
 -static int __devexit max77686_rtc_remove(struct platform_device *pdev)
 +static int max77686_rtc_remove(struct platform_device *pdev)
  {
   struct max77686_rtc_info *info = platform_get_drvdata(pdev);
  
 @@ -623,7 +623,7 @@ 

Re: [rtc-linux] [RESEND PATCH] RTC: MAX77686: Add Maxim 77686 driver

2012-12-18 Thread Joe Perches
On Tue, 2012-12-18 at 16:30 -0800, Andrew Morton wrote:
> On Wed, 28 Nov 2012 15:50:57 +0900
> Jonghwa Lee  wrote:
> 
> > Add driver for support max77686 rtc.
> > MAX77686 rtc support smpl and wtsr mode. It has two alarm register
> > which can be used for alarming to wake system up. This drvier uses regmap
> > to access its register.
> > 
> > ...
> >
> > +static inline int max77686_rtc_calculate_wday(u8 shifted)
> > +{
> > +   int counter = -1;
> > +   while (shifted) {
> > +   shifted >>= 1;
> > +   counter++;
> > +   }
> > +   return counter;
> > +}
> 
> We should be able to use log2() in here?

fls(shifted) - 1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [rtc-linux] [RESEND PATCH] RTC: MAX77686: Add Maxim 77686 driver

2012-12-18 Thread Andrew Morton
On Wed, 28 Nov 2012 15:50:57 +0900
Jonghwa Lee  wrote:

> Add driver for support max77686 rtc.
> MAX77686 rtc support smpl and wtsr mode. It has two alarm register
> which can be used for alarming to wake system up. This drvier uses regmap
> to access its register.
> 
> ...
>
> +static inline int max77686_rtc_calculate_wday(u8 shifted)
> +{
> + int counter = -1;
> + while (shifted) {
> + shifted >>= 1;
> + counter++;
> + }
> + return counter;
> +}

We should be able to use log2() in here?

>
> ...
>
> +static inline int max77686_rtc_update(struct max77686_rtc_info *info,
> + enum MAX77686_RTC_OP op)
> +{
> + int ret;
> + unsigned int data;
> +
> + switch (op) {
> + case MAX77686_RTC_WRITE:
> + data = 1 << RTC_UDR_SHIFT;
> + break;
> + case MAX77686_RTC_READ:
> + data = 1 << RTC_RBUDR_SHIFT;
> + break;
> + }
> +
> + ret = regmap_update_bits(info->max77686->rtc_regmap,
> +  MAX77686_RTC_UPDATE0, data, data);
> + if (ret < 0)
> + dev_err(info->dev, "%s: fail to write update reg(ret=%d, 
> data=0x%x)\n",
> + __func__, ret, data);
> + else {
> + /* Minimum 16ms delay required before RTC update. */
> + msleep(MAX77686_RTC_UPDATE_DELAY);
> + }
> +
> + return ret;
> +}

This function has about ten callers.  No way in the world do we want it
to be inlined!  Probably the compiler will just ignore your `inline',
but we should remove it to be sure.

>
> ...
>
> +static const struct rtc_class_ops max77686_rtc_ops = {
> + .read_time = max77686_rtc_read_time,
> + .set_time = max77686_rtc_set_time,
> + .read_alarm = max77686_rtc_read_alarm,
> + .set_alarm = max77686_rtc_set_alarm,
> + .alarm_irq_enable = max77686_rtc_alarm_irq_enable,
> +};
> +
> +#ifdef MAX77686_RTC_WTSR_SMPL

hm, this is always undefined.  Why is this code here?

> +static void max77686_rtc_enable_wtsr(struct max77686_rtc_info *info, bool 
> enable)
> +{
> + int ret;
> + unsigned int val, mask;
> +
> + if (enable)
> + val = (1 << WTSR_EN_SHIFT) | (3 << WTSRT_SHIFT);
> + else
> + val = 0;
> +
> + mask = WTSR_EN_MASK | WTSRT_MASK;
> +
> + dev_info(info->dev, "%s: %s WTSR\n", __func__,
> + enable ? "enable" : "disable");
> +
> + ret = regmap_update_bits(info->max77686->rtc_regmap,
> +  MAX77686_WTSR_SMPL_CNTL, mask, val);
> + if (ret < 0) {
> + dev_err(info->dev, "%s: fail to update WTSR reg(%d)\n",
> + __func__, ret);
> + return;
> + }
> +
> + max77686_rtc_update(info, MAX77686_RTC_WRITE);
> +}
> +
> +static void max77686_rtc_enable_smpl(struct max77686_rtc_info *info, bool 
> enable)
> +{
> + int ret;
> + unsigned int val, mask;
> +
> + if (enable)
> + val = (1 << SMPL_EN_SHIFT) | (0 << SMPLT_SHIFT);
> + else
> + val = 0;
> +
> + mask = SMPL_EN_MASK | SMPLT_MASK;
> +
> + dev_info(info->dev, "%s: %s SMPL\n", __func__,
> + enable ? "enable" : "disable");
> +
> + ret = regmap_update_bits(info->max77686->rtc_regmap,
> +  MAX77686_WTSR_SMPL_CNTL, mask, val);
> + if (ret < 0) {
> + dev_err(info->dev, "%s: fail to update SMPL reg(%d)\n",
> + __func__, ret);
> + return;
> + }
> +
> + max77686_rtc_update(info, MAX77686_RTC_WRITE);
> +
> + val = 0;
> + regmap_read(info->max77686->rtc_regmap, MAX77686_WTSR_SMPL_CNTL, );
> + pr_info("%s: WTSR_SMPL(0x%02x)\n", __func__, val);
> +}
> +#endif /* MAX77686_RTC_WTSR_SMPL */
>
> ...
>
> +static int __devinit max77686_rtc_probe(struct platform_device *pdev)

CONFIG_HOTPLUG is removed, so we should stop using devinit, devexit,
devinit_p, etc.

>
> ...
>

--- a/drivers/rtc/rtc-max77686.c~rtc-max77686-add-maxim-77686-driver-fix
+++ a/drivers/rtc/rtc-max77686.c
@@ -129,7 +129,7 @@ static int max77686_rtc_tm_to_data(struc
return 0;
 }
 
-static inline int max77686_rtc_update(struct max77686_rtc_info *info,
+static int max77686_rtc_update(struct max77686_rtc_info *info,
enum MAX77686_RTC_OP op)
 {
int ret;
@@ -501,7 +501,7 @@ static struct regmap_config max77686_rtc
.val_bits = 8,
 };
 
-static int __devinit max77686_rtc_probe(struct platform_device *pdev)
+static int max77686_rtc_probe(struct platform_device *pdev)
 {
struct max77686_dev *max77686 = dev_get_drvdata(pdev->dev.parent);
struct max77686_rtc_info *info;
@@ -575,7 +575,7 @@ out:
return ret;
 }
 
-static int __devexit max77686_rtc_remove(struct platform_device *pdev)
+static int max77686_rtc_remove(struct platform_device *pdev)
 {
struct max77686_rtc_info *info = platform_get_drvdata(pdev);
 
@@ -623,7 +623,7 @@ static struct 

Re: [rtc-linux] [RESEND PATCH] RTC: MAX77686: Add Maxim 77686 driver

2012-12-18 Thread Andrew Morton
On Wed, 28 Nov 2012 15:50:57 +0900
Jonghwa Lee jonghwa3@samsung.com wrote:

 Add driver for support max77686 rtc.
 MAX77686 rtc support smpl and wtsr mode. It has two alarm register
 which can be used for alarming to wake system up. This drvier uses regmap
 to access its register.
 
 ...

 +static inline int max77686_rtc_calculate_wday(u8 shifted)
 +{
 + int counter = -1;
 + while (shifted) {
 + shifted = 1;
 + counter++;
 + }
 + return counter;
 +}

We should be able to use log2() in here?


 ...

 +static inline int max77686_rtc_update(struct max77686_rtc_info *info,
 + enum MAX77686_RTC_OP op)
 +{
 + int ret;
 + unsigned int data;
 +
 + switch (op) {
 + case MAX77686_RTC_WRITE:
 + data = 1  RTC_UDR_SHIFT;
 + break;
 + case MAX77686_RTC_READ:
 + data = 1  RTC_RBUDR_SHIFT;
 + break;
 + }
 +
 + ret = regmap_update_bits(info-max77686-rtc_regmap,
 +  MAX77686_RTC_UPDATE0, data, data);
 + if (ret  0)
 + dev_err(info-dev, %s: fail to write update reg(ret=%d, 
 data=0x%x)\n,
 + __func__, ret, data);
 + else {
 + /* Minimum 16ms delay required before RTC update. */
 + msleep(MAX77686_RTC_UPDATE_DELAY);
 + }
 +
 + return ret;
 +}

This function has about ten callers.  No way in the world do we want it
to be inlined!  Probably the compiler will just ignore your `inline',
but we should remove it to be sure.


 ...

 +static const struct rtc_class_ops max77686_rtc_ops = {
 + .read_time = max77686_rtc_read_time,
 + .set_time = max77686_rtc_set_time,
 + .read_alarm = max77686_rtc_read_alarm,
 + .set_alarm = max77686_rtc_set_alarm,
 + .alarm_irq_enable = max77686_rtc_alarm_irq_enable,
 +};
 +
 +#ifdef MAX77686_RTC_WTSR_SMPL

hm, this is always undefined.  Why is this code here?

 +static void max77686_rtc_enable_wtsr(struct max77686_rtc_info *info, bool 
 enable)
 +{
 + int ret;
 + unsigned int val, mask;
 +
 + if (enable)
 + val = (1  WTSR_EN_SHIFT) | (3  WTSRT_SHIFT);
 + else
 + val = 0;
 +
 + mask = WTSR_EN_MASK | WTSRT_MASK;
 +
 + dev_info(info-dev, %s: %s WTSR\n, __func__,
 + enable ? enable : disable);
 +
 + ret = regmap_update_bits(info-max77686-rtc_regmap,
 +  MAX77686_WTSR_SMPL_CNTL, mask, val);
 + if (ret  0) {
 + dev_err(info-dev, %s: fail to update WTSR reg(%d)\n,
 + __func__, ret);
 + return;
 + }
 +
 + max77686_rtc_update(info, MAX77686_RTC_WRITE);
 +}
 +
 +static void max77686_rtc_enable_smpl(struct max77686_rtc_info *info, bool 
 enable)
 +{
 + int ret;
 + unsigned int val, mask;
 +
 + if (enable)
 + val = (1  SMPL_EN_SHIFT) | (0  SMPLT_SHIFT);
 + else
 + val = 0;
 +
 + mask = SMPL_EN_MASK | SMPLT_MASK;
 +
 + dev_info(info-dev, %s: %s SMPL\n, __func__,
 + enable ? enable : disable);
 +
 + ret = regmap_update_bits(info-max77686-rtc_regmap,
 +  MAX77686_WTSR_SMPL_CNTL, mask, val);
 + if (ret  0) {
 + dev_err(info-dev, %s: fail to update SMPL reg(%d)\n,
 + __func__, ret);
 + return;
 + }
 +
 + max77686_rtc_update(info, MAX77686_RTC_WRITE);
 +
 + val = 0;
 + regmap_read(info-max77686-rtc_regmap, MAX77686_WTSR_SMPL_CNTL, val);
 + pr_info(%s: WTSR_SMPL(0x%02x)\n, __func__, val);
 +}
 +#endif /* MAX77686_RTC_WTSR_SMPL */

 ...

 +static int __devinit max77686_rtc_probe(struct platform_device *pdev)

CONFIG_HOTPLUG is removed, so we should stop using devinit, devexit,
devinit_p, etc.


 ...


--- a/drivers/rtc/rtc-max77686.c~rtc-max77686-add-maxim-77686-driver-fix
+++ a/drivers/rtc/rtc-max77686.c
@@ -129,7 +129,7 @@ static int max77686_rtc_tm_to_data(struc
return 0;
 }
 
-static inline int max77686_rtc_update(struct max77686_rtc_info *info,
+static int max77686_rtc_update(struct max77686_rtc_info *info,
enum MAX77686_RTC_OP op)
 {
int ret;
@@ -501,7 +501,7 @@ static struct regmap_config max77686_rtc
.val_bits = 8,
 };
 
-static int __devinit max77686_rtc_probe(struct platform_device *pdev)
+static int max77686_rtc_probe(struct platform_device *pdev)
 {
struct max77686_dev *max77686 = dev_get_drvdata(pdev-dev.parent);
struct max77686_rtc_info *info;
@@ -575,7 +575,7 @@ out:
return ret;
 }
 
-static int __devexit max77686_rtc_remove(struct platform_device *pdev)
+static int max77686_rtc_remove(struct platform_device *pdev)
 {
struct max77686_rtc_info *info = platform_get_drvdata(pdev);
 
@@ -623,7 +623,7 @@ static struct platform_driver max77686_r
.owner  = THIS_MODULE,
},
.probe  = max77686_rtc_probe,
-   .remove = 

Re: [rtc-linux] [RESEND PATCH] RTC: MAX77686: Add Maxim 77686 driver

2012-12-18 Thread Joe Perches
On Tue, 2012-12-18 at 16:30 -0800, Andrew Morton wrote:
 On Wed, 28 Nov 2012 15:50:57 +0900
 Jonghwa Lee jonghwa3@samsung.com wrote:
 
  Add driver for support max77686 rtc.
  MAX77686 rtc support smpl and wtsr mode. It has two alarm register
  which can be used for alarming to wake system up. This drvier uses regmap
  to access its register.
  
  ...
 
  +static inline int max77686_rtc_calculate_wday(u8 shifted)
  +{
  +   int counter = -1;
  +   while (shifted) {
  +   shifted = 1;
  +   counter++;
  +   }
  +   return counter;
  +}
 
 We should be able to use log2() in here?

fls(shifted) - 1


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/