From: jogo@openwrt.org (Jonas Gorski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] net: mv643xx_eth: fix DT port device name
Date: Sun, 7 Jul 2013 23:43:41 +0200 [thread overview]
Message-ID: <20130707234341.0000721b@unknown> (raw)
In-Reply-To: <1373229231-8838-1-git-send-email-sebastian.hesselbarth@gmail.com>
On Sun, 7 Jul 2013 22:33:51 +0200
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote:
> Device tree support added to Marvell MV643xx ethernet driver registers
> port devices from port device nodes found on the corresponding controller
> node. The current port device name will cause the second controller to
> fail on registration because of two identical device names. This fixes
> the issue by taking the device node's name also as port device name.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Reported-by: Jonas Gorski <jogo@openwrt.org>
> ---
> Cc: Lennert Buytenhek <buytenh@wantstofly.org>
> Cc: Jonas Gorski <jogo@openwrt.org>
> Cc: netdev at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
> drivers/net/ethernet/marvell/mv643xx_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 6495bea..1f3a03d 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -2521,7 +2521,7 @@ static int mv643xx_eth_shared_of_add_port(struct platform_device *pdev,
> of_property_read_u32(pnp, "duplex", &ppd.duplex);
> }
>
> - ppdev = platform_device_alloc(MV643XX_ETH_NAME, ppd.port_number);
> + ppdev = platform_device_alloc(pnp->name, ppd.port_number);
> if (!ppdev)
> return -ENOMEM;
> ppdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
This breaks ethernet completely, as there is no platform driver
registered for pnp->name ("ethernetX-port"), only for MV643XX_ETH_NAME.
Also since I didn't see a patch for it and no mentioning of it:
There's still one further issue from having two ethernet-ports with
port_number 0, it causes a device leak:
static struct platform_device *port_platdev[3];
mv643xx_eth_shared_of_add_port()
{
...
port_platdev[ppd.port_number] = ppdev;
...
}
The second port at 0 will overwrite the first and thus will never be
deleted in
mv643xx_eth_shared_of_remove()
{
...
for (n = 0; n < 3; n++) {
platform_device_del(port_platdev[n]);
port_platdev[n] = NULL;
}
}
I doubt a insmod-rmmod-insmod will go well in that case ;-)
Regards
Jonas
WARNING: multiple messages have this Message-ID (diff)
From: Jonas Gorski <jogo@openwrt.org>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Lennert Buytenhek <buytenh@wantstofly.org>,
netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: mv643xx_eth: fix DT port device name
Date: Sun, 7 Jul 2013 23:43:41 +0200 [thread overview]
Message-ID: <20130707234341.0000721b@unknown> (raw)
In-Reply-To: <1373229231-8838-1-git-send-email-sebastian.hesselbarth@gmail.com>
On Sun, 7 Jul 2013 22:33:51 +0200
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote:
> Device tree support added to Marvell MV643xx ethernet driver registers
> port devices from port device nodes found on the corresponding controller
> node. The current port device name will cause the second controller to
> fail on registration because of two identical device names. This fixes
> the issue by taking the device node's name also as port device name.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Reported-by: Jonas Gorski <jogo@openwrt.org>
> ---
> Cc: Lennert Buytenhek <buytenh@wantstofly.org>
> Cc: Jonas Gorski <jogo@openwrt.org>
> Cc: netdev@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/net/ethernet/marvell/mv643xx_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 6495bea..1f3a03d 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -2521,7 +2521,7 @@ static int mv643xx_eth_shared_of_add_port(struct platform_device *pdev,
> of_property_read_u32(pnp, "duplex", &ppd.duplex);
> }
>
> - ppdev = platform_device_alloc(MV643XX_ETH_NAME, ppd.port_number);
> + ppdev = platform_device_alloc(pnp->name, ppd.port_number);
> if (!ppdev)
> return -ENOMEM;
> ppdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
This breaks ethernet completely, as there is no platform driver
registered for pnp->name ("ethernetX-port"), only for MV643XX_ETH_NAME.
Also since I didn't see a patch for it and no mentioning of it:
There's still one further issue from having two ethernet-ports with
port_number 0, it causes a device leak:
static struct platform_device *port_platdev[3];
mv643xx_eth_shared_of_add_port()
{
...
port_platdev[ppd.port_number] = ppdev;
...
}
The second port at 0 will overwrite the first and thus will never be
deleted in
mv643xx_eth_shared_of_remove()
{
...
for (n = 0; n < 3; n++) {
platform_device_del(port_platdev[n]);
port_platdev[n] = NULL;
}
}
I doubt a insmod-rmmod-insmod will go well in that case ;-)
Regards
Jonas
next prev parent reply other threads:[~2013-07-07 21:43 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-07 20:33 [PATCH] net: mv643xx_eth: fix DT port device name Sebastian Hesselbarth
2013-07-07 20:33 ` Sebastian Hesselbarth
2013-07-07 21:43 ` Jonas Gorski [this message]
2013-07-07 21:43 ` Jonas Gorski
2013-07-07 21:58 ` Sebastian Hesselbarth
2013-07-07 21:58 ` Sebastian Hesselbarth
2013-07-07 22:44 ` [PATCH] net: mv643xx_eth: do not use port number as platform device id Jonas Gorski
2013-07-07 22:44 ` Jonas Gorski
2013-07-07 22:05 ` [PATCH] net: mv643xx_eth: fix DT port device name Jonas Gorski
2013-07-07 22:05 ` Jonas Gorski
2013-07-10 10:29 ` Gerlando Falauto
2013-07-10 10:29 ` Gerlando Falauto
2013-07-10 22:59 ` Sebastian Hesselbarth
2013-07-10 22:59 ` Sebastian Hesselbarth
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=20130707234341.0000721b@unknown \
--to=jogo@openwrt.org \
--cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.