Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Wed, May 28, 2014 at 12:44:35PM -0700, Stephen Boyd wrote: > On 05/28/14 10:12, Mark Brown wrote: > > If the supply must always be physically present the bindings should be > > specified as it being mandatory and the code written in that fashion; as > > an extension Linux will put a dummy in but this is attempting to handle > > incorrect DTs. This means we have functional error handling in cases > > where there is something to worry about and simplifies the code using > > the regulator. > Ok, you're saying the opposite of Rob. Should it be required or optional > in the DT binding? I'm saying it should be required. The implementation accepts it as an extension (a recent extension at that). > Ok. Dave M has already picked up all these patches so I'll send a patch > to replace regulator_get_optional() with regulator_get() and fix up the > error handling unless I hear otherwise. Yes, please. I'm much more worried about the abuse of regulator_get_optional() than the binding. signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On 05/28/14 10:12, Mark Brown wrote: > On Wed, May 28, 2014 at 05:16:46PM +0200, Uwe Kleine-König wrote: >> On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: >>> On 05/24/14 05:48, Mark Brown wrote: So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. >> There is a difference between the supply being optional for the hardware >> to work and the need to specify it in the device tree, isn't it? My >> expectation is that when it's not specified there is just nothing the >> the software needs to care for. > If the supply must always be physically present the bindings should be > specified as it being mandatory and the code written in that fashion; as > an extension Linux will put a dummy in but this is attempting to handle > incorrect DTs. This means we have functional error handling in cases > where there is something to worry about and simplifies the code using > the regulator. Ok, you're saying the opposite of Rob. Should it be required or optional in the DT binding? > > regulator_get_optional() should *only* be used if the supply may be > omitted from the physical design and should generally always be > accompanied by code which does something substantially different such as > using an internal regulator or changing the source for a reference > voltage instead. > > Ok. Dave M has already picked up all these patches so I'll send a patch to replace regulator_get_optional() with regulator_get() and fix up the error handling unless I hear otherwise. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Wed, May 28, 2014 at 05:16:46PM +0200, Uwe Kleine-König wrote: > On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: > > On 05/24/14 05:48, Mark Brown wrote: > > > So, according to the datasheet I managed to find this device has a > > > supply VDD_IO (so normally written vdd-io-supply here), some other > > > supplies which are tied to VDD_IO (so can probably be omitted) and a > > > supply VDD_A3.3 none of which are optional. There is an internal > > > regulator which can be used to drop a higher voltage VDD_IO down for > > > some of the supplies tied to it but that's essentially a noop from > > > software as far as I can tell. None of these supplies are obviously > > > optional, though I've not read the datasheet in detail so I may have > > > missed something here. > There is a difference between the supply being optional for the hardware > to work and the need to specify it in the device tree, isn't it? My > expectation is that when it's not specified there is just nothing the > the software needs to care for. If the supply must always be physically present the bindings should be specified as it being mandatory and the code written in that fashion; as an extension Linux will put a dummy in but this is attempting to handle incorrect DTs. This means we have functional error handling in cases where there is something to worry about and simplifies the code using the regulator. regulator_get_optional() should *only* be used if the supply may be omitted from the physical design and should generally always be accompanied by code which does something substantially different such as using an internal regulator or changing the source for a reference voltage instead. > > > That said it looks like this is intended to be a supply for an external > > > PHY rather than the device itself, but even so my original question > > > about it being able to operate without power still applies. Looking at > > > the code it's certainly not doing any of the handling of a missing > > > supply that I would associate with using _optional(). > > I agree, both supplies don't look optional. Unfortunately > > efm32gg-dk3750.dts doesn't look to be listing any supply, and this > > driver only recently got support for the VDD_A3.3 supply that the omap > > board uses (adding Uwe for any comments on efm setup). I presume on > If I read the schematic correctly there is nothing to regulate on the > efm32 dev board. If you want to take a look on the schematic yourself, > it's contained in the documentation package available at > http://www.silabs.com/products/mcu/lowpower/pages/efm32gg-dk3750.aspx . > BDR3201A_A02_sch.pdf, page 3 of 22. That shows all the supplies connected to fixed voltage regulators (including the internal 1.8V LDO); the device tree should represent this accurately though the internal 1.8V regulator could be omitted for simplicity. It would be a remarkable device that was able to operate without power. signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Wed, May 28, 2014 at 10:16 AM, Uwe Kleine-König wrote: > Hello Stephen, > > On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: >> On 05/24/14 05:48, Mark Brown wrote: >> > On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: >> > >> >> Optional properties: >> >> -- vdd-supply: supply for Ethernet mac >> >> +- vdd-supply: analog 3.3V supply for Ethernet mac >> >> +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac >> > So, according to the datasheet I managed to find this device has a >> > supply VDD_IO (so normally written vdd-io-supply here), some other >> > supplies which are tied to VDD_IO (so can probably be omitted) and a >> > supply VDD_A3.3 none of which are optional. There is an internal >> > regulator which can be used to drop a higher voltage VDD_IO down for >> > some of the supplies tied to it but that's essentially a noop from >> > software as far as I can tell. None of these supplies are obviously >> > optional, though I've not read the datasheet in detail so I may have >> > missed something here. > There is a difference between the supply being optional for the hardware > to work and the need to specify it in the device tree, isn't it? My > expectation is that when it's not specified there is just nothing the > the software needs to care for. Yes, agreed. Of course you could have cases where a supply at the h/w level is optional like if a supply can be powered externally or via an internal regulator. Those cases will have to be made clear in the binding, but a heading "Optional properties" in a binding doc means properties which are optional to specify in DT. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Hello Stephen, On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: > On 05/24/14 05:48, Mark Brown wrote: > > On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: > > > >> Optional properties: > >> -- vdd-supply: supply for Ethernet mac > >> +- vdd-supply: analog 3.3V supply for Ethernet mac > >> +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac > > So, according to the datasheet I managed to find this device has a > > supply VDD_IO (so normally written vdd-io-supply here), some other > > supplies which are tied to VDD_IO (so can probably be omitted) and a > > supply VDD_A3.3 none of which are optional. There is an internal > > regulator which can be used to drop a higher voltage VDD_IO down for > > some of the supplies tied to it but that's essentially a noop from > > software as far as I can tell. None of these supplies are obviously > > optional, though I've not read the datasheet in detail so I may have > > missed something here. There is a difference between the supply being optional for the hardware to work and the need to specify it in the device tree, isn't it? My expectation is that when it's not specified there is just nothing the the software needs to care for. > > That said it looks like this is intended to be a supply for an external > > PHY rather than the device itself, but even so my original question > > about it being able to operate without power still applies. Looking at > > the code it's certainly not doing any of the handling of a missing > > supply that I would associate with using _optional(). > > I agree, both supplies don't look optional. Unfortunately > efm32gg-dk3750.dts doesn't look to be listing any supply, and this > driver only recently got support for the VDD_A3.3 supply that the omap > board uses (adding Uwe for any comments on efm setup). I presume on If I read the schematic correctly there is nothing to regulate on the efm32 dev board. If you want to take a look on the schematic yourself, it's contained in the documentation package available at http://www.silabs.com/products/mcu/lowpower/pages/efm32gg-dk3750.aspx . BDR3201A_A02_sch.pdf, page 3 of 22. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote:
> On 05/24/14 05:48, Mark Brown wrote:
> > That said it looks like this is intended to be a supply for an external
> > PHY rather than the device itself, but even so my original question
> > about it being able to operate without power still applies. Looking at
> > the code it's certainly not doing any of the handling of a missing
> > supply that I would associate with using _optional().
> I agree, both supplies don't look optional. Unfortunately
> efm32gg-dk3750.dts doesn't look to be listing any supply, and this
> driver only recently got support for the VDD_A3.3 supply that the omap
> board uses (adding Uwe for any comments on efm setup). I presume on
> these boards VDD_IO is tied to some always on power source that software
> doesn't want to deal with. Nishant, what's VDD_IO connected to on omap?
> What's the proper solution here? Should we use regulator_get() and check
> for EPROBE_DEFER and ignore other errors?
As an implementation extension if no supply is specified at all the
regulator API will happily substitute in a dummy if the board is using
DT or ACPI, or if it has specified full constraints.
> Should the get_optional() variant just drop the "Other consumers will
> be... " part and should the get_exclusive() variant say "obtain this
> regulator while this reference is held" ?
Yes.
> From: Stephen Boyd
> Subject: [PATCH] regulator: Fix regulator_get_{optional,exclusive}()
> documentation
Documentation/SubmittingPatches.
signature.asc
Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote:
On 05/24/14 05:48, Mark Brown wrote:
That said it looks like this is intended to be a supply for an external
PHY rather than the device itself, but even so my original question
about it being able to operate without power still applies. Looking at
the code it's certainly not doing any of the handling of a missing
supply that I would associate with using _optional().
I agree, both supplies don't look optional. Unfortunately
efm32gg-dk3750.dts doesn't look to be listing any supply, and this
driver only recently got support for the VDD_A3.3 supply that the omap
board uses (adding Uwe for any comments on efm setup). I presume on
these boards VDD_IO is tied to some always on power source that software
doesn't want to deal with. Nishant, what's VDD_IO connected to on omap?
What's the proper solution here? Should we use regulator_get() and check
for EPROBE_DEFER and ignore other errors?
As an implementation extension if no supply is specified at all the
regulator API will happily substitute in a dummy if the board is using
DT or ACPI, or if it has specified full constraints.
Should the get_optional() variant just drop the Other consumers will
be... part and should the get_exclusive() variant say obtain this
regulator while this reference is held ?
Yes.
From: Stephen Boyd sb...@codeaurora.org
Subject: [PATCH] regulator: Fix regulator_get_{optional,exclusive}()
documentation
Documentation/SubmittingPatches.
signature.asc
Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Hello Stephen, On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: On 05/24/14 05:48, Mark Brown wrote: On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: Optional properties: -- vdd-supply: supply for Ethernet mac +- vdd-supply: analog 3.3V supply for Ethernet mac +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. There is a difference between the supply being optional for the hardware to work and the need to specify it in the device tree, isn't it? My expectation is that when it's not specified there is just nothing the the software needs to care for. That said it looks like this is intended to be a supply for an external PHY rather than the device itself, but even so my original question about it being able to operate without power still applies. Looking at the code it's certainly not doing any of the handling of a missing supply that I would associate with using _optional(). I agree, both supplies don't look optional. Unfortunately efm32gg-dk3750.dts doesn't look to be listing any supply, and this driver only recently got support for the VDD_A3.3 supply that the omap board uses (adding Uwe for any comments on efm setup). I presume on If I read the schematic correctly there is nothing to regulate on the efm32 dev board. If you want to take a look on the schematic yourself, it's contained in the documentation package available at http://www.silabs.com/products/mcu/lowpower/pages/efm32gg-dk3750.aspx . BDR3201A_A02_sch.pdf, page 3 of 22. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Wed, May 28, 2014 at 10:16 AM, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: Hello Stephen, On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: On 05/24/14 05:48, Mark Brown wrote: On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: Optional properties: -- vdd-supply: supply for Ethernet mac +- vdd-supply: analog 3.3V supply for Ethernet mac +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. There is a difference between the supply being optional for the hardware to work and the need to specify it in the device tree, isn't it? My expectation is that when it's not specified there is just nothing the the software needs to care for. Yes, agreed. Of course you could have cases where a supply at the h/w level is optional like if a supply can be powered externally or via an internal regulator. Those cases will have to be made clear in the binding, but a heading Optional properties in a binding doc means properties which are optional to specify in DT. Rob -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Wed, May 28, 2014 at 05:16:46PM +0200, Uwe Kleine-König wrote: On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: On 05/24/14 05:48, Mark Brown wrote: So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. There is a difference between the supply being optional for the hardware to work and the need to specify it in the device tree, isn't it? My expectation is that when it's not specified there is just nothing the the software needs to care for. If the supply must always be physically present the bindings should be specified as it being mandatory and the code written in that fashion; as an extension Linux will put a dummy in but this is attempting to handle incorrect DTs. This means we have functional error handling in cases where there is something to worry about and simplifies the code using the regulator. regulator_get_optional() should *only* be used if the supply may be omitted from the physical design and should generally always be accompanied by code which does something substantially different such as using an internal regulator or changing the source for a reference voltage instead. That said it looks like this is intended to be a supply for an external PHY rather than the device itself, but even so my original question about it being able to operate without power still applies. Looking at the code it's certainly not doing any of the handling of a missing supply that I would associate with using _optional(). I agree, both supplies don't look optional. Unfortunately efm32gg-dk3750.dts doesn't look to be listing any supply, and this driver only recently got support for the VDD_A3.3 supply that the omap board uses (adding Uwe for any comments on efm setup). I presume on If I read the schematic correctly there is nothing to regulate on the efm32 dev board. If you want to take a look on the schematic yourself, it's contained in the documentation package available at http://www.silabs.com/products/mcu/lowpower/pages/efm32gg-dk3750.aspx . BDR3201A_A02_sch.pdf, page 3 of 22. That shows all the supplies connected to fixed voltage regulators (including the internal 1.8V LDO); the device tree should represent this accurately though the internal 1.8V regulator could be omitted for simplicity. It would be a remarkable device that was able to operate without power. signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On 05/28/14 10:12, Mark Brown wrote: On Wed, May 28, 2014 at 05:16:46PM +0200, Uwe Kleine-König wrote: On Tue, May 27, 2014 at 02:40:15PM -0700, Stephen Boyd wrote: On 05/24/14 05:48, Mark Brown wrote: So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. There is a difference between the supply being optional for the hardware to work and the need to specify it in the device tree, isn't it? My expectation is that when it's not specified there is just nothing the the software needs to care for. If the supply must always be physically present the bindings should be specified as it being mandatory and the code written in that fashion; as an extension Linux will put a dummy in but this is attempting to handle incorrect DTs. This means we have functional error handling in cases where there is something to worry about and simplifies the code using the regulator. Ok, you're saying the opposite of Rob. Should it be required or optional in the DT binding? regulator_get_optional() should *only* be used if the supply may be omitted from the physical design and should generally always be accompanied by code which does something substantially different such as using an internal regulator or changing the source for a reference voltage instead. Ok. Dave M has already picked up all these patches so I'll send a patch to replace regulator_get_optional() with regulator_get() and fix up the error handling unless I hear otherwise. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Wed, May 28, 2014 at 12:44:35PM -0700, Stephen Boyd wrote: On 05/28/14 10:12, Mark Brown wrote: If the supply must always be physically present the bindings should be specified as it being mandatory and the code written in that fashion; as an extension Linux will put a dummy in but this is attempting to handle incorrect DTs. This means we have functional error handling in cases where there is something to worry about and simplifies the code using the regulator. Ok, you're saying the opposite of Rob. Should it be required or optional in the DT binding? I'm saying it should be required. The implementation accepts it as an extension (a recent extension at that). Ok. Dave M has already picked up all these patches so I'll send a patch to replace regulator_get_optional() with regulator_get() and fix up the error handling unless I hear otherwise. Yes, please. I'm much more worried about the abuse of regulator_get_optional() than the binding. signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On 05/24/14 05:48, Mark Brown wrote:
> On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote:
>
>> Optional properties:
>> -- vdd-supply: supply for Ethernet mac
>> +- vdd-supply: analog 3.3V supply for Ethernet mac
>> +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac
> So, according to the datasheet I managed to find this device has a
> supply VDD_IO (so normally written vdd-io-supply here), some other
> supplies which are tied to VDD_IO (so can probably be omitted) and a
> supply VDD_A3.3 none of which are optional. There is an internal
> regulator which can be used to drop a higher voltage VDD_IO down for
> some of the supplies tied to it but that's essentially a noop from
> software as far as I can tell. None of these supplies are obviously
> optional, though I've not read the datasheet in detail so I may have
> missed something here.
>
> That said it looks like this is intended to be a supply for an external
> PHY rather than the device itself, but even so my original question
> about it being able to operate without power still applies. Looking at
> the code it's certainly not doing any of the handling of a missing
> supply that I would associate with using _optional().
I agree, both supplies don't look optional. Unfortunately
efm32gg-dk3750.dts doesn't look to be listing any supply, and this
driver only recently got support for the VDD_A3.3 supply that the omap
board uses (adding Uwe for any comments on efm setup). I presume on
these boards VDD_IO is tied to some always on power source that software
doesn't want to deal with. Nishant, what's VDD_IO connected to on omap?
What's the proper solution here? Should we use regulator_get() and check
for EPROBE_DEFER and ignore other errors?
By the way, the documentation for regulator_get_optional() and
regulator_get_exclusive() are confusing. It looks like we copy/pasted
the exclusive text (typo and all).
* regulator_get_optional - obtain optional access to a regulator.
* @dev: device for regulator "consumer"
* @id: Supply name or regulator ID.
*
* Returns a struct regulator corresponding to the regulator producer,
* or IS_ERR() condition containing errno. Other consumers will be
* unable to obtain this reference is held and the use count for the
* regulator will be initialised to reflect the current state of the
* regulator.
vs.
* regulator_get_exclusive - obtain exclusive access to a regulator.
* @dev: device for regulator "consumer"
* @id: Supply name or regulator ID.
*
* Returns a struct regulator corresponding to the regulator producer,
* or IS_ERR() condition containing errno. Other consumers will be
* unable to obtain this reference is held and the use count for the
* regulator will be initialised to reflect the current state of the
* regulator.
Should the get_optional() variant just drop the "Other consumers will
be... " part and should the get_exclusive() variant say "obtain this
regulator while this reference is held" ?
8<
From: Stephen Boyd
Subject: [PATCH] regulator: Fix regulator_get_{optional,exclusive}()
documentation
regulator_get_optional() doesn't hold an exclusive reference to
the regulator. Fix the documentation and reword the exclusive
documentation to fix the grammatical error "this reference is
held".
Signed-off-by: Stephen Boyd
---
drivers/regulator/core.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b97ffd2365d3..2fae21a9d0e5 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1430,9 +1430,9 @@ EXPORT_SYMBOL_GPL(regulator_get);
*
* Returns a struct regulator corresponding to the regulator producer,
* or IS_ERR() condition containing errno. Other consumers will be
- * unable to obtain this reference is held and the use count for the
- * regulator will be initialised to reflect the current state of the
- * regulator.
+ * unable to obtain this regulator while this reference is held and the
+ * use count for the regulator will be initialised to reflect the current
+ * state of the regulator.
*
* This is intended for use by consumers which cannot tolerate shared
* use of the regulator such as those which need to force the
@@ -1456,10 +1456,7 @@ EXPORT_SYMBOL_GPL(regulator_get_exclusive);
* @id: Supply name or regulator ID.
*
* Returns a struct regulator corresponding to the regulator producer,
- * or IS_ERR() condition containing errno. Other consumers will be
- * unable to obtain this reference is held and the use count for the
- * regulator will be initialised to reflect the current state of the
- * regulator.
+ * or IS_ERR() condition containing errno.
*
* This is intended for use by consumers for devices which can have
* some supplies unconnected in normal use, such as some MMC devices.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list:
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On 05/24/14 05:48, Mark Brown wrote:
On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote:
Optional properties:
-- vdd-supply: supply for Ethernet mac
+- vdd-supply: analog 3.3V supply for Ethernet mac
+- vdd-io-supply: digital 1.8V IO supply for Ethernet mac
So, according to the datasheet I managed to find this device has a
supply VDD_IO (so normally written vdd-io-supply here), some other
supplies which are tied to VDD_IO (so can probably be omitted) and a
supply VDD_A3.3 none of which are optional. There is an internal
regulator which can be used to drop a higher voltage VDD_IO down for
some of the supplies tied to it but that's essentially a noop from
software as far as I can tell. None of these supplies are obviously
optional, though I've not read the datasheet in detail so I may have
missed something here.
That said it looks like this is intended to be a supply for an external
PHY rather than the device itself, but even so my original question
about it being able to operate without power still applies. Looking at
the code it's certainly not doing any of the handling of a missing
supply that I would associate with using _optional().
I agree, both supplies don't look optional. Unfortunately
efm32gg-dk3750.dts doesn't look to be listing any supply, and this
driver only recently got support for the VDD_A3.3 supply that the omap
board uses (adding Uwe for any comments on efm setup). I presume on
these boards VDD_IO is tied to some always on power source that software
doesn't want to deal with. Nishant, what's VDD_IO connected to on omap?
What's the proper solution here? Should we use regulator_get() and check
for EPROBE_DEFER and ignore other errors?
By the way, the documentation for regulator_get_optional() and
regulator_get_exclusive() are confusing. It looks like we copy/pasted
the exclusive text (typo and all).
* regulator_get_optional - obtain optional access to a regulator.
* @dev: device for regulator consumer
* @id: Supply name or regulator ID.
*
* Returns a struct regulator corresponding to the regulator producer,
* or IS_ERR() condition containing errno. Other consumers will be
* unable to obtain this reference is held and the use count for the
* regulator will be initialised to reflect the current state of the
* regulator.
vs.
* regulator_get_exclusive - obtain exclusive access to a regulator.
* @dev: device for regulator consumer
* @id: Supply name or regulator ID.
*
* Returns a struct regulator corresponding to the regulator producer,
* or IS_ERR() condition containing errno. Other consumers will be
* unable to obtain this reference is held and the use count for the
* regulator will be initialised to reflect the current state of the
* regulator.
Should the get_optional() variant just drop the Other consumers will
be... part and should the get_exclusive() variant say obtain this
regulator while this reference is held ?
8
From: Stephen Boyd sb...@codeaurora.org
Subject: [PATCH] regulator: Fix regulator_get_{optional,exclusive}()
documentation
regulator_get_optional() doesn't hold an exclusive reference to
the regulator. Fix the documentation and reword the exclusive
documentation to fix the grammatical error this reference is
held.
Signed-off-by: Stephen Boyd sb...@codeaurora.org
---
drivers/regulator/core.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b97ffd2365d3..2fae21a9d0e5 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1430,9 +1430,9 @@ EXPORT_SYMBOL_GPL(regulator_get);
*
* Returns a struct regulator corresponding to the regulator producer,
* or IS_ERR() condition containing errno. Other consumers will be
- * unable to obtain this reference is held and the use count for the
- * regulator will be initialised to reflect the current state of the
- * regulator.
+ * unable to obtain this regulator while this reference is held and the
+ * use count for the regulator will be initialised to reflect the current
+ * state of the regulator.
*
* This is intended for use by consumers which cannot tolerate shared
* use of the regulator such as those which need to force the
@@ -1456,10 +1456,7 @@ EXPORT_SYMBOL_GPL(regulator_get_exclusive);
* @id: Supply name or regulator ID.
*
* Returns a struct regulator corresponding to the regulator producer,
- * or IS_ERR() condition containing errno. Other consumers will be
- * unable to obtain this reference is held and the use count for the
- * regulator will be initialised to reflect the current state of the
- * regulator.
+ * or IS_ERR() condition containing errno.
*
* This is intended for use by consumers for devices which can have
* some supplies unconnected in normal use, such as some MMC devices.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list:
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: > Optional properties: > -- vdd-supply:supply for Ethernet mac > +- vdd-supply: analog 3.3V supply for Ethernet mac > +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. That said it looks like this is intended to be a supply for an external PHY rather than the device itself, but even so my original question about it being able to operate without power still applies. Looking at the code it's certainly not doing any of the handling of a missing supply that I would associate with using _optional(). signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: > Optional properties: > -- vdd-supply:supply for Ethernet mac > +- vdd-supply: analog 3.3V supply for Ethernet mac > +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac > +- reset-gpios: reset_n input pin Are we *positive* that power is optional? I do note that there aren't any mandatory supplies for the device which seems very strange... signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: Optional properties: -- vdd-supply:supply for Ethernet mac +- vdd-supply: analog 3.3V supply for Ethernet mac +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac +- reset-gpios: reset_n input pin Are we *positive* that power is optional? I do note that there aren't any mandatory supplies for the device which seems very strange... signature.asc Description: Digital signature
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
On Fri, May 23, 2014 at 12:57:17PM -0700, Stephen Boyd wrote: Optional properties: -- vdd-supply:supply for Ethernet mac +- vdd-supply: analog 3.3V supply for Ethernet mac +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac So, according to the datasheet I managed to find this device has a supply VDD_IO (so normally written vdd-io-supply here), some other supplies which are tied to VDD_IO (so can probably be omitted) and a supply VDD_A3.3 none of which are optional. There is an internal regulator which can be used to drop a higher voltage VDD_IO down for some of the supplies tied to it but that's essentially a noop from software as far as I can tell. None of these supplies are obviously optional, though I've not read the datasheet in detail so I may have missed something here. That said it looks like this is intended to be a supply for an external PHY rather than the device itself, but even so my original question about it being able to operate without power still applies. Looking at the code it's certainly not doing any of the handling of a missing supply that I would associate with using _optional(). signature.asc Description: Digital signature
[PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
The ks8851 SPI ethernet wasn't documented, but we documented the optional regulator supply for it under the mll based ethernet chip. Furthermore, that compatible string needed another 'l'. Fix all of this and document the optional vdd-io and reset-gpios properties. Cc: Nishanth Menon Cc: Rob Herring Cc: Pawel Moll Cc: Mark Rutland Cc: Ian Campbell Cc: Kumar Gala Cc: Signed-off-by: Stephen Boyd --- Documentation/devicetree/bindings/net/micrel-ks8851.txt | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/micrel-ks8851.txt b/Documentation/devicetree/bindings/net/micrel-ks8851.txt index d54d0cc79487..bbdf9a7359a2 100644 --- a/Documentation/devicetree/bindings/net/micrel-ks8851.txt +++ b/Documentation/devicetree/bindings/net/micrel-ks8851.txt @@ -1,9 +1,18 @@ -Micrel KS8851 Ethernet mac +Micrel KS8851 Ethernet mac (MLL) Required properties: -- compatible = "micrel,ks8851-ml" of parallel interface +- compatible = "micrel,ks8851-mll" of parallel interface - reg : 2 physical address and size of registers for data and command - interrupts : interrupt connection +Micrel KS8851 Ethernet mac (SPI) + +Required properties: +- compatible = "micrel,ks8851" or the deprecated "ks8851" +- reg : chip select number +- interrupts : interrupt connection + Optional properties: -- vdd-supply: supply for Ethernet mac +- vdd-supply: analog 3.3V supply for Ethernet mac +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac +- reset-gpios: reset_n input pin -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
The ks8851 SPI ethernet wasn't documented, but we documented the optional regulator supply for it under the mll based ethernet chip. Furthermore, that compatible string needed another 'l'. Fix all of this and document the optional vdd-io and reset-gpios properties. Cc: Nishanth Menon n...@ti.com Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Cc: devicet...@vger.kernel.org Signed-off-by: Stephen Boyd sb...@codeaurora.org --- Documentation/devicetree/bindings/net/micrel-ks8851.txt | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/micrel-ks8851.txt b/Documentation/devicetree/bindings/net/micrel-ks8851.txt index d54d0cc79487..bbdf9a7359a2 100644 --- a/Documentation/devicetree/bindings/net/micrel-ks8851.txt +++ b/Documentation/devicetree/bindings/net/micrel-ks8851.txt @@ -1,9 +1,18 @@ -Micrel KS8851 Ethernet mac +Micrel KS8851 Ethernet mac (MLL) Required properties: -- compatible = micrel,ks8851-ml of parallel interface +- compatible = micrel,ks8851-mll of parallel interface - reg : 2 physical address and size of registers for data and command - interrupts : interrupt connection +Micrel KS8851 Ethernet mac (SPI) + +Required properties: +- compatible = micrel,ks8851 or the deprecated ks8851 +- reg : chip select number +- interrupts : interrupt connection + Optional properties: -- vdd-supply: supply for Ethernet mac +- vdd-supply: analog 3.3V supply for Ethernet mac +- vdd-io-supply: digital 1.8V IO supply for Ethernet mac +- reset-gpios: reset_n input pin -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
