2020. július 29., szerda 19:11 keltezéssel, Daniel Dadap <ddadap@xxxxxxxxxx> írta:
> [...]
I think
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
(or similar)
should be defined before any includes since you use pr_*().
Otherwise the messages won't be prefixed by the module name.
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/pci.h>
> +#include <linux/vga_switcheroo.h>
> +#include <linux/delay.h>
> +
> [...]
> +static void set_mux_state(acpi_handle handle, enum mux_state_command command)
> +{
> + acpi_helper(handle, MXDS, command);
> +}
> +
It's really a nitpick and a moot point, but I'd think set_mux_state() takes the state as
one of its arguments, not the command to set that state.
> [...]
> +static int __init mxds_gmux_init(void)
> +{
> + int ret = 0;
> + struct pci_dev *dev = NULL;
> +
> + while ((dev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, dev))) {
> + switch (dev->vendor) {
> + case 0x8086:
> + pci_dev_put(ig_dev);
> + ig_dev = pci_dev_get(dev);
> + break;
> + case 0x10de:
> + pci_dev_put(dg_dev);
> + dg_dev = pci_dev_get(dev);
> + break;
> + default:
> + break;
> + }
> + }
> +
I think I mentioned it last time, I personally dislike magic numbers,
so I'd use PCI_VENDOR_ID_{INTEL,NVIDIA} from <linux/pci_ids.h> here, but
I guess it's personal preference, so I don't want to bother you with that
anymore. If you use magic numbers, then I think comments would be much
appreciated.
> [...]
> +static void __exit mxds_gmux_exit(void)
> +{
> + if (vga_switcheroo_registered)
> + vga_switcheroo_unregister_handler();
The exit method of a module will not be run if it fails to load,
so I think "vga_switcheroo_registered" is not necessary.
> + pci_dev_put(ig_dev);
> + pci_dev_put(dg_dev);
> +}
> +module_exit(mxds_gmux_exit);
> --
> 2.18.4
Barnabás Pőcze
