linux-kernel - Re: [PATCH] ACPI video: Ignore AE_AML_PACKAGE_LIMIT errors after

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 11 Oct 2012 00:47:52 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Igor Murzov <e-mail@...e.by>
Cc:	linux-acpi@...r.kernel.org, Zhang Rui <rui.zhang@...el.com>,
	Len Brown <lenb@...nel.org>, Sergey V <sftp.mtuci@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ACPI video: Ignore AE_AML_PACKAGE_LIMIT errors after _DOD evaluation.

On Wednesday 10 of October 2012 04:43:40 Igor Murzov wrote:
> On Wed, 10 Oct 2012 00:58:10 +0200
> "Rafael J. Wysocki" <rjw@...k.pl> wrote:
> 
> > On Wednesday 10 of October 2012 02:19:06 Igor Murzov wrote:
> > > This should fix brightness controls on some laptops.
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=47861
> > > 
> > > Signed-off-by: Igor Murzov <e-mail@...e.by>
> > 
> > Put more detials into the changelog, please.  The BZ entry linked above
> > may or may not be accessible when the changelog is read by someone.
> > 
> > > ---
> > >  drivers/acpi/video.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> > > index 1e0a9e1..01bb58d 100644
> > > --- a/drivers/acpi/video.c
> > > +++ b/drivers/acpi/video.c
> > > @@ -1349,8 +1349,12 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,
> > >  	struct acpi_device *dev;
> > >  
> > >  	status = acpi_video_device_enumerate(video);
> > > -	if (status)
> > > -		return status;
> > > +	if (status) {
> > > +		if (status == AE_AML_PACKAGE_LIMIT)
> > > +			status = 0; /* Ignore this error */
> > > +		else
> > > +			return status;
> > > +	}
> > 
> > First off, please add a comment explaining _why_ we're ignoring the error.
> 
> The problem is that i'm not sure if it's ok to ignore
> AE_AML_PACKAGE_LIMIT here.

This should be written in a comment, then. :-)

> Stefan Wilkens in bugzilla
> claims that video module works fine on his Compaq 6720s
> in spite of the fact that acpi_video_device_enumerate()
> is not able to find any video device on his laptop.
> This fix is just the most obvious one, not necessarily
> the proper one.

So say something like "there are systems known to return AE_AML_PACKAGE_LIMIT
here, apparently for no valid reason" in a comment.  And give a reference to
the BZ entry in the changelog.

There are too many places where we have put workarounds like this without
documenting them properly and now we have problems when we need to change
something.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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/