FYI When you submit v3, you don't need to add "new patches on top" for
your feedbacks to the new driver, they can roll into the patch
introducing hp-cfg. Just make sure you include a changelog under your
cut line to indicate you changed these from vX->vY
I suspect that Hans will also want you to split the driver up into
smaller bite-size patches to make his review easier as well, but I'll
let him advise how he wants it done.
On 10/17/2022 09:11, Jorge Lopez wrote:
''Hi Mario,
Please see comments to previous source comments.
<snip>
Thanks. If you make this change for v2, I can make the matching change
in fwupd so that if it notices current_value permissions like this that
it shows read only there too.
Submitted the recommended changes for review in v2
Thanks, looks good.
Submitted a patch to improve the friendly display name for
few numbers of attributes associated with ‘Schedule Power-ON.’ BIOS
assign names such ‘Tuesday’ to an attribute. The name is correct, but
it is not descriptive enough for the user. Under those
conditions a portion of the path data value is appended to the attribute
name to create a user-friendly display name.
For instance, the attribute name is ‘Tuesday,’ and the display name
value is ‘Schedule Power-ON – Tuesday’
Looks good
Presumably if this is going into it's own directory you should move all
platform-x86 HP drivers to this directory earlier in the series too.
The other drivers named HP-WMI and HP_ACCEL were written by third
party members and not by HP. It is for this reason and because of
the number of files, only hp-bioscfg was placed in a separate
directory. Let me know If my reasoning is not valid enough and I
will keep the files in a separate directory and move the selection to
the main list. In addition, Moving HP-WMI and HP_ACCEL drivers
from x86 directories fall outside of the scope of these changes,
Correct?
There is no distinction who writes a driver. I think either you keep
this driver in the root of drivers/platform/x86 or you put all the HP
drivers in drivers/platform/x86/hp.
I think if you're going to put this driver in the sub-directory "hp",
then the first patch in this series should be to move those drivers to
that sub-directory. The second patch should be to introduce your new
driver.
The build process was tested with the latest drivers/platform/x86
from branch for-next.
Nonetheless, I will investigate.
I did my test on 6.0 rather than for-next. But given it's a header
issue I suspect you have a miss that works with the compiler I'm using.
I was using gcc 11.2.0 on Ubuntu 22.04.
<snip>
