Date: Tue, 12 Apr 2016 16:18:52 -0700
From: Tony Lindgren <tony@atomide.com>
To: Frank Rowand <frowand.list@gmail.com>
Cc: Grant Likely <grant.likely@linaro.org>, Rob Herring <robh+dt@kernel.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-omap@vger.kernel.org, Nishanth Menon <nm@ti.com>,
        Tero Kristo <t-kristo@ti.com>, Tom Rini <trini@konsulko.com>
Subject: Re: [PATCH] of: Add generic handling for hardware incomplete fail
 state
Message-ID: <20160412231852.GM5995@atomide.com>
References: <1460486275-12256-1-git-send-email-tony@atomide.com>
 <570D56FE.2070408@gmail.com>
 <570D7922.5020206@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <570D7922.5020206@gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1794
Lines: 56

* Frank Rowand <frowand.list@gmail.com> [160412 15:40]:
> On 4/12/2016 1:13 PM, Frank Rowand wrote:
> > Hi Tony,
> 
> < snip >
> 
> > With that change, the bulk of your patch looks good, with
> > minor changes:
> > 
> >   __of_device_is_available() would not need to change.
> > 
> >   __of_device_is_incomplete() would change to check the new
> >   boolean property.  (And I would suggest renaming it to
> >   something that conveys it is ok to power manage the
> >   device, but do not do anything else to the device.)
> > 
> > -Frank
> 
> One more thought...
> 
> Are there multiple drivers that need to follow this
> pattern, or just one at the moment?  If just one driver,
> then I would suggest open-coding accessing the property
> in the probe routine instead of adding the helper
> functions.  If more drivers appear with the same
> pattern then the helper functions could be added.

Well we already have several workarounds for devices to reset
them for idle by accessing the device registers:

omap_dss_reset
omap_hdq1w_reset
omap_i2c_reset

At least the three above are doing access to the device
registers partially duplicating the driver probe functionality.

Then the PM init code has some workarounds ti idle device
wrapper IP on SoCs that don't have the video or modem:

omap3xxx_prm_iva_idle
omap3_prm_reset_modem

And I'm just aware of the ones that we have in the mainline kernel
for PM to work. I bet Nishanth has more examples in mind where the
bootloader needs set devices to incomplete state based on
the SoC revision.

In general, I would rather see board specific dts files set
the unused devices to incomplete status rather than set them
to disabled. If they are set to disabled, from PM point of
view SoC specific workarounds are needed to idle them.

Regards,

Tony