RE: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Bryan.Whitehead]

> Hi Bryan
> 
> It is good to look at other drivers and see how they work. Ideally, your 
> driver
> wants to look similar to all other drivers, so making the maintenance easier.
> 
> You will find that most drivers have a set of goto statements for error
> handling, which jump to the end of the function, and do cleanup.
> No flags are maintained.
> 
>   Andrew

Thanks Andrew,

I understand now, I'll work on it.

Bryan

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Andrew Lunn]

> But the other case is when things fail anywhere during probe, or
> open. In these cases things are partially initialized and I assumed
> I needed to clean up whatever was initialized successfully before
> returning an error. I used flags so I could share a common clean up
> function for all possible error cases as well as the fully
> successful case. Without flags I would need to customize a clean-up
> sequence for each possible error point.

Hi Bryan

It is good to look at other drivers and see how they work. Ideally,
your driver wants to look similar to all other drivers, so making the
maintenance easier.

You will find that most drivers have a set of goto statements for
error handling, which jump to the end of the function, and do cleanup.
No flags are maintained.

  Andrew

RE: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Bryan.Whitehead]

> > Ok, but it seems to me that what I have is an example of "specific
> > book keeping private information". Can you clarify the style you prefer?
> >
> > In cases of allocation where I can just compare a pointer to null, I
> > can easily remove the flags. But in other cases I need a record of
> > which steps completed in order to clean up properly. In cases where I
> > need some sort of a flag would you prefer I avoid a bit mask, and have a
> standalone variable for each flag?
> 
> Hi Bryan
> 
> Often you know some thing has been done, because if it had not been
> done, you would of bombed out with an error. In the release function you
> can assume everything done in probe has been done, otherwise the probe
> would not be successful. In close, you can assume everything done in open
> was successful, otherwise the open would of failed
> 
> So probe does not need any flags. open does not need any flags.
> 
>Andrew

Hi Andrew,

OK, so there are two cases where clean-up is necessary. One is through call to 
remove or ndo_stop. For these cases I agree with you. Everything must have been 
set up correctly, so everything should be cleaned up, no flags required.

But the other case is when things fail anywhere during probe, or open. In these 
cases things are partially initialized and I assumed I needed to clean up 
whatever was initialized successfully before returning an error. I used flags 
so I could share a common clean up function for all possible error cases as 
well as the fully successful case. Without flags I would need to customize a 
clean-up sequence for each possible error point.

Or are you suggesting that I don't need to worry about clean up if an error 
happens during probe or open?

Bryan

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Andrew Lunn]

> Ok, but it seems to me that what I have is an example of "specific book 
> keeping
> private information". Can you clarify the style you prefer?
> 
> In cases of allocation where I can just compare a pointer to null, I can 
> easily remove
> the flags. But in other cases I need a record of which steps completed in 
> order to
> clean up properly. In cases where I need some sort of a flag would you prefer
> I avoid a bit mask, and have a standalone variable for each flag?

Hi Bryan

Often you know some thing has been done, because if it had not been
done, you would of bombed out with an error. In the release function
you can assume everything done in probe has been done, otherwise the
probe would not be successful. In close, you can assume everything
done in open was successful, otherwise the open would of failed

So probe does not need any flags. open does not need any flags.

   Andrew

RE: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Bryan.Whitehead]

Hi Florian,
Thanks for your review. I have the following questions/comments.

> On 02/21/2018 11:06 AM, Bryan Whitehead wrote:
> > Add main source files for new lan743x driver.
> >
> > Signed-off-by: Bryan Whitehead 
> > ---
> 
> > +lan743x-objs := lan743x_main.o
> 
> Should we assume that you have additional object files you would like to
> contribute at a later point? If that is the case, this is fine, if this is 
> going to be
> only file of this driver, consider renaming it so you don't even have to have
> this lan743x-objs line at all.

I am planning to add additional source files later. At the very least there will
be a lan743x_ethtool.o, and a lan743x_ptp.o. I didn't want to have to change
the name of the main file later, so I gave it the expected name now.

> > +
> > +static void lan743x_pci_cleanup(struct lan743x_adapter *adapter) {
> > +   struct lan743x_pci *pci = >pci;
> > +
> > +   if (pci->init_flags & INIT_FLAG_PCI_REGIONS_REQUESTED) {
> 
> There is a pattern throughout the driver of maintaining flags to track what
> was initialized and what was not, do you really need that, or can you check
> for specific book keeping private information. Maintaining flags is error 
> prone
> and requires you to keep adding new ones, that does not really scale.

Ok, but it seems to me that what I have is an example of "specific book keeping
private information". Can you clarify the style you prefer?

In cases of allocation where I can just compare a pointer to null, I can easily 
remove
the flags. But in other cases I need a record of which steps completed in order 
to
clean up properly. In cases where I need some sort of a flag would you prefer
I avoid a bit mask, and have a standalone variable for each flag?

[snip]

> > +   dmac_int_en &= ioc_bit;
> > +   dmac_int_sts &= dmac_int_en;
> > +   if (dmac_int_sts & ioc_bit) {
> > +   tasklet_schedule(>tx_isr_bottom_half);
> > +   enable_flag = 0;/* tasklet will re-enable later */
> > +   }
> 
> Consider migrating your TX buffer reclamation to a NAPI context. If you have
> one TX queue and one RX, the same NAPI context can be re-used, if you
> have separate RX/TX queues, you may create a NAPI context per RX/TX pair,
> or you may create separate NAPI contexts per TX queues and RX queues.

This may take some time to refactor, But I will see what I can do.

[snip]


> > +static int lan743x_phy_open(struct lan743x_adapter *adapter) {
> > +   struct lan743x_phy *phy = >phy;
> > +   struct phy_device *phydev;
> > +   struct net_device *netdev;
> > +   int ret = -EIO;
> > +   u32 mii_adv;
> > +
> > +   netdev = adapter->netdev;
> > +   phydev = phy_find_first(adapter->mdiobus);
> > +   if (!phydev) {
> > +   ret = -EIO;
> > +   goto clean_up;
> > +   }
> > +   ret = phy_connect_direct(netdev, phydev,
> > +lan743x_phy_link_status_change,
> > +PHY_INTERFACE_MODE_GMII);
> > +   if (ret)
> > +   goto clean_up;
> > +   phy->flags |= PHY_FLAG_ATTACHED;
> > +
> > +   /* MAC doesn't support 1000T Half */
> > +   phydev->supported &= ~SUPPORTED_1000baseT_Half;
> > +
> > +   /* support both flow controls */
> > +   phy->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX);
> > +   phydev->advertising &= ~(ADVERTISED_Pause |
> ADVERTISED_Asym_Pause);
> > +   mii_adv = (u32)mii_advertise_flowctrl(phy->fc_request_control);
> > +   phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
> > +   phy->fc_autoneg = phydev->autoneg;
> 
> No driver does things like that, that appears to be really wrong.
Can you clarify? What is wrong and how should it be?

[snip]

> > +static int lan743x_dmac_init(struct lan743x_adapter *adapter) {
> > +   struct lan743x_dmac *dmac = >dmac;
> > +   u32 data = 0;
> > +
> > +   dmac->flags = 0;
> > +   dmac->descriptor_spacing = DEFAULT_DMA_DESCRIPTOR_SPACING;
> > +   lan743x_csr_write(adapter, DMAC_CMD, DMAC_CMD_SWR_);
> > +   lan743x_csr_wait_for_bit(adapter, DMAC_CMD,
> DMAC_CMD_SWR_,
> > +0, 1000, 2, 100);
> > +   switch (dmac->descriptor_spacing) {
> > +   case DMA_DESCRIPTOR_SPACING_16:
> > +   data = DMAC_CFG_MAX_DSPACE_16_;
> > +   break;
> > +   case DMA_DESCRIPTOR_SPACING_32:
> > +   data = DMAC_CFG_MAX_DSPACE_32_;
> > +   break;
> > +   case DMA_DESCRIPTOR_SPACING_64:
> > +   data = DMAC_CFG_MAX_DSPACE_64_;
> > +   break;
> > +   case DMA_DESCRIPTOR_SPACING_128:
> > +   data = DMAC_CFG_MAX_DSPACE_128_;
> > +   break;
> > +   default:
> > +   return -EPERM;
> 
> -EINVAL maybe?
I think -EPERM is more appropriate because it reflects an unresolvable error. 
In other words the platform is in compatible with the device.
Would you prefer I use a preprocessor error instead, such as

#if invalid_setting
#error incompatible setting
#endif

> 
> [snip]
> 
> > +
> > +done:
> > +   

RE: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Bryan.Whitehead]

> On Thu, Feb 22, 2018 at 10:45:34PM +0100, Andrew Lunn wrote:
> > > Also I'm allocating interrupt resources on interface up, and freeing
> > > resources on interface down. So if there is an up, down, up sequence
> > > then the driver will allocate resources twice. In order for devm to
> > > work properly, should I move all resource allocation into the probe
> function?
> >
> > Hi Bryan
> >
> > It is better to fail early if the resource is not available, so yes, i
> > would register the interrupt handler in probe.
> 
> And we maintainers don't always agree with each other :-)
> 
> Doing irq handling in open/close without devm_ is also O.K.
> 
>Andrew

Thanks Andrew, and Florian,

Moving irq allocation and free, to probe and remove, will require a bit of 
refactoring and possibly introduce new issues. For now I will keep IRQ handling 
in open/close without devm_.

Other resource allocations are already in probe/remove so I will apply your 
suggestions in the next patch revision.

Thanks,
Bryan

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Florian Fainelli]

On 02/21/2018 11:06 AM, Bryan Whitehead wrote:
> Add main source files for new lan743x driver.
> 
> Signed-off-by: Bryan Whitehead 
> ---

> +lan743x-objs := lan743x_main.o

Should we assume that you have additional object files you would like to
contribute at a later point? If that is the case, this is fine, if this
is going to be only file of this driver, consider renaming it so you
don't even have to have this lan743x-objs line at all.

> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c 
> b/drivers/net/ethernet/microchip/lan743x_main.c
> new file mode 100644
> index 000..3de39e1
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -0,0 +1,2757 @@
> +/*
> + * Copyright (C) 2018 Microchip Technology

You should consider the SPDX license tags to reduce the license
boilerplate standard disclaimer.

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see .
> + */
> +
> +#include "lan743x_main.h"

This is not ideal, having all your header dependencies resolved by a
main header file usually leads to unnecessary inclusions when fewer are
needed.

> +
> +static void lan743x_pci_cleanup(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_pci *pci = >pci;
> +
> + if (pci->init_flags & INIT_FLAG_PCI_REGIONS_REQUESTED) {

There is a pattern throughout the driver of maintaining flags to track
what was initialized and what was not, do you really need that, or can
you check for specific book keeping private information. Maintaining
flags is error prone and requires you to keep adding new ones, that does
not really scale.

[snip]

> +static void lan743x_intr_software_isr(void *context)
> +{
> + struct lan743x_adapter *adapter = context;
> + struct lan743x_intr *intr = >intr;
> + u32 int_sts;
> +
> + int_sts = lan743x_csr_read(adapter, INT_STS);
> + if (int_sts & INT_BIT_SW_GP_) {
> + lan743x_csr_write(adapter, INT_STS, INT_BIT_SW_GP_);
> + intr->software_isr_flag = 1;
> + }
> +}
> +
> +static void lan743x_tx_isr(void *context, u32 int_sts, u32 flags)
> +{
> + struct lan743x_tx *tx = context;
> + struct lan743x_adapter *adapter = tx->adapter;
> + int enable_flag = 1;

This is inherently a boolean type.

> + u32 int_en = 0;
> +
> + int_en = lan743x_csr_read(adapter, INT_EN_SET);
> + if (flags & LAN743X_VECTOR_FLAG_SOURCE_ENABLE_CLEAR) {
> + lan743x_csr_write(adapter, INT_EN_CLR,
> +   INT_BIT_DMA_TX_(tx->channel_number));
> + }
> + if (int_sts & INT_BIT_DMA_TX_(tx->channel_number)) {
> + u32 ioc_bit = DMAC_INT_BIT_TX_IOC_(tx->channel_number);
> + u32 dmac_int_sts;
> + u32 dmac_int_en;
> +
> + if (flags & LAN743X_VECTOR_FLAG_SOURCE_STATUS_READ)
> + dmac_int_sts = lan743x_csr_read(adapter, DMAC_INT_STS);
> + else
> + dmac_int_sts = ioc_bit;
> + if (flags & LAN743X_VECTOR_FLAG_SOURCE_ENABLE_CHECK)
> + dmac_int_en = lan743x_csr_read(adapter,
> +DMAC_INT_EN_SET);
> + else
> + dmac_int_en = ioc_bit;
> +
> + dmac_int_en &= ioc_bit;
> + dmac_int_sts &= dmac_int_en;
> + if (dmac_int_sts & ioc_bit) {
> + tasklet_schedule(>tx_isr_bottom_half);
> + enable_flag = 0;/* tasklet will re-enable later */
> + }

Consider migrating your TX buffer reclamation to a NAPI context. If you
have one TX queue and one RX, the same NAPI context can be re-used, if
you have separate RX/TX queues, you may create a NAPI context per RX/TX
pair, or you may create separate NAPI contexts per TX queues and RX queues.

> + }
> + if (enable_flag)
> + /* enable isr */
> + lan743x_csr_write(adapter, INT_EN_SET,
> +   INT_BIT_DMA_TX_(tx->channel_number));
> +}
> +
> +static void lan743x_rx_isr(void *context, u32 int_sts, u32 flags)
> +{
> + struct lan743x_rx *rx = context;
> + struct lan743x_adapter *adapter = rx->adapter;
> + int enable_flag = 1;
> +
> + if (flags & LAN743X_VECTOR_FLAG_SOURCE_ENABLE_CLEAR) {
> + lan743x_csr_write(adapter, INT_EN_CLR,
> +

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Andrew Lunn]

On Thu, Feb 22, 2018 at 10:45:34PM +0100, Andrew Lunn wrote:
> > Also I'm allocating interrupt resources on interface up, and freeing 
> > resources
> > on interface down. So if there is an up, down, up sequence then the driver
> > will allocate resources twice. In order for devm to work properly, should I
> > move all resource allocation into the probe function?
> 
> Hi Bryan
> 
> It is better to fail early if the resource is not available, so yes, i
> would register the interrupt handler in probe.

And we maintainers don't always agree with each other :-)

Doing irq handling in open/close without devm_ is also O.K.

 Andrew

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Andrew Lunn]

> Also I'm allocating interrupt resources on interface up, and freeing resources
> on interface down. So if there is an up, down, up sequence then the driver
> will allocate resources twice. In order for devm to work properly, should I
> move all resource allocation into the probe function?

Hi Bryan

It is better to fail early if the resource is not available, so yes, i
would register the interrupt handler in probe.

  Andrew

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Florian Fainelli]

On 02/22/2018 01:31 PM, bryan.whiteh...@microchip.com wrote:
>>> +static void lan743x_intr_unregister_isr(struct lan743x_adapter *adapter,
>>> +   int vector_index)
>>> +{
>>> +   struct lan743x_vector *vector = >intr.vector_list
>>> +   [vector_index];
>>> +
>>> +   devm_free_irq(>pci.pdev->dev, vector->irq, vector);
>>
>> Hu Bryan
>>
>> The point of devm_ is that you don't need to free resources you have
>> allocated using devm_. The core will release them when the device is
>> removed.
> 
> Hi Andrew,
> 
> When I remove the call devm_free_irq, I get a segmentation fault on close
> in pci_disable_msix. Did I do something else wrong?
> 
> Also I'm allocating interrupt resources on interface up, and freeing resources
> on interface down. So if there is an up, down, up sequence then the driver
> will allocate resources twice. In order for devm to work properly, should I
> move all resource allocation into the probe function?

No, most network drivers request their interrupt line in the open
function and free it in the close function. Because you are balancing
each devm_request_irq() with a devm_free_irq(), just don't just devm_*
functions, just the normal request_irq() and free_irq() functions.
-- 
Florian

RE: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Bryan.Whitehead]

> > +static void lan743x_intr_unregister_isr(struct lan743x_adapter *adapter,
> > +   int vector_index)
> > +{
> > +   struct lan743x_vector *vector = >intr.vector_list
> > +   [vector_index];
> > +
> > +   devm_free_irq(>pci.pdev->dev, vector->irq, vector);
> 
> Hu Bryan
> 
> The point of devm_ is that you don't need to free resources you have
> allocated using devm_. The core will release them when the device is
> removed.

Hi Andrew,

When I remove the call devm_free_irq, I get a segmentation fault on close
in pci_disable_msix. Did I do something else wrong?

Also I'm allocating interrupt resources on interface up, and freeing resources
on interface down. So if there is an up, down, up sequence then the driver
will allocate resources twice. In order for devm to work properly, should I
move all resource allocation into the probe function?

Bryan

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[kbuild test robot]

Hi Bryan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Bryan-Whitehead/lan743x-Add-new-lan743x-driver/20180222-225510
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/net/ethernet/microchip/lan743x_main.c:68:5: sparse: symbol 
>> 'lan743x_csr_read' was not declared. Should it be
>> drivers/net/ethernet/microchip/lan743x_main.c:73:6: sparse: symbol 
>> 'lan743x_csr_write' was not declared. Should it be

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Andrew Lunn]

> +static void lan743x_intr_unregister_isr(struct lan743x_adapter *adapter,
> + int vector_index)
> +{
> + struct lan743x_vector *vector = >intr.vector_list
> + [vector_index];
> +
> + devm_free_irq(>pci.pdev->dev, vector->irq, vector);

Hu Bryan

The point of devm_ is that you don't need to free resources you have
allocated using devm_. The core will release them when the device is
removed.

In this case, you might want to ensure the hardware will not generate
any more interrupts, but you can leave the core to call free_irq().

Please look at all your devm_*free() like calls, and remove them if
they are not needed. 

> +static void lan743x_mdiobus_cleanup(struct lan743x_adapter *adapter)
> +{
> + if (adapter->init_flags & LAN743X_INIT_FLAG_MDIOBUS_REGISTERED) {
> + mdiobus_unregister(adapter->mdiobus);
> + adapter->init_flags &= ~LAN743X_INIT_FLAG_MDIOBUS_REGISTERED;
> + }
> + if (adapter->init_flags & LAN743X_INIT_FLAG_MDIOBUS_ALLOCATED) {
> + devm_mdiobus_free(>pci.pdev->dev, adapter->mdiobus);
> + adapter->mdiobus = NULL;
> + adapter->init_flags &= ~LAN743X_INIT_FLAG_MDIOBUS_ALLOCATED;
> + }
> +}

So you can delete devm_mdiobus_free(). That probably means you can also remove 
LAN743X_INIT_FLAG_MDIOBUS_ALLOCATED.

> +
> +static void lan743x_full_cleanup(struct lan743x_adapter *adapter)
> +{
> + if (adapter->init_flags & LAN743X_INIT_FLAG_NETDEV_REGISTERED) {
> + unregister_netdev(adapter->netdev);
> + adapter->init_flags &= ~LAN743X_INIT_FLAG_NETDEV_REGISTERED;
> + }
> + lan743x_mdiobus_cleanup(adapter);
> + lan743x_hardware_cleanup(adapter);
> + if (adapter->init_flags & LAN743X_COMPONENT_FLAG_PCI) {
> + lan743x_pci_cleanup(adapter);
> + adapter->init_flags &= ~LAN743X_COMPONENT_FLAG_PCI;
> + }
> +
> + /* would have freed netdev here.
> +  * but netdev was allocated with devm_alloc_etherdev.
> +  * and devm_free_netdev is not accessible.
> +  * so it is expected to be freed by the devm subsystem.
> +  */

And this comment can go.

Andrew

[PATCH v2 net-next 1/2] lan743x: Add main source files for new lan743x driver

[Bryan Whitehead]

Add main source files for new lan743x driver.

Signed-off-by: Bryan Whitehead 
---
 drivers/net/ethernet/microchip/Kconfig|   10 +
 drivers/net/ethernet/microchip/Makefile   |3 +
 drivers/net/ethernet/microchip/lan743x_main.c | 2757 +
 drivers/net/ethernet/microchip/lan743x_main.h |  686 ++
 4 files changed, 3456 insertions(+)
 create mode 100644 drivers/net/ethernet/microchip/lan743x_main.c
 create mode 100644 drivers/net/ethernet/microchip/lan743x_main.h

diff --git a/drivers/net/ethernet/microchip/Kconfig 
b/drivers/net/ethernet/microchip/Kconfig
index 36a09d9..71dca8b 100644
--- a/drivers/net/ethernet/microchip/Kconfig
+++ b/drivers/net/ethernet/microchip/Kconfig
@@ -42,4 +42,14 @@ config ENCX24J600
   To compile this driver as a module, choose M here. The module will be
   called encx24j600.
 
+config LAN743X
+   tristate "LAN743x support"
+   depends on PCI
+   select PHYLIB
+   ---help---
+ Support for the Microchip LAN743x PCI Express Gigabit Ethernet chip
+
+ To compile this driver as a module, choose M here. The module will be
+ called lan743x.
+
 endif # NET_VENDOR_MICROCHIP
diff --git a/drivers/net/ethernet/microchip/Makefile 
b/drivers/net/ethernet/microchip/Makefile
index ff78f62..2e982cc 100644
--- a/drivers/net/ethernet/microchip/Makefile
+++ b/drivers/net/ethernet/microchip/Makefile
@@ -4,3 +4,6 @@
 
 obj-$(CONFIG_ENC28J60) += enc28j60.o
 obj-$(CONFIG_ENCX24J600) += encx24j600.o encx24j600-regmap.o
+obj-$(CONFIG_LAN743X) += lan743x.o
+
+lan743x-objs := lan743x_main.o
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c 
b/drivers/net/ethernet/microchip/lan743x_main.c
new file mode 100644
index 000..3de39e1
--- /dev/null
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -0,0 +1,2757 @@
+/*
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include "lan743x_main.h"
+
+static void lan743x_pci_cleanup(struct lan743x_adapter *adapter)
+{
+   struct lan743x_pci *pci = >pci;
+
+   if (pci->init_flags & INIT_FLAG_PCI_REGIONS_REQUESTED) {
+   pci_release_selected_regions(pci->pdev,
+pci_select_bars(pci->pdev,
+IORESOURCE_MEM));
+   pci->init_flags &= ~INIT_FLAG_PCI_REGIONS_REQUESTED;
+   }
+   if (pci->init_flags & INIT_FLAG_PCI_DEVICE_ENABLED) {
+   pci_disable_device(pci->pdev);
+   pci->init_flags &= ~INIT_FLAG_PCI_DEVICE_ENABLED;
+   }
+}
+
+static int lan743x_pci_init(struct lan743x_adapter *adapter,
+   struct pci_dev *pdev)
+{
+   struct lan743x_pci *pci = >pci;
+   unsigned long bars = 0;
+   int ret;
+
+   pci->pdev = pdev;
+   ret = pci_enable_device_mem(pdev);
+   if (ret)
+   goto clean_up;
+   pci->init_flags |= INIT_FLAG_PCI_DEVICE_ENABLED;
+
+   netif_info(adapter, probe, adapter->netdev,
+  "PCI: Vendor ID = 0x%04X, Device ID = 0x%04X\n",
+  pdev->vendor, pdev->device);
+   bars = pci_select_bars(pdev, IORESOURCE_MEM);
+   if (!test_bit(0, ))
+   goto clean_up;
+
+   ret = pci_request_selected_regions(pdev, bars, DRIVER_NAME);
+   if (ret)
+   goto clean_up;
+   pci->init_flags |= INIT_FLAG_PCI_REGIONS_REQUESTED;
+
+   pci_set_master(pdev);
+   return 0;
+clean_up:
+   lan743x_pci_cleanup(adapter);
+   return ret;
+}
+
+u32 lan743x_csr_read(struct lan743x_adapter *adapter, int offset)
+{
+   return ioread32(>csr.csr_address[offset]);
+}
+
+void lan743x_csr_write(struct lan743x_adapter *adapter, int offset, u32 data)
+{
+   iowrite32(data, >csr.csr_address[offset]);
+}
+
+#define LAN743X_CSR_READ_OP(offset)lan743x_csr_read(adapter, offset)
+
+static int lan743x_csr_light_reset(struct lan743x_adapter *adapter)
+{
+   u32 data;
+
+   data = lan743x_csr_read(adapter, HW_CFG);
+   data |= HW_CFG_LRST_;
+   lan743x_csr_write(adapter, HW_CFG, data);
+
+   return readx_poll_timeout(LAN743X_CSR_READ_OP, HW_CFG, data,
+ !(data & HW_CFG_LRST_), 10, 1000);
+}
+
+static int lan743x_csr_wait_for_bit(struct