lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <48332796-1b9a-c897-c695-e66b116386be@redhat.com>
Date:   Fri, 6 Aug 2021 15:42:40 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Eric Piel <eric.piel@...mplin-utc.net>,
        Mark Gross <mgross@...ux.intel.com>
Subject: Re: [RFT, PATCH v1 1/1] platform/x86: hp_accel: Convert to be a
 platform driver

Hi,

On 8/3/21 10:08 PM, Andy Shevchenko wrote:
> ACPI core in conjunction with platform driver core provides
> an infrastructure to enumerate ACPI devices. Use it in order
> to remove a lot of boilerplate code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
> 
> Not sure what buys us to run _INI on PM calls. It's against the spec AFAICT.
> In any case ACPICA runs _INI as per specification when devices are
> instantiated.

_INI used to also be ran on resume for some reason, but that was recently
changed.

You're right that calling it is no longer necessary now that we no longer
do that.

But the changes related to this are really separate from the platform
driver conversion, please split this into 2 patches.

Also for the next version please Cc: Kai-Heng Feng <kai.heng.feng@...onical.com>
and ask him to test, I think he has access to hardware to test this.

Regards,

Hans


> 
>  drivers/platform/x86/hp_accel.c | 74 +++++++--------------------------
>  1 file changed, 14 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp_accel.c b/drivers/platform/x86/hp_accel.c
> index 8c0867bda828..69f86b761c7f 100644
> --- a/drivers/platform/x86/hp_accel.c
> +++ b/drivers/platform/x86/hp_accel.c
> @@ -29,7 +29,6 @@
>  #include "../../misc/lis3lv02d/lis3lv02d.h"
>  
>  #define DRIVER_NAME     "hp_accel"
> -#define ACPI_MDPS_CLASS "accelerometer"
>  
>  /* Delayed LEDs infrastructure ------------------------------------ */
>  
> @@ -78,7 +77,6 @@ static const struct acpi_device_id lis3lv02d_device_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids);
>  
> -
>  /**
>   * lis3lv02d_acpi_init - ACPI _INI method: initialize the device.
>   * @lis3: pointer to the device struct
> @@ -87,14 +85,6 @@ MODULE_DEVICE_TABLE(acpi, lis3lv02d_device_ids);
>   */
>  static int lis3lv02d_acpi_init(struct lis3lv02d *lis3)
>  {
> -	struct acpi_device *dev = lis3->bus_priv;
> -	if (!lis3->init_required)
> -		return 0;
> -
> -	if (acpi_evaluate_object(dev->handle, METHOD_NAME__INI,
> -				 NULL, NULL) != AE_OK)
> -		return -EINVAL;
> -
>  	return 0;
>  }
>  
> @@ -278,30 +268,6 @@ static struct delayed_led_classdev hpled_led = {
>  	.set_brightness = hpled_set,
>  };
>  
> -static acpi_status
> -lis3lv02d_get_resource(struct acpi_resource *resource, void *context)
> -{
> -	if (resource->type == ACPI_RESOURCE_TYPE_EXTENDED_IRQ) {
> -		struct acpi_resource_extended_irq *irq;
> -		u32 *device_irq = context;
> -
> -		irq = &resource->data.extended_irq;
> -		*device_irq = irq->interrupts[0];
> -	}
> -
> -	return AE_OK;
> -}
> -
> -static void lis3lv02d_enum_resources(struct acpi_device *device)
> -{
> -	acpi_status status;
> -
> -	status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> -					lis3lv02d_get_resource, &lis3_dev.irq);
> -	if (ACPI_FAILURE(status))
> -		printk(KERN_DEBUG DRIVER_NAME ": Error getting resources\n");
> -}
> -
>  static bool hp_accel_i8042_filter(unsigned char data, unsigned char str,
>  				  struct serio *port)
>  {
> @@ -331,23 +297,19 @@ static bool hp_accel_i8042_filter(unsigned char data, unsigned char str,
>  	return false;
>  }
>  
> -static int lis3lv02d_add(struct acpi_device *device)
> +static int lis3lv02d_probe(struct platform_device *device)
>  {
>  	int ret;
>  
> -	if (!device)
> -		return -EINVAL;
> -
> -	lis3_dev.bus_priv = device;
> +	lis3_dev.bus_priv = ACPI_COMPANION(&device->dev);
>  	lis3_dev.init = lis3lv02d_acpi_init;
>  	lis3_dev.read = lis3lv02d_acpi_read;
>  	lis3_dev.write = lis3lv02d_acpi_write;
> -	strcpy(acpi_device_name(device), DRIVER_NAME);
> -	strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
> -	device->driver_data = &lis3_dev;
>  
>  	/* obtain IRQ number of our device from ACPI */
> -	lis3lv02d_enum_resources(device);
> +	ret = platform_get_irq_optional(device, 0);
> +	if (ret > 0)
> +		lis3_dev.irq = ret;
>  
>  	/* If possible use a "standard" axes order */
>  	if (lis3_dev.ac.x && lis3_dev.ac.y && lis3_dev.ac.z) {
> @@ -359,7 +321,6 @@ static int lis3lv02d_add(struct acpi_device *device)
>  	}
>  
>  	/* call the core layer do its init */
> -	lis3_dev.init_required = true;
>  	ret = lis3lv02d_init_device(&lis3_dev);
>  	if (ret)
>  		return ret;
> @@ -381,11 +342,8 @@ static int lis3lv02d_add(struct acpi_device *device)
>  	return ret;
>  }
>  
> -static int lis3lv02d_remove(struct acpi_device *device)
> +static int lis3lv02d_remove(struct platform_device *device)
>  {
> -	if (!device)
> -		return -EINVAL;
> -
>  	i8042_remove_filter(hp_accel_i8042_filter);
>  	lis3lv02d_joystick_disable(&lis3_dev);
>  	lis3lv02d_poweroff(&lis3_dev);
> @@ -396,7 +354,6 @@ static int lis3lv02d_remove(struct acpi_device *device)
>  	return lis3lv02d_remove_fs(&lis3_dev);
>  }
>  
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int lis3lv02d_suspend(struct device *dev)
>  {
> @@ -407,14 +364,12 @@ static int lis3lv02d_suspend(struct device *dev)
>  
>  static int lis3lv02d_resume(struct device *dev)
>  {
> -	lis3_dev.init_required = false;
>  	lis3lv02d_poweron(&lis3_dev);
>  	return 0;
>  }
>  
>  static int lis3lv02d_restore(struct device *dev)
>  {
> -	lis3_dev.init_required = true;
>  	lis3lv02d_poweron(&lis3_dev);
>  	return 0;
>  }
> @@ -434,17 +389,16 @@ static const struct dev_pm_ops hp_accel_pm = {
>  #endif
>  
>  /* For the HP MDPS aka 3D Driveguard */
> -static struct acpi_driver lis3lv02d_driver = {
> -	.name  = DRIVER_NAME,
> -	.class = ACPI_MDPS_CLASS,
> -	.ids   = lis3lv02d_device_ids,
> -	.ops = {
> -		.add     = lis3lv02d_add,
> -		.remove  = lis3lv02d_remove,
> +static struct platform_driver lis3lv02d_driver = {
> +	.probe	= lis3lv02d_probe,
> +	.remove	= lis3lv02d_remove,
> +	.driver	= {
> +		.name	= DRIVER_NAME,
> +		.pm	= HP_ACCEL_PM,
> +		.acpi_match_table = lis3lv02d_device_ids,
>  	},
> -	.drv.pm = HP_ACCEL_PM,
>  };
> -module_acpi_driver(lis3lv02d_driver);
> +module_platform_driver(lis3lv02d_driver);
>  
>  MODULE_DESCRIPTION("Glue between LIS3LV02Dx and HP ACPI BIOS and support for disk protection LED.");
>  MODULE_AUTHOR("Yan Burman, Eric Piel, Pavel Machek");
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ