ks8851 DT/regulator/gpio updates
Subsystems (5)
v2 cover ks8851 DT/regulator/gpio updates
This set of patches properly documents the micrel ks8851 spi ethernet controller, converts to devm_regulator_get_optional() to make error paths slightly simpler, and finally adds supports for another optional regulator and a reset gpio. This allows me to use the ks8851 on my MSM8960 CDP board.
Changes since v1:
* Dropped vendor prefix patch as that should go through DT tree
Stephen Boyd (4):
devicetree: bindings: Properly document micrel ks8851 SPI chips
net: ks8851: Use devm_regulator_get_optional()
net: ks8851: Add optional vdd_io regulator and reset gpio
net: ks8851: Add of match table
.../devicetree/bindings/net/micrel-ks8851.txt | 15 ++++- drivers/net/ethernet/micrel/ks8851.c | 71 +++++++++++++++++++--- 2 files changed, 74 insertions(+), 12 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
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.
--- Documentation/devicetree/bindings/net/micrel-ks8851.txt | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/net/micrel-ks8851.txt +12 -3
@@ -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
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Mark Brown
maintainerRe: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Quoted 5 lines · Stephen Boyd
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...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140524/50bafebb/attachment.sig>
Mark Brown
maintainerRe: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
In reply to Stephen BoydQuoted 4 lines · Stephen Boyd
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().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140524/6b0d3e8c/attachment-0001.sig>
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Quoted 16 lines · Mark Brown
Quoted 4 lines · Stephen Boyd
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 macSo, 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 toVDD_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 voltageVDD_IOdown 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.
Quoted 5 lines
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 <[email protected]>
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".
--- drivers/regulator/core.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/regulator/core.c +4 -7
@@ -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.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Mark Brown
maintainerRe: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Quoted 15 lines · Stephen Boyd
Quoted 5 lines · Mark Brown
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.dtsdoesn'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 boardsVDD_IOis tied to some always on power source that software doesn't want to deal with. Nishant, what'sVDD_IOconnected to on omap?
Quoted 2 lines
What's the proper solution here? Should we use
regulator_get()and check forEPROBE_DEFERand 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.
Quoted 3 lines
Should the
get_optional()variant just drop the "Other consumers will be... " part and should theget_exclusive()variant say "obtain this regulator while this reference is held" ?
Yes.
Quoted 3 lines
From: Stephen Boyd <[email protected]>
Subject: [PATCH] regulator: Fix regulator_get_{optional,exclusive}()
documentation
Documentation/SubmittingPatches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140528/d1b26f8b/attachment.sig>
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
In reply to Stephen BoydHello Stephen,
Quoted 7 lines · Stephen Boyd
Quoted 6 lines · Mark Brown
Quoted 4 lines · Stephen Boyd
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
Quoted 9 lines
Quoted 9 lines
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 toVDD_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 voltageVDD_IOdown 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.
Quoted 10 lines
Quoted 5 lines
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.dtsdoesn'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
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Quoted 19 lines · Uwe Kleine-K?nig
Hello Stephen,
Quoted 7 lines · Stephen Boyd
Quoted 6 lines · Mark Brown
Quoted 4 lines · Stephen Boyd
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 macQuoted 9 lines
Quoted 9 lines
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 toVDD_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 voltageVDD_IOdown 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.
Quoted 4 lines
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
Mark Brown
maintainerRe: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
In reply to Uwe Kleine-KönigQuoted 13 lines · Uwe Kleine-K?nig
Quoted 11 lines · Stephen Boyd
Quoted 9 lines · Mark Brown
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 toVDD_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 voltageVDD_IOdown 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.
Quoted 4 lines
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.
Quoted 11 lines
Quoted 10 lines
Quoted 5 lines
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.dtsdoesn'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
Quoted 5 lines
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140528/de74c033/attachment.sig>
Re: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Quoted 16 lines · Mark Brown
Quoted 11 lines · Uwe Kleine-K?nig
Quoted 10 lines · Stephen Boyd
Quoted 9 lines · Mark Brown
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 toVDD_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 voltageVDD_IOdown 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.Quoted 4 lines
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.
Quoted 6 lines
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?
Quoted 8 lines
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.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Mark Brown
maintainerRe: [PATCH v2 1/4] devicetree: bindings: Properly document micrel ks8851 SPI chips
Quoted 11 lines · Stephen Boyd
Quoted 6 lines · Mark Brown
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).
Quoted 3 lines
Ok. Dave M has already picked up all these patches so I'll send a patch to replace
regulator_get_optional()withregulator_get()and fix up the error handling unless I hear otherwise.
Yes, please. I'm much more worried about the abuse ofregulator_get_optional() than the binding.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140528/2dfa8b45/attachment-0001.sig>
PATCH v2 2/4 net: ks8851: Use devm_regulator_get_optional()
In reply to Stephen BoydThis simplifies error paths and removes the need to regulator_put().
--- drivers/net/ethernet/micrel/ks8851.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/net/ethernet/micrel/ks8851.c +3 -8
@@ -1417,7 +1417,7 @@ static int ks8851_probe(struct spi_device *spi) ks->spidev = spi; ks->tx_space =6144 ; - ks->vdd_reg =regulator_get_optional (&spi->dev,"vdd" ); + ks->vdd_reg =devm_regulator_get_optional (&spi->dev,"vdd" );if (IS_ERR (ks->vdd_reg)) { ret =PTR_ERR (ks->vdd_reg);if (ret == -EPROBE_DEFER )
@@ -1427,7 +1427,7 @@ static int ks8851_probe(struct spi_device *spi)if (ret) {dev_err (&spi->dev,"regulator enable fail: %d\n" , ret); - goto err_reg_en; + goto err_reg; } }
@@ -1530,9 +1530,6 @@ err_irq: err_id:if (!IS_ERR (ks->vdd_reg))regulator_disable (ks->vdd_reg); -err_reg_en: -if (!IS_ERR (ks->vdd_reg)) -regulator_put (ks->vdd_reg); err_reg:free_netdev (ndev);return ret;
@@ -1547,10 +1544,8 @@ static int ks8851_remove(struct spi_device *spi)unregister_netdev (priv->netdev);free_irq (spi->irq, priv); -if (!IS_ERR (priv->vdd_reg)) { +if (!IS_ERR (priv->vdd_reg))regulator_disable (priv->vdd_reg); -regulator_put (priv->vdd_reg); - }free_netdev (priv->netdev);return 0 ;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 3/4 net: ks8851: Add optional vdd_io regulator and reset gpio
In reply to Stephen BoydAllow the ks8851 driver to enable an optional 1.8V vdd_io regulator and assert the reset pin to the phy if a reset gpio is present in device tree.
--- drivers/net/ethernet/micrel/ks8851.c | 54 +++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-)
drivers/net/ethernet/micrel/ks8851.c +53 -1
@@ -26,6 +26,8 @@#include <linux/regulator/consumer.h> #include <linux/spi/spi.h> +#include <linux/gpio.h> +#include <linux/of_gpio.h> #include "ks8851.h"
@@ -85,6 +87,8 @@ union ks8851_tx_hdr {* @eeprom_size: Companion eeprom size in Bytes, 0 if no eeprom * @eeprom: 93CX6 EEPROM state for accessing on-board EEPROM. * @vdd_reg: Optional regulator supplying the chip +* @vdd_io: Optional digital power supply for IO +* @gpio: Optional reset_n gpio * * The @lock ensures that the chip is protected when certain operations are * in progress. When the read or write packet transfer is in progress, most
@@ -133,6 +137,8 @@ struct ks8851_net {struct eeprom_93cx6 eeprom;struct regulator *vdd_reg; +struct regulator *vdd_io; +int gpio; };static int msg_enable;
@@ -1404,6 +1410,7 @@ static int ks8851_probe(struct spi_device *spi)struct ks8851_net *ks;int ret;unsigned cider; +int gpio; ndev =alloc_etherdev (sizeof (struct ks8851_net ));if (!ndev)
@@ -1417,6 +1424,37 @@ static int ks8851_probe(struct spi_device *spi) ks->spidev = spi; ks->tx_space =6144 ; + gpio =of_get_named_gpio_flags (spi->dev.of_node,"reset-gpios" , +0 ,NULL ); +if (gpio == -EPROBE_DEFER ) { + ret = gpio; + goto err_gpio; + } + + ks->gpio = gpio; +if (gpio_is_valid (gpio)) { + ret =devm_gpio_request_one (&spi->dev, gpio, +GPIOF_OUT_INIT_LOW ,"ks8851_rst_n" ); +if (ret) { +dev_err (&spi->dev,"reset gpio request failed\n" ); + goto err_gpio; + } + } + + ks->vdd_io =devm_regulator_get_optional (&spi->dev,"vdd-io" ); +if (IS_ERR (ks->vdd_io)) { + ret =PTR_ERR (ks->vdd_io); +if (ret == -EPROBE_DEFER ) + goto err_reg_io; + }else { + ret =regulator_enable (ks->vdd_io); +if (ret) { +dev_err (&spi->dev,"regulator vdd_io enable fail: %d\n" , + ret); + goto err_reg_io; + } + } + ks->vdd_reg =devm_regulator_get_optional (&spi->dev,"vdd" );if (IS_ERR (ks->vdd_reg)) { ret =PTR_ERR (ks->vdd_reg);
@@ -1425,12 +1463,16 @@ static int ks8851_probe(struct spi_device *spi) }else { ret =regulator_enable (ks->vdd_reg);if (ret) { -dev_err (&spi->dev,"regulator enable fail: %d\n" , +dev_err (&spi->dev,"regulator vdd enable fail: %d\n" , ret); goto err_reg; } } +if (gpio_is_valid (gpio)) { +usleep_range (10000 ,11000 ); +gpio_set_value (gpio,1 ); + }mutex_init (&ks->lock);spin_lock_init (&ks->statelock);
@@ -1527,10 +1569,16 @@ err_netdev:free_irq (ndev->irq, ks); err_irq: +if (gpio_is_valid (gpio)) +gpio_set_value (gpio,0 ); err_id:if (!IS_ERR (ks->vdd_reg))regulator_disable (ks->vdd_reg); err_reg: +if (!IS_ERR (ks->vdd_io)) +regulator_disable (ks->vdd_io); +err_reg_io: +err_gpio:free_netdev (ndev);return ret; }
@@ -1544,8 +1592,12 @@ static int ks8851_remove(struct spi_device *spi)unregister_netdev (priv->netdev);free_irq (spi->irq, priv); +if (gpio_is_valid (priv->gpio)) +gpio_set_value (priv->gpio,0 );if (!IS_ERR (priv->vdd_reg))regulator_disable (priv->vdd_reg); +if (!IS_ERR (priv->vdd_io)) +regulator_disable (priv->vdd_io);free_netdev (priv->netdev);return 0 ;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 4/4 net: ks8851: Add of match table
In reply to Stephen BoydUsers are currently just providing "ks8851" as the compatible for this driver in device tree. Add a compatible string that provides the vendor name along with the device name to be more explicit.
--- drivers/net/ethernet/micrel/ks8851.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/ethernet/micrel/ks8851.c +6 -0
@@ -1603,9 +1603,15 @@ static int ks8851_remove(struct spi_device *spi)return 0 ; } +static const struct of_device_id ks8851_match_table[] = { + { .compatible ="micrel,ks8851" }, + { } +}; +static struct spi_driver ks8851_driver = { .driver = { .name ="ks8851" , + .of_match_table = ks8851_match_table, .owner =THIS_MODULE , .pm = &ks8851_pm_ops, },
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
David Miller
maintainerFrom: Stephen Boyd <[email protected]>
Date: Fri, 23 May 2014 12:57:16 -0700
Quoted 8 lines
This set of patches properly documents the micrel ks8851 spi ethernet controller, converts to
devm_regulator_get_optional()to make error paths slightly simpler, and finally adds supports for another optional regulator and a reset gpio. This allows me to use the ks8851 on my MSM8960 CDP board.Changes since v1:
* Dropped vendor prefix patch as that should go through DT tree
Series applied, thanks Stephen!