Message ID | 20220727025255.61232-1-jrdr.linux@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: dsa: microchip: remove of_match_ptr() from ksz9477_dt_ids | expand |
On Wed, Jul 27, 2022 at 08:22:55AM +0530, Souptick Joarder wrote: > From: "Souptick Joarder (HPE)" <jrdr.linux@gmail.com> > > >> drivers/net/dsa/microchip/ksz9477_i2c.c:89:34: > warning: 'ksz9477_dt_ids' defined but not used [-Wunused-const-variable=] > 89 | static const struct of_device_id ksz9477_dt_ids[] = { > | ^~~~~~~~~~~~~~ > > Removed of_match_ptr() from ksz9477_dt_ids. > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Souptick Joarder (HPE) <jrdr.linux@gmail.com> > --- I have to say, this is a major fail for what of_match_ptr() intended to do. commit 3a1e362e3f3cd571b3974b8d44b8e358ec7a098c Author: Ben Dooks <ben-linux@fluff.org> Date: Wed Aug 3 10:11:42 2011 +0100 OF: Add of_match_ptr() macro Add a macro of_match_ptr() that allows the .of_match_table entry in the driver structures to be assigned without having an #ifdef xxx NULL for the case that OF is not enabled Signed-off-by: Ben Dooks <ben-linux@fluff.org> Signed-off-by: Grant Likely <grant.likely@secretlab.ca> Should we also depend on CONFIG_OF? > drivers/net/dsa/microchip/ksz9477_i2c.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c > index 99966514d444..c967a03a22c6 100644 > --- a/drivers/net/dsa/microchip/ksz9477_i2c.c > +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c > @@ -118,7 +118,7 @@ MODULE_DEVICE_TABLE(of, ksz9477_dt_ids); > static struct i2c_driver ksz9477_i2c_driver = { > .driver = { > .name = "ksz9477-switch", > - .of_match_table = of_match_ptr(ksz9477_dt_ids), > + .of_match_table = ksz9477_dt_ids, > }, > .probe = ksz9477_i2c_probe, > .remove = ksz9477_i2c_remove, > -- > 2.25.1 >
On Thu, Jul 28, 2022 at 5:46 AM Vladimir Oltean <olteanv@gmail.com> wrote: > > On Wed, Jul 27, 2022 at 08:22:55AM +0530, Souptick Joarder wrote: > > From: "Souptick Joarder (HPE)" <jrdr.linux@gmail.com> > > > > >> drivers/net/dsa/microchip/ksz9477_i2c.c:89:34: > > warning: 'ksz9477_dt_ids' defined but not used [-Wunused-const-variable=] > > 89 | static const struct of_device_id ksz9477_dt_ids[] = { > > | ^~~~~~~~~~~~~~ > > > > Removed of_match_ptr() from ksz9477_dt_ids. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Souptick Joarder (HPE) <jrdr.linux@gmail.com> > > --- > > I have to say, this is a major fail for what of_match_ptr() intended to do. > > commit 3a1e362e3f3cd571b3974b8d44b8e358ec7a098c > Author: Ben Dooks <ben-linux@fluff.org> > Date: Wed Aug 3 10:11:42 2011 +0100 > > OF: Add of_match_ptr() macro > > Add a macro of_match_ptr() that allows the .of_match_table > entry in the driver structures to be assigned without having > an #ifdef xxx NULL for the case that OF is not enabled > > Signed-off-by: Ben Dooks <ben-linux@fluff.org> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca> > > Should we also depend on CONFIG_OF? Few other drivers threw similar warnings and the suggestion was to remove of_match_ptr(). Anyway both approaches look fine. > > > drivers/net/dsa/microchip/ksz9477_i2c.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c > > index 99966514d444..c967a03a22c6 100644 > > --- a/drivers/net/dsa/microchip/ksz9477_i2c.c > > +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c > > @@ -118,7 +118,7 @@ MODULE_DEVICE_TABLE(of, ksz9477_dt_ids); > > static struct i2c_driver ksz9477_i2c_driver = { > > .driver = { > > .name = "ksz9477-switch", > > - .of_match_table = of_match_ptr(ksz9477_dt_ids), > > + .of_match_table = ksz9477_dt_ids, > > }, > > .probe = ksz9477_i2c_probe, > > .remove = ksz9477_i2c_remove, > > -- > > 2.25.1 > >
On Thu, Jul 28, 2022 at 06:16:50AM +0530, Souptick Joarder wrote: > Few other drivers threw similar warnings and the suggestion was to > remove of_match_ptr(). Anyway both approaches look fine. I don't disagree with this change, I'm just saying that as a next step we may block this driver from even being compiled if CONFIG_OF=n.
On Wed, 2022-07-27 at 08:22 +0530, Souptick Joarder wrote: > From: "Souptick Joarder (HPE)" <jrdr.linux@gmail.com> > > > > drivers/net/dsa/microchip/ksz9477_i2c.c:89:34: > warning: 'ksz9477_dt_ids' defined but not used [-Wunused-const-variable=] > 89 | static const struct of_device_id ksz9477_dt_ids[] = { > | ^~~~~~~~~~~~~~ > > Removed of_match_ptr() from ksz9477_dt_ids. > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Souptick Joarder (HPE) <jrdr.linux@gmail.com> As this looks like a fix, could you please post a new revision of the patch including into the commit message a proper 'Fixes' tag? Thanks! Paolo
On Thu, Jul 28, 2022 at 6:45 PM Paolo Abeni <pabeni@redhat.com> wrote: > > On Wed, 2022-07-27 at 08:22 +0530, Souptick Joarder wrote: > > From: "Souptick Joarder (HPE)" <jrdr.linux@gmail.com> > > > > > > drivers/net/dsa/microchip/ksz9477_i2c.c:89:34: > > warning: 'ksz9477_dt_ids' defined but not used [-Wunused-const-variable=] > > 89 | static const struct of_device_id ksz9477_dt_ids[] = { > > | ^~~~~~~~~~~~~~ > > > > Removed of_match_ptr() from ksz9477_dt_ids. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Souptick Joarder (HPE) <jrdr.linux@gmail.com> > > As this looks like a fix, could you please post a new revision of the > patch including into the commit message a proper 'Fixes' tag? Sure. > > Thanks! > > Paolo >
diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c index 99966514d444..c967a03a22c6 100644 --- a/drivers/net/dsa/microchip/ksz9477_i2c.c +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c @@ -118,7 +118,7 @@ MODULE_DEVICE_TABLE(of, ksz9477_dt_ids); static struct i2c_driver ksz9477_i2c_driver = { .driver = { .name = "ksz9477-switch", - .of_match_table = of_match_ptr(ksz9477_dt_ids), + .of_match_table = ksz9477_dt_ids, }, .probe = ksz9477_i2c_probe, .remove = ksz9477_i2c_remove,