lkml.org 
[lkml]   [2016]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCHv2] of: Add generic handling for ePAPR 1.1 fail-sss states
From
Date
On 09/08/16 08:58, Tony Lindgren wrote:
> * Rob Herring <robh+dt@kernel.org> [160908 06:38]:
>> On Wed, Aug 31, 2016 at 4:41 PM, Tony Lindgren <tony@atomide.com> wrote:
>>> * Frank Rowand <frowand.list@gmail.com> [160831 13:51]:
>>>> 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.
>
> Yeah I think we have to assume that.
>
>>>> 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.
>>
>>>> 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.
>
> Both status and and a separate property work for me.
>
> If no other considerations, we should probably pick something with a
> a limited set of states to avoid it getting out of control and being
> misused for something weird like driver probe order..

I do not want to add another property that conveys state. The property
that I was proposing does not convey state -- it describes the hardware.


> For example, just status = "fail" would be enough for the cases I've
> seen. That would still allow probe the device, then PM runtime idle
> it and bail out with -ENODEV.
>
> For whatever warnings or errors the driver needs to show, the driver
> could probably figure it out. I don't know if we want to or need to
> pass any informational messages with the incomplete status or
> property :)
>
>> 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?
>
> Well there may be no way to check if something is pinned out based on
> the hardware. The same SoC can be packaged with different pins. In that
> case only the die id or serial number for each produced chip is
> different, not the revision numbers. So for cases like that the dtb
> file or kernel cmdline is the only information available. And we can't
> assume pinctrl is required as it's perfectly fine to do that only in
> the bootloader for the static cases to save memory.
>
> Just to consider other ways of doing it, we could use the compatible
> flag to tag devices that need to be just idled on probe, but that does
> not seem like generic solution to me.

Yuck. Again overloading a property to convey multiple pieces of
information.


>
> Regards,
>
> Tony
>

\
 
 \ /
  Last update: 2016-09-17 09:58    [W:0.091 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site