linux-kernel - Re: [PATCHv2] of: Add generic handling for ePAPR 1.1 fail-sss states

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: <20160831214151.wzq7y54xxs2qj422@atomide.com>
Date:   Wed, 31 Aug 2016 14:41:52 -0700
From:   Tony Lindgren <tony@...mide.com>
To:     Frank Rowand <frowand.list@...il.com>
Cc:     Grant Likely <grant.likely@...aro.org>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
        Nishanth Menon <nm@...com>, Tero Kristo <t-kristo@...com>,
        Tom Rini <trini@...sulko.com>
Subject: Re: [PATCHv2] of: Add generic handling for ePAPR 1.1 fail-sss states

* Frank Rowand <frowand.list@...il.com> [160831 13:51]:
> On 08/29/16 15:35, Tony Lindgren wrote:
> > 	if (of_device_is_incomplete(pdev->dev.of_node, status)) {
> > 		if (!strcmp("hw-incomplete-pins", status)) {
> > 			dev_info(&pdev->dev,
> > 				 "Unusable hardware: Not pinned out\n");
> > 			err = -ENODEV;
> > 			goto out;
> > 		}
> > 		if (!strcmp("hw-missing-daughter-card")) {
> > 			err = -EPROBE_DEFER;
> > 			goto out;
> > 		}
> > 		if (!strcmp("hw-buggy-dma")) {
> > 			dev_warn(&pdev->dev,
> > 				 "Replace hardware for working DMA\n");
> > 		}
> > 	}
> 
> What if the device has two issues to be reported?  You can not
> specify two different values for the status property.

That's a good point.

> What if the firmware wants to report that the hardware failed
> self-test (thus status = "fail-sss") but is already using
> status to describe the hardware?

Yeah that's true. Do you know what the "sss" stands for here?
Status Self teSt, or Side Scan Sonar? :)

> > - Make more generic as suggested by Frank but stick with
> >   "operational status of a device" approch most people seem
> >   to prefer that
> 
> I am still opposed to using the status property for this purpose.
> 
> The status property is intended to report an operational problem with
> a device or a device that the kernel can cause to be operational (such
> as a quiescent cpu being enabled).  It is the only property I am aware
> of to report _state_.
> 
> It is unfortunate that Linux has adopted the practice of overloading status
> to determine whether a piece of hardware exists or does not exist.  This
> is extremely useful for the way we structure the .dts and .dtsi files but
> should have used a new property name.  We are stuck with that choice of
> using the status property for two purposes, first the state of a device,
> and secondly the hardware description of existing or not existing.
> 
> Why not just create a new property that describes the hardware?
> Perhaps something like:
> 
>    incomplete = "pins_output", "buggy_dma";

New property for incomplete works for me. Rob, got any comments here?

> > + *  __of_device_is_incomplete - check if a device is incomplete
> 
> It is not checking if a device is incomplete.  It is checking whether the
> device is operational _or_ incomplete.
> 
> This is conflating concepts and likely to be confusing.  This is the problem
> with overloading the status property for yet another purpose.

Sure that's a valid point.

Regards,

Tony