* [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings
@ 2013-03-25 15:03 Florian Fainelli
2013-03-25 15:03 ` [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells Florian Fainelli
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
Hello David,
Please find here 3 minor bugfixes related to my recent device tree bindings
additions to DSA. Thanks!
Florian Fainelli (3):
dsa: fix device tree binding documentation typo on #address-cells
dsa: factor freeing of dsa_platform_data
dsa: fix freeing of sparse port allocation
Documentation/devicetree/bindings/net/dsa/dsa.txt | 2 +-
net/dsa/dsa.c | 40 ++++++++++-----------
2 files changed, 21 insertions(+), 21 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells
2013-03-25 15:03 [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings Florian Fainelli
@ 2013-03-25 15:03 ` Florian Fainelli
2013-03-25 15:03 ` [PATCH 2/3] dsa: factor freeing of dsa_platform_data Florian Fainelli
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
The device tree binding documentation for dsa explicitely states that a
DSA node should have its #address-cells property set to 2, yet the
example still used 1, fix that typo.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
Documentation/devicetree/bindings/net/dsa/dsa.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.txt b/Documentation/devicetree/bindings/net/dsa/dsa.txt
index db92f55..49f4f7a 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.txt
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.txt
@@ -43,7 +43,7 @@ Example:
dsa@0 {
compatible = "marvell,dsa";
- #address-cells = <1>;
+ #address-cells = <2>;
#size-cells = <0>;
interrupts = <10>;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] dsa: factor freeing of dsa_platform_data
2013-03-25 15:03 [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings Florian Fainelli
2013-03-25 15:03 ` [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells Florian Fainelli
@ 2013-03-25 15:03 ` Florian Fainelli
2013-03-25 15:03 ` [PATCH 3/3] dsa: fix freeing of sparse port allocation Florian Fainelli
2013-03-25 16:23 ` [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings David Miller
3 siblings, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
This patch factors the freeing of the struct dsa_platform_data
manipulated by the driver identically in two places to a single
function.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
net/dsa/dsa.c | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 908bc11..aa2ff58 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -343,6 +343,21 @@ out:
return ret;
}
+static void dsa_of_free_platform_data(struct dsa_platform_data *pd)
+{
+ int i;
+ int port_index;
+
+ for (i = 0; i < pd->nr_chips; i++) {
+ port_index = 0;
+ while (pd->chip[i].port_names &&
+ pd->chip[i].port_names[++port_index])
+ kfree(pd->chip[i].port_names[port_index]);
+ kfree(pd->chip[i].rtable);
+ }
+ kfree(pd->chip);
+}
+
static int dsa_of_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -354,7 +369,7 @@ static int dsa_of_probe(struct platform_device *pdev)
const char *port_name;
int chip_index, port_index;
const unsigned int *sw_addr, *port_reg;
- int ret, i;
+ int ret;
mdio = of_parse_phandle(np, "dsa,mii-bus", 0);
if (!mdio)
@@ -439,14 +454,7 @@ static int dsa_of_probe(struct platform_device *pdev)
return 0;
out_free_chip:
- for (i = 0; i < pd->nr_chips; i++) {
- port_index = 0;
- while (pd->chip[i].port_names &&
- pd->chip[i].port_names[++port_index])
- kfree(pd->chip[i].port_names[port_index]);
- kfree(pd->chip[i].rtable);
- }
- kfree(pd->chip);
+ dsa_of_free_platform_data(pd);
out_free:
kfree(pd);
pdev->dev.platform_data = NULL;
@@ -456,21 +464,11 @@ out_free:
static void dsa_of_remove(struct platform_device *pdev)
{
struct dsa_platform_data *pd = pdev->dev.platform_data;
- int i;
- int port_index;
if (!pdev->dev.of_node)
return;
- for (i = 0; i < pd->nr_chips; i++) {
- port_index = 0;
- while (pd->chip[i].port_names &&
- pd->chip[i].port_names[++port_index])
- kfree(pd->chip[i].port_names[port_index]);
- kfree(pd->chip[i].rtable);
- }
-
- kfree(pd->chip);
+ dsa_of_free_platform_data(pd);
kfree(pd);
}
#else
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] dsa: fix freeing of sparse port allocation
2013-03-25 15:03 [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings Florian Fainelli
2013-03-25 15:03 ` [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells Florian Fainelli
2013-03-25 15:03 ` [PATCH 2/3] dsa: factor freeing of dsa_platform_data Florian Fainelli
@ 2013-03-25 15:03 ` Florian Fainelli
2013-03-25 16:03 ` David Laight
2013-03-25 16:23 ` [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings David Miller
3 siblings, 1 reply; 7+ messages in thread
From: Florian Fainelli @ 2013-03-25 15:03 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
If we have defined a sparse port allocation which is non-contiguous and
contains gaps, the code freeing port_names will just stop when it
encouters a first NULL port_names, which is not right, we should iterate
over all possible number of ports (DSA_MAX_PORTS) until we are done.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
net/dsa/dsa.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index aa2ff58..0eb5d5e 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -350,9 +350,11 @@ static void dsa_of_free_platform_data(struct dsa_platform_data *pd)
for (i = 0; i < pd->nr_chips; i++) {
port_index = 0;
- while (pd->chip[i].port_names &&
- pd->chip[i].port_names[++port_index])
- kfree(pd->chip[i].port_names[port_index]);
+ while (port_index < DSA_MAX_PORTS) {
+ if (pd->chip[i].port_names[port_index])
+ kfree(pd->chip[i].port_names[port_index]);
+ port_index++;
+ }
kfree(pd->chip[i].rtable);
}
kfree(pd->chip);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH 3/3] dsa: fix freeing of sparse port allocation
2013-03-25 15:03 ` [PATCH 3/3] dsa: fix freeing of sparse port allocation Florian Fainelli
@ 2013-03-25 16:03 ` David Laight
2013-03-25 19:55 ` Florian Fainelli
0 siblings, 1 reply; 7+ messages in thread
From: David Laight @ 2013-03-25 16:03 UTC (permalink / raw)
To: Florian Fainelli, davem; +Cc: netdev
> If we have defined a sparse port allocation which is non-contiguous and
> contains gaps, the code freeing port_names will just stop when it
> encouters a first NULL port_names, which is not right, we should iterate
> over all possible number of ports (DSA_MAX_PORTS) until we are done.
...
> port_index = 0;
> - while (pd->chip[i].port_names &&
> - pd->chip[i].port_names[++port_index])
> - kfree(pd->chip[i].port_names[port_index]);
> + while (port_index < DSA_MAX_PORTS) {
> + if (pd->chip[i].port_names[port_index])
> + kfree(pd->chip[i].port_names[port_index]);
> + port_index++;
> + }
A couple of 'odd' differences:
The old code checked pd->chip[i].port_names (as if it
might be a pointer) that is absent from the new version.
(If it is separately allocated it is leaked).
The new code tests and frees pd->chip[i].port_names[0]
whereas the old code ignored the 0th entry.
These may, or may not, be relevant!
David
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings
2013-03-25 15:03 [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings Florian Fainelli
` (2 preceding siblings ...)
2013-03-25 15:03 ` [PATCH 3/3] dsa: fix freeing of sparse port allocation Florian Fainelli
@ 2013-03-25 16:23 ` David Miller
3 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2013-03-25 16:23 UTC (permalink / raw)
To: florian; +Cc: netdev
From: Florian Fainelli <florian@openwrt.org>
Date: Mon, 25 Mar 2013 16:03:37 +0100
> Please find here 3 minor bugfixes related to my recent device tree bindings
> additions to DSA. Thanks!
Series applied, thanks Florian.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] dsa: fix freeing of sparse port allocation
2013-03-25 16:03 ` David Laight
@ 2013-03-25 19:55 ` Florian Fainelli
0 siblings, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2013-03-25 19:55 UTC (permalink / raw)
To: David Laight; +Cc: davem, netdev
Le lundi 25 mars 2013 17:03:23, David Laight a écrit :
> > If we have defined a sparse port allocation which is non-contiguous and
> > contains gaps, the code freeing port_names will just stop when it
> > encouters a first NULL port_names, which is not right, we should iterate
> > over all possible number of ports (DSA_MAX_PORTS) until we are done.
>
> ...
>
> > port_index = 0;
> >
> > - while (pd->chip[i].port_names &&
> > - pd->chip[i].port_names[++port_index])
> > - kfree(pd->chip[i].port_names[port_index]);
> > + while (port_index < DSA_MAX_PORTS) {
> > + if (pd->chip[i].port_names[port_index])
> > + kfree(pd->chip[i].port_names[port_index]);
> > + port_index++;
> > + }
>
> A couple of 'odd' differences:
>
> The old code checked pd->chip[i].port_names (as if it
> might be a pointer) that is absent from the new version.
> (If it is separately allocated it is leaked).
>
> The new code tests and frees pd->chip[i].port_names[0]
> whereas the old code ignored the 0th entry.
The old code was wrong, it was off by one for the first array index, and would
stop whenever it encountered the first NULL port_names[index] so we would not
skip other these and free the possibly non-NULL next one.
I think the current code is now correct, but thanks for the review!
--
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-03-25 19:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-25 15:03 [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings Florian Fainelli
2013-03-25 15:03 ` [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells Florian Fainelli
2013-03-25 15:03 ` [PATCH 2/3] dsa: factor freeing of dsa_platform_data Florian Fainelli
2013-03-25 15:03 ` [PATCH 3/3] dsa: fix freeing of sparse port allocation Florian Fainelli
2013-03-25 16:03 ` David Laight
2013-03-25 19:55 ` Florian Fainelli
2013-03-25 16:23 ` [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings David Miller
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.