From: Andrew Lunn <andrew@lunn.ch>
To: Jiawen Wu <jiawenwu@trustnetic.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net-next v2] net: txgbe: Add build support for txgbe
Date: Wed, 25 May 2022 14:56:19 +0200 [thread overview]
Message-ID: <Yo4ncweml1gRDlhC@lunn.ch> (raw)
In-Reply-To: <01d001d86fe7$a4f4ba20$eede2e60$@trustnetic.com>
> > > + /* setup the private structure */
> > > + err = txgbe_sw_init(adapter);
> > > + if (err)
> > > + goto err_sw_init;
> > > +
> > > + if (pci_using_dac)
> > > + netdev->features |= NETIF_F_HIGHDMA;
> >
> > There should probably be a return 0; here, so the probe is successful.
> Without
> > that, you cannot test the remove function.
> >
>
> I find that when I execute 'rmmod txgbe', it causes a segmentation fault
> which prints 'iounmap: bad address'.
> But when I try to do 'iounmap' before 'return 0' in the probe function,
> there is no error.
> Could you please tell me the reason for this?
I'm assuming it is this code which is doing the print:
https://elixir.bootlin.com/linux/v5.18/source/arch/x86/mm/ioremap.c#L469
Which suggests the area you are trying to unmap is not actually
mapped.
Your code is a bit confusing:
in probe you have:
+ hw->hw_addr = ioremap(pci_resource_start(pdev, 0),
+ pci_resource_len(pdev, 0));
and remove:
+ iounmap(adapter->io_addr);
There is an assignment adapter->io_addr = hw->hw_addr; but this is
enough suggestion your structure of adapter and hw is not correct.
What i also notice is that release would normally things in the
opposite order to probe. That is not the case for your code.
> > > +static bool txgbe_check_cfg_remove(struct txgbe_hw *hw, struct
> > > +pci_dev *pdev) {
> > > + u16 value;
> > > +
> > > + pci_read_config_word(pdev, PCI_VENDOR_ID, &value);
> > > + if (value == TXGBE_FAILED_READ_CFG_WORD) {
> > > + txgbe_remove_adapter(hw);
> > > + return true;
> > > + }
> > > + return false;
> >
> > This needs a comment to explain what is happening here, because it is not
> > clear to me.
> >
>
> It means some kind of problem occur on PCI.
Which does not explain what this function is doing.
It seems like you have two cases to cover:
A PCI problem during probe. This is probably the more likely case. You
just fail the probe.
A PCI problem during run time. What sort of recovery are you going to
do? Just print a warning and keep going, hope for the best?
Andrew
prev parent reply other threads:[~2022-05-25 12:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-17 9:21 [PATCH net-next v2] net: txgbe: Add build support for txgbe Jiawen Wu
2022-05-17 21:28 ` kernel test robot
2022-05-18 3:12 ` Andrew Lunn
[not found] ` <01d001d86fe7$a4f4ba20$eede2e60$@trustnetic.com>
2022-05-25 12:56 ` Andrew Lunn [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yo4ncweml1gRDlhC@lunn.ch \
--to=andrew@lunn.ch \
--cc=jiawenwu@trustnetic.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).