linux-kernel - Re: [PATCH V5 05/12] ACPI: check and enumerate CMOS RTC devices explicitly

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: <1398649833.2443.6.camel@rzhang1-mobl4>
Date:	Mon, 28 Apr 2014 09:50:33 +0800
From:	Zhang Rui <rui.zhang@...el.com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	bhelgaas@...gle.com, matthew.garrett@...ula.com,
	rafael.j.wysocki@...el.com, dmitry.torokhov@...il.com
Subject: Re: [PATCH V5 05/12] ACPI: check and enumerate CMOS RTC devices
 explicitly

On Mon, 2014-04-28 at 00:19 +0200, Rafael J. Wysocki wrote:
> On Monday, April 28, 2014 12:17:49 AM Rafael J. Wysocki wrote:
> > On Tuesday, April 08, 2014 12:06:52 AM Zhang Rui wrote:
> > > For CMOS RTC devices, the acpi pnp scan handler does not work because
> > > there is already a cmos rtc scan handler installed, thus we need to
> > > check those devices and enumerate them to PNP bus explicitly.
> > > Plus, the cmos rtc scan handler needs to return 1 so that it will not
> > > be enumerated to platform bus.
> > > 
> > > Note: the CMOS RTC device id is not removed from ACPI pnp scan handler
> > >       id list, thus, if cmos rtc scan handler is compiled out, the ACPI
> > >       pnp scan handler will be attached to the CMOS RTC devices instead,
> > >       to prevent these devices from being created to platform bus.
> > > 
> > > Signed-off-by: Zhang Rui <rui.zhang@...el.com>
> > > ---
> > >  drivers/acpi/acpi_cmos_rtc.c |    2 +-
> > >  drivers/acpi/acpi_pnp.c      |   22 +++++++++++++++++++++-
> > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/acpi_cmos_rtc.c b/drivers/acpi/acpi_cmos_rtc.c
> > > index 961b45d..2da8660 100644
> > > --- a/drivers/acpi/acpi_cmos_rtc.c
> > > +++ b/drivers/acpi/acpi_cmos_rtc.c
> > > @@ -68,7 +68,7 @@ static int acpi_install_cmos_rtc_space_handler(struct acpi_device *adev,
> > >  		return -ENODEV;
> > >  	}
> > >  
> > > -	return 0;
> > > +	return 1;
> > >  }
> > >  
> > >  static void acpi_remove_cmos_rtc_space_handler(struct acpi_device *adev)
> > > diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> > > index 8cf7d9d..387b150 100644
> > > --- a/drivers/acpi/acpi_pnp.c
> > > +++ b/drivers/acpi/acpi_pnp.c
> > > @@ -349,9 +349,29 @@ static struct acpi_scan_handler acpi_pnp_handler = {
> > >  	.attach = acpi_pnp_scan_handler_attach,
> > >  };
> > >  
> > > +/*
> > > + * For CMOS RTC devices, the acpi pnp spcan handler does not work because
> > > + * there is already a cmos rtc scan handler installed, thus we need to
> > > + * check those devices and enumerate them to PNP bus explicitly.
> > > + */
> > > +static int is_cmos_rtc_device(struct acpi_device *adev)
> > > +{
> > > +	struct acpi_device_id ids[] = {
> > > +		{ "PNP0B00" },
> > > +		{ "PNP0B01" },
> > > +		{ "PNP0B02" },
> > > +		{""},
> > > +	};
> > > +	return !acpi_match_device_ids(adev, ids);
> > > +}
> > > +
> > >  bool acpi_is_pnp_device(struct acpi_device *device)
> > >  {
> > > -	return device->handler == &acpi_pnp_handler;
> > > +	if (device->handler == &acpi_pnp_handler)
> > > +		return true;
> > > +	if (is_cmos_rtc_device(device))
> > > +		return true;
> > > +	return false;
> > 
> > What about doing
> > 
> > 	return device->handler == &acpi_pnp_handler || is_cmos_rtc_device(device);
> > 
> > instead?
> > 
okay.

> > >  }
> > >  EXPORT_SYMBOL_GPL(acpi_is_pnp_device);
> 
> Moreover, it looks like this patch should be folded into [2/12] to avoid
> breaking bisection.
> 
well, I wanted to make the patch clean and cleasr so I preferred to do
one thing in one patch and fix the side effect in a later patch as long
as it builds okay.
But you're right, will merge them in next version.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/