Re: [PATCH v4 1/6] acerhdf: Adding support for "manual mode"

Andreas Mohr
2014-07-20T20:20:53Z

Hi,

On Sun, Jul 20, 2014 at 12:42:53PM +0200, Peter Feuerer wrote:
> Hi Andreas,
>
> Andreas Mohr writes:
> >On Sun, Jul 20, 2014 at 02:51:15AM +0200, Peter Feuerer wrote:
> >>@@ -127,105 +138,107 @@ struct bios_settings_t {
> >> unsigned char fanreg;
> >> unsigned char tempreg;
> >> struct fancmd cmd;
> >>+ int mcmd_enable;
> >> };
> >>+
> >> /* Register addresses and values for different BIOS versions */
> >> static const struct bios_settings_t bios_tbl[] = {
> >
> >Change-unrelated content here, might want to move away.
>
> You mean this newline?

Yup.

> >Suggestion (if ok from a commit revision/history POV):
> >git rebase -i s:/'acerhdf: Adding support for "manual mode"'~1
> ><"e"dit first commit>
> >git checkout -p s:/'acerhdf: Adding support for "manual mode"'
> ><grab hunk [split as needed]>
> >git commit -m "FIXUP"
> >git rebase --continue
> >git rebase -i s:/'acerhdf: Adding support for "manual mode"'~1
> ><move FIXUP line post [PATCH v4 6/6] acerhdf: minor clean up>
> ><do "f"ixup>
>
> If I understand this correctly, you want me to move this new line
> to patch 6, correct?

Yup, to a suitable "other stuff" location, which patch 6 likely would be.

> >Sorry that I didn't find worse breakage during my review
> >than this si{mp|ng}le line ;)
>
> No problem, as long as you give your Acked-by / Reviewed-by ;)

Hmm, I'm unsure whether my involvement is sufficient for a full Reviewed-by
(think full replacement of core driver mechanisms as in bang-bang governor,
etc.).

But yeah, at least I can say I did review things
and I did not see any obvious issues.

Thanks,

Andreas