From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Florian Fainelli <florian@openwrt.org>
Cc: davem@davemloft.net, Grant Likely <grant.likely@secretlab.ca>,
Rob Herring <rob.herring@calxeda.com>,
Rob Landley <rob@landley.net>,
Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
Russell King <linux@arm.linux.org.uk>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Lennert Buytenhek <buytenh@wantstofly.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linuxppc-dev@lists.ozlabs.org, netdev@vger.kernel.org
Subject: Re: [PATCH 3/5] net: mvmdio: enhance driver to support SMI error/done interrupts
Date: Tue, 29 Jan 2013 16:39:57 +0100 [thread overview]
Message-ID: <20130129163957.79815ff7@skate> (raw)
In-Reply-To: <1359473048-26551-4-git-send-email-florian@openwrt.org>
Dear Florian Fainelli,
On Tue, 29 Jan 2013 16:24:06 +0100, Florian Fainelli wrote:
> #define MVMDIO_SMI_DATA_SHIFT 0
> #define MVMDIO_SMI_PHY_ADDR_SHIFT 16
> @@ -36,12 +40,28 @@
> #define MVMDIO_SMI_WRITE_OPERATION 0
> #define MVMDIO_SMI_READ_VALID BIT(27)
> #define MVMDIO_SMI_BUSY BIT(28)
> +#define MVMDIO_ERR_INT_CAUSE 0x007C
> +#define MVMDIO_ERR_INT_SMI_DONE 0x00000010
> +#define MVMDIO_ERR_INT_MASK 0x0080
>
> struct orion_mdio_dev {
> struct mutex lock;
> void __iomem *regs;
> + /*
> + * If we have access to the error interrupt pin (which is
> + * somewhat misnamed as it not only reflects internal errors
> + * but also reflects SMI completion), use that to wait for
> + * SMI access completion instead of polling the SMI busy bit.
> + */
> + int err_interrupt;
> + wait_queue_head_t smi_busy_wait;
> };
>
> +static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
> +{
> + return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
> +}
> +
> /* Wait for the SMI unit to be ready for another operation
> */
> static int orion_mdio_wait_ready(struct mii_bus *bus)
> @@ -50,19 +70,30 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
> int count;
> u32 val;
>
> - count = 0;
> - while (1) {
> - val = readl(dev->regs);
> - if (!(val & MVMDIO_SMI_BUSY))
> - break;
> -
> - if (count > 100) {
> - dev_err(bus->parent, "Timeout: SMI busy for too long\n");
> - return -ETIMEDOUT;
> + if (dev->err_interrupt == NO_IRQ) {
> + count = 0;
> + while (1) {
> + val = readl(dev->regs);
> + if (!(val & MVMDIO_SMI_BUSY))
> + break;
What about using your new orion_mdio_smi_is_done() function here?
> +
> + if (count > 100) {
> + dev_err(bus->parent,
> + "Timeout: SMI busy for too long\n");
> + return -ETIMEDOUT;
> + }
> +
> + udelay(10);
> + count++;
> }
> + }
>
> - udelay(10);
> - count++;
> + if (!orion_mdio_smi_is_done(dev)) {
Maybe it should be in an else if block so that the waitqueue case is
only considered if there is an IRQ registered? Of course practically
speaking, it's OK because if there is no IRQ, we'll wait in the polling
loop above, and either exit from the function on timeout, or continue
on success. But it still would make the code a little bit clearer, I'd
say.
> static int orion_mdio_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> @@ -181,6 +227,19 @@ static int orion_mdio_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + dev->err_interrupt = NO_IRQ;
Not needed, you already do dev->err_interrupt = something() below.
> + init_waitqueue_head(&dev->smi_busy_wait);
> +
> + dev->err_interrupt = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + if (dev->err_interrupt != NO_IRQ) {
> + ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
> + orion_mdio_err_irq,
> + IRQF_SHARED, pdev->name, dev);
> + if (!ret)
> + writel(MVMDIO_ERR_INT_SMI_DONE,
> + dev->regs + MVMDIO_ERR_INT_MASK);
> + }
> +
> mutex_init(&dev->lock);
>
> ret = of_mdiobus_register(bus, np);
> @@ -202,6 +261,8 @@ static int orion_mdio_remove(struct platform_device *pdev)
> struct mii_bus *bus = platform_get_drvdata(pdev);
> struct orion_mdio_dev *dev = bus->priv;
>
> + writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
> + free_irq(dev->err_interrupt, dev);
free_irq() not needed since the IRQ handler is registered with
devm_request_irq().
> mdiobus_unregister(bus);
> kfree(bus->irq);
> mdiobus_free(bus);
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
next prev parent reply other threads:[~2013-01-29 15:40 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-29 15:24 [PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver Florian Fainelli
2013-01-29 15:24 ` [PATCH 1/5] net: mvmdio: unmap base register address at driver removal Florian Fainelli
2013-01-29 15:32 ` Thomas Petazzoni
2013-01-29 15:35 ` Florian Fainelli
2013-01-29 15:24 ` [PATCH 2/5] net: mvmdio: rename base register cookie from smireg to regs Florian Fainelli
2013-01-29 15:34 ` Thomas Petazzoni
2013-01-29 15:24 ` [PATCH 3/5] net: mvmdio: enhance driver to support SMI error/done interrupts Florian Fainelli
2013-01-29 15:39 ` Thomas Petazzoni [this message]
2013-01-29 15:24 ` [PATCH 4/5] net: mvmdio: allow Device Tree and platform device to coexist Florian Fainelli
2013-01-29 15:48 ` Thomas Petazzoni
2013-01-29 17:59 ` Jason Gunthorpe
2013-01-29 20:41 ` Florian Fainelli
2013-01-29 15:24 ` [PATCH 5/5] mv643xx_eth: convert to use the Marvell Orion MDIO driver Florian Fainelli
2013-01-29 16:01 ` Thomas Petazzoni
2013-01-29 16:27 ` Florian Fainelli
2013-01-29 16:46 ` Thomas Petazzoni
2013-01-29 18:13 ` Jason Gunthorpe
2013-01-29 20:41 ` Florian Fainelli
2013-03-14 17:25 ` [PATCH 0/5] mv643xx_eth: use mvmdio MDIO bus driver Jason Cooper
2013-03-14 18:09 ` Florian Fainelli
2013-03-14 18:16 ` Jason Cooper
2013-03-14 18:08 ` [PATCH 0/4] " Florian Fainelli
2013-03-14 18:08 ` [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver Florian Fainelli
2013-03-14 19:02 ` Jason Cooper
2013-03-14 19:32 ` Florian Fainelli
2013-03-14 20:36 ` Jason Cooper
2013-03-14 18:08 ` [PATCH 2/4 v2] net: mvmdio: rename base register cookie from smireg to regs Florian Fainelli
2013-03-14 18:08 ` [PATCH 3/4 v2] net: mvmdio: enhance driver to support SMI error/done interrupts Florian Fainelli
2013-03-15 18:05 ` Russell King - ARM Linux
2013-03-14 18:08 ` [PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver Florian Fainelli
2013-03-15 11:07 ` Florian Fainelli
2013-03-15 11:42 ` Thomas Petazzoni
2013-03-14 18:11 ` [PATCH 1/4 v2] net: mvmdio: allow platform device style registration Florian Fainelli
2013-03-15 12:53 ` [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver David Miller
2013-03-15 12:51 ` Florian Fainelli
2013-03-15 12:55 ` David Miller
2013-03-15 12:53 ` Florian Fainelli
2013-03-15 13:05 ` David Miller
2013-03-15 13:03 ` Florian Fainelli
2013-03-22 13:39 ` [PATCH 0/4 v3] " Florian Fainelli
2013-03-22 13:39 ` [PATCH 1/4 v3] net: mvmdio: allow platform device style registration Florian Fainelli
2013-03-22 13:39 ` [PATCH 2/4 v3] net: mvmdio: rename base register cookie from smireg to regs Florian Fainelli
2013-03-22 13:39 ` [PATCH 3/4 v3] net: mvmdio: enhance driver to support SMI error/done interrupts Florian Fainelli
2013-03-22 13:39 ` [PATCH 4/4 v3] mv643xx_eth: convert to use the Marvell Orion MDIO driver Florian Fainelli
2013-03-22 13:53 ` [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver Thomas Petazzoni
2013-03-22 14:14 ` Jason Cooper
2013-03-22 14:24 ` Florian Fainelli
2013-03-22 14:29 ` Jason Cooper
2013-03-22 14:31 ` Florian Fainelli
2013-03-22 14:26 ` David Miller
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=20130129163957.79815ff7@skate \
--to=thomas.petazzoni@free-electrons.com \
--cc=andrew@lunn.ch \
--cc=benh@kernel.crashing.org \
--cc=buytenh@wantstofly.org \
--cc=davem@davemloft.net \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=florian@openwrt.org \
--cc=grant.likely@secretlab.ca \
--cc=gregkh@linuxfoundation.org \
--cc=jason@lakedaemon.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=netdev@vger.kernel.org \
--cc=paulus@samba.org \
--cc=rob.herring@calxeda.com \
--cc=rob@landley.net \
/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).