Re: [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
To: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
Cc: Florian Fainelli
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>,
	Herbert Xu
	<herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Ray Jui <rjui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>,
	Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>,
	Krzysztof Kozlowski
	<krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>,
	Harald Freudenberger
	<freude-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..."
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	"open list:HARDWARE RANDOM NUMBER GENERATOR CORE" <l
Subject: Re: [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock
Date: Sat, 4 Nov 2017 20:37:31 +0100	[thread overview]
Message-ID: <20171104193731.GA751@lunn.ch> (raw)
In-Reply-To: <547786283.132202.1509819754396-7tX72C7vayboQLBSYMtkGA@public.gmane.org>

Hi Florian

> > >> +	/* Clock is optional on most platforms */
> > >> +	priv->clk = devm_clk_get(dev, NULL);
> > >> +	if (IS_ERR(priv->clk))
> > >> +		priv->clk = NULL;
> > > 
> > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> > 
> > Good point, so more like:
> > 
> > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
> 

> Unfortunately we need to return the error in all other cases. Please
> take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to
> take care of ENXIO in our case.

A few subsystems have a get_optional() call, e.g.
devm_phy_optional_get(). It does not return an error when the phy is
not supposed to exist, but in all other cases, it does.

Maybe consider adding devm_clk_get_optional()?

      Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)

From: andrew@lunn.ch (Andrew Lunn)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock
Date: Sat, 4 Nov 2017 20:37:31 +0100	[thread overview]
Message-ID: <20171104193731.GA751@lunn.ch> (raw)
In-Reply-To: <547786283.132202.1509819754396@email.1und1.de>

Hi Florian

> > >> +	/* Clock is optional on most platforms */
> > >> +	priv->clk = devm_clk_get(dev, NULL);
> > >> +	if (IS_ERR(priv->clk))
> > >> +		priv->clk = NULL;
> > > 
> > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> > 
> > Good point, so more like:
> > 
> > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
> 

> Unfortunately we need to return the error in all other cases. Please
> take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to
> take care of ENXIO in our case.

A few subsystems have a get_optional() call, e.g.
devm_phy_optional_get(). It does not return an error when the phy is
not supposed to exist, but in all other cases, it does.

Maybe consider adding devm_clk_get_optional()?

      Andrew

WARNING: multiple messages have this Message-ID (diff)

From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
To: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
Cc: Florian Fainelli
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>,
	Herbert Xu
	<herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Ray Jui <rjui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Matt Mackall <mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org>,
	Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>,
	Krzysztof Kozlowski
	<krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>,
	Harald Freudenberger
	<freude-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..."
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>"open
	list:HARDWARE RANDOM NUMBER GENERATOR CORE" <l>
Subject: Re: [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock
Date: Sat, 4 Nov 2017 20:37:31 +0100	[thread overview]
Message-ID: <20171104193731.GA751@lunn.ch> (raw)
In-Reply-To: <547786283.132202.1509819754396-7tX72C7vayboQLBSYMtkGA@public.gmane.org>

Hi Florian

> > >> +	/* Clock is optional on most platforms */
> > >> +	priv->clk = devm_clk_get(dev, NULL);
> > >> +	if (IS_ERR(priv->clk))
> > >> +		priv->clk = NULL;
> > > 
> > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> > 
> > Good point, so more like:
> > 
> > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
> 

> Unfortunately we need to return the error in all other cases. Please
> take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to
> take care of ENXIO in our case.

A few subsystems have a get_optional() call, e.g.
devm_phy_optional_get(). It does not return an error when the phy is
not supposed to exist, but in all other cases, it does.

Maybe consider adding devm_clk_get_optional()?

      Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)

From: Andrew Lunn <andrew@lunn.ch>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Sean Wang <sean.wang@mediatek.com>,
	Martin Kaiser <martin@kaiser.cx>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Scott Branden <sbranden@broadcom.com>,
	Ray Jui <rjui@broadcom.com>, Matt Mackall <mpm@selenic.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux-kernel@vger.kernel.org, Eric Anholt <eric@anholt.net>,
	Harald Freudenberger <freude@linux.vnet.ibm.com>,
	Rob Herring <robh+dt@kernel.org>,
	"maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..." 
	<bcm-kernel-feedback-list@broadcom.com>,
	"open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
	<linux-crypto@vger.kernel.org>,
	PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>,
	Steffen Trumtrar <s.trumtrar@pengutronix.de>,
	"moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" 
	<linux-rpi-kernel@lists.infradead.org>
Subject: Re: [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock
Date: Sat, 4 Nov 2017 20:37:31 +0100	[thread overview]
Message-ID: <20171104193731.GA751@lunn.ch> (raw)
In-Reply-To: <547786283.132202.1509819754396@email.1und1.de>

Hi Florian

> > >> +	/* Clock is optional on most platforms */
> > >> +	priv->clk = devm_clk_get(dev, NULL);
> > >> +	if (IS_ERR(priv->clk))
> > >> +		priv->clk = NULL;
> > > 
> > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock?
> > 
> > Good point, so more like:
> > 
> > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)?
> 

> Unfortunately we need to return the error in all other cases. Please
> take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to
> take care of ENXIO in our case.

A few subsystems have a get_optional() call, e.g.
devm_phy_optional_get(). It does not return an error when the phy is
not supposed to exist, but in all other cases, it does.

Maybe consider adding devm_clk_get_optional()?

      Andrew

next prev parent reply	other threads:[~2017-11-04 19:37 UTC|newest]

Thread overview: 119+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-02  1:03 [PATCH 00/12] bcm63xx-rng conversion to bcm2835-rng Florian Fainelli
2017-11-02  1:03 ` Florian Fainelli
2017-11-02  1:03 ` Florian Fainelli
2017-11-02  1:03 ` Florian Fainelli
     [not found] ` <20171102010408.27736-1-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-02  1:03   ` [PATCH 01/12] hwrng: bcm2835-rng: Obtain base register via resource Florian Fainelli
2017-11-02  1:03     ` Florian Fainelli
2017-11-02  1:03     ` Florian Fainelli
2017-11-02  1:03     ` Florian Fainelli
2017-11-02  1:04   ` [PATCH 06/12] hwrng: bcm2835-rng: Rework interrupt masking Florian Fainelli
2017-11-02  1:04     ` Florian Fainelli
2017-11-02  1:04     ` Florian Fainelli
2017-11-02  1:04     ` Florian Fainelli
2017-11-02  1:04   ` [PATCH 08/12] hwrng: bcm2835-rng: Abstract I/O accessors Florian Fainelli
2017-11-02  1:04     ` Florian Fainelli
2017-11-02  1:04     ` Florian Fainelli
2017-11-02  1:04     ` Florian Fainelli
     [not found]     ` <20171102010408.27736-9-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-03 20:19       ` Eric Anholt
2017-11-03 20:19         ` Eric Anholt
2017-11-03 20:19         ` Eric Anholt
2017-11-03 20:19         ` Eric Anholt
     [not found]         ` <874lqbru7i.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
2017-11-03 22:04           ` Florian Fainelli
2017-11-03 22:04             ` Florian Fainelli
2017-11-03 22:04             ` Florian Fainelli
2017-11-03 22:04             ` Florian Fainelli
2017-11-02 19:00   ` [PATCH 00/12] bcm63xx-rng conversion to bcm2835-rng Stefan Wahren
2017-11-02 19:00     ` Stefan Wahren
2017-11-02 19:00     ` Stefan Wahren
2017-11-02 19:00     ` Stefan Wahren
2017-11-02  1:03 ` [PATCH 02/12] hwrng: bcm2835-rng: Define a driver private context Florian Fainelli
2017-11-02  1:03   ` Florian Fainelli
2017-11-02  1:03   ` Florian Fainelli
2017-11-02  1:03   ` Florian Fainelli
2017-11-02  1:03 ` [PATCH 03/12] hwrng: bcm2835-rng: Move enabling to hwrng::init Florian Fainelli
2017-11-02  1:03   ` Florian Fainelli
2017-11-02  1:03   ` Florian Fainelli
2017-11-02  1:03   ` Florian Fainelli
2017-11-02  1:04 ` [PATCH 04/12] hwrng: bcm2835-rng: Implementation cleanup callback Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04 ` [PATCH 05/12] hwrng: bcm2835-rng: Use device managed helpers Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04 ` [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-04 13:50   ` Stefan Wahren
2017-11-04 13:50     ` Stefan Wahren
2017-11-04 13:50     ` Stefan Wahren
     [not found]     ` <944505791.223880.1509803454887-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
2017-11-04 17:59       ` Florian Fainelli
2017-11-04 17:59         ` Florian Fainelli
2017-11-04 17:59         ` Florian Fainelli
2017-11-04 17:59         ` Florian Fainelli
2017-11-04 18:22         ` Stefan Wahren
2017-11-04 18:22           ` Stefan Wahren
2017-11-04 18:22           ` Stefan Wahren
     [not found]           ` <547786283.132202.1509819754396-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
2017-11-04 19:37             ` Andrew Lunn [this message]
2017-11-04 19:37               ` Andrew Lunn
2017-11-04 19:37               ` Andrew Lunn
2017-11-04 19:37               ` Andrew Lunn
     [not found]               ` <20171104193731.GA751-g2DYL2Zd6BY@public.gmane.org>
2017-11-04 20:08                 ` Russell King - ARM Linux
2017-11-04 20:08                   ` Russell King - ARM Linux
2017-11-04 20:08                   ` Russell King - ARM Linux
2017-11-04 20:08                   ` Russell King - ARM Linux
2017-11-04 20:17                   ` Andrew Lunn
2017-11-04 20:17                     ` Andrew Lunn
2017-11-04 20:17                     ` Andrew Lunn
2017-11-04 20:17                     ` Andrew Lunn
2017-11-02  1:04 ` [PATCH 09/12] hwrng: bcm2835-rng: Add Broadcom MIPS I/O accessors Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04 ` [PATCH 10/12] dt-bindings: rng: Incorporate brcm,bcm6368.txt binding Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-06 21:38   ` Rob Herring
2017-11-06 21:38     ` Rob Herring
2017-11-06 21:38     ` Rob Herring
2017-11-06 21:38     ` Rob Herring
2017-11-02  1:04 ` [PATCH 11/12] hwrng: bcm2835-rng: Enable BCM2835 RNG to work on BCM63xx platforms Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-04 18:27   ` Stefan Wahren
2017-11-04 18:27     ` Stefan Wahren
2017-11-04 18:27     ` Stefan Wahren
2017-11-06 20:16     ` Florian Fainelli
2017-11-06 20:16       ` Florian Fainelli
2017-11-06 20:16       ` Florian Fainelli
2017-11-06 20:16       ` Florian Fainelli
2017-11-07  6:45       ` Stefan Wahren
2017-11-07  6:45         ` Stefan Wahren
2017-11-07  6:45         ` Stefan Wahren
2017-11-07  6:45         ` Stefan Wahren
2017-11-07 23:27         ` Stefan Wahren
2017-11-07 23:27           ` Stefan Wahren
2017-11-07 23:27           ` Stefan Wahren
2017-11-02  1:04 ` [PATCH 12/12] hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
2017-11-02  1:04   ` Florian Fainelli
     [not found]   ` <20171102010408.27736-13-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-03 20:18     ` Eric Anholt
2017-11-03 20:18       ` Eric Anholt
2017-11-03 20:18       ` Eric Anholt
2017-11-03 20:18       ` Eric Anholt
     [not found]       ` <877ev7ru97.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
2017-11-03 21:58         ` Florian Fainelli
2017-11-03 21:58           ` Florian Fainelli
2017-11-03 21:58           ` Florian Fainelli
2017-11-03 21:58           ` Florian Fainelli
2017-11-02 19:01 ` [PATCH 00/12] bcm63xx-rng conversion to bcm2835-rng Stefan Wahren
2017-11-02 19:01   ` Stefan Wahren
2017-11-02 19:01   ` Stefan Wahren
     [not found]   ` <1329593033.127526.1509649260041-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
2017-11-02 19:06     ` Florian Fainelli
2017-11-02 19:06       ` Florian Fainelli
2017-11-02 19:06       ` Florian Fainelli
2017-11-02 19:06       ` Florian Fainelli

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=20171104193731.GA751@lunn.ch \
    --to=andrew-g2dyl2zd6by@public.gmane.org \
    --cc=bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org \
    --cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=freude-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \
    --cc=herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org \
    --cc=krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org \
    --cc=mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ@public.gmane.org \
    --cc=rjui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org \
    --cc=stefan.wahren-eS4NqCHxEME@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.