[lm-sensors] #2073: VT1211 Driver review + docs

[lm-sensors] #2073: VT1211 Driver review + docs

[Juerg Haefliger]

Rudolf,

I know you're busy but is there a chance that someone gives the vt1211
a final review and merge it into main line soon? It's been around for
a while now.
Is there anything I can do to help?

Thanks
...juerg


On 6/4/06, lm-sensors <lm-sensors-notify at lm-sensors.org> wrote:
> #2073: VT1211 Driver review + docs
> --------------------+-------------------------------------------------------
>  Reporter:  ruik    |       Owner:  ruik
>      Type:  task    |      Status:  new
>  Priority:  major   |   Milestone:
> Component:  kernel  |     Version:
>  Keywords:          |
> --------------------+-------------------------------------------------------
>  [http://lists.lm-sensors.org/pipermail/lm-sensors/2006-June/016329.html
>  VT1211 driver review]
>
>  Needs review. Khali thinks not yet for 2.6.18
>
> --
> Ticket URL: <http://www.lm-sensors.org/ticket/2073>
> lm-sensors <http://lm-sensors.org/>
> Linux hardware monitoring

[lm-sensors] #2073: VT1211 Driver review + docs

[Jim Cromie]

Juerg Haefliger wrote:
> Rudolf,
>
> I know you're busy but is there a chance that someone gives the vt1211
> a final review and merge it into main line soon? It's been around for
> a while now.
> Is there anything I can do to help?
>
> Thanks
> ...juerg
>
>   
> On 6/4/06, lm-sensors <lm-sensors-notify at lm-sensors.org> wrote:
>   
>> #2073: VT1211 Driver review + docs
>> --------------------+-------------------------------------------------------
>>  Reporter:  ruik    |       Owner:  ruik
>>      Type:  task    |      Status:  new
>>  Priority:  major   |   Milestone:
>> Component:  kernel  |     Version:
>>  Keywords:          |
>> --------------------+-------------------------------------------------------
>>  [http://lists.lm-sensors.org/pipermail/lm-sensors/2006-June/016329.html
>>  VT1211 driver review]
>>
>>     

your patch passes the leading, trailing whitespace tests (which you know):
    cat -TE attachment-0001.obj | grep '^+ ' |more
    cat -TE attachment-0001.obj | grep ' $' |more



2 trivial notes:

+	default:
+		printk(KERN_ERR DRVNAME ": Unknown attr fetch\n");


since these are in your callbacks, which have a struct device *dev,
you could change them to 

	dev_err(dev, "Unknown attr.....
+	}



Also, just below here:

+		/* pwm & temp auto point registers */
+		data->pwm_auto_pwm[0][0] = 255;	/* hard wired */
+		data->pwm_auto_pwm[0][1] = vt1211_read8(data,



you could have done those initializations in a loop. 
Im assuming this would shrink the object by a teeny amount.



And one 'idle-curiosity' question:
what happens when you rmmod  then modprobe ? any surprises ?

[lm-sensors] #2073: VT1211 Driver review + docs

[Juerg Haefliger]

> your patch passes the leading, trailing whitespace tests (which you know):
>     cat -TE attachment-0001.obj | grep '^+ ' |more
>     cat -TE attachment-0001.obj | grep ' $' |more

good to know :-)


> 2 trivial notes:
>
> +       default:
> +               printk(KERN_ERR DRVNAME ": Unknown attr fetch\n");
>
>
> since these are in your callbacks, which have a struct device *dev,
> you could change them to
>
>         dev_err(dev, "Unknown attr.....
> +       }
>
>
> Also, just below here:
>
> +               /* pwm & temp auto point registers */
> +               data->pwm_auto_pwm[0][0] = 255; /* hard wired */
> +               data->pwm_auto_pwm[0][1] = vt1211_read8(data,
>
> you could have done those initializations in a loop.
> Im assuming this would shrink the object by a teeny amount.
>

Those issues are minor enough that I waive them at this point. Once
the patch is in the main line I'm happy to do increments. But I want
this thing in now :-)


> And one 'idle-curiosity' question:
> what happens when you rmmod  then modprobe ? any surprises ?

No prob, works fine.

Thanks
...juerg

[lm-sensors] #2073: VT1211 Driver review + docs

[Rudolf Marek]

Hi Juerg,

Sorry now it is Jean turn, and he decided that it should wait until 2.6.18 is
out. Moreover I did not have free time to do anything for lm-sensors ;(

Please be patient Jean knows it but again the reviews are very time consumpting.

Regards
Rudolf