From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756808AbcH3Aj3 (ORCPT ); Mon, 29 Aug 2016 20:39:29 -0400 Received: from muru.com ([72.249.23.125]:46697 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754688AbcH3Aj1 (ORCPT ); Mon, 29 Aug 2016 20:39:27 -0400 Date: Mon, 29 Aug 2016 17:39:22 -0700 From: Tony Lindgren To: Rob Herring Cc: Frank Rowand , Grant Likely , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , linux-omap , Nishanth Menon , Tero Kristo , Tom Rini Subject: Re: [PATCHv2] of: Add generic handling for ePAPR 1.1 fail-sss states Message-ID: <20160830003922.462byzmpnq277h2u@atomide.com> References: <20160829223542.18871-1-tony@atomide.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2-neo (2016-07-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Rob Herring [160829 17:24]: > On Mon, Aug 29, 2016 at 5:35 PM, Tony Lindgren wrote: > > It seems we can use the ePAPR 1.1 status fail-sss to do this. > > Quoting "Table 2-4 Values for status property" we have "fail-sss": > > > > "Indicates that the device is not operational. A serious error was > > detected in the device and it is unlikely to become operational > > without repair. The sss portion of the value is specific to the > > device and indicates the error condition detected." > > I had read this as 'sss' is just 3 characters, but I guess that doesn't matter. Yeah I'd assume it does not matter for the length. > > We can handle these fail states can be handled in a generic > > way. So let's introduce a generic status = "fail-" that > > describes a situation where a device driver probe should just > > shut down or idle the device if possible and then bail out. > > This allows the SoC variant and board specific dts files to set > > the status as needed. > > > > The suggested usage in a device driver probe is: > > > > static int foo_probe(struct platform_device *pdev) > > { > > int err; > > const char *status; > > ... > > > > pm_runtime_enable(&pdev->dev); > > pm_runtime_get_sync(&pdev->dev); > > pm_runtime_use_autosuspend(&pdev->dev); > > ... > > > > /* Configure device, load firmware, idle device */ > > ... > > > > if (of_device_is_incomplete(pdev->dev.of_node, status)) { > > &status Oops, I guess I should compile test the example. > > 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; > > This implies we're going to change this on the fly? I guess > disabled->okay can already happen. Well let's assume the bootloader sets some i2c controlled daughter board with "fail-hw-missing-daughter-card", then in theory kernel could probe it if it pops up on the i2c bus later on. Not sure if we want to do this, but it seems we could.. > > +static bool __of_device_is_incomplete(const struct device_node *device, > > + const char **status) > > +{ > > + const char *s, *m = "fail-"; > > + int slen, mlen; > > + > > + if (!device) > > + return false; > > + > > + s = __of_get_property(device, "status", &slen); > > You can use the string helper function here (or is the lock a problem?). I'll check. > Overall, seems okay to me. OK thanks for looking. Regards, Tony