* [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data()
@ 2016-03-01 16:37 Wolfram Sang
2016-03-01 16:37 ` [PATCH 2/2] net: ethernet: renesas: sh_eth: " Wolfram Sang
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Wolfram Sang @ 2016-03-01 16:37 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Sergei Shtylyov, netdev
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
This change will also make Coverity happy by avoiding a theoretical NULL
pointer dereference; yet another reason is to use the above helper function
to tighten the code and make it more readable.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Compile tested only. I am on the road and don't have acces to my Gen3 board.
If someone could test it, that would be much appreciated. Or I'll do it next
week. The pattern worked for other drivers I could actually test, though.
drivers/net/ethernet/renesas/ravb_main.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index c936682aae68df..b8613a611a6278 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1706,7 +1706,6 @@ static int ravb_set_gti(struct net_device *ndev)
static int ravb_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- const struct of_device_id *match;
struct ravb_private *priv;
enum ravb_chip_id chip_id;
struct net_device *ndev;
@@ -1738,8 +1737,7 @@ static int ravb_probe(struct platform_device *pdev)
ndev->base_addr = res->start;
ndev->dma = -1;
- match = of_match_device(of_match_ptr(ravb_match_table), &pdev->dev);
- chip_id = (enum ravb_chip_id)match->data;
+ chip_id = (enum ravb_chip_id)of_device_get_match_data(&pdev->dev);
if (chip_id == RCAR_GEN3)
irq = platform_get_irq_byname(pdev, "ch22");
--
2.7.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/2] net: ethernet: renesas: sh_eth: don't open code of_device_get_match_data()
2016-03-01 16:37 [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data() Wolfram Sang
@ 2016-03-01 16:37 ` Wolfram Sang
2016-03-02 1:21 ` Simon Horman
2016-03-03 21:39 ` David Miller
2016-03-02 1:19 ` [PATCH 1/2] net: ethernet: renesas: ravb_main: " Simon Horman
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Wolfram Sang @ 2016-03-01 16:37 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Sergei Shtylyov, netdev
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
This change will also make Coverity happy by avoiding a theoretical NULL
pointer dereference; yet another reason is to use the above helper function
to tighten the code and make it more readable.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Tested on a Lager board.
drivers/net/ethernet/renesas/sh_eth.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 0a150b2289146f..8b6c07fe3d407d 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3056,15 +3056,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
mdp->ether_link_active_low = pd->ether_link_active_low;
/* set cpu data */
- if (id) {
+ if (id)
mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
- } else {
- const struct of_device_id *match;
+ else
+ mdp->cd = (struct sh_eth_cpu_data *)of_device_get_match_data(&pdev->dev);
- match = of_match_device(of_match_ptr(sh_eth_match_table),
- &pdev->dev);
- mdp->cd = (struct sh_eth_cpu_data *)match->data;
- }
mdp->reg_offset = sh_eth_get_register_offset(mdp->cd->register_type);
if (!mdp->reg_offset) {
dev_err(&pdev->dev, "Unknown register type (%d)\n",
--
2.7.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] net: ethernet: renesas: sh_eth: don't open code of_device_get_match_data()
2016-03-01 16:37 ` [PATCH 2/2] net: ethernet: renesas: sh_eth: " Wolfram Sang
@ 2016-03-02 1:21 ` Simon Horman
2016-03-02 6:46 ` Wolfram Sang
2016-03-03 21:39 ` David Miller
1 sibling, 1 reply; 9+ messages in thread
From: Simon Horman @ 2016-03-02 1:21 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev
On Tue, Mar 01, 2016 at 05:37:59PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This change will also make Coverity happy by avoiding a theoretical NULL
> pointer dereference; yet another reason is to use the above helper function
> to tighten the code and make it more readable.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Tested on a Lager board.
>
> drivers/net/ethernet/renesas/sh_eth.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 0a150b2289146f..8b6c07fe3d407d 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -3056,15 +3056,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> mdp->ether_link_active_low = pd->ether_link_active_low;
>
> /* set cpu data */
> - if (id) {
> + if (id)
> mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
> - } else {
> - const struct of_device_id *match;
> + else
> + mdp->cd = (struct sh_eth_cpu_data *)of_device_get_match_data(&pdev->dev);
Is the cast needed here? of_device_get_match_data returns void *
>
> - match = of_match_device(of_match_ptr(sh_eth_match_table),
> - &pdev->dev);
> - mdp->cd = (struct sh_eth_cpu_data *)match->data;
> - }
> mdp->reg_offset = sh_eth_get_register_offset(mdp->cd->register_type);
> if (!mdp->reg_offset) {
> dev_err(&pdev->dev, "Unknown register type (%d)\n",
> --
> 2.7.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] net: ethernet: renesas: sh_eth: don't open code of_device_get_match_data()
2016-03-02 1:21 ` Simon Horman
@ 2016-03-02 6:46 ` Wolfram Sang
2016-03-03 0:04 ` Simon Horman
0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2016-03-02 6:46 UTC (permalink / raw)
To: Simon Horman; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev
[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]
On Wed, Mar 02, 2016 at 10:21:34AM +0900, Simon Horman wrote:
> On Tue, Mar 01, 2016 at 05:37:59PM +0100, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > This change will also make Coverity happy by avoiding a theoretical NULL
> > pointer dereference; yet another reason is to use the above helper function
> > to tighten the code and make it more readable.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >
> > Tested on a Lager board.
> >
> > drivers/net/ethernet/renesas/sh_eth.c | 10 +++-------
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> > index 0a150b2289146f..8b6c07fe3d407d 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> > @@ -3056,15 +3056,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> > mdp->ether_link_active_low = pd->ether_link_active_low;
> >
> > /* set cpu data */
> > - if (id) {
> > + if (id)
> > mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
> > - } else {
> > - const struct of_device_id *match;
> > + else
> > + mdp->cd = (struct sh_eth_cpu_data *)of_device_get_match_data(&pdev->dev);
>
> Is the cast needed here? of_device_get_match_data returns void *
The compiler complains about a const mismatch without the cast. To keep
things simple, I decided to leave the cast.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] net: ethernet: renesas: sh_eth: don't open code of_device_get_match_data()
2016-03-02 6:46 ` Wolfram Sang
@ 2016-03-03 0:04 ` Simon Horman
0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2016-03-03 0:04 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev
On Wed, Mar 02, 2016 at 07:46:24AM +0100, Wolfram Sang wrote:
> On Wed, Mar 02, 2016 at 10:21:34AM +0900, Simon Horman wrote:
> > On Tue, Mar 01, 2016 at 05:37:59PM +0100, Wolfram Sang wrote:
> > > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > >
> > > This change will also make Coverity happy by avoiding a theoretical NULL
> > > pointer dereference; yet another reason is to use the above helper function
> > > to tighten the code and make it more readable.
> > >
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > ---
> > >
> > > Tested on a Lager board.
> > >
> > > drivers/net/ethernet/renesas/sh_eth.c | 10 +++-------
> > > 1 file changed, 3 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> > > index 0a150b2289146f..8b6c07fe3d407d 100644
> > > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> > > @@ -3056,15 +3056,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> > > mdp->ether_link_active_low = pd->ether_link_active_low;
> > >
> > > /* set cpu data */
> > > - if (id) {
> > > + if (id)
> > > mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
> > > - } else {
> > > - const struct of_device_id *match;
> > > + else
> > > + mdp->cd = (struct sh_eth_cpu_data *)of_device_get_match_data(&pdev->dev);
> >
> > Is the cast needed here? of_device_get_match_data returns void *
>
> The compiler complains about a const mismatch without the cast. To keep
> things simple, I decided to leave the cast.
Ok, that makes sense.
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] net: ethernet: renesas: sh_eth: don't open code of_device_get_match_data()
2016-03-01 16:37 ` [PATCH 2/2] net: ethernet: renesas: sh_eth: " Wolfram Sang
2016-03-02 1:21 ` Simon Horman
@ 2016-03-03 21:39 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2016-03-03 21:39 UTC (permalink / raw)
To: wsa; +Cc: linux-renesas-soc, sergei.shtylyov, netdev
From: Wolfram Sang <wsa@the-dreams.de>
Date: Tue, 1 Mar 2016 17:37:59 +0100
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This change will also make Coverity happy by avoiding a theoretical NULL
> pointer dereference; yet another reason is to use the above helper function
> to tighten the code and make it more readable.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Tested on a Lager board.
>
> drivers/net/ethernet/renesas/sh_eth.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 0a150b2289146f..8b6c07fe3d407d 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -3056,15 +3056,11 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> mdp->ether_link_active_low = pd->ether_link_active_low;
>
> /* set cpu data */
> - if (id) {
> + if (id)
> mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
> - } else {
> - const struct of_device_id *match;
> + else
> + mdp->cd = (struct sh_eth_cpu_data *)of_device_get_match_data(&pdev->dev);
Applied, although mdp->cd should probably be made const. It is not legal to make
modifications to the objects returned from this interface.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data()
2016-03-01 16:37 [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data() Wolfram Sang
2016-03-01 16:37 ` [PATCH 2/2] net: ethernet: renesas: sh_eth: " Wolfram Sang
@ 2016-03-02 1:19 ` Simon Horman
2016-03-02 9:11 ` Geert Uytterhoeven
2016-03-03 21:39 ` David Miller
3 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2016-03-02 1:19 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev
On Tue, Mar 01, 2016 at 05:37:58PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This change will also make Coverity happy by avoiding a theoretical NULL
> pointer dereference; yet another reason is to use the above helper function
> to tighten the code and make it more readable.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data()
2016-03-01 16:37 [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data() Wolfram Sang
2016-03-01 16:37 ` [PATCH 2/2] net: ethernet: renesas: sh_eth: " Wolfram Sang
2016-03-02 1:19 ` [PATCH 1/2] net: ethernet: renesas: ravb_main: " Simon Horman
@ 2016-03-02 9:11 ` Geert Uytterhoeven
2016-03-03 21:39 ` David Miller
3 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2016-03-02 9:11 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Sergei Shtylyov, netdev@vger.kernel.org
On Tue, Mar 1, 2016 at 5:37 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This change will also make Coverity happy by avoiding a theoretical NULL
> pointer dereference; yet another reason is to use the above helper function
> to tighten the code and make it more readable.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data()
2016-03-01 16:37 [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data() Wolfram Sang
` (2 preceding siblings ...)
2016-03-02 9:11 ` Geert Uytterhoeven
@ 2016-03-03 21:39 ` David Miller
3 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2016-03-03 21:39 UTC (permalink / raw)
To: wsa; +Cc: linux-renesas-soc, sergei.shtylyov, netdev
From: Wolfram Sang <wsa@the-dreams.de>
Date: Tue, 1 Mar 2016 17:37:58 +0100
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This change will also make Coverity happy by avoiding a theoretical NULL
> pointer dereference; yet another reason is to use the above helper function
> to tighten the code and make it more readable.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Applied.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-03-03 21:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-01 16:37 [PATCH 1/2] net: ethernet: renesas: ravb_main: don't open code of_device_get_match_data() Wolfram Sang
2016-03-01 16:37 ` [PATCH 2/2] net: ethernet: renesas: sh_eth: " Wolfram Sang
2016-03-02 1:21 ` Simon Horman
2016-03-02 6:46 ` Wolfram Sang
2016-03-03 0:04 ` Simon Horman
2016-03-03 21:39 ` David Miller
2016-03-02 1:19 ` [PATCH 1/2] net: ethernet: renesas: ravb_main: " Simon Horman
2016-03-02 9:11 ` Geert Uytterhoeven
2016-03-03 21:39 ` David Miller
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).