From: Frank Rowand <frowand.list@gmail.com>
To: Rob Herring <robh+dt@kernel.org>, Tony Lindgren <tony@atomide.com>
Cc: Grant Likely <grant.likely@linaro.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
linux-omap <linux-omap@vger.kernel.org>,
Nishanth Menon <nm@ti.com>, Tero Kristo <t-kristo@ti.com>,
Tom Rini <trini@konsulko.com>
Subject: Re: [PATCHv2] of: Add generic handling for ePAPR 1.1 fail-sss states
Date: Thu, 8 Sep 2016 12:05:12 -0700 [thread overview]
Message-ID: <57D1B668.3070001@gmail.com> (raw)
In-Reply-To: <CAL_Jsq+dm5HMkX8DNNZNB2QF-VPg96Esb+Capv88zys9nAPksw@mail.gmail.com>
On 09/08/16 06:38, Rob Herring wrote:
> On Wed, Aug 31, 2016 at 4:41 PM, Tony Lindgren <tony@atomide.com> wrote:
>> * Frank Rowand <frowand.list@gmail.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? :)
>
> String String String!!!?
>
> No clue for me.
>
>>
>>>> - 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_.
>
> Yes, in theory a device can go from disabled to okay, but that's
> generally never been supported. Linux takes the simple approach of
> "disabled" means ignore it. I think we'll see that change with
> overlays.
>
>>> 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.
>
> I don't agree. Generally, disabled means the h/w is there, but don't
> use it. There may be some cases where the hardware doesn't exist for
> the convenience of having a single dts, but that's the exception.
That it is not an exception, but instead a frequent pattern for .dtsi files.
A quick look in arm:
$ grep status *.dtsi | wc -l
4842
$ grep status *.dtsi | grep '"disabled"' | wc -l
3431
>>> 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?
>
> Pins not muxed out or connected on the board has to be the #1 reason
> for disabled status. I don't think we need or want another way to
> express that.
How is that expressed now?
> We may have discussed this, but why can't the driver that checks fail
> state just check whatever was used to set the device to fail in the
> first place?
>
> Rob
>
next prev parent reply other threads:[~2016-09-08 19:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-29 22:35 [PATCHv2] of: Add generic handling for ePAPR 1.1 fail-sss states Tony Lindgren
2016-08-30 0:23 ` Rob Herring
2016-08-30 0:39 ` Tony Lindgren
2016-08-31 17:47 ` Tony Lindgren
2016-08-31 20:50 ` Frank Rowand
2016-08-31 21:41 ` Tony Lindgren
2016-09-08 13:38 ` Rob Herring
2016-09-08 14:20 ` Nishanth Menon
2016-09-08 15:58 ` Tony Lindgren
2016-09-08 19:09 ` Frank Rowand
2016-09-08 19:17 ` Frank Rowand
2016-09-08 20:19 ` Tony Lindgren
2016-09-08 19:05 ` Frank Rowand [this message]
2016-09-09 2:43 ` Rob Herring
2016-09-09 14:10 ` Tom Rini
2016-09-10 1:11 ` Matthijs van Duin
2016-09-12 13:35 ` Tom Rini
2016-09-12 13:46 ` Matthijs van Duin
2016-09-12 13:49 ` Tom Rini
2016-09-12 13:38 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57D1B668.3070001@gmail.com \
--to=frowand.list@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=grant.likely@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=nm@ti.com \
--cc=robh+dt@kernel.org \
--cc=t-kristo@ti.com \
--cc=tony@atomide.com \
--cc=trini@konsulko.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).